vm_object: drop memq field

[dougm]

Summary

This is a first draft, a work in progress. All the reviewers automatically added don't need to rush to review it.

The memq field in vm_object is used to maintain a list of mapped pages, sorted by pindex. Remove that field, and stop maintaining the list. Change all iterations that use the list to use pctrie iterators instead. Change the tailq field in vm_page so that it is part of a union that overlaps the {pindex, object) pair, and remains in use only for the buddy allocator in vm_phys.c and the maintenance of temporary lists in uma_core.c. Where a predecessor page is passed to a function solely to help with insertion into the memq list, drop that parameter.

[dougm]

A code reformat to (slightly) reduce the diff size.

[dougm]

Define and use VM_RADIX_FOREACH macros. Initialize a couple of iterators before acquiring read locks.

[markj]

sys/amd64/sgx/sgx.c

396

Rather than lifting out the vm_page_remove() calls, perhaps pass the iterator to sgx_page_remove() as well, and have it decide whether to call vm_page_remove() or vm_page_iter_remove() based on whether the iterator pointer is NULL.

sys/vm/vm_glue.c

625

Do you have some other uses for NOCREAT_LT planned? This function is called rarely (kernel stacks are cached by UMA and reused many times without going through this function), so the extra complexity feels a bit unnecessary.

sys/vm/vm_page.h

224

Not quite related to this change, but once it lands, I'd like to add one 8-byte field to plinks. I believe that'd be enough to let us start embedding UMA slab headers (i.e. uma_slab_t) within the vm_page structure. This is an alternative approach to D22768.

sys/vm/vm_radix.h

272

Bikeshedding, but VM_RADIX_FOREACH_FROM reads a bit more naturally to me, and we already have TAILQ_FOREACH_FROM etc..

sys/vm/vm_reserv.c

528

This is more expensive, but at some point I presume it should be possible to pass an interator down from the page allocator and use that instead of doing a full lookup here.

[dougm]

sys/vm/vm_glue.c

625

I have no other uses planned. I was just looking for a way to avoid a redundant lookup.

sys/vm/vm_radix.h

272

I don't object to using FROM, but the reason I didn't is that FOREACH_FROM takes two arguments, and this macro takes three. FOREACH_FROM assumes that the lookup has already happened and FOREACH_START has to do the lookup. Those differences are the reason that I chose to use a distinct name. But it doesn't matter much to me.

sys/vm/vm_reserv.c

528

I agree that it would be possible.

[markj]

Doug, how would you like to proceed with the patch? Since quite a few pieces of it are independent, I imagine they can be reviewed and peeled off one by one, especially in areas that aren't performance-critical.

Are there still some concerns about the overhead of switching to iterators in some of the already-committed changes? I've lost track a bit, I'm sorry.

sys/kern/kern_kcov.c

411

Can't we use vm_radix_iter_next() here?

sys/kern/subr_pctrie.c

550

This can presumably be committed on its own. It looks reasonable to me.

sys/x86/iommu/amd_idpgtbl.c

123

The assertion can come first, as in dmar_domain_free_pgtbl().

sys/x86/iommu/intel_idpgtbl.c

266–268

Elsewhere you are careful to initialize the iterator outside the scope of the object lock, but not here.

268

Does it make sense to have a macro for this usage too? Perhaps we could use VM_RADIX_FOREACH and assert that pindices are consecutive?

[dougm]

In D48007#1102734, @markj wrote:

Doug, how would you like to proceed with the patch? Since quite a few pieces of it are independent, I imagine they can be reviewed and peeled off one by one, especially in areas that aren't performance-critical.

Some of them have been reviewed separately, and approved by @kib, but blocked by comments from @alc. His comment on D47718 is why I stopped creeping along a bit at a time and threw it all out for consideration at once. I expect that his approval depends on some kind of performance testing. He's been drawn away from BSD considerations by other life events lately.

Are there still some concerns about the overhead of switching to iterators in some of the already-committed changes? I've lost track a bit, I'm sorry.

@alc has concerns. He's reported performance worsened a bit by some of the page management changes already made, and worsened a bit by some other changes that have been under review for a while.

sys/x86/iommu/intel_idpgtbl.c

268

I don't know. I'll add a VM_RADIX_FORALL macro for this purpose, and you can decide how you feel about it.

[dougm]

Use VM_RADIX_FORALL_FROM one more place.