2022-10-30 13:37:20

by Dawei Li

[permalink] [raw]
Subject: [PATCH v2] vfs: Make vfs_get_super() internal

For now there are no external callers of vfs_get_super(),
so just make it an internal API.

v1: https://lore.kernel.org/all/TYCP286MB2323D37F4F6400FD07D7C7F7CA319@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM/

v2: move vfs_get_super_keying to super.c, as the suggestion
from Christian Brauner.

base-commit: 3aca47127a646165965ff52803e2b269eed91afc

Signed-off-by: Dawei Li <[email protected]>
---
fs/super.c | 13 +++++++++++--
include/linux/fs_context.h | 14 --------------
2 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 6a82660e1adb..24e31e458552 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1111,6 +1111,16 @@ static int test_single_super(struct super_block *s, struct fs_context *fc)
return 1;
}

+/*
+ * sget() wrappers to be called from the ->get_tree() op.
+ */
+enum vfs_get_super_keying {
+ vfs_get_single_super, /* Only one such superblock may exist */
+ vfs_get_single_reconf_super, /* As above, but reconfigure if it exists */
+ vfs_get_keyed_super, /* Superblocks with different s_fs_info keys may exist */
+ vfs_get_independent_super, /* Multiple independent superblocks may exist */
+};
+
/**
* vfs_get_super - Get a superblock with a search key set in s_fs_info.
* @fc: The filesystem context holding the parameters
@@ -1136,7 +1146,7 @@ static int test_single_super(struct super_block *s, struct fs_context *fc)
* A permissions check is made by sget_fc() unless we're getting a superblock
* for a kernel-internal mount or a submount.
*/
-int vfs_get_super(struct fs_context *fc,
+static int vfs_get_super(struct fs_context *fc,
enum vfs_get_super_keying keying,
int (*fill_super)(struct super_block *sb,
struct fs_context *fc))
@@ -1189,7 +1199,6 @@ int vfs_get_super(struct fs_context *fc,
deactivate_locked_super(sb);
return err;
}
-EXPORT_SYMBOL(vfs_get_super);

int get_tree_nodev(struct fs_context *fc,
int (*fill_super)(struct super_block *sb,
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 13fa6f3df8e4..87a34f2fa68d 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -145,20 +145,6 @@ extern void fc_drop_locked(struct fs_context *fc);
int reconfigure_single(struct super_block *s,
int flags, void *data);

-/*
- * sget() wrappers to be called from the ->get_tree() op.
- */
-enum vfs_get_super_keying {
- vfs_get_single_super, /* Only one such superblock may exist */
- vfs_get_single_reconf_super, /* As above, but reconfigure if it exists */
- vfs_get_keyed_super, /* Superblocks with different s_fs_info keys may exist */
- vfs_get_independent_super, /* Multiple independent superblocks may exist */
-};
-extern int vfs_get_super(struct fs_context *fc,
- enum vfs_get_super_keying keying,
- int (*fill_super)(struct super_block *sb,
- struct fs_context *fc));
-
extern int get_tree_nodev(struct fs_context *fc,
int (*fill_super)(struct super_block *sb,
struct fs_context *fc));
--
2.25.1



2022-10-30 20:45:58

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH v2] vfs: Make vfs_get_super() internal

On Sun, Oct 30, 2022 at 09:06:08PM +0800, Dawei Li wrote:
> For now there are no external callers of vfs_get_super(),
> so just make it an internal API.
>
> v1: https://lore.kernel.org/all/TYCP286MB2323D37F4F6400FD07D7C7F7CA319@TYCP286MB2323.JPNP286.PROD.OUTLOOK.COM/
>
> v2: move vfs_get_super_keying to super.c, as the suggestion
> from Christian Brauner.
>
> base-commit: 3aca47127a646165965ff52803e2b269eed91afc
>
> Signed-off-by: Dawei Li <[email protected]>
> ---
> fs/super.c | 13 +++++++++++--
> include/linux/fs_context.h | 14 --------------
> 2 files changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/fs/super.c b/fs/super.c
> index 6a82660e1adb..24e31e458552 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1111,6 +1111,16 @@ static int test_single_super(struct super_block *s, struct fs_context *fc)
> return 1;
> }
>
> +/*
> + * sget() wrappers to be called from the ->get_tree() op.
> + */
> +enum vfs_get_super_keying {
> + vfs_get_single_super, /* Only one such superblock may exist */
> + vfs_get_single_reconf_super, /* As above, but reconfigure if it exists */
> + vfs_get_keyed_super, /* Superblocks with different s_fs_info keys may exist */
> + vfs_get_independent_super, /* Multiple independent superblocks may exist */
> +};

I would rather kill the "keying" thing completely.

Seriously, what does it buy us? Consider e.g.
return vfs_get_super(fc, vfs_get_independent_super, fill_super);
in get_tree_nodev(). If you expand vfs_get_super() there, you'll get
this:
{
struct super_block *sb = sget_fc(fc, NULL, set_anon_super_fc);
if (IS_ERR(sb))
return PTR_ERR(sb);

if (!sb->s_root) {
int err = fill_super(sb, fc);
if (unlikely(err)) {
deactivate_locked_super(sb);
return err;
}
sb->s_flags |= SB_ACTIVE;
}
fc->root = dget(sb->s_root);
return 0;
}

Sure, it's several lines long, but it's much easier to follow than
vfs_get_super(). And duplication between these vfs_get_super()
callers is, IMO, not worth eliminating...

2022-10-31 08:45:02

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v2] vfs: Make vfs_get_super() internal

On Sun, Oct 30, 2022 at 08:28:39PM +0000, Al Viro wrote:
> I would rather kill the "keying" thing completely.

And I replied with a patch doing just that to his v1. Guess it's time
to resend it formally.