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

[Nikolay Borisov <nborisov@suse.com>]

On 16.05.22 г. 11:04 ч., Dan Carpenter wrote:
> Hello Gabriel Niebler,
> 
> The patch 06a79e50ff00: "btrfs: turn fs_roots_radix in btrfs_fs_info
> into an XArray" from May 3, 2022, leads to the following Smatch
> static checker warning:
> 
> 	fs/btrfs/disk-io.c:4560 btrfs_cleanup_fs_roots()
> 	warn: ignoring unreachable code.
> 
> fs/btrfs/disk-io.c
>      4520 int btrfs_cleanup_fs_roots(struct btrfs_fs_info *fs_info)
>      4521 {
>      4522         struct btrfs_root *roots[8];
>      4523         unsigned long index = 0;
>      4524         int i;
>      4525         int err = 0;
>      4526         int grabbed;
>      4527
>      4528         while (1) {
>      4529                 struct btrfs_root *root;
>      4530
>      4531                 spin_lock(&fs_info->fs_roots_lock);
>      4532                 if (!xa_find(&fs_info->fs_roots, &index, ULONG_MAX, XA_PRESENT)) {
>      4533                         spin_unlock(&fs_info->fs_roots_lock);
>      4534                         return err;
>      4535                 }
>      4536
>      4537                 grabbed = 0;
>      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;
>      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;
>      4554                         btrfs_put_root(roots[i]);
>      4555                 }
>      4556                 index++;
>      4557         }
>      4558
>      4559         /* Release the roots that remain uncleaned due to error */
> --> 4560         for (; i < grabbed; i++) {
> 
> This code is unreachable now.

How is it unreachable, if we error in the middle of 
btrfs_orphan_cleanup, we'd break and this loop will cleanup the rest of 
the roots,

> 
>      4561                 if (roots[i])
>      4562                         btrfs_put_root(roots[i]);
>      4563         }
>      4564         return err;
>      4565 }
> 
> regards,
> dan carpenter
>