| Message ID | 20241009033151.2334888-1-hsiangkao@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] fs/super.c: introduce get_tree_bdev_flags() | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed 09-10-24 11:31:50, Gao Xiang wrote: > As Allison reported [1], currently get_tree_bdev() will store > "Can't lookup blockdev" error message. Although it makes sense for > pure bdev-based fses, this message may mislead users who try to use > EROFS file-backed mounts since get_tree_nodev() is used as a fallback > then. > > Add get_tree_bdev_flags() to specify extensible flags [2] and > GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message > since it's misleading to EROFS file-backed mounts now. > > [1] https://lore.kernel.org/r/CAOYeF9VQ8jKVmpy5Zy9DNhO6xmWSKMB-DO8yvBB0XvBE7=3Ugg@mail.gmail.com > [2] https://lore.kernel.org/r/ZwUkJEtwIpUA4qMz@infradead.org > Suggested-by: Christoph Hellwig <hch@infradead.org> > Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > v1: https://lore.kernel.org/r/20241008095606.990466-1-hsiangkao@linux.alibaba.com > change since v1: > - add get_tree_bdev_flags() suggested by Christoph. > > fs/super.c | 26 ++++++++++++++++++++------ > include/linux/fs_context.h | 6 ++++++ > 2 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/fs/super.c b/fs/super.c > index 1db230432960..c9c7223bc2a2 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -1596,13 +1596,14 @@ int setup_bdev_super(struct super_block *sb, int sb_flags, > EXPORT_SYMBOL_GPL(setup_bdev_super); > > /** > - * get_tree_bdev - Get a superblock based on a single block device > + * get_tree_bdev_flags - Get a superblock based on a single block device > * @fc: The filesystem context holding the parameters > * @fill_super: Helper to initialise a new superblock > + * @flags: GET_TREE_BDEV_* flags > */ > -int get_tree_bdev(struct fs_context *fc, > - int (*fill_super)(struct super_block *, > - struct fs_context *)) > +int get_tree_bdev_flags(struct fs_context *fc, > + int (*fill_super)(struct super_block *sb, > + struct fs_context *fc), unsigned int flags) > { > struct super_block *s; > int error = 0; > @@ -1613,10 +1614,10 @@ int get_tree_bdev(struct fs_context *fc, > > error = lookup_bdev(fc->source, &dev); > if (error) { > - errorf(fc, "%s: Can't lookup blockdev", fc->source); > + if (!(flags & GET_TREE_BDEV_QUIET_LOOKUP)) > + errorf(fc, "%s: Can't lookup blockdev", fc->source); > return error; > } > - > fc->sb_flags |= SB_NOSEC; > s = sget_dev(fc, dev); > if (IS_ERR(s)) > @@ -1644,6 +1645,19 @@ int get_tree_bdev(struct fs_context *fc, > fc->root = dget(s->s_root); > return 0; > } > +EXPORT_SYMBOL_GPL(get_tree_bdev_flags); > + > +/** > + * get_tree_bdev - Get a superblock based on a single block device > + * @fc: The filesystem context holding the parameters > + * @fill_super: Helper to initialise a new superblock > + */ > +int get_tree_bdev(struct fs_context *fc, > + int (*fill_super)(struct super_block *, > + struct fs_context *)) > +{ > + return get_tree_bdev_flags(fc, fill_super, 0); > +} > EXPORT_SYMBOL(get_tree_bdev); > > static int test_bdev_super(struct super_block *s, void *data) > diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h > index c13e99cbbf81..4b4bfef6f053 100644 > --- a/include/linux/fs_context.h > +++ b/include/linux/fs_context.h > @@ -160,6 +160,12 @@ extern int get_tree_keyed(struct fs_context *fc, > > int setup_bdev_super(struct super_block *sb, int sb_flags, > struct fs_context *fc); > + > +#define GET_TREE_BDEV_QUIET_LOOKUP 0x0001 > +int get_tree_bdev_flags(struct fs_context *fc, > + int (*fill_super)(struct super_block *sb, > + struct fs_context *fc), unsigned int flags); > + > extern int get_tree_bdev(struct fs_context *fc, > int (*fill_super)(struct super_block *sb, > struct fs_context *fc)); > -- > 2.43.5 >
On Wed, 09 Oct 2024 11:31:50 +0800, Gao Xiang wrote: > As Allison reported [1], currently get_tree_bdev() will store > "Can't lookup blockdev" error message. Although it makes sense for > pure bdev-based fses, this message may mislead users who try to use > EROFS file-backed mounts since get_tree_nodev() is used as a fallback > then. > > Add get_tree_bdev_flags() to specify extensible flags [2] and > GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message > since it's misleading to EROFS file-backed mounts now. > > [...] Applied to the vfs.misc branch of the vfs/vfs.git tree. Patches in the vfs.misc branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.misc [1/2] fs/super.c: introduce get_tree_bdev_flags() https://git.kernel.org/vfs/vfs/c/f54acb32dff2 [2/2] erofs: use get_tree_bdev_flags() to avoid misleading messages https://git.kernel.org/vfs/vfs/c/83e6e973d9c9
Hi Christian, On 2024/10/10 17:48, Christian Brauner wrote: > On Wed, 09 Oct 2024 11:31:50 +0800, Gao Xiang wrote: >> As Allison reported [1], currently get_tree_bdev() will store >> "Can't lookup blockdev" error message. Although it makes sense for >> pure bdev-based fses, this message may mislead users who try to use >> EROFS file-backed mounts since get_tree_nodev() is used as a fallback >> then. >> >> Add get_tree_bdev_flags() to specify extensible flags [2] and >> GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message >> since it's misleading to EROFS file-backed mounts now. >> >> [...] > > Applied to the vfs.misc branch of the vfs/vfs.git tree. > Patches in the vfs.misc branch should appear in linux-next soon. > > Please report any outstanding bugs that were missed during review in a > new review to the original patch series allowing us to drop it. > > It's encouraged to provide Acked-bys and Reviewed-bys even though the > patch has now been applied. If possible patch trailers will be updated. > > Note that commit hashes shown below are subject to change due to rebase, > trailer updates or similar. If in doubt, please check the listed branch. > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git > branch: vfs.misc > > [1/2] fs/super.c: introduce get_tree_bdev_flags() > https://git.kernel.org/vfs/vfs/c/f54acb32dff2 > [2/2] erofs: use get_tree_bdev_flags() to avoid misleading messages > https://git.kernel.org/vfs/vfs/c/83e6e973d9c9 Anyway, I'm not sure what's your thoughts about this, so I try to write an email again. As Allison suggested in the email [1], "..so probably it should get fixed before the final release.". Although I'm pretty fine to leave it in "vfs.misc" for the next merge window (6.13) instead, it could cause an unnecessary backport to the stable kernel. Or if there is some other potential concern about these two patches? Also I hope my previous reply about a redundant blank line removal in the first patch might be useful too [2]. [1] https://lore.kernel.org/r/CAOYeF9VQ8jKVmpy5Zy9DNhO6xmWSKMB-DO8yvBB0XvBE7=3Ugg@mail.gmail.com [2] https://lore.kernel.org/r/8ec1896f-93da-4eca-ab69-8ae9d1645181@linux.alibaba.com Thanks, Gao Xiang
On Mon, Oct 21, 2024 at 03:54:12PM +0800, Gao Xiang wrote: > Hi Christian, > > On 2024/10/10 17:48, Christian Brauner wrote: > > On Wed, 09 Oct 2024 11:31:50 +0800, Gao Xiang wrote: > > > As Allison reported [1], currently get_tree_bdev() will store > > > "Can't lookup blockdev" error message. Although it makes sense for > > > pure bdev-based fses, this message may mislead users who try to use > > > EROFS file-backed mounts since get_tree_nodev() is used as a fallback > > > then. > > > > > > Add get_tree_bdev_flags() to specify extensible flags [2] and > > > GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message > > > since it's misleading to EROFS file-backed mounts now. > > > > > > [...] > > > > Applied to the vfs.misc branch of the vfs/vfs.git tree. > > Patches in the vfs.misc branch should appear in linux-next soon. > > > > Please report any outstanding bugs that were missed during review in a > > new review to the original patch series allowing us to drop it. > > > > It's encouraged to provide Acked-bys and Reviewed-bys even though the > > patch has now been applied. If possible patch trailers will be updated. > > > > Note that commit hashes shown below are subject to change due to rebase, > > trailer updates or similar. If in doubt, please check the listed branch. > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git > > branch: vfs.misc > > > > [1/2] fs/super.c: introduce get_tree_bdev_flags() > > https://git.kernel.org/vfs/vfs/c/f54acb32dff2 > > [2/2] erofs: use get_tree_bdev_flags() to avoid misleading messages > > https://git.kernel.org/vfs/vfs/c/83e6e973d9c9 > > Anyway, I'm not sure what's your thoughts about this, so I try to > write an email again. > > As Allison suggested in the email [1], "..so probably it should get > fixed before the final release.". Although I'm pretty fine to leave > it in "vfs.misc" for the next merge window (6.13) instead, it could > cause an unnecessary backport to the stable kernel. Oh, the file changes have been merged during the v6.12 merge window? Sorry, that wasn't clear. Well, this is a bit annoying but yes, we can get that fixed upstream then. I'll move it to vfs.fixes...
On Wed, 09 Oct 2024 11:31:50 +0800, Gao Xiang wrote: > As Allison reported [1], currently get_tree_bdev() will store > "Can't lookup blockdev" error message. Although it makes sense for > pure bdev-based fses, this message may mislead users who try to use > EROFS file-backed mounts since get_tree_nodev() is used as a fallback > then. > > Add get_tree_bdev_flags() to specify extensible flags [2] and > GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message > since it's misleading to EROFS file-backed mounts now. > > [...] Applied to the vfs.fixes branch of the vfs/vfs.git tree. Patches in the vfs.fixes branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.fixes [1/2] fs/super.c: introduce get_tree_bdev_flags() https://git.kernel.org/vfs/vfs/c/4021e685139d [2/2] erofs: use get_tree_bdev_flags() to avoid misleading messages https://git.kernel.org/vfs/vfs/c/14c2d97265ea
On 2024/10/21 20:27, Christian Brauner wrote: > On Mon, Oct 21, 2024 at 03:54:12PM +0800, Gao Xiang wrote: >> Hi Christian, >> >> On 2024/10/10 17:48, Christian Brauner wrote: >>> On Wed, 09 Oct 2024 11:31:50 +0800, Gao Xiang wrote: >>>> As Allison reported [1], currently get_tree_bdev() will store >>>> "Can't lookup blockdev" error message. Although it makes sense for >>>> pure bdev-based fses, this message may mislead users who try to use >>>> EROFS file-backed mounts since get_tree_nodev() is used as a fallback >>>> then. >>>> >>>> Add get_tree_bdev_flags() to specify extensible flags [2] and >>>> GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message >>>> since it's misleading to EROFS file-backed mounts now. >>>> >>>> [...] >>> >>> Applied to the vfs.misc branch of the vfs/vfs.git tree. >>> Patches in the vfs.misc branch should appear in linux-next soon. >>> >>> Please report any outstanding bugs that were missed during review in a >>> new review to the original patch series allowing us to drop it. >>> >>> It's encouraged to provide Acked-bys and Reviewed-bys even though the >>> patch has now been applied. If possible patch trailers will be updated. >>> >>> Note that commit hashes shown below are subject to change due to rebase, >>> trailer updates or similar. If in doubt, please check the listed branch. >>> >>> tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git >>> branch: vfs.misc >>> >>> [1/2] fs/super.c: introduce get_tree_bdev_flags() >>> https://git.kernel.org/vfs/vfs/c/f54acb32dff2 >>> [2/2] erofs: use get_tree_bdev_flags() to avoid misleading messages >>> https://git.kernel.org/vfs/vfs/c/83e6e973d9c9 >> >> Anyway, I'm not sure what's your thoughts about this, so I try to >> write an email again. >> >> As Allison suggested in the email [1], "..so probably it should get >> fixed before the final release.". Although I'm pretty fine to leave >> it in "vfs.misc" for the next merge window (6.13) instead, it could >> cause an unnecessary backport to the stable kernel. > > Oh, the file changes have been merged during the v6.12 merge window? > Sorry, that wasn't clear. > > Well, this is a bit annoying but yes, we can get that fixed upstream > then. I'll move it to vfs.fixes... Many thanks for the reply! Yeah, the feature is already usable [1] and it will be used for several use cases, yet the unexpected message might be confusing. Anyway, both options are fine to me, but "vfs.fixes" may be better to avoid unnecesary backporting. [1] https://lwn.net/Articles/990750 Thanks, Gao Xiang
diff --git a/fs/super.c b/fs/super.c index 1db230432960..c9c7223bc2a2 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1596,13 +1596,14 @@ int setup_bdev_super(struct super_block *sb, int sb_flags, EXPORT_SYMBOL_GPL(setup_bdev_super); /** - * get_tree_bdev - Get a superblock based on a single block device + * get_tree_bdev_flags - Get a superblock based on a single block device * @fc: The filesystem context holding the parameters * @fill_super: Helper to initialise a new superblock + * @flags: GET_TREE_BDEV_* flags */ -int get_tree_bdev(struct fs_context *fc, - int (*fill_super)(struct super_block *, - struct fs_context *)) +int get_tree_bdev_flags(struct fs_context *fc, + int (*fill_super)(struct super_block *sb, + struct fs_context *fc), unsigned int flags) { struct super_block *s; int error = 0; @@ -1613,10 +1614,10 @@ int get_tree_bdev(struct fs_context *fc, error = lookup_bdev(fc->source, &dev); if (error) { - errorf(fc, "%s: Can't lookup blockdev", fc->source); + if (!(flags & GET_TREE_BDEV_QUIET_LOOKUP)) + errorf(fc, "%s: Can't lookup blockdev", fc->source); return error; } - fc->sb_flags |= SB_NOSEC; s = sget_dev(fc, dev); if (IS_ERR(s)) @@ -1644,6 +1645,19 @@ int get_tree_bdev(struct fs_context *fc, fc->root = dget(s->s_root); return 0; } +EXPORT_SYMBOL_GPL(get_tree_bdev_flags); + +/** + * get_tree_bdev - Get a superblock based on a single block device + * @fc: The filesystem context holding the parameters + * @fill_super: Helper to initialise a new superblock + */ +int get_tree_bdev(struct fs_context *fc, + int (*fill_super)(struct super_block *, + struct fs_context *)) +{ + return get_tree_bdev_flags(fc, fill_super, 0); +} EXPORT_SYMBOL(get_tree_bdev); static int test_bdev_super(struct super_block *s, void *data) diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h index c13e99cbbf81..4b4bfef6f053 100644 --- a/include/linux/fs_context.h +++ b/include/linux/fs_context.h @@ -160,6 +160,12 @@ extern int get_tree_keyed(struct fs_context *fc, int setup_bdev_super(struct super_block *sb, int sb_flags, struct fs_context *fc); + +#define GET_TREE_BDEV_QUIET_LOOKUP 0x0001 +int get_tree_bdev_flags(struct fs_context *fc, + int (*fill_super)(struct super_block *sb, + struct fs_context *fc), unsigned int flags); + extern int get_tree_bdev(struct fs_context *fc, int (*fill_super)(struct super_block *sb, struct fs_context *fc));
As Allison reported [1], currently get_tree_bdev() will store "Can't lookup blockdev" error message. Although it makes sense for pure bdev-based fses, this message may mislead users who try to use EROFS file-backed mounts since get_tree_nodev() is used as a fallback then. Add get_tree_bdev_flags() to specify extensible flags [2] and GET_TREE_BDEV_QUIET_LOOKUP to silence "Can't lookup blockdev" message since it's misleading to EROFS file-backed mounts now. [1] https://lore.kernel.org/r/CAOYeF9VQ8jKVmpy5Zy9DNhO6xmWSKMB-DO8yvBB0XvBE7=3Ugg@mail.gmail.com [2] https://lore.kernel.org/r/ZwUkJEtwIpUA4qMz@infradead.org Suggested-by: Christoph Hellwig <hch@infradead.org> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com> --- v1: https://lore.kernel.org/r/20241008095606.990466-1-hsiangkao@linux.alibaba.com change since v1: - add get_tree_bdev_flags() suggested by Christoph. fs/super.c | 26 ++++++++++++++++++++------ include/linux/fs_context.h | 6 ++++++ 2 files changed, 26 insertions(+), 6 deletions(-)