Re: [PATCH] vfs: Pre-allocate superblock in sget_fc() if !test

From: Christian Brauner
Date: Mon Jan 24 2022 - 06:38:08 EST


On Fri, Jan 21, 2022 at 01:52:55PM -0500, Waiman Long wrote:
> When the test function is not defined in sget_fc(), we always need
> to allocate a new superblock. So there is no point in acquiring the
> sb_lock twice in this case. Optimize the !test case by pre-allocating
> the superblock first before acquring the lock.
>
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> ---
> fs/super.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/super.c b/fs/super.c
> index a6405d44d4ca..c2bd5c34a826 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -520,6 +520,8 @@ struct super_block *sget_fc(struct fs_context *fc,
> struct user_namespace *user_ns = fc->global ? &init_user_ns : fc->user_ns;
> int err;
>
> + if (!test)
> + s = alloc_super(fc->fs_type, fc->sb_flags, user_ns);

Shouldn't we treat this allocation failure as "fatal" right away and not
bother taking locks, walking lists and so on? Seems strange to treat it
as fatal below but not here.

(The code-flow in here has always been a bit challenging to follow imho.
So not super keen to see more special-cases in there. Curious: do you
see any noticeable performance impact from that lock being taken and
dropped for the !test case?)