From: Neal Gompa <neal@gompa.dev>
To: Kent Overstreet <kent.overstreet@linux.dev>
Cc: linux-fsdevel@vger.kernel.org, linux-bcachefs@vger.kernel.org,
linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
Josef Bacik <josef@toxicpanda.com>,
Miklos Szeredi <mszeredi@redhat.com>,
Christian Brauner <brauner@kernel.org>,
David Howells <dhowells@redhat.com>
Subject: Re: [PATCH v2] statx: stx_subvol
Date: Fri, 8 Mar 2024 06:42:27 -0500 [thread overview]
Message-ID: <CAEg-Je96OKs_LOXorNVj1a1=e+1f=-gw34v4VWNOmfKXc6PLSQ@mail.gmail.com> (raw)
In-Reply-To: <20240308022914.196982-1-kent.overstreet@linux.dev>
On Thu, Mar 7, 2024 at 9:29 PM Kent Overstreet
<kent.overstreet@linux.dev> wrote:
>
> Add a new statx field for (sub)volume identifiers, as implemented by
> btrfs and bcachefs.
>
> 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>
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> ---
> 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..6a542ed43e2c 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->subvol = inode->ei_subvol;
> + stat->result_mask |= STATX_SUBVOL;
> +
> 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..70bd3e888cfa 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_subvol = stat->subvol;
>
> return copy_to_user(buffer, &tmp, sizeof(tmp)) ? -EFAULT : 0;
> }
> diff --git a/include/linux/stat.h b/include/linux/stat.h
> index 52150570d37a..bf92441dbad2 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 subvol;
> };
>
> /* 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..67626d535316 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_subvol; /* 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_SUBVOL 0x00008000U /* Want/got stx_subvol */
>
> #define STATX__RESERVED 0x80000000U /* Reserved for future struct statx expansion */
>
> --
> 2.43.0
>
>
I think it's generally expected that patches that touch different
layers are split up. That is, we should have a patch that adds the
capability and a separate patch that enables it in bcachefs. This also
helps make it clearer to others how a new feature should be plumbed
into a filesystem.
I would prefer it to be split up in this manner for this reason.
--
真実はいつも一つ!/ Always, there's only one truth!
next prev parent reply other threads:[~2024-03-08 11:43 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-08 2:29 [PATCH v2] statx: stx_subvol Kent Overstreet
2024-03-08 11:42 ` Neal Gompa [this message]
2024-03-08 16:34 ` Kent Overstreet
2024-03-08 16:44 ` Neal Gompa
2024-03-08 16:48 ` Kent Overstreet
2024-03-08 16:56 ` Darrick J. Wong
2024-03-08 17:13 ` Kent Overstreet
2024-03-09 11:46 ` Jeff Layton
2024-03-09 12:15 ` Kent Overstreet
2024-03-11 2:17 ` Dave Chinner
2024-03-11 5:30 ` Miklos Szeredi
2024-03-11 5:49 ` Kent Overstreet
2024-03-11 13:42 ` Christian Brauner
2024-03-11 8:12 ` Johannes Thumshirn
2024-03-11 13:43 ` Christian Brauner
2024-03-11 20:15 ` Kent Overstreet
2024-03-12 14:27 ` Christian Brauner
2024-03-11 22:43 ` David Sterba
2024-03-12 14:27 ` Christian Brauner
2024-03-12 17:17 ` Neal Gompa
2024-03-12 2:13 ` Eric Biggers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAEg-Je96OKs_LOXorNVj1a1=e+1f=-gw34v4VWNOmfKXc6PLSQ@mail.gmail.com' \
--to=neal@gompa.dev \
--cc=brauner@kernel.org \
--cc=dhowells@redhat.com \
--cc=josef@toxicpanda.com \
--cc=kent.overstreet@linux.dev \
--cc=linux-bcachefs@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mszeredi@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).