On Wed, Mar 24, 2021 at 2:26 PM Naohiro Aota 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
> Cc: Johannes Thumshirn
> Signed-off-by: Naohiro Aota
Reviewed-by: Filipe Manana
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.”
