Fix atime handling and relatime #4482

tuxoko · 2016-04-01T21:34:47Z

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. 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().

rlaager · 2016-04-01T21:41:33Z

What's the status on merging inodes and znodes, or at least eliminating the duplicated fields?

tuxoko · 2016-04-01T21:43:08Z

@rlaager
I don't think I've seen anyone working on it yet.

tuxoko · 2016-04-02T00:31:23Z

Strange, atime_002_neg keeps failing. But I haven't failed it on my box.

spacelama · 2016-04-02T03:35:44Z

On Fri, 1 Apr 2016, tuxoko wrote:

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().
You can view, comment on, or merge this pull request online at:

Thankyou! I had been thinking atimes were broken for a while despite my
earlier bug about atime/relatime handling. Never had time to look into
(still don't have time to test :( )

Tim Connors

behlendorf · 2016-04-05T16:17:10Z

module/zfs/zfs_sa.c

@@ -309,6 +309,7 @@ zfs_sa_upgrade(sa_handle_t *hdl, dmu_tx_t *tx)

 	/* First do a bulk query of the attributes that aren't cached */
 	bulk = kmem_alloc(sizeof (sa_bulk_attr_t) * 20, KM_SLEEP);


Not a defect in this patch (or necessarily in the existing code), but it's interesting that bulk is already oversized here.

behlendorf · 2016-04-05T17:25:43Z

@rlaager I'm not aware of anyone working on it right now. But it definitely needs to happen.

@tuxoko the approach taken in the patch looks good to me, I made a few fairly minor comment inline. In addition, I strongly suggest adding a few new test cases to tests/zfs-tests/tests/functional/atime/ which verify that the Linux atime mount options are set properly and behave as expected.

I'm glad to see this long standing bit of terribleness finally get addressed!

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 <david.chen@osnexus.com>

Linux 4.0 introduces lazytime. The idea is that when we update the atime, we delay writing it to disk for as long as it is reasonably possible. When lazytime is enabled, dirty_inode will be called with only I_DIRTY_TIME flag whenever i_atime is updated. So under such condition, we will set z_atime_dirty. We will only write it to disk if file is closed, inode is evicted or setattr is called. Ideally, we should also write it whenever SA is going to be updated, but it is left for future improvement. There's one thing that we should take care of now that we allow i_atime to be dirty. In original implementation, whenever SA is modified, zfs_inode_update will be called to overwrite every thing in inode. This will cause dirty i_atime to be discarded. We fix this by don't overwrite i_atime in zfs_inode_update. We only overwrite i_atime when allocating new inode or doing zfs_rezget with zfs_inode_update_new. Signed-off-by: Chunwei Chen <david.chen@osnexus.com>

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com>

tuxoko · 2016-04-05T19:19:30Z

Update:

Fix thing according to review.
Remove ZFS_ACCESSTIME_STAMP in zfs_getpage
Fix MNTOPT_LAZYTIME comment.

behlendorf · 2016-04-05T23:45:12Z

Great, it looks like the only thing remaining is to add a test case. It looks like it would be trivial to add a atime_003_pos.ksh test case which verifies the relatime property using atime_001_pos.ksh as a base. The helper functions already exist to easy check the atime and verify it.

Add atime_003_pos to test relatime=on, we do check_atime_updated twice, the first time should success and the second time should fail. We also modify atime_001_pos to do check_atime_updated twice and both times should succeed. Signed-off-by: Chunwei Chen <david.chen@osnexus.com>

tuxoko · 2016-04-06T00:56:31Z

Update: add atime_003_pos to test relatime.

behlendorf · 2016-04-06T16:47:14Z

This LGTM, thanks for making those fixes and adding the test case. I see you also were able to drop ZFS_ACCESSTIME_STAMP entirely which is great. @tuxoko if your happy with this as a final version let me know and I'll merge it.

tuxoko · 2016-04-06T17:23:46Z

Yeah, I'm good as it is now.

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 <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

Linux 4.0 introduces lazytime. The idea is that when we update the atime, we delay writing it to disk for as long as it is reasonably possible. When lazytime is enabled, dirty_inode will be called with only I_DIRTY_TIME flag whenever i_atime is updated. So under such condition, we will set z_atime_dirty. We will only write it to disk if file is closed, inode is evicted or setattr is called. Ideally, we should also write it whenever SA is going to be updated, but it is left for future improvement. There's one thing that we should take care of now that we allow i_atime to be dirty. In original implementation, whenever SA is modified, zfs_inode_update will be called to overwrite every thing in inode. This will cause dirty i_atime to be discarded. We fix this by don't overwrite i_atime in zfs_inode_update. We only overwrite i_atime when allocating new inode or doing zfs_rezget with zfs_inode_update_new. Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

behlendorf · 2016-04-06T17:47:06Z

Merged as:

2b54cb1 Add zfs-tests for relatime
6760077 Make zfs mount according to relatime config in dataset
704cd07 Enable lazytime semantic for atime
0df9673 Fix atime handling and relatime

ThomasWaldmann · 2016-06-15T22:48:02Z

So, does this PR fix this?: borgbackup/borg#1172

Will this fix somehow get into what ubuntu has in 16.04LTS?

tuxoko · 2016-06-15T22:58:00Z

@ThomasWaldmann
Yes, it should work with O_NOATIME. Unfortunately, this patch set is not included in 0.6.5 stable branch.

ThomasWaldmann · 2016-06-15T23:00:12Z

OK, at least it is fixed. Somewhere.

behlendorf · 2016-06-15T23:49:09Z

This is something we could consider back porting to 0.6.5.

Add atime_003_pos to test relatime=on, we do check_atime_updated twice, the first time should success and the second time should fail. We also modify atime_001_pos to do check_atime_updated twice and both times should succeed. Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Closes openzfs#4482

tuxoko · 2016-11-08T19:44:47Z

@behlendorf
Can you queue this in the 0.6.5.9 project, thanks.

behlendorf · 2016-11-08T20:53:14Z

I've added it to the 0.6.5.9 project for @nedbass's consideration.

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 <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

Linux 4.0 introduces lazytime. The idea is that when we update the atime, we delay writing it to disk for as long as it is reasonably possible. When lazytime is enabled, dirty_inode will be called with only I_DIRTY_TIME flag whenever i_atime is updated. So under such condition, we will set z_atime_dirty. We will only write it to disk if file is closed, inode is evicted or setattr is called. Ideally, we should also write it whenever SA is going to be updated, but it is left for future improvement. There's one thing that we should take care of now that we allow i_atime to be dirty. In original implementation, whenever SA is modified, zfs_inode_update will be called to overwrite every thing in inode. This will cause dirty i_atime to be discarded. We fix this by don't overwrite i_atime in zfs_inode_update. We only overwrite i_atime when allocating new inode or doing zfs_rezget with zfs_inode_update_new. Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue openzfs#4482

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 <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #4482

Linux 4.0 introduces lazytime. The idea is that when we update the atime, we delay writing it to disk for as long as it is reasonably possible. When lazytime is enabled, dirty_inode will be called with only I_DIRTY_TIME flag whenever i_atime is updated. So under such condition, we will set z_atime_dirty. We will only write it to disk if file is closed, inode is evicted or setattr is called. Ideally, we should also write it whenever SA is going to be updated, but it is left for future improvement. There's one thing that we should take care of now that we allow i_atime to be dirty. In original implementation, whenever SA is modified, zfs_inode_update will be called to overwrite every thing in inode. This will cause dirty i_atime to be discarded. We fix this by don't overwrite i_atime in zfs_inode_update. We only overwrite i_atime when allocating new inode or doing zfs_rezget with zfs_inode_update_new. Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #4482

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #4482

tuxoko force-pushed the atime branch from ed1e91b to 338564a Compare April 1, 2016 22:11

tuxoko force-pushed the atime branch from 338564a to 0b49e62 Compare April 2, 2016 00:56

behlendorf reviewed Apr 5, 2016
View reviewed changes

Chunwei Chen added 3 commits April 5, 2016 12:03

Make zfs mount according to relatime config in dataset

69b7af3

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com>

tuxoko force-pushed the atime branch from 0b49e62 to 69b7af3 Compare April 5, 2016 19:16

behlendorf closed this in 2b54cb1 Apr 6, 2016

jumbi77 mentioned this pull request Feb 3, 2017

Proposed 0.6.5.9 tag [WIP] #5729

Closed

2 tasks

behlendorf pushed a commit that referenced this pull request Feb 3, 2017

Make zfs mount according to relatime config in dataset

10fbf7c

Also enable lazytime in mount.zfs Signed-off-by: Chunwei Chen <david.chen@osnexus.com> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov> Issue #4482

fcicq mentioned this pull request Mar 24, 2018

feature: LAZYTIME mount option is not handled in #4482 #7340

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix atime handling and relatime #4482

Fix atime handling and relatime #4482

tuxoko commented Apr 1, 2016

rlaager commented Apr 1, 2016

tuxoko commented Apr 1, 2016

tuxoko commented Apr 2, 2016

spacelama commented Apr 2, 2016

behlendorf Apr 5, 2016

behlendorf commented Apr 5, 2016

tuxoko commented Apr 5, 2016

behlendorf commented Apr 5, 2016

tuxoko commented Apr 6, 2016

behlendorf commented Apr 6, 2016

tuxoko commented Apr 6, 2016

behlendorf commented Apr 6, 2016

ThomasWaldmann commented Jun 15, 2016

tuxoko commented Jun 15, 2016

ThomasWaldmann commented Jun 15, 2016

behlendorf commented Jun 15, 2016

tuxoko commented Nov 8, 2016

behlendorf commented Nov 8, 2016

		@@ -309,6 +309,7 @@ zfs_sa_upgrade(sa_handle_t hdl, dmu_tx_t tx)

		/* First do a bulk query of the attributes that aren't cached */
		bulk = kmem_alloc(sizeof (sa_bulk_attr_t) * 20, KM_SLEEP);

Fix atime handling and relatime #4482

Fix atime handling and relatime #4482

Conversation

tuxoko commented Apr 1, 2016

rlaager commented Apr 1, 2016

tuxoko commented Apr 1, 2016

tuxoko commented Apr 2, 2016

spacelama commented Apr 2, 2016

behlendorf Apr 5, 2016

Choose a reason for hiding this comment

behlendorf commented Apr 5, 2016

tuxoko commented Apr 5, 2016

behlendorf commented Apr 5, 2016

tuxoko commented Apr 6, 2016

behlendorf commented Apr 6, 2016

tuxoko commented Apr 6, 2016

behlendorf commented Apr 6, 2016

ThomasWaldmann commented Jun 15, 2016

tuxoko commented Jun 15, 2016

ThomasWaldmann commented Jun 15, 2016

behlendorf commented Jun 15, 2016

tuxoko commented Nov 8, 2016

behlendorf commented Nov 8, 2016