linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] statx: stx_vol
@ 2024-03-02 22:02 Kent Overstreet
  2024-03-03 18:52 ` John Stoffel
  2024-03-04  9:18 ` Christian Brauner
  0 siblings, 2 replies; 6+ messages in thread
From: Kent Overstreet @ 2024-03-02 22:02 UTC (permalink / raw)
  To: linux-bcachefs, linux-fsdevel, linux-kernel
  Cc: Kent Overstreet, Josef Bacik, Miklos Szeredi, Christian Brauner,
	David Howells

Add a new statx field for (sub)volume identifiers.

This includes bcachefs support; we'll definitely want btrfs support as
well.

Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: Miklos Szeredi <mszeredi@redhat.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: David Howells <dhowells@redhat.com>
---
 fs/bcachefs/fs.c          | 3 +++
 fs/stat.c                 | 1 +
 include/linux/stat.h      | 1 +
 include/uapi/linux/stat.h | 4 +++-
 4 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 3f073845bbd7..d82f7f3f0670 100644
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -840,6 +840,9 @@ static int bch2_getattr(struct mnt_idmap *idmap,
 	stat->blksize	= block_bytes(c);
 	stat->blocks	= inode->v.i_blocks;
 
+	stat->vol	= inode->ei_subvol;
+	stat->result_mask |= STATX_VOL;
+
 	if (request_mask & STATX_BTIME) {
 		stat->result_mask |= STATX_BTIME;
 		stat->btime = bch2_time_to_timespec(c, inode->ei_inode.bi_otime);
diff --git a/fs/stat.c b/fs/stat.c
index 77cdc69eb422..80d5f7502d99 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -658,6 +658,7 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer)
 	tmp.stx_mnt_id = stat->mnt_id;
 	tmp.stx_dio_mem_align = stat->dio_mem_align;
 	tmp.stx_dio_offset_align = stat->dio_offset_align;
+	tmp.stx_vol = stat->vol;
 
 	return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;
 }
diff --git a/include/linux/stat.h b/include/linux/stat.h
index 52150570d37a..9dc1b493ef1f 100644
--- a/include/linux/stat.h
+++ b/include/linux/stat.h
@@ -53,6 +53,7 @@ struct kstat {
 	u32		dio_mem_align;
 	u32		dio_offset_align;
 	u64		change_cookie;
+	u64		vol;
 };
 
 /* These definitions are internal to the kernel for now. Mainly used by nfsd. */
diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
index 2f2ee82d5517..ae090d67946d 100644
--- a/include/uapi/linux/stat.h
+++ b/include/uapi/linux/stat.h
@@ -126,8 +126,9 @@ struct statx {
 	__u64	stx_mnt_id;
 	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
 	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
+	__u64	stx_vol;	/* Subvolume identifier */
 	/* 0xa0 */
-	__u64	__spare3[12];	/* Spare space for future expansion */
+	__u64	__spare3[11];	/* Spare space for future expansion */
 	/* 0x100 */
 };
 
