Start implementing queue state updates using fcmpset loops. [1/5]

[markj]

Summary

This is in preparation for eliminating the use of the vm_page lock for
protecting queue state operations.

Introduce the vm_page_pqstate_commit_*() functions. These functions act
as helpers around vm_page_astate_fcmpset() and are specialized for
specific types of operations. All of them use
vm_page_pqstate_fcmpset(), which counts update failures in
vm.stats.page.pqstate_commit_aborts.

Convert vm_pqbatch_process_page() to use the pqstate helpers.

Convert vm_page_dequeue() and vm_page_dequeue_deferred() to use the
helpers. Remove vm_page_dequeue_deferred_free(), which is now the same
as vm_page_dequeue_deferred(). Remove vm_page_dequeue_complete().

Convert vm_page_swapqueue() to use the helpers.

[jeff]

sys/vm/vm_page.c

3238 ↗

(On Diff #65520)

abort makes sense for operations that won't loop but some will. I guess a single counter is ok but it may be interesting to know how much we are spinning.

3279 ↗

(On Diff #65520)

This might read more easily if the two if (queued) blocks were together and you instead had an else with a duplicate fcmpset call.

You could even do
if (!queued)

return(fcmpset());

3389 ↗

(On Diff #65520)

If you have a sample output for these counters after buildworld or stress2 or something taxing I would love to see them.

3396 ↗

(On Diff #65520)

if new = old in both cases can we assign above for clarity?

sys/vm/vm_page.h

923 ↗

(On Diff #65520)

Can you describe why the fence was needed before and why it is not now?

[markj]

sys/vm/vm_page.c

3238 ↗

(On Diff #65520)

Maybe commit_retry instead?

3389 ↗

(On Diff #65520)

Here's a snapshot from slightly over 24h of poudriere:

vm.stats.page.queue_nops: 782408604
vm.stats.page.queue_ops: 30467297356
vm.stats.page.pqstate_commit_aborts: 75944

I'll try to get one for buildworld as well.

Most nops are a result of per-CPU caching. vm_page_free_prep() requests a deferred dequeue. If the page goes through vm_phys it is likely that the dequeue will complete before the page is allocated again. However, if the page goes through the LIFO per-CPU caches, it is likely that it will be dequeued by vm_page_alloc() instead before the batch is processed.

3396 ↗

(On Diff #65520)

Yes.

[markj]

sys/vm/vm_page.h

923 ↗

(On Diff #65520)

Previously it synchronized with vm_page_dequeue_complete() (also removed in this patch), which is called without the page lock held. vm_page_dequeue_complete() sets m->queue = PQ_NONE and clears PGA_DEQUEUE as separate operations. The fence is needed to ensure that reordering of those operations does not allow a vm_page_queue() caller to observe m->queue != PQ_NONE and (m->aflags & PGA_DEQUEUE) == 0 and incorrectly deduce that the page belongs to a queue.

The fence is no longer needed because now the queue flags and queue index are always updated in a single atomic operation. Note though that with the removal of the page lock the return value of vm_page_queue() is always stale.

[markj]

Address feedback and reorganize a bit.

[bdrewery]

head/sys/vm/vm_page.c

3964

Oops vm_page_release always gets noreuse == false now; noreuse is unused in this function.

[markj]

head/sys/vm/vm_page.c

3964

Thanks. I think this has a fairly small effect: it means that if vm_page_release(VPR_TRYFREE) fails to free the page (e.g., because it is mapped), we will put it at the end of the inactive queue rather than near the head. How did you notice?

[bdrewery]

head/sys/vm/vm_page.c

3964

Added a failpoint with temp noreuse and hit compile error. We're hunting for corruption of page object listq. Max suggested this FP, I am guessing to speedup reclamation.