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

[alc]

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.

[markj]

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

Leftover debug print.

[alc]

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

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

[markj]

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]

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.