Re: [PATCH 5/5] fs: skip unborn superblocks in drop_caches and Q_SYNC

[Jan Kara]

On Wed 21-10-26 16:47:01, Josef Bacik wrote:
> Writing to /proc/sys/vm/drop_caches and quotactl(Q_SYNC) without a device
> walk all the superblocks with iterate_supers(), and like sync(2) they hang,
> unkillably, on one whose mount is stuck before SB_BORN.
> 
> Neither gets anything out of the wait.  A filesystem which isn't mounted
> yet has no page cache anybody would miss.  It may well have quota turned
> on, ext4 and XFS do that while they are being mounted, but nobody has used
> it yet.  The only changes to the quota so far are the filesystem's own,
> from cleaning up orphans say, and it has to get those to disk whether or
> not anybody calls Q_SYNC.
> 
> Add iterate_supers_born(), which skips such superblocks, and use it for
> the two.
> 
> iterate_supers() itself keeps waiting.  Its remaining user needs that:
> selinux_complete_init() sets up the superblocks which came before the
> policy was loaded.  NFS calls security_sb_set_mnt_opts() from its
> ->get_tree(), so before SB_BORN, and without a policy SELinux puts the work
> off until that walk.  Skipping the superblock there would leave it without
> labels for good.
> 
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Makes sense. Feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

								Honza

> ---
>  fs/drop_caches.c |  2 +-
>  fs/internal.h    |  1 +
>  fs/quota/quota.c |  2 +-
>  fs/super.c       | 15 +++++++++++++++
>  4 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/drop_caches.c b/fs/drop_caches.c
> index 49f56a598ecb..7ba726837075 100644
> --- a/fs/drop_caches.c
> +++ b/fs/drop_caches.c
> @@ -61,7 +61,7 @@ static int drop_caches_sysctl_handler(const struct ctl_table *table, int write,
>  
>  		if (sysctl_drop_caches & 1) {
>  			lru_add_drain_all();
> -			iterate_supers(drop_pagecache_sb, NULL);
> +			iterate_supers_born(drop_pagecache_sb, NULL);
>  			count_vm_event(DROP_PAGECACHE);
>  		}
>  		if (sysctl_drop_caches & 2) {
> diff --git a/fs/internal.h b/fs/internal.h
> index 71ab0ae11941..dd29e98e05fb 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -137,6 +137,7 @@ extern int reconfigure_super(struct fs_context *);
>  extern bool super_trylock_shared(struct super_block *sb);
>  struct super_block *user_get_super(dev_t, bool excl);
>  void put_super(struct super_block *sb);
> +void iterate_supers_born(void (*f)(struct super_block *, void *), void *arg);
>  int iterate_supers_sync(void (*f)(struct super_block *, void *), void *arg);
>  void __init super_dev_init(void);
>  extern bool mount_capable(struct fs_context *);
> diff --git a/fs/quota/quota.c b/fs/quota/quota.c
> index 33bacd707580..713e66e4d00f 100644
> --- a/fs/quota/quota.c
> +++ b/fs/quota/quota.c
> @@ -66,7 +66,7 @@ static int quota_sync_all(int type)
>  
>  	ret = security_quotactl(Q_SYNC, type, 0, NULL);
>  	if (!ret)
> -		iterate_supers(quota_sync_one, &type);
> +		iterate_supers_born(quota_sync_one, &type);
>  	return ret;
>  }
>  
> diff --git a/fs/super.c b/fs/super.c
> index b8d6f1c969ce..1cfacd71a0d8 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1110,6 +1110,21 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg)
>  	__iterate_supers(f, arg, 0);
>  }
>  
> +/**
> + * iterate_supers_born - iterate_supers() without the wait
> + * @f: function to call
> + * @arg: argument to pass to it
> + *
> + * Doesn't wait for superblocks which are still being set up, they are not
> + * passed to @f.  For callers which are after what has piled up in a filesystem
> + * in use.  Not for callers which have to get at every superblock there is: one
> + * which is skipped may be in use a moment later.
> + */
> +void iterate_supers_born(void (*f)(struct super_block *, void *), void *arg)
> +{
> +	__iterate_supers(f, arg, SUPER_ITER_SKIP_UNBORN);
> +}
> +
>  /**
>   * iterate_supers_sync - iterate_supers() for syncing all filesystems
>   * @f: function to call
> 
> -- 
> 2.55.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR