| Message ID | 20200617145310.GK3183@techsingularity.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs, pseudo: Do not update atime for pseudo inodes | expand |
On Wed, Jun 17, 2020 at 5:53 PM Mel Gorman <mgorman@techsingularity.net> wrote: > > The kernel uses internal mounts created by kern_mount() and populated > with files with no lookup path by alloc_file_pseudo() for a variety of > reasons. An relevant example is anonymous pipes because every vfs_write > also checks if atime needs to be updated even though it is unnecessary. > Most of the relevant users for alloc_file_pseudo() either have no statfs > helper or use simple_statfs which does not return st_atime. The closest st_atime is returned by simple_getattr() > proxy measure is the proc fd representations of such inodes which do not > appear to change once they are created. This patch sets the S_NOATIME > on inode->i_flags for inodes created by new_inode_pseudo() so that atime > will not be updated. > new_inode() calls new_inode_pseudo() ... You need to factor out a new helper. Either you can provide callers analysis of all new_inode_pseudo() users or use a new helper to set S_NOATIME and call it from the relevant users (pipe, socket). How about S_NOCMTIME while you are at it? Doesn't file_update_time() show in profiling? Is there a valid use case for updating c/mtime of anonymous socket/pipe? Thanks, Amir.
On Fri, Jun 19, 2020 at 04:45:09PM +0300, Amir Goldstein wrote: > > proxy measure is the proc fd representations of such inodes which do not > > appear to change once they are created. This patch sets the S_NOATIME > > on inode->i_flags for inodes created by new_inode_pseudo() so that atime > > will not be updated. > > > > new_inode() calls new_inode_pseudo() ... > You need to factor out a new helper. > You're right, it's broken as it stands. Even though I don't think direct users of alloc_file_pseudo use simple_getattr, it doesn't matter. > Either you can provide callers analysis of all new_inode_pseudo() users > or use a new helper to set S_NOATIME and call it from the relevant users > (pipe, socket). > Relevant users makes sense as it's less likely to cause surprises. > How about S_NOCMTIME while you are at it? > Doesn't file_update_time() show in profiling? I can look into it, I don't recall seeing file_update_time() in the profile but maybe it was just too small. > Is there a valid use case for updating c/mtime of anonymous socket/pipe? > Not that I can think of but that could be a failure of imagination.
diff --git a/fs/inode.c b/fs/inode.c index 72c4c347afb7..6d4ea0c9fe3e 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -930,6 +930,7 @@ struct inode *new_inode_pseudo(struct super_block *sb) if (inode) { spin_lock(&inode->i_lock); inode->i_state = 0; + inode->i_flags |= S_NOATIME; spin_unlock(&inode->i_lock); INIT_LIST_HEAD(&inode->i_sb_list); }
The kernel uses internal mounts created by kern_mount() and populated with files with no lookup path by alloc_file_pseudo() for a variety of reasons. An relevant example is anonymous pipes because every vfs_write also checks if atime needs to be updated even though it is unnecessary. Most of the relevant users for alloc_file_pseudo() either have no statfs helper or use simple_statfs which does not return st_atime. The closest proxy measure is the proc fd representations of such inodes which do not appear to change once they are created. This patch sets the S_NOATIME on inode->i_flags for inodes created by new_inode_pseudo() so that atime will not be updated. The test motivating this was "perf bench sched messaging --pipe" where atime-related functions were noticeable in the profiles. On a single-socket machine using threads the difference of the patch was as follows. The difference in performance was 5.8.0-rc1 5.8.0-rc1 vanilla pseudoatime-v1r1 Amean 1 0.4807 ( 0.00%) 0.4623 * 3.81%* Amean 3 1.5543 ( 0.00%) 1.4610 ( 6.00%) Amean 5 2.5647 ( 0.00%) 2.5183 ( 1.81%) Amean 7 3.7407 ( 0.00%) 3.7120 ( 0.77%) Amean 12 5.9900 ( 0.00%) 5.5233 ( 7.79%) Amean 18 8.8727 ( 0.00%) 6.8353 * 22.96%* Amean 24 11.1510 ( 0.00%) 8.9123 * 20.08%* Amean 30 13.9330 ( 0.00%) 10.8743 * 21.95%* Amean 32 14.2177 ( 0.00%) 10.9923 * 22.69%* Note that I consider the impact to be disproportionate and so it may not be universal. On a profiled run for just *one* group, the difference in perf profiles for atime-related functions were 0.23% -0.18% [kernel.vmlinux] [k] atime_needs_update 0.13% -0.02% [kernel.vmlinux] [k] touch_atime So there is a large reduction in atime overhead which on this particular machine must have gotten incrementally worse as the group count increased. I could measure it specifically but I think it's reasonable to reduce atime overhead for pseudo files unconditionally. Signed-off-by: Mel Gorman <mgorman@techsingularity.net> --- fs/inode.c | 1 + 1 file changed, 1 insertion(+)