[PATCH 0/3] bit more FS_IOC_GETFSUUID, FS_IOC_GETFSSYSFSPATH

[PATCH 0/3] bit more FS_IOC_GETFSUUID, FS_IOC_GETFSSYSFSPATH

[Kent Overstreet]

implement FS_IOC_GETFSUUID, FS_IOC_GETFSSYSFSPATH a bit more

also: https://evilpiepirate.org/git/bcachefs.git/commit/?h=bcachefs-sysfs-ioctls

Kent Overstreet (3):
  bcachefs: Switch to uuid_to_fsid()
  bcachefs: Initialize super_block->s_uuid
  ext4: Add support for FS_IOC_GETFSSYSFSPATH

 fs/bcachefs/fs.c | 9 ++++-----
 fs/ext4/super.c  | 1 +
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH 1/3] bcachefs: Switch to uuid_to_fsid()

[Kent Overstreet]

switch the statfs code from something horrible and open coded to the
more standard uuid_to_fsid()

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 fs/bcachefs/fs.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 77ae65542db9..ec9cf4b8faf1 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1572,7 +1572,6 @@ static int bch2_statfs(struct dentry *dentry, struct kstatfs *buf)
 	 * number:
 	 */
 	u64 avail_inodes = ((usage.capacity - usage.used) << 3);
-	u64 fsid;
 
 	buf->f_type	= BCACHEFS_STATFS_MAGIC;
 	buf->f_bsize	= sb->s_blocksize;
@@ -1583,10 +1582,7 @@ static int bch2_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_files	= usage.nr_inodes + avail_inodes;
 	buf->f_ffree	= avail_inodes;
 
-	fsid = le64_to_cpup((void *) c->sb.user_uuid.b) ^
-	       le64_to_cpup((void *) c->sb.user_uuid.b + sizeof(u64));
-	buf->f_fsid.val[0] = fsid & 0xFFFFFFFFUL;
-	buf->f_fsid.val[1] = (fsid >> 32) & 0xFFFFFFFFUL;
+	buf->f_fsid	= uuid_to_fsid(c->sb.user_uuid.b);
 	buf->f_namelen	= BCH_NAME_MAX;
 
 	return 0;
-- 
2.43.0

[PATCH 2/3] bcachefs: Initialize super_block->s_uuid

[Kent Overstreet]

Need to fix this oversight for the new UUID/sysfspath ioctls; also,
initialize the sysfs path.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
---
 fs/bcachefs/fs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index ec9cf4b8faf1..cfc9d90ab179 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -1881,6 +1881,9 @@ static struct dentry *bch2_mount(struct file_system_type *fs_type,
 	c->vfs_sb		= sb;
 	strscpy(sb->s_id, c->name, sizeof(sb->s_id));
 
+	super_set_uuid(sb, c->sb.user_uuid.b, 16);
+	super_set_sysfs_name_uuid(sb);
+
 	ret = super_setup_bdi(sb);
 	if (ret)
 		goto err_put_super;
-- 
2.43.0

[PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Kent Overstreet]

the new sysfs path ioctl lets us get the /sys/fs/ path for a given
filesystem in a fs agnostic way, potentially nudging us towards
standarizing some of our reporting.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: "Theodore Ts'o" <tytso@mit.edu>
Cc: Andreas Dilger <adilger.kernel@dilger.ca>
Cc: linux-ext4@vger.kernel.org
---
 fs/ext4/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index f5e5a44778cf..cb82b23a4d98 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
 #endif
 	super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
+	super_set_sysfs_name_bdev(sb);
 
 	INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
 	mutex_init(&sbi->s_orphan_lock);
-- 
2.43.0

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Theodore Ts'o]

On Thu, Mar 14, 2024 at 11:53:02PM -0400, Kent Overstreet wrote:
> the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> filesystem in a fs agnostic way, potentially nudging us towards
> standarizing some of our reporting.
> 
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
>  #endif
>  	super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> +	super_set_sysfs_name_bdev(sb);

Should we perhaps be hoisting this call up to the VFS layer, so that
all file systems would benefit?

					- Ted

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Kent Overstreet]

