{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/4482","repository_url":"https://api.github.com/repos/zfsonlinux/zfs","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/4482/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/4482/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/4482/events","html_url":"https://github.com/zfsonlinux/zfs/pull/4482","id":145289751,"number":4482,"title":"Fix atime handling and relatime","user":{"login":"tuxoko","id":119672,"avatar_url":"https://avatars.githubusercontent.com/u/119672?v=3","gravatar_id":"","url":"https://api.github.com/users/tuxoko","html_url":"https://github.com/tuxoko","followers_url":"https://api.github.com/users/tuxoko/followers","following_url":"https://api.github.com/users/tuxoko/following{/other_user}","gists_url":"https://api.github.com/users/tuxoko/gists{/gist_id}","starred_url":"https://api.github.com/users/tuxoko/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tuxoko/subscriptions","organizations_url":"https://api.github.com/users/tuxoko/orgs","repos_url":"https://api.github.com/users/tuxoko/repos","events_url":"https://api.github.com/users/tuxoko/events{/privacy}","received_events_url":"https://api.github.com/users/tuxoko/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-04-01T21:34:47Z","updated_at":"2016-04-01T21:43:09Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/zfsonlinux/zfs/pulls/4482","html_url":"https://github.com/zfsonlinux/zfs/pull/4482","diff_url":"https://github.com/zfsonlinux/zfs/pull/4482.diff","patch_url":"https://github.com/zfsonlinux/zfs/pull/4482.patch"},"body":"The problem for atime:\r\n\r\nWe have 3 places for atime: inode->i_atime, znode->z_atime and SA. And its\r\nhandling is a mess. A huge part of mess regarding atime comes from\r\nzfs_tstamp_update_setup, zfs_inode_update, and zfs_getattr, which behave\r\ninconsistently with those three values.\r\n\r\nzfs_tstamp_update_setup clears z_atime_dirty unconditionally as long as you\r\ndon't pass ATTR_ATIME. Which means every write(2) operation which only updates\r\nctime and mtime will cause atime changes to not be written to disk.\r\n\r\nAlso zfs_inode_update from write(2) will replace inode->i_atime with what's\r\ninside SA(stale). But doesn't touch z_atime. So after read(2) and write(2).\r\nYou'll have i_atime(stale), z_atime(new), SA(stale) and z_atime_dirty=0.\r\n\r\nNow, if you do stat(2), zfs_getattr will actually replace i_atime with what's\r\ninside, z_atime. So you will have now you'll have i_atime(new), z_atime(new),\r\nSA(stale) and z_atime_dirty=0. These will all gone after umount. And you'll\r\nleave with a stale atime.\r\n\r\nThe problem for relatime:\r\n\r\nWe do have a relatime config inside ZFS dataset, but how it should interact\r\nwith the mount flag MS_RELATIME is not well defined. It seems it wanted\r\nrelatime mount option to override the dataset config by showing it as\r\ntemporary in `zfs get`. But at the same time, `zfs set relatime=on|off` would\r\nalso seems to want to override the mount option. Not to mention that\r\nMS_RELATIME flag is actually never passed into ZFS, so it never really worked.\r\n\r\nHow Linux handles atime:\r\n\r\nThe Linux kernel actually handles atime completely in VFS, except for writing\r\nit to disk. So if we remove the atime handling in ZFS, things would just work,\r\nno matter it's strictatime, relatime, noatime. And whenever VFS updates the\r\ni_atime, it will notify the underlying filesystem via sb->dirty_inode().\r\n\r\nAnd also there's one thing to note about atime flags like MS_RELATIME and\r\nother flags like MS_NODEV, etc. They are mount point flags rather than\r\nfilesystem(sb) flags. Since native linux filesystem can be mounted at multiple\r\nplaces at the same time, they can all have different atime settings. So these\r\nflags are never passed down to filesystem drivers.\r\n\r\nWhat this patch tries to do:\r\n\r\nWe remove znode->z_atime, since we won't gain anything from it. We remove most\r\nof the atime handling and leave it to VFS. The only thing we do with atime is\r\nto write it when dirty_inode() or setattr() is called. We also add\r\nfile_accessed() in zpl_read() since it's not provided in vfs_read()."},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/204575937","html_url":"https://github.com/zfsonlinux/zfs/pull/4482#issuecomment-204575937","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/4482","id":204575937,"user":{"login":"tuxoko","id":119672,"avatar_url":"https://avatars.githubusercontent.com/u/119672?v=3","gravatar_id":"","url":"https://api.github.com/users/tuxoko","html_url":"https://github.com/tuxoko","followers_url":"https://api.github.com/users/tuxoko/followers","following_url":"https://api.github.com/users/tuxoko/following{/other_user}","gists_url":"https://api.github.com/users/tuxoko/gists{/gist_id}","starred_url":"https://api.github.com/users/tuxoko/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tuxoko/subscriptions","organizations_url":"https://api.github.com/users/tuxoko/orgs","repos_url":"https://api.github.com/users/tuxoko/repos","events_url":"https://api.github.com/users/tuxoko/events{/privacy}","received_events_url":"https://api.github.com/users/tuxoko/received_events","type":"User","site_admin":false},"created_at":"2016-04-01T21:43:08Z","updated_at":"2016-04-01T21:43:08Z","body":"@rlaager \r\nI don't think I've seen anyone working on it yet."}}