tmpfs: restore atime updates for reads from page cache.

[kib]

Summary

Split TMPFS_NODE_ACCCESSED bit into dedicated byte that can be updated atomically without locks or (locked) atomics.

tn_update_getattr() change also contains unrelated bug fix.

Reported by: lwhsu
PR: 249362

(Not tested).

[mjg]

The tmpfs_set_accessed call in pgread has to move up prior to vfs_smr_exit. I can't make an inline comment for some reason.

On a more general note perhaps this can finally move to just getnanotime() on access and updating the stored value only if it goes forward. While this would incur some loss of concurrency it may be small enough to be more than tolerable, all while finally providing reasonable timestamp accuracy. Note as it is atime is updated who-knows-when. Same goes for other counters.

[kib]

In D26451#588463, @mjg wrote:

The tmpfs_set_accessed call in pgread has to move up prior to vfs_smr_exit. I can't make an inline comment for some reason.

Done.

On a more general note perhaps this can finally move to just getnanotime() on access and updating the stored value only if it goes forward. While this would incur some loss of concurrency it may be small enough to be more than tolerable, all while finally providing reasonable timestamp accuracy. Note as it is atime is updated who-knows-when. Same goes for other counters.

At least we do not write torn values to Xtime fields. I do not want to take node lock in tmpfs_pgcache_read(), after all efforts I put to remove object and node referencing.

[kib]

Move write to tn_accesses before vfs_smr_exit().

[markj]

sys/fs/tmpfs/tmpfs.h

175 ↗

(On Diff #77098)

I would add a sentence explaining why the field is split.

sys/fs/tmpfs/tmpfs_subr.c

1866 ↗

(On Diff #77098)

atomic_store_8()?

[mjg]

In D26451#588469, @kib wrote:

In D26451#588463, @mjg wrote:

On a more general note perhaps this can finally move to just getnanotime() on access and updating the stored value only if it goes forward. While this would incur some loss of concurrency it may be small enough to be more than tolerable, all while finally providing reasonable timestamp accuracy. Note as it is atime is updated who-knows-when. Same goes for other counters.

At least we do not write torn values to Xtime fields. I do not want to take node lock in tmpfs_pgcache_read(), after all efforts I put to remove object and node referencing.

But we don't have to have torn writes or any locking. The most aggressive take on it would just store the 4 byte ticks and make sure something eventually takes care of it before a potential wrap around. The general problem is that atime updates from reads on all filesystems is drastically inaccurate - timestamp is updated many seconds later. That said, fixing even if only for tmpfs may be beyond the scope of this patch.

[mjg]

sys/fs/tmpfs/tmpfs_subr.c

1864 ↗

(On Diff #77098)

Can we skip tm_ronly checks? Instead it can be asserted that tn_status never gets TMPFS_NODE_ACCESSED nor TMPFS_NODE_MODIFIED bits set and the tn_accessed field can be ignored in tmpfs_itimes.

[kib]

In D26451#588524, @mjg wrote:

In D26451#588469, @kib wrote:

In D26451#588463, @mjg wrote:

On a more general note perhaps this can finally move to just getnanotime() on access and updating the stored value only if it goes forward. While this would incur some loss of concurrency it may be small enough to be more than tolerable, all while finally providing reasonable timestamp accuracy. Note as it is atime is updated who-knows-when. Same goes for other counters.

At least we do not write torn values to Xtime fields. I do not want to take node lock in tmpfs_pgcache_read(), after all efforts I put to remove object and node referencing.

But we don't have to have torn writes or any locking. The most aggressive take on it would just store the 4 byte ticks and make sure something eventually takes care of it before a potential wrap around. The general problem is that atime updates from reads on all filesystems is drastically inaccurate - timestamp is updated many seconds later. That said, fixing even if only for tmpfs may be beyond the scope of this patch.

Storing timecounter raw value means that we need to do active scan each second.

sys/fs/tmpfs/tmpfs_subr.c

1864 ↗

(On Diff #77098)

It is not that simple, right now code is organized so that we must process all times even after tm_ronly and MNT_RDONLY are set.

I might look at restructuring rw->ro remount, then it could be possible with some adjustments.

[kib]

Add comment about tn_accessed.
Use atomic_store_8.
Unrelated style changes will be committed separately.