Don't update per-page activation counts in the swapout code.
ClosedPublic
Actions

Authored by markj on Dec 4 2019, 4:37 PM.

Details

Reviewers

alc
kib
jeff
dougm

Commits

rS356154: Don't update per-page activation counts in the swapout code.

Summary

This is the work of the page daemon. Now that the page daemon does slow
and steady scanning of the active queue, I don't think it makes much
sense to replicate that algorithm here. Moreover, it was doing so
inconsistently:

PGA_REFERENCED is not counted in act_count unless pmap_ts_referenced() returned 0, but the page daemon always counts PGA_REFERENCED.
The swapout daemon always activates a referenced page, but the page daemon only does so when the containing object is mapped at least once.

The main purpose of swapout_deactivate_pages() is to shrink the number
of pages mapped into a given pmap. To do this without unmapping active
pages, use the non-destructive pmap_is_referenced() instead of the
destructive pmap_ts_referenced() and deactivate pages accordingly.

This function is racy before and after this change: a reference bit
which is set between the pmap_is_referenced() and
vm_page_try_remove_all() calls is lost. We do not have a pmap interface
for removing only unreferenced mappings.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27941
Build 26107: arc lint + arc unit

Event Timeline

markj created this revision.Dec 4 2019, 4:37 PM

Harbormaster completed remote builds in B27941: Diff 65213.Dec 4 2019, 4:37 PM

markj added reviewers: alc, kib, jeff, dougm.Dec 4 2019, 4:38 PM

Re-add page locking which I removed prematurely.

Harbormaster completed remote builds in B27986: Diff 65275.Dec 5 2019, 5:48 PM

kib accepted this revision.Dec 6 2019, 3:00 PM

kib added inline comments.

sys/vm/vm_swapout.c
183	Does it make sense to move these checks before tryxbusy() ? Same for pmap_is_referenced() call below ?

This revision is now accepted and ready to land.Dec 6 2019, 3:00 PM

markj added inline comments.Dec 6 2019, 6:29 PM

sys/vm/vm_swapout.c
183	The wired check must come after the busy (the holder of a page busy is allowed to wire the page), but otherwise I think this is reasonable.

kib added inline comments.Dec 6 2019, 8:33 PM

sys/vm/vm_swapout.c
183	I suspect that the checks should be repeated after xbusy succeeded. At least exists_quick(), and then wired().

jeff added inline comments.Dec 6 2019, 8:41 PM

sys/vm/vm_swapout.c
183	What are you trying to save by reordering?

kib added inline comments.Dec 6 2019, 8:59 PM

sys/vm/vm_swapout.c
183	I want to execute tests that do not change the page' state before we modify the page (xbusy). I do think that in some situations the page_exists_quick() or wired() checking before xbusy would save a lot.

jeff added inline comments.Dec 6 2019, 9:03 PM

sys/vm/vm_swapout.c
183	page_exists_quick() is still much more expensive than busy. If we have to repeat it it seems like a loss to me. busy is only an atomic. For wired you could check before and after with little cost.

markj added inline comments.Dec 6 2019, 9:14 PM

sys/vm/vm_swapout.c
183	I think we could check wired and exists_quick() before busy, and repeat the wired check after. I think the exists_quick() check is probably too expensive to repeat.