Re: [PATCH 1/2] btrfs: scrub: maintain the unlock order in scrub thread

[Nikolay Borisov <nborisov@suse.com>]

On 26.11.18 г. 11:07 ч., Anand Jain wrote:
> The fs_info::device_list_mutex and fs_info::scrub_lock creates a
> nested locks in btrfs_scrub_dev(). During the lock acquire the
> hierarchy is fs_info::device_list_mutex and then fs_info::scrub_lock,
> so following the same reverse order during unlock, that is
> fs_info::scrub_lock and then fs_info::device_list_mutex.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/scrub.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 902819d3cf41..b1c2d1cdbd4b 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -3865,7 +3865,6 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
>  	}
>  	sctx->readonly = readonly;
>  	dev->scrub_ctx = sctx;
> -	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
>  
>  	/*
>  	 * checking @scrub_pause_req here, we can avoid
> @@ -3875,15 +3874,14 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start,
>  	atomic_inc(&fs_info->scrubs_running);
>  	mutex_unlock(&fs_info->scrub_lock);
>  
> -	if (!is_dev_replace) {
> -		/*
> -		 * by holding device list mutex, we can
> -		 * kick off writing super in log tree sync.
> -		 */
> -		mutex_lock(&fs_info->fs_devices->device_list_mutex);
> +	/*
> +	 * by holding device list mutex, we can kick off writing super in log
> +	 * tree sync.
> +	 */
> +	if (!is_dev_replace)
>  		ret = scrub_supers(sctx, dev);
> -		mutex_unlock(&fs_info->fs_devices->device_list_mutex);
> -	}
> +
> +	mutex_unlock(&fs_info->fs_devices->device_list_mutex);

Have you considered whether this change will have any negative impact
due to the fact that __scrtub_blocked_if_needed can go to sleep for
arbitrary time with device_list_mutex held now ?

>  
>  	if (!ret)
>  		ret = scrub_enumerate_chunks(sctx, dev, start, end);
>