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

[Dan Carpenter <dan.carpenter@oracle.com>]

On Mon, May 16, 2022 at 01:39:52PM +0300, Nikolay Borisov wrote:
> 
> 
> 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;

breaks out of xa_for_each_start() 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.

> >      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,

There are breaks inside the inner loops but nothing breaks out of the
while (1) loop.

regards,
dan carpenter