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
F136943059: D22081.id63451.diff
Thu, Nov 20, 6:54 PM
F136942725: D22081.id63452.diff
Thu, Nov 20, 6:52 PM
F136942073: D22081.id63454.diff
Thu, Nov 20, 6:49 PM
F136942031: D22081.id.diff
Thu, Nov 20, 6:49 PM
F136941829: D22081.id63537.diff
Thu, Nov 20, 6:47 PM
F136940340: D22081.diff
Thu, Nov 20, 6:36 PM
Unknown Object (File)
Wed, Nov 19, 3:21 PM
Unknown Object (File)
Mon, Nov 17, 10:07 PM
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

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 27110
Build 25389: arc lint + arc unit

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
sys/
vm/
22 lines

Diff 63451

View Options

sys/vm/uma_core.c

Loading...