On Fri, Mar 15, 2024 at 12:45:50PM -0400, Theodore Ts'o wrote:
> On Thu, Mar 14, 2024 at 11:53:02PM -0400, Kent Overstreet wrote:
> > the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> > filesystem in a fs agnostic way, potentially nudging us towards
> > standarizing some of our reporting.
> > 
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> >  	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> >  #endif
> >  	super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> > +	super_set_sysfs_name_bdev(sb);
> 
> Should we perhaps be hoisting this call up to the VFS layer, so that
> all file systems would benefit?

I did as much hoisting as I could. For some filesystems (single device
filesystems) the sysfs name is the block device, for the multi device
filesystems I've looked at it's the UUID.

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Kiselev, Oleg]

On 3/15/24, 09:51, "Kent Overstreet" <kent.overstreet@linux.dev <mailto:kent.overstreet@linux.dev>> wrote:
> On Fri, Mar 15, 2024 at 12:45:50PM -0400, Theodore Ts'o wrote:
> > On Thu, Mar 14, 2024 at 11:53:02PM -0400, Kent Overstreet wrote:
> > > the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> > > filesystem in a fs agnostic way, potentially nudging us towards
> > > standarizing some of our reporting.
> > >
> > > --- a/fs/ext4/super.c
> > > +++ b/fs/ext4/super.c
> > > @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> > > sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> > > #endif
> > > super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> > > + super_set_sysfs_name_bdev(sb);
> >
> > Should we perhaps be hoisting this call up to the VFS layer, so that
> > all file systems would benefit?
>
>
> I did as much hoisting as I could. For some filesystems (single device
> filesystems) the sysfs name is the block device, for the multi device
> filesystems I've looked at it's the UUID.

Why not use the fs UUID for all cases, single device and multi device?
-- 
Oleg Kiselev 

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Dave Chinner]

On Mon, Mar 18, 2024 at 09:51:04PM +0000, Kiselev, Oleg wrote:
> On 3/15/24, 09:51, "Kent Overstreet" <kent.overstreet@linux.dev <mailto:kent.overstreet@linux.dev>> wrote:
> > On Fri, Mar 15, 2024 at 12:45:50PM -0400, Theodore Ts'o wrote:
> > > On Thu, Mar 14, 2024 at 11:53:02PM -0400, Kent Overstreet wrote:
> > > > the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> > > > filesystem in a fs agnostic way, potentially nudging us towards
> > > > standarizing some of our reporting.
> > > >
> > > > --- a/fs/ext4/super.c
> > > > +++ b/fs/ext4/super.c
> > > > @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> > > > sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> > > > #endif
> > > > super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> > > > + super_set_sysfs_name_bdev(sb);
> > >
> > > Should we perhaps be hoisting this call up to the VFS layer, so that
> > > all file systems would benefit?
> >
> >
> > I did as much hoisting as I could. For some filesystems (single device
> > filesystems) the sysfs name is the block device, for the multi device
> > filesystems I've looked at it's the UUID.
> 
> Why not use the fs UUID for all cases, single device and multi device?

Because the sysfs directory heirachy has already been defined for
many filesystems, and technically sysfs represents a KABI that we
can't just break for the hell of it.

e.g. changing everything to use uuid will break fstests
infrastructure because it assumes that it can derive the sysfs dir
location for the filesystem from the *device name* the filesystem is
mounted on. btrfs has a special implementation of that derivation
that runs the btrfs command to retreive the UUID of the filesysem.

So, yes, while we could technically change it, we break anything in
userspace that has introduced a dependency on bdev name as the sysfs
fs identifier. We're not going to break userspace like this,
especially as it is trivial to avoid.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Darrick J. Wong]

