Re: [PATCH 4/6] Btrfs: remove unused check of skip_locking

[Nikolay Borisov <nborisov@suse.com>]

On 15.05.2018 20:52, Liu Bo wrote:
> The check is superfluous since all of callers who set search_for_commit
> also have skip_locking set.

This is false. For example btrfs_qgroup_rescan_worker sets
search_commit_root = 1 but doesn't set skip locking. So either your
assumption is wrong or the code in btrfs_qgroup_rescan_worker has missed
that (which seems more likely).

I think this change necessitates either:

a) ASSERT(p->skip_locking) inside the if (p->search_commit_root) branch

or

b) Unconditionally setting ->skip_locking if we have set
search_commit_root and removing opencoded set of skip_locking alongside
search_commit_root.

I think b) is the better option since it provides "fire and forget"
semantics when you want to search the commit root, since it's only read
only.

> 
> Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
> ---
>  fs/btrfs/ctree.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 399839df7a8f..cf34eca41d4e 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -2623,8 +2623,6 @@ static struct extent_buffer *btrfs_search_slot_get_root(struct btrfs_root *root,
>  		level = btrfs_header_level(b);
>  		if (p->need_commit_sem)
>  			up_read(&fs_info->commit_root_sem);
> -		if (!p->skip_locking)
> -			btrfs_tree_read_lock(b);
>  
>  		goto out;
>  	}
>