On Tue, Jan 15, 2019 at 01:13:06PM -0800, Christoph Hellwig wrote:
> On Tue, Jan 15, 2019 at 09:08:49AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Refactor xfs_vn_getattr to use generic_fillattr to fill out parts of the
> > kstat structure instead of open-coding the same pieces. This eliminates
> > redundant code and fixes a bug where we fail to set the AUTOMOUNT
> > attribute. Obviously, we retain all the xfs-specific parts.
>
> I find the idea of writing the same values twice for something like
> stat that is pretty performane critical rather odd.
>
> I'd much rather just fix the XFS version up to set the automount
> attribute, and clear STATX_ATIME for noatime inodes.
Actually, the proper fix is to move these checks to what actually
is common code, something like this:
--
>From 3f82b9fce150a8af6b98bae686bbd24cbe3388ab Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@xxxxxx>
Date: Tue, 15 Jan 2019 22:31:50 +0100
Subject: fs: move generic stat response attr handling to vfs_getattr_nosec
generic_fillattr is an optional helper that isn't used by all
filesystems, move handling purely based on inode flags to
vfs_getattr_nosec, which is common code.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
fs/stat.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/fs/stat.c b/fs/stat.c
index adbfcd86c81b..9600ff1ea8df 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -45,11 +45,6 @@ void generic_fillattr(struct inode *inode, struct kstat *stat)
stat->ctime = inode->i_ctime;
stat->blksize = i_blocksize(inode);
stat->blocks = inode->i_blocks;
-
- if (IS_NOATIME(inode))
- stat->result_mask &= ~STATX_ATIME;
- if (IS_AUTOMOUNT(inode))
- stat->attributes |= STATX_ATTR_AUTOMOUNT;
}
EXPORT_SYMBOL(generic_fillattr);
@@ -75,11 +70,19 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat,
stat->result_mask |= STATX_BASIC_STATS;
request_mask &= STATX_ALL;
query_flags &= KSTAT_QUERY_FLAGS;
- if (inode->i_op->getattr)
- return inode->i_op->getattr(path, stat, request_mask,
- query_flags);
+ if (inode->i_op->getattr) {
+ int ret = inode->i_op->getattr(path, stat, request_mask,
+ query_flags);
+ if (ret)
+ return ret;
+ } else {
+ generic_fillattr(inode, stat);
+ }
- generic_fillattr(inode, stat);
+ if (IS_NOATIME(inode))
+ stat->result_mask &= ~STATX_ATIME;
+ if (IS_AUTOMOUNT(inode))
+ stat->attributes |= STATX_ATTR_AUTOMOUNT;
return 0;
}
EXPORT_SYMBOL(vfs_getattr_nosec);
--
2.20.1
