[PATCH] fs: move generic stat response attr handling to vfs_getattr_nosec

[PATCH] fs: move generic stat response attr handling to vfs_getattr_nosec

[Christoph Hellwig]

generic_fillattr is an optional helper that isn't used by all
file systems, move handling purely based on inode flags to
vfs_getattr_nosec, which is common code.

This fixes setting this flag for file systems not using generic_fillattr
like xfs.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 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

Re: [PATCH] fs: move generic stat response attr handling to vfs_getattr_nosec

[David Howells]

Christoph Hellwig <hch@lst.de> wrote:

> generic_fillattr is an optional helper that isn't used by all
> file systems, move handling purely based on inode flags to
> vfs_getattr_nosec, which is common code.
> 
> This fixes setting this flag for file systems not using generic_fillattr
> like xfs.

The only problem I have with this change is that you've moved the flag setting
to after the ->getattr() call, which means that the filesystem cannot override
it.  I'm not sure that particularly matters in the case of these two flags,
but it might encourage similar things to be put there where it could matter.

David