Page MenuHomeFreeBSD
Differential D25859

vm_page_xbusy_claim(): Use atomics to update busy lock state.
ClosedPublic
Actions

Authored by markj on Jul 28 2020, 3:08 PM.
Tags
None
Referenced Files
F103538977: D25859.id75089.diff
Tue, Nov 26, 6:30 AM
Unknown Object (File)
Sat, Nov 23, 6:00 AM
Unknown Object (File)
Tue, Nov 5, 5:20 PM
Unknown Object (File)
Oct 4 2024, 4:37 AM
Unknown Object (File)
Oct 2 2024, 5:00 PM
Unknown Object (File)
Oct 1 2024, 4:29 AM
Unknown Object (File)
Sep 30 2024, 9:03 AM
Unknown Object (File)
Sep 27 2024, 1:54 AM
View All 32 Files
Subscribers
cy
emaste
imp

Details

Reviewers
alc
kib
jeff
vangyzen
Commits
rS363654: vm_page_xbusy_claim(): Use atomics to update busy lock state.
Summary

vm_page_xbusy_claim() could clobber the waiter bit. For its original
use, kernel memory pages, this was not a problem since nothing would
ever block on the busy lock for such pages. r363607 introduced a new
use where this could in principle be a problem.

Fix the problem by using atomic_cmpset. Since this macro is defined
only for INVARIANTS kernels, the extra overhead doesn't seem
prohibitive.

Reported by: vangyzen

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 32621
Build 30077: arc lint + arc unit

Event Timeline

markj requested review of this revision.Jul 28 2020, 3:08 PM
markj created this revision.
Harbormaster completed remote builds in B32614: Diff 75069.Jul 28 2020, 3:08 PM
markj added reviewers: alc, kib, jeff, vangyzen.Jul 28 2020, 3:12 PM
cy added a subscriber: cy.Jul 28 2020, 3:21 PM
kib accepted this revision.Jul 28 2020, 3:43 PM
kib added inline comments.
sys/vm/vm_page.h
775

I think you want to use _busy_lock as a name. If user code (in kernel) defines a busy_lock macro, we have conflict. The _[a-z0-9] namespace is reserved by std for file-local impl symbols.

This revision is now accepted and ready to land.Jul 28 2020, 3:43 PM
emaste added a subscriber: emaste.Jul 28 2020, 3:53 PM
markj updated this revision to Diff 75078.Jul 28 2020, 4:08 PM
markj marked an inline comment as done.

Put the local var in the file-local namespace.

This revision now requires review to proceed.Jul 28 2020, 4:08 PM
Harbormaster completed remote builds in B32621: Diff 75078.Jul 28 2020, 4:08 PM
kib accepted this revision.Jul 28 2020, 4:22 PM
This revision is now accepted and ready to land.Jul 28 2020, 4:22 PM
vangyzen accepted this revision.Jul 28 2020, 4:22 PM
vangyzen added inline comments.
sys/vm/vm_page.h
780

I was expecting fcmpset, but this works, too.

markj added inline comments.Jul 28 2020, 4:37 PM
sys/vm/vm_page.h
780

I initially wrote it with fcmpset but I find this is a bit clearer. I don't see any real reason to prefer one over the other.

alc accepted this revision.Jul 28 2020, 6:19 PM
Closed by commit rS363654: vm_page_xbusy_claim(): Use atomics to update busy lock state. (authored by markj). · Explain WhyJul 28 2020, 7:51 PM
This revision was automatically updated to reflect the committed changes.
markj added a commit: rS363654: vm_page_xbusy_claim(): Use atomics to update busy lock state..
Herald added a subscriber: imp. · View Herald TranscriptJul 28 2020, 7:51 PM

Revision Contents
Changeset List

PathSize
sys/
vm/
7 lines

Diff 75078

View Options

sys/vm/vm_page.h

Loading...