Fix atime handling and relatime

The problem for atime:

We have 3 places for atime: inode->i_atime, znode->z_atime and SA. And its
handling is a mess. A huge part of mess regarding atime comes from
zfs_tstamp_update_setup, zfs_inode_update, and zfs_getattr, which behave
inconsistently with those three values.

zfs_tstamp_update_setup clears z_atime_dirty unconditionally as long as you
don't pass ATTR_ATIME. Which means every write(2) operation which only updates
ctime and mtime will cause atime changes to not be written to disk.

Also zfs_inode_update from write(2) will replace inode->i_atime with what's
inside SA(stale). But doesn't touch z_atime. So after read(2) and write(2).
You'll have i_atime(stale), z_atime(new), SA(stale) and z_atime_dirty=0.

Now, if you do stat(2), zfs_getattr will actually replace i_atime with what's
inside, z_atime. So you will have now you'll have i_atime(new), z_atime(new),
SA(stale) and z_atime_dirty=0. These will all gone after umount. And you'll
leave with a stale atime.

The problem for relatime:

We do have a relatime config inside ZFS dataset, but how it should interact
with the mount flag MS_RELATIME is not well defined. It seems it wanted
relatime mount option to override the dataset config by showing it as
temporary in `zfs get`. But at the same time, `zfs set relatime=on|off` would
also seems to want to override the mount option. Not to mention that
MS_RELATIME flag is actually never passed into ZFS, so it never really worked.

How Linux handles atime:

The Linux kernel actually handles atime completely in VFS, except for writing
it to disk. So if we remove the atime handling in ZFS, things would just work,
no matter it's strictatime, relatime, noatime, or even O_NOATIME. And whenever
VFS updates the i_atime, it will notify the underlying filesystem via
sb->dirty_inode().

And also there's one thing to note about atime flags like MS_RELATIME and
other flags like MS_NODEV, etc. They are mount point flags rather than
filesystem(sb) flags. Since native linux filesystem can be mounted at multiple
places at the same time, they can all have different atime settings. So these
flags are never passed down to filesystem drivers.

What this patch tries to do:

We remove znode->z_atime, since we won't gain anything from it. We remove most
of the atime handling and leave it to VFS. The only thing we do with atime is
to write it when dirty_inode() or setattr() is called. We also add
file_accessed() in zpl_read() since it's not provided in vfs_read().

After this patch, only the MS_RELATIME flag will have effect. The setting in
dataset won't do anything. We will make zfstuil to mount ZFS with MS_RELATIME
set according to the setting in dataset in future patch.

Signed-off-by: Chunwei Chen <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Issue #4482

2 files changed, 3 insertions, 11 deletions

diff --git a/include/sys/trace_acl.h b/include/sys/trace_acl.h
index b94cb79c6..87ffecb4e 100644
--- a/include/sys/trace_acl.h
+++ b/include/sys/trace_acl.h
@@ -56,7 +56,6 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
 	    __field(uint64_t,		z_mapcnt)
 	    __field(uint64_t,		z_gen)
 	    __field(uint64_t,		z_size)
-	    __array(uint64_t,		z_atime, 2)
 	    __field(uint64_t,		z_links)
 	    __field(uint64_t,		z_pflags)
 	    __field(uint64_t,		z_uid)
@@ -95,8 +94,6 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
 	    __entry->z_mapcnt		= zn->z_mapcnt;
 	    __entry->z_gen		= zn->z_gen;
 	    __entry->z_size		= zn->z_size;
-	    __entry->z_atime[0]		= zn->z_atime[0];
-	    __entry->z_atime[1]		= zn->z_atime[1];
 	    __entry->z_links		= zn->z_links;
 	    __entry->z_pflags		= zn->z_pflags;
 	    __entry->z_uid		= zn->z_uid;
@@ -126,7 +123,7 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
 	),
 	TP_printk("zn { id %llu unlinked %u atime_dirty %u "
 	    "zn_prefetch %u moved %u blksz %u seq %u "
-	    "mapcnt %llu gen %llu size %llu atime 0x%llx:0x%llx "
+	    "mapcnt %llu gen %llu size %llu "
 	    "links %llu pflags %llu uid %llu gid %llu "
 	    "sync_cnt %u mode 0x%x is_sa %d is_zvol %d "
 	    "is_mapped %d is_ctldir %d is_stale %d inode { "
@@ -136,7 +133,7 @@ DECLARE_EVENT_CLASS(zfs_ace_class,
 	    __entry->z_id, __entry->z_unlinked, __entry->z_atime_dirty,
 	    __entry->z_zn_prefetch, __entry->z_moved, __entry->z_blksz,
 	    __entry->z_seq, __entry->z_mapcnt, __entry->z_gen,
-	    __entry->z_size, __entry->z_atime[0], __entry->z_atime[1],
+	    __entry->z_size,
 	    __entry->z_links, __entry->z_pflags, __entry->z_uid,
 	    __entry->z_gid, __entry->z_sync_cnt, __entry->z_mode,
 	    __entry->z_is_sa, __entry->z_is_zvol, __entry->z_is_mapped,
diff --git a/include/sys/zfs_znode.h b/include/sys/zfs_znode.h
index c03bef5c7..65fa10399 100644
--- a/include/sys/zfs_znode.h
+++ b/include/sys/zfs_znode.h
@@ -198,7 +198,6 @@ typedef struct znode {
 	uint64_t	z_mapcnt;	/* number of pages mapped to file */
 	uint64_t	z_gen;		/* generation (cached) */
 	uint64_t	z_size;		/* file size (cached) */
-	uint64_t	z_atime[2];	/* atime (cached) */
 	uint64_t	z_links;	/* file links (cached) */
 	uint64_t	z_pflags;	/* pflags (cached) */
 	uint64_t	z_uid;		/* uid fuid (cached) */
@@ -306,16 +305,12 @@ extern unsigned int zfs_object_mutex_size;
 #define	STATE_CHANGED		(ATTR_CTIME)
 #define	CONTENT_MODIFIED	(ATTR_MTIME | ATTR_CTIME)
 
-#define	ZFS_ACCESSTIME_STAMP(zsb, zp) \
-	if ((zsb)->z_atime && !(zfs_is_readonly(zsb))) \
-		zfs_tstamp_update_setup(zp, ACCESSED, NULL, NULL, B_FALSE);
-
 extern int	zfs_init_fs(zfs_sb_t *, znode_t **);
 extern void	zfs_set_dataprop(objset_t *);
 extern void	zfs_create_fs(objset_t *os, cred_t *cr, nvlist_t *,
     dmu_tx_t *tx);
 extern void	zfs_tstamp_update_setup(znode_t *, uint_t, uint64_t [2],
-    uint64_t [2], boolean_t);
+    uint64_t [2]);
 extern void	zfs_grow_blocksize(znode_t *, uint64_t, dmu_tx_t *);
 extern int	zfs_freesp(znode_t *, uint64_t, uint64_t, int, boolean_t);
 extern void	zfs_znode_init(void);