vm_page: Consolidate page busy sleep mechanisms
ClosedPublic
Actions

Authored by markj on Nov 11 2021, 5:34 PM.

Details

Reviewers

alc
kib
jeff

Group Reviewers

manpages

Commits

rG87b646630c48: vm_page: Consolidate page busy sleep mechanisms

Summary

Modify vm_page_busy_sleep() and vm_page_busy_sleep_unlocked() to take a VM_ALLOC_* flag indicating whether to sleep on shared-busy, and fix up callers.
Modify vm_page_busy_sleep() to return a status indicating whether the object lock was dropped, and fix up callers.
Convert callers of vm_page_sleep_if_busy() to use vm_page_busy_sleep() instead.
Remove vm_page_sleep_if_(x)busy().

Obtained from: jeff (object_concurrency patches)

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 42766
Build 39654: arc lint + arc unit

Event Timeline

markj created this revision.Nov 11 2021, 5:34 PM

Herald added a reviewer: manpages. · View Herald TranscriptNov 11 2021, 5:34 PM

Herald added a subscriber: imp. · View Herald Transcript

markj requested review of this revision.Nov 11 2021, 5:34 PM

markj added a parent revision: D32946: vm_pager: Micro-optimize an assertion.

markj added a child revision: D32948: vm_page: Remove vm_page_sbusy() and vm_page_xbusy().

Harbormaster completed remote builds in B42734: Diff 98372.Nov 11 2021, 5:34 PM

kib added inline comments.Nov 12 2021, 6:30 PM

sys/vm/vm_page.c
1019–1020	It is somewhat strange API design. Is it planned to allow other values for allocflags later on? If you decide to reuse allocflags there, why not allow any flags and just operate on the (allocflags & VM_ALLOC_IGN_SBUSY) != 0? Or not reuse and then make (keep) allocflags bool argunment.

markj added inline comments.Nov 12 2021, 6:42 PM

sys/vm/vm_page.c
1019–1020	I don't think there's any plan to permit other flags for now, but this way consumers use a standard terminology at least. I find it a bit nicer than a naked bool. I could simply remove the assertion. Note that _vm_page_busy_sleep() does not have any assertion since some other calls will pass vm_page_grab* flags in unmodified.

kib added inline comments.Nov 12 2021, 6:48 PM

sys/vm/vm_page.c
1019–1020	To me, removing the assertion looks reasonable.

Drop assertions.

Harbormaster completed remote builds in B42766: Diff 98437.Nov 12 2021, 7:07 PM

kib accepted this revision.Nov 12 2021, 7:17 PM

This revision is now accepted and ready to land.Nov 12 2021, 7:17 PM

Closed by commit rG87b646630c48: vm_page: Consolidate page busy sleep mechanisms (authored by markj). · Explain WhyFeb 16 2022, 10:48 PM

This revision was automatically updated to reflect the committed changes.

markj added a commit: rG87b646630c48: vm_page: Consolidate page busy sleep mechanisms.

Only seeing this now (I hadn't yet joined manpages when this was last touched).

To make this manual page easier to understand, I would draw a much more explicit connection between "busied" and "currently marked busy", and likewise between "busies" and "the kernel marks the page busy".

(Sorry for the earlier version, which said much more about English grammar than needed for this. That was me hitting submit too soon after thinking in writing.)