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

2018-05-28 Thread Nikolay Borisov


On 28.05.2018 17:21, David Sterba wrote:
> On Fri, May 18, 2018 at 01:27:50PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年05月18日 11:00, Liu Bo wrote:
>>> The check is superfluous since all of callers who set search_for_commit
>>> also have skip_locking set.
>>>
>>> Signed-off-by: Liu Bo 
>>
>> Reviewed-by: Qu Wenruo 
>>
>> Although more obvious comment about search_commit_root and skip_locking
>> in ctree.h will be much better.
> 
> Not only a comment but also an ASSERT, this is too easy to get wrong.
> That all currenct callers do search_commit_root + skip_locking will not
> catch any future callers. And there was an example reported.

How about my initial suggestion of setting skip)_locking in
btrfs_search_slot if we see commit_root set? Let's try and make the life
of callers as easier as possible.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2018-05-28 Thread David Sterba
On Fri, May 18, 2018 at 01:27:50PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年05月18日 11:00, Liu Bo wrote:
> > The check is superfluous since all of callers who set search_for_commit
> > also have skip_locking set.
> > 
> > Signed-off-by: Liu Bo 
> 
> Reviewed-by: Qu Wenruo 
> 
> Although more obvious comment about search_commit_root and skip_locking
> in ctree.h will be much better.

Not only a comment but also an ASSERT, this is too easy to get wrong.
That all currenct callers do search_commit_root + skip_locking will not
catch any future callers. And there was an example reported.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2018-05-17 Thread Qu Wenruo


On 2018年05月18日 11:00, Liu Bo wrote:
> The check is superfluous since all of callers who set search_for_commit
> also have skip_locking set.
> 
> Signed-off-by: Liu Bo 

Reviewed-by: Qu Wenruo 

Although more obvious comment about search_commit_root and skip_locking
in ctree.h will be much better.

Thanks,
Qu

> ---
>  fs/btrfs/ctree.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index d12fc0474e21..8d3b09038f37 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;
>   }
> 



signature.asc
Description: OpenPGP digital signature


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

2018-05-17 Thread Liu Bo
The check is superfluous since all of callers who set search_for_commit
also have skip_locking set.

Signed-off-by: Liu Bo 
---
 fs/btrfs/ctree.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index d12fc0474e21..8d3b09038f37 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;
}
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html