On Tue, Mar 19, 2024 at 09:22:55AM +1100, Dave Chinner wrote:
> On Mon, Mar 18, 2024 at 09:51:04PM +0000, Kiselev, Oleg wrote:
> > On 3/15/24, 09:51, "Kent Overstreet" <kent.overstreet@linux.dev <mailto:kent.overstreet@linux.dev>> wrote:
> > > On Fri, Mar 15, 2024 at 12:45:50PM -0400, Theodore Ts'o wrote:
> > > > On Thu, Mar 14, 2024 at 11:53:02PM -0400, Kent Overstreet wrote:
> > > > > the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> > > > > filesystem in a fs agnostic way, potentially nudging us towards
> > > > > standarizing some of our reporting.
> > > > >
> > > > > --- a/fs/ext4/super.c
> > > > > +++ b/fs/ext4/super.c
> > > > > @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> > > > > sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> > > > > #endif
> > > > > super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> > > > > + super_set_sysfs_name_bdev(sb);
> > > >
> > > > Should we perhaps be hoisting this call up to the VFS layer, so that
> > > > all file systems would benefit?
> > >
> > >
> > > I did as much hoisting as I could. For some filesystems (single device
> > > filesystems) the sysfs name is the block device, for the multi device
> > > filesystems I've looked at it's the UUID.
> > 
> > Why not use the fs UUID for all cases, single device and multi device?
> 
> Because the sysfs directory heirachy has already been defined for
> many filesystems, and technically sysfs represents a KABI that we
> can't just break for the hell of it.
> 
> e.g. changing everything to use uuid will break fstests
> infrastructure because it assumes that it can derive the sysfs dir
> location for the filesystem from the *device name* the filesystem is
> mounted on. btrfs has a special implementation of that derivation
> that runs the btrfs command to retreive the UUID of the filesysem.
> 
> So, yes, while we could technically change it, we break anything in
> userspace that has introduced a dependency on bdev name as the sysfs
> fs identifier. We're not going to break userspace like this,
> especially as it is trivial to avoid.

Wellll... some day in the far kumbaya future, everyone will call
GETSYSFSPATH and they won't have to know or care what each fs does under
the covers.  So who cares? 8-)

How about using sysfs_create_link for non -o nouuid filesystems so that
/sys/fs/xfs/$uuid actually goes somewhere?

<shrug> Don't really care much myself either way.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> 

Re: [PATCH 3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]

On Mar 14, 2024, at 9:53 PM, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> 
> the new sysfs path ioctl lets us get the /sys/fs/ path for a given
> filesystem in a fs agnostic way, potentially nudging us towards
> standarizing some of our reporting.

I find it ironic that we are adding an ioctl to be able to get the
sysfs path, which was originally created to avoid adding ioctls...
But, the days of jumping through hoops to find stuff in sysfs for
each filesystem arrived long ago, so we may as well make it easier. :-)

Cheers, Andreas

> 
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: "Theodore Ts'o" <tytso@mit.edu>
> Cc: Andreas Dilger <adilger.kernel@dilger.ca>
> Cc: linux-ext4@vger.kernel.org
> ---
> fs/ext4/super.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f5e5a44778cf..cb82b23a4d98 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -5346,6 +5346,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
> 	sb->s_quota_types = QTYPE_MASK_USR | QTYPE_MASK_GRP | QTYPE_MASK_PRJ;
> #endif
> 	super_set_uuid(sb, es->s_uuid, sizeof(es->s_uuid));
> +	super_set_sysfs_name_bdev(sb);
> 
> 	INIT_LIST_HEAD(&sbi->s_orphan); /* unlinked but open files */
> 	mutex_init(&sbi->s_orphan_lock);
> --
> 2.43.0
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 0/3] bit more FS_IOC_GETFSUUID, FS_IOC_GETFSSYSFSPATH

[Theodore Ts'o]

On Thu, 14 Mar 2024 23:52:59 -0400, Kent Overstreet wrote:
> implement FS_IOC_GETFSUUID, FS_IOC_GETFSSYSFSPATH a bit more
> 
> also: https://evilpiepirate.org/git/bcachefs.git/commit/?h=bcachefs-sysfs-ioctls
> 
> Kent Overstreet (3):
>   ext4: Add support for FS_IOC_GETFSSYSFSPATH
> 
> [...]

Applied, thanks!

[3/3] ext4: Add support for FS_IOC_GETFSSYSFSPATH
      commit: fb092d407262eb4278f3d1ca24da54396a038c62

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>