Re: [PATCH 2/2] btrfs: Use list_for_each_entry_safe in free_reloc_roots

Qu Wenruo
2020-02-21T13:23:30Z

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);
>
>