btrfs: do not utilize goto to implement delayed inode ref deletion
authorQu Wenruo <wqu@suse.com>
Mon, 30 Oct 2023 21:07:20 +0000 (07:37 +1030)
committerDavid Sterba <dsterba@suse.com>
Fri, 15 Dec 2023 19:27:00 +0000 (20:27 +0100)
[PROBLEM]
The function __btrfs_update_delayed_inode() is doing something not
meeting the code standard of today:

path->slots[0]++
if (path->slots[0] >= btrfs_header_nritems(leaf))
goto search;
again:
if (!is_the_target_inode_ref())
goto out;
ret = btrfs_delete_item();
/* Some cleanup. */
return ret;

search:
ret = search_for_the_last_inode_ref();
goto again;

With the tag named "again", it's pretty common to think it's a loop, but
the truth is, we only need to do the search once, to locate the last
(also the first, since there should only be one INODE_REF or
INODE_EXTREF now) ref of the inode.

[FIX]
Instead of the weird jumps, just do them in a stream-lined fashion.
This removes those weird labels, and add extra comments on why we can do
the different searches.

Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/delayed-inode.c

index 7381241334e8dbcf713843373b1f03ded8698b9c..91159dd7355b36dc97b73070bf4f3cb7abd45e9c 100644 (file)
@@ -1036,14 +1036,33 @@ static int __btrfs_update_delayed_inode(struct btrfs_trans_handle *trans,
        if (!test_bit(BTRFS_DELAYED_NODE_DEL_IREF, &node->flags))
                goto out;
 
-       path->slots[0]++;
-       if (path->slots[0] >= btrfs_header_nritems(leaf))
-               goto search;
-again:
+       /*
+        * Now we're going to delete the INODE_REF/EXTREF, which should be the
+        * only one ref left.  Check if the next item is an INODE_REF/EXTREF.
+        *
+        * But if we're the last item already, release and search for the last
+        * INODE_REF/EXTREF.
+        */
+       if (path->slots[0] + 1 >= btrfs_header_nritems(leaf)) {
+               key.objectid = node->inode_id;
+               key.type = BTRFS_INODE_EXTREF_KEY;
+               key.offset = (u64)-1;
+
+               btrfs_release_path(path);
+               ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
+               if (ret < 0)
+                       goto err_out;
+               ASSERT(ret > 0);
+               ASSERT(path->slots[0] > 0);
+               ret = 0;
+               path->slots[0]--;
+               leaf = path->nodes[0];
+       } else {
+               path->slots[0]++;
+       }
        btrfs_item_key_to_cpu(leaf, &key, path->slots[0]);
        if (key.objectid != node->inode_id)
                goto out;
-
        if (key.type != BTRFS_INODE_REF_KEY &&
            key.type != BTRFS_INODE_EXTREF_KEY)
                goto out;
@@ -1070,22 +1089,6 @@ err_out:
                btrfs_abort_transaction(trans, ret);
 
        return ret;
-
-search:
-       btrfs_release_path(path);
-
-       key.type = BTRFS_INODE_EXTREF_KEY;
-       key.offset = -1;
-
-       ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
-       if (ret < 0)
-               goto err_out;
-       ASSERT(ret);
-
-       ret = 0;
-       leaf = path->nodes[0];
-       path->slots[0]--;
-       goto again;
 }
 
 static inline int btrfs_update_delayed_inode(struct btrfs_trans_handle *trans,