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. >
FYI, btrfs_search_forward() doesn't need keep_locks's semantics as all of its callers only access path->nodes[0], thus I'm planning to remove keep_locks setting inside it, too. 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