| Message ID | 20240302220203.623614-1-kent.overstreet@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | statx: stx_vol | expand |
>>>>> "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
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 >
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
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?
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.
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 */
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(-)