Tmpfs nomtime mount flag.

[kib]

Summary

Extend vm_object_page_clean() to accept swap objects that backs tmpfs regular files. The result is just the removal of write permissions of the mappings. This should make mtime updates due to the later writes through the mmaped memory working.

Add nomtime mount flag to disable mtime updates due to writes from the mmaped regions. I think it does not make sense to imply the meaning of disabling mtime updates from normal writes.

[mjg]

Works with my test program.

I would argue the MNOSYNC flag should be left alone as it disables vfs_msync, which is not wanted here anyway.

Instead perhaps ->tm_nomtime or similar can be added (similar to how nonc option is handled).

[kib]

Stop messing with MNTK_NOMSYNC.

[jeff]

sys/vm/vm_object.c

1045 ↗

(On Diff #67533)

If we're not doing mtime we're still going to pay the price of unmapping the pages and faulting again. It might be nice to avoid that altogether.

Rather than a bunch of type checks, we could have a single VM_OBJECT_ flag that specifies whether the object is participating in this algorithm or not.

[kib]

sys/vm/vm_object.c

1045 ↗

(On Diff #67533)

It would only induce the cost at the place where the function is called, and right now the only place where it is called that way is vinactivef(). All other vm_object_page_clean() callers either only do it for OBJT_VNODE objects, or occur inside the check for MNTK_!NOMSYNC.

I do not like an object flag for several reasons:

1. we have only one free flag left. Can be 'fixed' by introducing flag2 or extending flag, but do we really need it ?
2. On tmpfs mount -u which changes nomtime, we would need to iterate over all nodes (not vnodes).

[markj]

sys/vm/vm_object.c

1125 ↗

(On Diff #67533)

Maybe also assert object->type == VNODE || (object->type == SWAP && (object->flags & OBJ_TMPFS_NODE)).

1160 ↗

(On Diff #67533)

Is eio left uninitialized?

[kib]

Handle Mark' notes.

[markj]

sys/fs/tmpfs/tmpfs_vfsops.c

106 ↗

(On Diff #67566)

This comment doesn't seem to be accurate anymore.

sys/vm/vm_object.c

1127 ↗

(On Diff #67566)

Add an explicit "!= 0"?

1163 ↗

(On Diff #67566)

Won't we attempt to unmap some pages multiple times? Suppose we unmap a range [a, b]. Then we advance the object iterator and unmap b+1. Since we cluster behind this page and the busy state is released immediately, we will attempt to busy and unmap the page at b again.

IMO it is a bit strange to perform clustering in the tmpfs case. It does not provide any benefit that I can see.

[kib]

sys/fs/tmpfs/tmpfs_vfsops.c

106 ↗

(On Diff #67566)

You mean that updates might be disabled ? I mentioned this. If you mean something else, please state.

[kib]

Stop trying to cluster for swap objects. Update comment.

[markj]

sys/fs/tmpfs/tmpfs_vfsops.c

106 ↗

(On Diff #67566)

Sorry, I misunderstood what the comment was saying.

sys/vm/vm_object.h

326 ↗

(On Diff #67578)

As a micro-optimization I think the test for type == SWAP could be omitted since we should not be setting OBJ_TMPFS_NODE on a non-swap object.

[kib]

Microoptimize vm_object_mightbedirty().

[kib]

Collect all the fixes so far.

Compilation errors due to KASSERT() requiring sys/systm.h.
Leak of the busy state.
"nomtime" tmpfs mount option parsing.

[markj]

sys/vm/vm_object.h

328 ↗

(On Diff #67674)

This is already in an #ifdef _KERNEL block.

[kib]

One more fix for parsing nomtime option on mount -u.
Update comment for tm_nomtime mentioning that only mmaped writes are ignored.
Do not update cleangeneration for tmpfs swap objects in vm_object_page_clean(), tmpfs_update_mtime() must see the advanced generation to work.
Remove excessive _KERNEL define check.