[PATCH] fs, pseudo: Do not update atime for pseudo inodes

[PATCH] fs, pseudo: Do not update atime for pseudo inodes

[Mel Gorman]

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

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);
 	}

Re: [PATCH] fs, pseudo: Do not update atime for pseudo inodes

[Amir Goldstein]

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.

Re: [PATCH] fs, pseudo: Do not update atime for pseudo inodes

[Mel Gorman]

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.

-- 
Mel Gorman
SUSE Labs