On Wed, Nov 08, 2023 at 05:16:38PM +0100, Christian Brauner wrote: > On Wed, Nov 08, 2023 at 06:08:09AM -0800, Christoph Hellwig wrote: > > On Wed, Nov 08, 2023 at 09:27:44AM +0100, Christian Brauner wrote: > > > > What is that flag going to buy us? > > > > > > The initial list that Josef provided in > > > https://lore.kernel.org/linux-btrfs/20231025210654.GA2892534@perftesting > > > asks to give users a way to figure out whether a file is located on a > > > subvolume. Which I think is reasonable and there's a good chunk of > > > software out there that could really benefit from this. Now all of the I explained myself badly here. What I mean and what is immediately useful is to add STATX_ATTR_SUBVOLUME_ROOT which works for both btrfs and bcachefs and makes it easy for userspace to figure out whether an inode is the root of a subvolume: (This won't compile obviously.) diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c index 166d8d8abe68..fce8603d37b0 100644 --- a/fs/bcachefs/fs.c +++ b/fs/bcachefs/fs.c @@ -776,6 +776,10 @@ static int bch2_getattr(struct mnt_idmap *idmap, stat->attributes |= STATX_ATTR_NODUMP; stat->attributes_mask |= STATX_ATTR_NODUMP; + if (BTRFS_is_subvol_root(inode)) + stat->attributes_mask |= STATX_ATTR_SUBVOLUME_ROOT; + stat->attributes_mask |= STATX_ATTR_SUBVOLUME_ROOT; + return 0; } diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 5e3fccddde0c..c339a9a08d7e 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -8657,10 +8657,14 @@ static int btrfs_getattr(struct mnt_idmap *idmap, if (bi_ro_flags & BTRFS_INODE_RO_VERITY) stat->attributes |= STATX_ATTR_VERITY; + if (BCH2_is_subvol_root(inode)) + stat->attributes |= STATX_ATTR_SUBVOLUME_ROOT; + stat->attributes_mask |= (STATX_ATTR_APPEND | STATX_ATTR_COMPRESSED | STATX_ATTR_IMMUTABLE | - STATX_ATTR_NODUMP); + STATX_ATTR_NODUMP | + STATX_ATTR_SUBVOLUME_ROOT); generic_fillattr(idmap, request_mask, inode, stat); stat->dev = BTRFS_I(inode)->root->anon_dev; diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h index 7cab2c65d3d7..24d493babe63 100644 --- a/include/uapi/linux/stat.h +++ b/include/uapi/linux/stat.h @@ -187,6 +187,7 @@ struct statx { #define STATX_ATTR_ENCRYPTED 0x00000800 /* [I] File requires key to decrypt in fs */ #define STATX_ATTR_AUTOMOUNT 0x00001000 /* Dir: Automount trigger */ #define STATX_ATTR_MOUNT_ROOT 0x00002000 /* Root of a mount */ +#define STATX_ATTR_SUBVOLUME_ROOT 0x00004000 /* Root of a subvolume */ #define STATX_ATTR_VERITY 0x00100000 /* [I] Verity protected file */ #define STATX_ATTR_DAX 0x00200000 /* File is currently in DAX state */ This would be a pretty big help for userspace already. Right now all code needs to do a stat() and a statfs() and then check the inode number. And that likely only works for btrfs. This would also allow tools that want to to detect when they're crossing into a new subvolume - be it on btrfs or bcachefs - and take appropriate measures deciding what they want to do just relying on statx() without any additional system calls. And I think that's something we should do.
