Re: [PATCH 4/5] fs: don't make sync(2) wait for unborn superblocks

[Jan Kara]

On Wed 21-10-26 16:47:00, Josef Bacik wrote:
> A mount which is stuck reading from its device or waiting for its server
> hangs every sync(2) on the machine, and the emergency sync as well, and
> they can't be killed.
> 
> A superblock is on the list sync(2) walks from sget_fc() on, but isn't
> usable until vfs_get_tree() sets SB_BORN, and super_lock() waits for that
> uninterruptibly.
> 
> There is nothing in that wait for them.  sync(2) is about what was written
> before it was called, and nobody can have written to a filesystem which
> isn't mounted yet.  What the filesystem writes on its own while it is being
> mounted, replaying a journal say, is the mount's business: it has to get
> that to disk whether or not anybody calls sync(2).
> 
> Add a flag for __iterate_supers() to skip superblocks without SB_BORN and
> pass it from iterate_supers_sync().
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/super.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/super.c b/fs/super.c
> index ecba64ed0c2a..b8d6f1c969ce 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1041,6 +1041,7 @@ enum super_iter_flags_t {
>  	SUPER_ITER_UNLOCKED	= (1U << 1),
>  	SUPER_ITER_REVERSE	= (1U << 2),
>  	SUPER_ITER_WAIT_SHUTDOWN = (1U << 3),
> +	SUPER_ITER_SKIP_UNBORN	= (1U << 4),
>  };
>  
>  static inline struct super_block *first_super(enum super_iter_flags_t flags)
> @@ -1073,6 +1074,10 @@ static int __iterate_supers(void (*f)(struct super_block *, void *), void *arg,
>  		if (super_flags(sb, SB_DYING))
>  			continue;
>  
> +		if ((flags & SUPER_ITER_SKIP_UNBORN) &&
> +		    !super_flags(sb, SB_BORN))
> +			continue;
> +
>  		if (!refcount_inc_not_zero(&sb->s_passive))
>  			continue;
>  
> @@ -1114,12 +1119,16 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
>   * down writes it out.  Wait for that, so that it is on disk as well by the time
>   * the walk is over.
>   *
> + * A superblock which is still being set up isn't passed to @f either, and
> + * isn't waited for.  Nobody can have written to it yet.
> + *
>   * Return: 0 if all the superblocks were seen, -ERESTARTSYS if the task was
>   *         killed while it waited for one.
>   */
>  int iterate_supers_sync(void (*f)(struct super_block *, void *), void *arg)
>  {
> -	return __iterate_supers(f, arg, SUPER_ITER_WAIT_SHUTDOWN);
> +	return __iterate_supers(f, arg, SUPER_ITER_WAIT_SHUTDOWN |
> +					SUPER_ITER_SKIP_UNBORN);
>  }
>  
>  /**
> 
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR