Page MenuHomeFreeBSD
Differential D22081

Fix pointer loads in uma_vm_zone_stats().
ClosedPublic
Actions

Authored by markj on Oct 18 2019, 4:51 PM.
Tags
None
Referenced Files
F122623805: D22081.id63454.diff
Sun, Jul 6, 8:42 PM
F122593549: D22081.id63454.diff
Sun, Jul 6, 1:02 PM
Unknown Object (File)
Mon, Jun 30, 9:41 AM
Unknown Object (File)
Sat, Jun 28, 5:26 PM
Unknown Object (File)
Fri, Jun 27, 1:25 PM
Unknown Object (File)
Thu, Jun 26, 6:34 AM
Unknown Object (File)
Thu, Jun 26, 6:04 AM
Unknown Object (File)
Mon, Jun 23, 4:46 AM
View All Files
Subscribers
imp
pho

Details

Reviewers
alc
jeff
kib
dougm
Commits
rS353886: Avoid reloading bucket pointers in uma_vm_zone_stats().
Summary

Ensure that each bucket pointer is loaded only once.
This happens already in practice, but without a change
like this one the compiler is permitted to issue two loads,
resulting in a race condition. This can happen when compiling
with -O0, for instance.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj created this revision.Oct 18 2019, 4:51 PM
Harbormaster completed remote builds in B27110: Diff 63451.Oct 18 2019, 4:51 PM
markj mentioned this in D20635: release multiple swap blocks at a time in swp_pager_force_pagein.Oct 18 2019, 4:54 PM
markj updated this revision to Diff 63452.Oct 18 2019, 4:56 PM

Restore the original loop, we need to always zero the structure.

Harbormaster completed remote builds in B27111: Diff 63452.Oct 18 2019, 4:56 PM
markj updated this revision to Diff 63453.Oct 18 2019, 4:57 PM

Fix the right loop this time.

Harbormaster completed remote builds in B27112: Diff 63453.Oct 18 2019, 4:57 PM
markj updated this revision to Diff 63454.Oct 18 2019, 4:57 PM

One more try.

Harbormaster completed remote builds in B27113: Diff 63454.Oct 18 2019, 4:57 PM
markj edited the summary of this revision. (Show Details)Oct 18 2019, 5:15 PM
markj added reviewers: alc, jeff, kib, dougm.
markj added a subscriber: pho.Oct 18 2019, 5:22 PM

Peter, could you post the disassembly of uma_vm_zone_stats() from your -O0 kernel?

kib accepted this revision.Oct 18 2019, 5:33 PM

Perhaps adding a sentence to the preceding comment explaining why atomic_load would be useful.

This revision is now accepted and ready to land.Oct 18 2019, 5:33 PM
markj updated this revision to Diff 63458.Oct 18 2019, 5:39 PM

Add a comment.

This revision now requires review to proceed.Oct 18 2019, 5:39 PM
Harbormaster completed remote builds in B27115: Diff 63458.Oct 18 2019, 5:39 PM
pho added a comment.Oct 18 2019, 5:54 PM
In D22081#482480, @markj wrote:

Peter, could you post the disassembly of uma_vm_zone_stats() from your -O0 kernel?

Sure.

mark.txt10 KBDownload

markj added a comment.Oct 18 2019, 6:00 PM
In D22081#482499, @pho wrote:
In D22081#482480, @markj wrote:

Peter, could you post the disassembly of uma_vm_zone_stats() from your -O0 kernel?

Sure.

mark.txt10 KBDownload

Thanks. Indeed, that code contains a race that should be fixed by this diff.

pho added a comment.Oct 19 2019, 7:42 AM

I tested 63458 for 17 hours with a "-O0" build of uma_core.c. LGTM.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 22 2019, 2:20 PM
Closed by commit rS353886: Avoid reloading bucket pointers in uma_vm_zone_stats(). (authored by markj). · Explain Why
This revision was automatically updated to reflect the committed changes.
markj added a commit: rS353886: Avoid reloading bucket pointers in uma_vm_zone_stats()..
Herald added a subscriber: imp. · View Herald TranscriptOct 22 2019, 2:20 PM

Revision Contents
Changeset List

PathSize
head/
sys/
vm/
32 lines

Diff 63537

View Options

head/sys/vm/uma_core.c

Loading...