On Wed, Mar 24, 2021 at 2:26 PM Naohiro Aota <naohiro.a...@wdc.com> wrote:
>
> Commit 6e37d2459941 ("btrfs: zoned: fix deadlock on log sync") pointed out
> a deadlock warning and removed
> mutex_{lock,unlock}(&fs_info->tree_root->log_mutex). While it looks like it
> always cause a deadlock, we didn't see actual deadlock in fstests runs. The
> reason is log_root_tree->log_mutex != fs_info->tree_root->log_mutex, not
> taking the same lock. So, the warning was actually a false-positive.
>
> Since btrfs_alloc_log_tree_node() is protected only by
> fs_info->tree_root->log_mutex, we can (and should) move the code out of the
> lock scope of log_root_tree->log_mutex and silence the warning.
>
> Cc: Filipe Manana <fdman...@suse.com>
> Cc: Johannes Thumshirn <johannes.thumsh...@wdc.com>
> Signed-off-by: Naohiro Aota <naohiro.a...@wdc.com>

Reviewed-by: Filipe Manana <fdman...@suse.com>

Looks good, thanks.

> ---
>  fs/btrfs/tree-log.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 92a368627791..72c4b66ed516 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -3165,20 +3165,22 @@ int btrfs_sync_log(struct btrfs_trans_handle *trans,
>          */
>         mutex_unlock(&root->log_mutex);
>
> -       btrfs_init_log_ctx(&root_log_ctx, NULL);
> -
> -       mutex_lock(&log_root_tree->log_mutex);
> -
>         if (btrfs_is_zoned(fs_info)) {
> +               mutex_lock(&fs_info->tree_root->log_mutex);
>                 if (!log_root_tree->node) {
>                         ret = btrfs_alloc_log_tree_node(trans, log_root_tree);
>                         if (ret) {
> -                               mutex_unlock(&log_root_tree->log_mutex);
> +                               mutex_unlock(&fs_info->tree_log_mutex);
>                                 goto out;
>                         }
>                 }
> +               mutex_unlock(&fs_info->tree_root->log_mutex);
>         }
>
> +       btrfs_init_log_ctx(&root_log_ctx, NULL);
> +
> +       mutex_lock(&log_root_tree->log_mutex);
> +
>         index2 = log_root_tree->log_transid % 2;
>         list_add_tail(&root_log_ctx.list, &log_root_tree->log_ctxs[index2]);
>         root_log_ctx.log_transid = log_root_tree->log_transid;
> --
> 2.31.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Reply via email to