Augment vm_page_grab to support more cases that want to acquire busy locks.
AbandonedPublic
Actions

Authored by jeff on Aug 20 2019, 1:04 AM.

Details

Reviewers

alc
kib
markj
dougm

Summary

This patch adds two new flags, VM_ALLOC_NOCREAT & VM_ALLOC_VALID, so that we can move more consumers to grab() and eliminate hand-rolled loops with sensitive synchronization.

NOCREAT does the lookup/busy/wire in a loop but fails if it has to allocate a page.
VALID only returns existing valid pages.

There are other callers which can be refactored into this paradigm later.

The second part of the patch implements vm_page_busy_acquire() which does a looping busy acquire so callers are not coding their own sleep_if_busy() loops or blindly calling vm_page_xbusy() and expecting it to succeed.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25969
Build 24525: arc lint + arc unit

Event Timeline

jeff created this revision.Aug 20 2019, 1:04 AM

Herald added a subscriber: delphij. · View Herald TranscriptAug 20 2019, 1:04 AM

Harbormaster completed remote builds in B25969: Diff 61025.Aug 20 2019, 1:04 AM

jeff retitled this revision from Augment vm_page_grab to support more cases that want to acquire busy locks. Add a new function, vm_page_busy_acquire(), to support others. to Augment vm_page_grab to support more cases that want to acquire busy locks..Aug 20 2019, 1:09 AM

jeff edited the summary of this revision. (Show Details)

jeff added reviewers: alc, kib, markj, dougm.

kib added inline comments.Aug 20 2019, 10:25 AM

sys/vm/vm_page.c
3895	Why do you changed the setting of PGA_REFERENCED ?
3921	It is somewhat strange semantic. If only ALLOC_VALID was specified, without ALLOC_NOCREAT, why zeroed page cannot satisfy the caller ?

jeff added inline comments.Aug 20 2019, 8:16 PM

sys/vm/vm_page.c
3895	I flipped this the wrong way. It should be NOCREAT == 0. Truncate, for example, wants to grab the last page to zero it, but only if it already exists. If not the pager will do it properly when it's created and you just waste time paging in an unnecessary page.
3921	I overloaded the VALID name. I think we could use a different name for VM_ALLOC_VALID. It basically means, only return an existing valid page, but do no work to make it valid. There are some other weirdnesses. For example, vm_page_grab_pages() will set the page bits valid when it zeros a page. vm_page_grab() does not. I like this api layer that allows us to centralize synchronization but I feel at some point it could be simplified and split into a few smaller functions. The swiss army knife of flags always gets confusing.

jeff added inline comments.Aug 20 2019, 8:23 PM

sys/vm/vm_page.c
3895	How about ALLOC_IFVALID

kib added inline comments.Aug 21 2019, 1:12 PM

sys/vm/vm_page.c
3895	I still do not see why would we remove protection against pagedaemon during the sleep. We did found the page, so why NOCREAT/IFVALID should make it more likely for the page to be reclaimed under us, while we sleep for busy ?
3921	I understand this, my point is that allocating a page is much more work than validate it by zeroing.