On Fri, Apr 27, 2018 at 2:06 AM, David Sterba <dste...@suse.cz> wrote:
> On Thu, Apr 26, 2018 at 09:27:43AM +0800, Liu Bo wrote:
>> path->keep_lock may force to lock tree root and higher nodes, and make
>> lock contention worse, thus it needs to be avoided as much as
>> possible.
>>
>> In btrfs_degrag_leaves, path->keep_lock is set but @path immediatley
>> gets released, which is not necessary at all.
>
> So what is the effect of this? The locks are held throughout
> btrfs_search_forward and released after that. Thi
>
>> Signed-off-by: Liu Bo <bo....@linux.alibaba.com>
>> ---
>> v2: update commit log with more details.
>>
>>  fs/btrfs/tree-defrag.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-defrag.c b/fs/btrfs/tree-defrag.c
>> index 3c0987ab587d..c12747904d4c 100644
>> --- a/fs/btrfs/tree-defrag.c
>> +++ b/fs/btrfs/tree-defrag.c
>> @@ -65,8 +65,6 @@ int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
>>               memcpy(&key, &root->defrag_progress, sizeof(key));
>>       }
>>
>> -     path->keep_locks = 1;
>> -
>>       ret = btrfs_search_forward(root, &key, path, BTRFS_OLDEST_GENERATION);
>
> What does btrfs_search_forward do as the first statement:
>
> 5115 int btrfs_search_forward(struct btrfs_root *root, struct btrfs_key 
> *min_key,
> 5116                          struct btrfs_path *path,
> 5117                          u64 min_trans)
> 5118 {
> .... declarations
> 5128
> 5129         path->keep_locks = 1;
>
> So even if removed from above, there will be no change. The value of
> keep_locks is preserved after btrfs_path_release.
>

It will set it back,

out:
        path->keep_locks = keep_locks;


thanks,
liubo
>>       if (ret < 0)
>>               goto out;
>> @@ -81,6 +79,7 @@ int btrfs_defrag_leaves(struct btrfs_trans_handle *trans,
>>        * a deadlock (attempting to write lock an already write locked leaf).
>>        */
>>       path->lowest_level = 1;
>> +     path->keep_locks = 1;
>>       wret = btrfs_search_slot(trans, root, &key, path, 0, 1);
>>
>>       if (wret < 0) {
>> --
>> 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
> --
> 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

Reply via email to