Page MenuHomeFreeBSD

Change vm_pageout_scan() to return a value indicating whether the free page target was met
ClosedPublic

Authored by alc on Oct 1 2016, 5:07 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 19 2024, 3:32 PM
Unknown Object (File)
Mar 19 2024, 3:19 PM
Unknown Object (File)
Mar 19 2024, 5:05 AM
Unknown Object (File)
Mar 19 2024, 3:11 AM
Unknown Object (File)
Feb 8 2024, 2:36 PM
Unknown Object (File)
Dec 24 2023, 7:10 AM
Unknown Object (File)
Dec 20 2023, 12:23 AM
Unknown Object (File)
Oct 30 2023, 7:34 AM
Subscribers
None

Details

Summary

Currently, vm_pageout_worker() itself checks the length of the free page queues to determine whether vm_pageout_scan(pass >= 1)'s inactive queue scan freed enough pages to meet the free page target. Specifically, vm_pageout_worker() uses vm_paging_needed(). The trouble with vm_paging_needed() is that it compares the length of the free page queues to the wakeup threshold for the page daemon, which is much lower than the free page target. Consequently, vm_pageout_worker() can conclude that the inactive queue scan succeeded in meeting its free page target when in fact it did not; and rather than immediately triggering an all out laundering pass over the inactive queue, vm_pageout_worker() goes back to sleep waiting for the free page count to fall below the page daemon wakeup threshold again at which point it will perform another limited (pass == 1) scan over the inactive queue.

Changing vm_pageout_worker() to use vm_page_count_target() instead of vm_paging_needed() won't work because any page allocations that happen concurrently with the inactive queue scan will result in the free page count being below the target at the end of a successful scan. Instead, having vm_pageout_scan() return a value indicating success or failure is the most straightforward fix.

Test Plan

I have verified using the included debugging printf() that this scenario does happen. That said, it's not that common.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

alc retitled this revision from to Change vm_pageout_scan() to return a value indicating whether the free page target was met.
alc updated this object.
alc edited the test plan for this revision. (Show Details)
alc added reviewers: kib, markj.
alc updated this object.
kib edited edge metadata.
This revision is now accepted and ready to land.Oct 1 2016, 6:23 PM
markj edited edge metadata.

I'm a bit confused about the intent behind this. With PQ_LAUNDRY, this change won't have much of an effect since vm_pageout_scan() doesn't distinguish between pass == 1 and pass > 1.

vm/vm_pageout.c
1561 โ†—(On Diff #20913)

Leftover debug print.

alc marked an inline comment as done.Oct 2 2016, 11:36 PM
In D8111#167865, @markj wrote:

I'm a bit confused about the intent behind this. With PQ_LAUNDRY, this change won't have much of an effect since vm_pageout_scan() doesn't distinguish between pass == 1 and pass > 1.

Under PQ_LAUNDRY, this change affects whether we perform a 1/2 second pause (with page daemon wakeups disabled) or reenable page daemon wakeups. Specifically, we're sometimes reenabling page daemon wakeups when we should have paused.

vm/vm_pageout.c
1561 โ†—(On Diff #20913)

Ya, I left this printf() in the uploaded patch to show how I determined that mistakes were occurring.

In D8111#168126, @alc wrote:
In D8111#167865, @markj wrote:

I'm a bit confused about the intent behind this. With PQ_LAUNDRY, this change won't have much of an effect since vm_pageout_scan() doesn't distinguish between pass == 1 and pass > 1.

Under PQ_LAUNDRY, this change affects whether we perform a 1/2 second pause (with page daemon wakeups disabled) or reenable page daemon wakeups. Specifically, we're sometimes reenabling page daemon wakeups when we should have paused.

Hm, I note that we only do the 1/2 second pause after the first "shortfall pass". That is, we'll immediately scan the inactive queue a second time upon failing to meet the target. I think that doesn't make sense anymore on PQ_LAUNDRY since we'll wake up the laundry thread after the first scan.

alc marked an inline comment as done.Oct 3 2016, 1:27 AM
In D8111#168151, @markj wrote:
In D8111#168126, @alc wrote:
In D8111#167865, @markj wrote:

I'm a bit confused about the intent behind this. With PQ_LAUNDRY, this change won't have much of an effect since vm_pageout_scan() doesn't distinguish between pass == 1 and pass > 1.

Under PQ_LAUNDRY, this change affects whether we perform a 1/2 second pause (with page daemon wakeups disabled) or reenable page daemon wakeups. Specifically, we're sometimes reenabling page daemon wakeups when we should have paused.

Hm, I note that we only do the 1/2 second pause after the first "shortfall pass". That is, we'll immediately scan the inactive queue a second time upon failing to meet the target. I think that doesn't make sense anymore on PQ_LAUNDRY since we'll wake up the laundry thread after the first scan.

Agreed. I'll address that issue when I merge this change into PQ_LAUNDRY.

This revision was automatically updated to reflect the committed changes.