vm: streamline uses of OBJT_SWAP and tmpfs swap objects

[kib]

Summary

This patch extends vm_pager' table of virtual functions and adds new pager type OBJT_SWAP_TMPFS. In total it eliminates OBJ_TMPFS_NODE flag, replacing conditionals with the pager' method calls. It also eliminate several other checks for OBJT_SWAP and direct calls into swap_* functions.

I still needed some common attribute for OBJT_SWAP and OBJT_SWAP_TMPFS objects, so OBJ_TMPFS_NODE flag is repurposed as OBJ_SWAPPING, indicating any type of the swap object.

I booted the patched kernel multiuser with /tmp on tmpfs, but no other testing was done.

I did not looked at specific tests for other object types, there are definitely more opportunities to simplify the code with another set of vm_pager methods, but only OBJT_SWAP for now.
Interesting next step would be to allow dynamically loadable pagers, so OBJT_SWAP_TMPFS only appers after kldload tmpfs.ko.

[markj]

sys/vm/vm_object.c

243 ↗

(On Diff #88473)

(flags & OBJ_SWAPPING) != 0?

sys/vm/vm_object.h

204 ↗

(On Diff #88473)

"swapping" sounds transient, like (flags & OBJ_SWAPPING) != 0 implies paging_in_progress > 0. I think OBJ_SWAP would be better.

sys/vm/vm_pager.h

240 ↗

(On Diff #88473)

If INVARIANTS is off, this function will silently do nothing. I think callers handle it more or less sanely, but I wonder why we don't just assume method != NULL and call method unconditionally.

Ah I see it was written this way before too, ok.

[kib]

sys/vm/vm_object.c

243 ↗

(On Diff #88473)

It is too early to check the flag, with the current code structure. I set OBJ_SWAPPING in swap_pager_alloc_init() right after vm_object_allocate() call.

I can move setting of the flag into vm_object_allocate().

[kib]

OBJ_SWAPPING->OBJ_SWAP
Set OBJ_SWAP in vm_object_allocate(), and test it in _vm_object_allocate() instead of checking types.

[markj]

sys/vm/swap_pager.c

767

Should we clear OBJ_SWAP as well? Otherwise the checks of object->type that were converted to checks of object->flags need to be audited to make sure they handle dead objects correctly.

[kib]

sys/vm/swap_pager.c

767

Do you think that such places could exist?

swap_pager_dealloc() is called when object refcount == 0, which for non-vnode objects must imply that no one else except our caller has a pointer to the object.

Hmm, I think we can only reach destroyed swap object through the vm_object_list iteration, and then the only such function would be swap_pager_swapoff(). It rechecks OBJ_DEAD after obtaining object lock.

Ok, I will add the cleaning of the flag.

sys/vm/vm_pager.h

240 ↗

(On Diff #88473)

In fact it triggers from e.g. vm_object_get_vnode(). I removed the assert.

[kib]

Clear OBJ_SWAP on dealloc
Tolerate NULL pgo_getvp()

[markj]

sys/vm/swap_pager.c

437

I guess pagerops should be annotated __read_mostly.

767

It is not obvious to me that the object checks in vm_page_reclaim_run() are sufficient, for example. vm_object_terminate() does not necessarily remove all pages (see the OBJ_PG_DTOR flag). This is a contrived example though, OBJ_PG_DTOR has only a single user today. Really my point is that some state is now spread around multiple fields, so we should try to minimize the set of valid combinations.

To be honest, I somewhat dislike OBJ_SWAP. Instead of having a new object type, did you consider using a tuple <obj->type, obj->flags> to select pager methods? Or, is it really painful to check obj->type == OBJT_SWAP || obj->type == OBJT_SWAP_TMPFS inline?

[kib]

sys/vm/swap_pager.c

437

I think they should be marked const and go into .rodata instead.

767

Plan is to (try to) make OBJT_SWAP_TMPFS pager kldloadable from tmpfs.ko. Hardcoding the type in sys/vm would be undesirable.

[kib]

Constify vm_pager ops, and ops for phys and managed cdevs.
Mark pageropstab[] array as read_mostly.

[kib]

Bug fixes:

• typo in testing OBJ_SWAPPING
• locking for vm_pager_getvp::swap_tmpfs_pager_getvp

Passed Peter' testing.