| Message ID | 20200221131124.24105-2-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] btrfs: Remove superflous lock acquisition in __del_reloc_root | expand |
On 2020/2/21 下午9:11, Nikolay Borisov wrote: > The function always works on a local copy of the reloc root list, which > cannot be modified outside of it so using list_for_each_entry is fine. > Additionally the macro handles empty lists so drop list_empty checks of > callers. No semantic changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/relocation.c | 16 +++++----------- > 1 file changed, 5 insertions(+), 11 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index e5cb64409f7c..f13b79adf6b0 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2523,13 +2523,10 @@ int prepare_to_merge(struct reloc_control *rc, int err) > static noinline_for_stack > void free_reloc_roots(struct list_head *list) > { > - struct btrfs_root *reloc_root; > + struct btrfs_root *reloc_root, *tmp; > > - while (!list_empty(list)) { > - reloc_root = list_entry(list->next, struct btrfs_root, > - root_list); > + list_for_each_entry_safe(reloc_root, tmp, list, root_list) > __del_reloc_root(reloc_root); > - } > } > > static noinline_for_stack > @@ -2588,15 +2585,13 @@ void merge_reloc_roots(struct reloc_control *rc) > out: > if (ret) { > btrfs_handle_fs_error(fs_info, ret, NULL); > - if (!list_empty(&reloc_roots)) > - free_reloc_roots(&reloc_roots); > + free_reloc_roots(&reloc_roots); > > /* new reloc root may be added */ > mutex_lock(&fs_info->reloc_mutex); > list_splice_init(&rc->reloc_roots, &reloc_roots); > mutex_unlock(&fs_info->reloc_mutex); > - if (!list_empty(&reloc_roots)) > - free_reloc_roots(&reloc_roots); > + free_reloc_roots(&reloc_roots); > } > > BUG_ON(!RB_EMPTY_ROOT(&rc->reloc_root_tree.rb_root)); > @@ -4689,8 +4684,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) > out_free: > kfree(rc); > out: > - if (!list_empty(&reloc_roots)) > - free_reloc_roots(&reloc_roots); > + free_reloc_roots(&reloc_roots); > > btrfs_free_path(path); > >
On Fri, Feb 21, 2020 at 03:11:24PM +0200, Nikolay Borisov wrote: > The function always works on a local copy of the reloc root list, which > cannot be modified outside of it so using list_for_each_entry is fine. > Additionally the macro handles empty lists so drop list_empty checks of > callers. No semantic changes. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> This is still relevant for misc-next, so I'm applying it to misc-next.
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index e5cb64409f7c..f13b79adf6b0 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2523,13 +2523,10 @@ int prepare_to_merge(struct reloc_control *rc, int err) static noinline_for_stack void free_reloc_roots(struct list_head *list) { - struct btrfs_root *reloc_root; + struct btrfs_root *reloc_root, *tmp; - while (!list_empty(list)) { - reloc_root = list_entry(list->next, struct btrfs_root, - root_list); + list_for_each_entry_safe(reloc_root, tmp, list, root_list) __del_reloc_root(reloc_root); - } } static noinline_for_stack @@ -2588,15 +2585,13 @@ void merge_reloc_roots(struct reloc_control *rc) out: if (ret) { btrfs_handle_fs_error(fs_info, ret, NULL); - if (!list_empty(&reloc_roots)) - free_reloc_roots(&reloc_roots); + free_reloc_roots(&reloc_roots); /* new reloc root may be added */ mutex_lock(&fs_info->reloc_mutex); list_splice_init(&rc->reloc_roots, &reloc_roots); mutex_unlock(&fs_info->reloc_mutex); - if (!list_empty(&reloc_roots)) - free_reloc_roots(&reloc_roots); + free_reloc_roots(&reloc_roots); } BUG_ON(!RB_EMPTY_ROOT(&rc->reloc_root_tree.rb_root)); @@ -4689,8 +4684,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) out_free: kfree(rc); out: - if (!list_empty(&reloc_roots)) - free_reloc_roots(&reloc_roots); + free_reloc_roots(&reloc_roots); btrfs_free_path(path);
The function always works on a local copy of the reloc root list, which cannot be modified outside of it so using list_for_each_entry is fine. Additionally the macro handles empty lists so drop list_empty checks of callers. No semantic changes. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/relocation.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-)