Re: [bug report] btrfs: turn fs_roots_radix in btrfs_fs_info into an XArray

[David Sterba <dsterba@suse.cz>]

On Mon, May 16, 2022 at 02:45:57PM +0300, Nikolay Borisov wrote:
> On 16.05.22 г. 14:02 ч., Dan Carpenter wrote:
> > On Mon, May 16, 2022 at 01:39:52PM +0300, Nikolay Borisov wrote:
> >> On 16.05.22 г. 11:04 ч., Dan Carpenter wrote:
> >>>       4538                 xa_for_each_start(&fs_info->fs_roots, index, root, index) {
> >>>       4539                         /* Avoid grabbing roots in dead_roots */
> >>>       4540                         if (btrfs_root_refs(&root->root_item) > 0)
> >>>       4541                                 roots[grabbed++] = btrfs_grab_root(root);
> >>>       4542                         if (grabbed >= ARRAY_SIZE(roots))
> >>>       4543                                 break;
> > 
> > breaks out of xa_for_each_start() loop.
> 
> This is fine, as when we fill root[] then we'd process it in the 
> subsequent for loop.
> 
> > 
> >>>       4544                 }
> >>>       4545                 spin_unlock(&fs_info->fs_roots_lock);
> >>>       4546
> >>>       4547                 for (i = 0; i < grabbed; i++) {
> >>>       4548                         if (!roots[i])
> >>>       4549                                 continue;
> >>>       4550                         index = roots[i]->root_key.objectid;
> >>>       4551                         err = btrfs_orphan_cleanup(roots[i]);
> >>>       4552                         if (err)
> >>>       4553                                 break;
> > 
> > breaks out of for loop.
> 
> However, this is indeed a genuine bug, and the fix for it is:
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index fe309db9f5ff..f33093513360 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4549,12 +4549,13 @@ int btrfs_cleanup_fs_roots(struct btrfs_fs_info 
> *fs_info)
>                          index = roots[i]->root_key.objectid;
>                          err = btrfs_orphan_cleanup(roots[i]);
>                          if (err)
> -                               break;
> +                               goto out;
>                          btrfs_put_root(roots[i]);
>                  }
>                  index++;
>          }
> 
> +out:
>          /* Release the roots that remain uncleaned due to error */
>          for (; i < grabbed; i++) {
>                  if (roots[i])

Thanks Dan for the report, fixup folded and pushed.