2020-12-01 17:03:43

by Eric Sandeen

Subject: [PATCH 2/2] statx: move STATX_ATTR_DAX attribute handling to filesystems

It's a bit odd to set STATX_ATTR_DAX into the statx attributes in the VFS;
while the VFS can detect the current DAX state, it is the filesystem which
actually sets S_DAX on the inode, and the filesystem is the place that
knows whether DAX is something that the "filesystem actually supports" [1]
so that the statx attributes_mask can be properly set.

So, move STATX_ATTR_DAX attribute setting to the individual dax-capable
filesystems, and update the attributes_mask there as well.

[1] 3209f68b3ca4 statx: Include a mask for stx_attributes in struct statx

Signed-off-by: Eric Sandeen <[email protected]>
---
fs/ext2/inode.c | 6 +++++-
fs/ext4/inode.c | 5 ++++-
fs/stat.c | 3 ---
fs/xfs/xfs_iops.c | 5 ++++-
4 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 11c5c6fe75bb..3550783a6ea0 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -1653,11 +1653,15 @@ int ext2_getattr(const struct path *path, struct kstat *stat,
stat->attributes |= STATX_ATTR_IMMUTABLE;
if (flags & EXT2_NODUMP_FL)
stat->attributes |= STATX_ATTR_NODUMP;
+ if (IS_DAX(inode))
+ stat->attributes |= STATX_ATTR_DAX;
+
stat->attributes_mask |= (STATX_ATTR_APPEND |
STATX_ATTR_COMPRESSED |
STATX_ATTR_ENCRYPTED |
STATX_ATTR_IMMUTABLE |
- STATX_ATTR_NODUMP);
+ STATX_ATTR_NODUMP |
+ STATX_ATTR_DAX);

generic_fillattr(inode, stat);
return 0;
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0d8385aea898..848a0f2b154e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5550,13 +5550,16 @@ int ext4_getattr(const struct path *path, struct kstat *stat,
stat->attributes |= STATX_ATTR_NODUMP;
if (flags & EXT4_VERITY_FL)
stat->attributes |= STATX_ATTR_VERITY;
+ if (IS_DAX(inode))
+ stat->attributes |= STATX_ATTR_DAX;

stat->attributes_mask |= (STATX_ATTR_APPEND |
STATX_ATTR_COMPRESSED |
STATX_ATTR_ENCRYPTED |
STATX_ATTR_IMMUTABLE |
STATX_ATTR_NODUMP |
- STATX_ATTR_VERITY);
+ STATX_ATTR_VERITY |
+ STATX_ATTR_DAX);

generic_fillattr(inode, stat);
return 0;
diff --git a/fs/stat.c b/fs/stat.c
index dacecdda2e79..5bd90949c69b 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -80,9 +80,6 @@ int vfs_getattr_nosec(const struct path *path, struct kstat *stat,
if (IS_AUTOMOUNT(inode))
stat->attributes |= STATX_ATTR_AUTOMOUNT;

- if (IS_DAX(inode))
- stat->attributes |= STATX_ATTR_DAX;
-
if (inode->i_op->getattr)
return inode->i_op->getattr(path, stat, request_mask,
query_flags);
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 1414ab79eacf..56deda7042fd 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -575,10 +575,13 @@ xfs_vn_getattr(
stat->attributes |= STATX_ATTR_APPEND;
if (ip->i_d.di_flags & XFS_DIFLAG_NODUMP)
stat->attributes |= STATX_ATTR_NODUMP;
+ if (IS_DAX(inode))
+ stat->attributes |= STATX_ATTR_DAX;

stat->attributes_mask |= (STATX_ATTR_IMMUTABLE |
STATX_ATTR_APPEND |
- STATX_ATTR_NODUMP);
+ STATX_ATTR_NODUMP |
+ STATX_ATTR_DAX);

switch (inode->i_mode & S_IFMT) {
case S_IFBLK:
--
2.17.0

2020-12-01 21:07:50

by David Howells

Subject: Re: [PATCH 2/2] statx: move STATX_ATTR_DAX attribute handling to filesystems

Linus Torvalds <[email protected]> wrote:

> And if IS_DAX() is correct, then why shouldn't this just be done in
> generic code? Why move it to every individual filesystem?

One way of looking at it is that the check is then done for every filesystem -
most of which don't support it. Not sure whether that's a big enough problem
to worry about. The same is true of the automount test too, I suppose...

David