Re: [PATCH 1/8] btrfs: cleanup the locking in btrfs_next_old_leaf

[Filipe Manana <fdmanana@gmail.com>]

On Fri, Nov 6, 2020 at 9:29 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> We are carrying around this next_rw_lock from when we would do spinning
> vs blocking read locks.  Now that we have the rwsem locking we can
> simply use the read lock flag unconditionally and the read lock helpers.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

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

Looks good, thanks.

> ---
>  fs/btrfs/ctree.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index d2d5854d51a7..3a01e6e048c0 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -5327,7 +5327,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>         struct btrfs_key key;
>         u32 nritems;
>         int ret;
> -       int next_rw_lock = 0;
>
>         nritems = btrfs_header_nritems(path->nodes[0]);
>         if (nritems == 0)
> @@ -5337,7 +5336,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>  again:
>         level = 1;
>         next = NULL;
> -       next_rw_lock = 0;
>         btrfs_release_path(path);
>
>         path->keep_locks = 1;
> @@ -5401,12 +5399,11 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>                 }
>
>                 if (next) {
> -                       btrfs_tree_unlock_rw(next, next_rw_lock);
> +                       btrfs_tree_read_unlock(next);
>                         free_extent_buffer(next);
>                 }
>
>                 next = c;
> -               next_rw_lock = path->locks[level];
>                 ret = read_block_for_search(root, path, &next, level,
>                                             slot, &key);
>                 if (ret == -EAGAIN)
> @@ -5437,7 +5434,6 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>                                                        BTRFS_NESTING_RIGHT,
>                                                        path->recurse);
>                         }
> -                       next_rw_lock = BTRFS_READ_LOCK;
>                 }
>                 break;
>         }
> @@ -5446,13 +5442,13 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>                 level--;
>                 c = path->nodes[level];
>                 if (path->locks[level])
> -                       btrfs_tree_unlock_rw(c, path->locks[level]);
> +                       btrfs_tree_read_unlock(c);
>
>                 free_extent_buffer(c);
>                 path->nodes[level] = next;
>                 path->slots[level] = 0;
>                 if (!path->skip_locking)
> -                       path->locks[level] = next_rw_lock;
> +                       path->locks[level] = BTRFS_READ_LOCK;
>                 if (!level)
>                         break;
>
> @@ -5466,11 +5462,9 @@ int btrfs_next_old_leaf(struct btrfs_root *root, struct btrfs_path *path,
>                         goto done;
>                 }
>
> -               if (!path->skip_locking) {
> +               if (!path->skip_locking)
>                         __btrfs_tree_read_lock(next, BTRFS_NESTING_RIGHT,
>                                                path->recurse);
> -                       next_rw_lock = BTRFS_READ_LOCK;
> -               }
>         }
>         ret = 0;
>  done:
> --
> 2.26.2
>


-- 
Filipe David Manana,

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