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@xxxxxxxxxxxxxxxxx> > --- > 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; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
