I think that there is another edge case that isn't being handled. Similar to my comment about the vm_page_dequeue after vm_reserv_alloc_page, suppose that a page is allocated from a reservation via vm_page_alloc(), validated, mapped, added to a paging queue, and then later freed back to the reservation. Now, suppose that the reservation is broken and the recently-freed page is passed to the buddy allocator. The lazy dequeue may not have completed yet. I think that reservation breaking will need to perform the vm_page_dequeue on each of the pages being passed to the buddy allocator.

The key here is that all page allocation functions call vm_page_dequeue(), completing any lingering page queue operations that involve the plinks.q field.

Are you going to update the patch to include vm_page_grab_pages()?

Aren't all of the changes to vm_page.h valid? Can we have a standalone patch with just those changes?

In D49957#1140976, @alc wrote:

Now that iterators are much smaller, consisting of only four 64-bit fields, I think we should seriously consider embedding an iterator in the vm_object for use by operations that require a write lock on the object. If that iterator is used by the functions that insert and remove pages from the trie, then it will always be consistent, and most of the pctrie_iter_reset()s can be eliminated. With the removal of the memq from the object, this would only grow the size of the vm_object by 2 64-bit fields. (If there was some way to avoid having both the root of the pctrie and the iterator's pointer to that root within the vm_object, then the size of the vm_object would only grow by 1 64-bit field.)

The operations, like vm_map_pmap_enter(), that utilize a read lock will still need to define and use their own private iterator.

That all said, I think that we should complete the removal of the memq, and then consider embedding an iterator in the object.

Now that iterators are much smaller, consisting of only four 64-bit fields, I think we should seriously consider embedding an iterator in the vm_object for use by operations that require a write lock on the object. If that iterator is used by the functions that insert and remove pages from the trie, then it will always be consistent, and most of the pctrie_iter_reset()s can be eliminated. With the removal of the memq from the object, this would only grow the size of the vm_object by 2 64-bit fields. (If there was some way to avoid having both the root of the pctrie and the iterator's pointer to that root within the vm_object, then the size of the vm_object would only grow by 1 64-bit field.)

The performance results in the TEST PLAN are much improved over what I was seeing last fall for a similar set of changes, in particular, the results for vm_page_alloc(). Clearly, the improvements to the implementation of iterators in the last few months, particularly I suspect the elimination of the rather large array on the stack, have made a difference. Modulo any low-level changes/fixes to this patch that are needed, I would be very happy to see it committed. This is really the most critical step in my mind to the elimination of the two linked list pointers from struct vm_page, which I hope will yield a small but measurable performance improvement, particularly for pages allocated from superpage reservations.

In D49391#1126158, @alc wrote:

The vm_page.h comment incorrectly says vm_page_alloc accepts VM_ALLOC_WAITOK.

The vm_page.h comment incorrectly says vm_page_alloc accepts VM_ALLOC_WAITOK.

This change increased the cycles spent in both vm_object_split() and vm_object_collapse_scan() during a buildworld. It appears that we're not moving enough pages to overcome the startup costs of the iterator.

Shouldn't this be abandoned?

Update in response to reviewer comments.

In D47829#1090358, @dougm wrote:

Comments in vm_reserv.c need to be updated to reflect this change.

If you have measured the improvement, how big is it?

Update in response to reviewer comments.

Let's limit this change to vm_page_iter_free. It reduces the average cycles in _kmem_unback and vm_object_page_remove by 7% and 5.3%, respectively.

By itself, this is going to be slower. This should be a part of a larger patch that removes the memq, so that we can evaluate whether the elimination of the memq makes up for the higher cost of the iterators.