@@ -155,6 +156,7 @@ struct statx {
 #define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
 #define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
 #define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
+#define STATX_VOL		0x00008000U	/* Want/got stx_vol */
 
 #define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] statx: stx_vol
  2024-03-02 22:02 [PATCH] statx: stx_vol Kent Overstreet
@ 2024-03-03 18:52 ` John Stoffel
  2024-03-04  9:18 ` Christian Brauner
  1 sibling, 0 replies; 6+ messages in thread
From: John Stoffel @ 2024-03-03 18:52 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: linux-bcachefs, linux-fsdevel, linux-kernel, Josef Bacik,
	Miklos Szeredi, Christian Brauner, David Howells

>>>>> "Kent" == Kent Overstreet <kent.overstreet@linux.dev> writes:

> Add a new statx field for (sub)volume identifiers.
> This includes bcachefs support; we'll definitely want btrfs support as
> well.

> Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: Miklos Szeredi <mszeredi@redhat.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: David Howells <dhowells@redhat.com>
> ---
>  fs/bcachefs/fs.c          | 3 +++
>  fs/stat.c                 | 1 +
>  include/linux/stat.h      | 1 +
>  include/uapi/linux/stat.h | 4 +++-
>  4 files changed, 8 insertions(+), 1 deletion(-)

> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 3f073845bbd7..d82f7f3f0670 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -840,6 +840,9 @@ static int bch2_getattr(struct mnt_idmap *idmap,
stat-> blksize	= block_bytes(c);
stat-> blocks	= inode->v.i_blocks;
 
> +	stat->vol	= inode->ei_subvol;
> +	stat->result_mask |= STATX_VOL;
> +
>  	if (request_mask & STATX_BTIME) {
stat-> result_mask |= STATX_BTIME;
stat-> btime = bch2_time_to_timespec(c, inode->ei_inode.bi_otime);
> diff --git a/fs/stat.c b/fs/stat.c
> index 77cdc69eb422..80d5f7502d99 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -658,6 +658,7 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer)
>  	tmp.stx_mnt_id = stat->mnt_id;
>  	tmp.stx_dio_mem_align = stat->dio_mem_align;
>  	tmp.stx_dio_offset_align = stat->dio_offset_align;
> +	tmp.stx_vol = stat->vol;

So this bugs me that you use subvol and vol sorta interchangeably
here.  Shouldn't it always be 'subvol'?  It's not a seperate volume,
it's a snapshot/whatever that is a part of the parent volume, but
should be treated seperately. 
 
>  	return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;
>  }
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index 52150570d37a..9dc1b493ef1f 100644
> --- a/include/linux/stat.h
> +++ b/include/linux/stat.h
> @@ -53,6 +53,7 @@ struct kstat {
>  	u32		dio_mem_align;
>  	u32		dio_offset_align;
>  	u64		change_cookie;
> +	u64		vol;
>  };

Again, should this be subvol? And comments added for what it really means?
 
>  /* These definitions are internal to the kernel for now. Mainly used by nfsd. */
> diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
> index 2f2ee82d5517..ae090d67946d 100644
> --- a/include/uapi/linux/stat.h
> +++ b/include/uapi/linux/stat.h
> @@ -126,8 +126,9 @@ struct statx {
>  	__u64	stx_mnt_id;
>  	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
>  	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
> +	__u64	stx_vol;	/* Subvolume identifier */

Again, then call it 'stx_subvol' if that's what it is!

>  	/* 0xa0 */
> -	__u64	__spare3[12];	/* Spare space for future expansion */
> +	__u64	__spare3[11];	/* Spare space for future expansion */
>  	/* 0x100 */
>  };
 
> @@ -155,6 +156,7 @@ struct statx {
>  #define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
>  #define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
>  #define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
> +#define STATX_VOL		0x00008000U	/* Want/got stx_vol */
 
>  #define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
 
> -- 
> 2.43.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] statx: stx_vol
  2024-03-02 22:02 [PATCH] statx: stx_vol Kent Overstreet
  2024-03-03 18:52 ` John Stoffel
@ 2024-03-04  9:18 ` Christian Brauner
  2024-03-06 19:59   ` Josef Bacik
  2024-03-07 17:39   ` Kent Overstreet
  1 sibling, 2 replies; 6+ messages in thread
From: Christian Brauner @ 2024-03-04  9:18 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: linux-bcachefs, linux-fsdevel, linux-kernel, Josef Bacik,
	Miklos Szeredi, David Howells

On Sat, Mar 02, 2024 at 05:02:03PM -0500, Kent Overstreet wrote:
> Add a new statx field for (sub)volume identifiers.
> 
> This includes bcachefs support; we'll definitely want btrfs support as
> well.
> 
> Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: Miklos Szeredi <mszeredi@redhat.com>
> Cc: Christian Brauner <brauner@kernel.org>
> Cc: David Howells <dhowells@redhat.com>
> ---

As I've said many times before I'm supportive of this and would pick up
a patch like this. There's definitely a lot of userspace that would make
use of this that I'm aware of. If the btrfs people could provide an Ack
on this to express their support here that would be great.

And it would be lovely if we could expand the commit message a bit and
do some renaming/bikeshedding. Imho, STATX_SUBVOLUME_ID is great and
then stx_subvolume_id or stx_subvol_id. And then subvolume_id or
subvol_id for the field in struct kstat.

>  fs/bcachefs/fs.c          | 3 +++
>  fs/stat.c                 | 1 +
>  include/linux/stat.h      | 1 +
>  include/uapi/linux/stat.h | 4 +++-
>  4 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
> index 3f073845bbd7..d82f7f3f0670 100644
> --- a/fs/bcachefs/fs.c
> +++ b/fs/bcachefs/fs.c
> @@ -840,6 +840,9 @@ static int bch2_getattr(struct mnt_idmap *idmap,
>  	stat->blksize	= block_bytes(c);
>  	stat->blocks	= inode->v.i_blocks;
>  
> +	stat->vol	= inode->ei_subvol;
> +	stat->result_mask |= STATX_VOL;
> +
>  	if (request_mask & STATX_BTIME) {
>  		stat->result_mask |= STATX_BTIME;
>  		stat->btime = bch2_time_to_timespec(c, inode->ei_inode.bi_otime);
> diff --git a/fs/stat.c b/fs/stat.c
> index 77cdc69eb422..80d5f7502d99 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -658,6 +658,7 @@ cp_statx(const struct kstat *stat, struct statx __user *buffer)
>  	tmp.stx_mnt_id = stat->mnt_id;
>  	tmp.stx_dio_mem_align = stat->dio_mem_align;
>  	tmp.stx_dio_offset_align = stat->dio_offset_align;
> +	tmp.stx_vol = stat->vol;
>  
>  	return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;
>  }
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index 52150570d37a..9dc1b493ef1f 100644
> --- a/include/linux/stat.h
> +++ b/include/linux/stat.h
> @@ -53,6 +53,7 @@ struct kstat {
>  	u32		dio_mem_align;
>  	u32		dio_offset_align;
>  	u64		change_cookie;
> +	u64		vol;
>  };
>  
>  /* These definitions are internal to the kernel for now. Mainly used by nfsd. */
> diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h
> index 2f2ee82d5517..ae090d67946d 100644
> --- a/include/uapi/linux/stat.h
> +++ b/include/uapi/linux/stat.h
> @@ -126,8 +126,9 @@ struct statx {
>  	__u64	stx_mnt_id;
>  	__u32	stx_dio_mem_align;	/* Memory buffer alignment for direct I/O */
>  	__u32	stx_dio_offset_align;	/* File offset alignment for direct I/O */
> +	__u64	stx_vol;	/* Subvolume identifier */
>  	/* 0xa0 */
> -	__u64	__spare3[12];	/* Spare space for future expansion */
> +	__u64	__spare3[11];	/* Spare space for future expansion */
>  	/* 0x100 */
>  };
>  
> @@ -155,6 +156,7 @@ struct statx {
>  #define STATX_MNT_ID		0x00001000U	/* Got stx_mnt_id */
>  #define STATX_DIOALIGN		0x00002000U	/* Want/got direct I/O alignment info */
>  #define STATX_MNT_ID_UNIQUE	0x00004000U	/* Want/got extended stx_mount_id */
> +#define STATX_VOL		0x00008000U	/* Want/got stx_vol */
>  
>  #define STATX__RESERVED		0x80000000U	/* Reserved for future struct statx expansion */
>  
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] statx: stx_vol
  2024-03-04  9:18 ` Christian Brauner
@ 2024-03-06 19:59   ` Josef Bacik
  2024-03-07 17:39   ` Kent Overstreet
  1 sibling, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2024-03-06 19:59 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Kent Overstreet, linux-bcachefs, linux-fsdevel, linux-kernel,
	Miklos Szeredi, David Howells

On Mon, Mar 04, 2024 at 10:18:22AM +0100, Christian Brauner wrote:
> On Sat, Mar 02, 2024 at 05:02:03PM -0500, Kent Overstreet wrote:
> > Add a new statx field for (sub)volume identifiers.
> > 
> > This includes bcachefs support; we'll definitely want btrfs support as
> > well.
> > 
> > Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> > Cc: Josef Bacik <josef@toxicpanda.com>
> > Cc: Miklos Szeredi <mszeredi@redhat.com>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: David Howells <dhowells@redhat.com>
> > ---
> 
> As I've said many times before I'm supportive of this and would pick up
> a patch like this. There's definitely a lot of userspace that would make
> use of this that I'm aware of. If the btrfs people could provide an Ack
> on this to express their support here that would be great.
> 
> And it would be lovely if we could expand the commit message a bit and
> do some renaming/bikeshedding. Imho, STATX_SUBVOLUME_ID is great and
> then stx_subvolume_id or stx_subvol_id. And then subvolume_id or
> subvol_id for the field in struct kstat.

Sorry I had my head down in some NFS problems.  This works for me, I agree with
the naming suggestions you've made.  Kent, when you send a new version I'll
review it and then followup with a btrfs patch.  Thanks for getting this ball
rolling,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] statx: stx_vol
  2024-03-04  9:18 ` Christian Brauner
  2024-03-06 19:59   ` Josef Bacik
@ 2024-03-07 17:39   ` Kent Overstreet
  2024-03-08 10:59     ` Christian Brauner
  1 sibling, 1 reply; 6+ messages in thread
From: Kent Overstreet @ 2024-03-07 17:39 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-bcachefs, linux-fsdevel, linux-kernel, Josef Bacik,
	Miklos Szeredi, David Howells

On Mon, Mar 04, 2024 at 10:18:22AM +0100, Christian Brauner wrote:
> On Sat, Mar 02, 2024 at 05:02:03PM -0500, Kent Overstreet wrote:
> > Add a new statx field for (sub)volume identifiers.
> > 
> > This includes bcachefs support; we'll definitely want btrfs support as
> > well.
> > 
> > Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
> > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> > Cc: Josef Bacik <josef@toxicpanda.com>
> > Cc: Miklos Szeredi <mszeredi@redhat.com>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: David Howells <dhowells@redhat.com>
> > ---
> 
> As I've said many times before I'm supportive of this and would pick up
> a patch like this. There's definitely a lot of userspace that would make
> use of this that I'm aware of. If the btrfs people could provide an Ack
> on this to express their support here that would be great.
> 
> And it would be lovely if we could expand the commit message a bit and
> do some renaming/bikeshedding. Imho, STATX_SUBVOLUME_ID is great and
> then stx_subvolume_id or stx_subvol_id. And then subvolume_id or
> subvol_id for the field in struct kstat.

_id is too redundant for me, can we just do STATX_SUBVOL/statx.subvol?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] statx: stx_vol
  2024-03-07 17:39   ` Kent Overstreet
@ 2024-03-08 10:59     ` Christian Brauner
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2024-03-08 10:59 UTC (permalink / raw)
  To: Kent Overstreet
  Cc: linux-bcachefs, linux-fsdevel, linux-kernel, Josef Bacik,
	Miklos Szeredi, David Howells

On Thu, Mar 07, 2024 at 12:39:58PM -0500, Kent Overstreet wrote:
> On Mon, Mar 04, 2024 at 10:18:22AM +0100, Christian Brauner wrote:
> > On Sat, Mar 02, 2024 at 05:02:03PM -0500, Kent Overstreet wrote:
> > > Add a new statx field for (sub)volume identifiers.
> > > 
> > > This includes bcachefs support; we'll definitely want btrfs support as
> > > well.
> > > 
> > > Link: https://lore.kernel.org/linux-fsdevel/2uvhm6gweyl7iyyp2xpfryvcu2g3padagaeqcbiavjyiis6prl@yjm725bizncq/
> > > Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> > > Cc: Josef Bacik <josef@toxicpanda.com>
> > > Cc: Miklos Szeredi <mszeredi@redhat.com>
> > > Cc: Christian Brauner <brauner@kernel.org>
> > > Cc: David Howells <dhowells@redhat.com>
> > > ---
> > 
> > As I've said many times before I'm supportive of this and would pick up
> > a patch like this. There's definitely a lot of userspace that would make
> > use of this that I'm aware of. If the btrfs people could provide an Ack
> > on this to express their support here that would be great.
> > 
> > And it would be lovely if we could expand the commit message a bit and
> > do some renaming/bikeshedding. Imho, STATX_SUBVOLUME_ID is great and
> > then stx_subvolume_id or stx_subvol_id. And then subvolume_id or
> > subvol_id for the field in struct kstat.
> 
> _id is too redundant for me, can we just do STATX_SUBVOL/statx.subvol?

Fine by me.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-03-08 10:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-02 22:02 [PATCH] statx: stx_vol Kent Overstreet
2024-03-03 18:52 ` John Stoffel
2024-03-04  9:18 ` Christian Brauner
2024-03-06 19:59   ` Josef Bacik
2024-03-07 17:39   ` Kent Overstreet
2024-03-08 10:59     ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).