diff mbox series

[6/7] btrfs: tree-log: Simplify log_new_ancestors

Message ID 20210804184854.10696-7-mpdesouza@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: Use btrfs_find_item whenever possible | expand

Commit Message

Marcos Paulo de Souza Aug. 4, 2021, 6:48 p.m. UTC 
The search_key variable was being used only as argument of
btrfs_search_slot. This can be simplified by calling btrfs_find_item,
which also handles the next leaf condition as well.

No functional changes.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 fs/btrfs/tree-log.c | 40 ++++++++++++----------------------------
 1 file changed, 12 insertions(+), 28 deletions(-)

Comments

Filipe Manana Aug. 5, 2021, 9 a.m. UTC | #1 
On Wed, Aug 4, 2021 at 10:07 PM Marcos Paulo de Souza
<mpdesouza@suse.com> wrote:
>
> The search_key variable was being used only as argument of
> btrfs_search_slot. This can be simplified by calling btrfs_find_item,
> which also handles the next leaf condition as well.
>
> No functional changes.
>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
>  fs/btrfs/tree-log.c | 40 ++++++++++++----------------------------
>  1 file changed, 12 insertions(+), 28 deletions(-)
>
> diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> index 567adc3de11a..22417cd32347 100644
> --- a/fs/btrfs/tree-log.c
> +++ b/fs/btrfs/tree-log.c
> @@ -5929,31 +5929,30 @@ static int btrfs_log_all_parents(struct btrfs_trans_handle *trans,
>         return ret;
>  }
>
> +/*
> + * Iterate over the given and all it's parent directories, logging them if
> + * needed.
> + *
> + * Return 0 if we reach the toplevel directory, or < 0 if error.
> + */
>  static int log_new_ancestors(struct btrfs_trans_handle *trans,
>                              struct btrfs_root *root,
>                              struct btrfs_path *path,
>                              struct btrfs_log_ctx *ctx)
>  {
> +       struct btrfs_fs_info *fs_info = root->fs_info;
>         struct btrfs_key found_key;
> +       u64 ino;
>
>         btrfs_item_key_to_cpu(path->nodes[0], &found_key, path->slots[0]);
>
>         while (true) {
> -               struct btrfs_fs_info *fs_info = root->fs_info;
> -               struct extent_buffer *leaf = path->nodes[0];
> -               int slot = path->slots[0];
> -               struct btrfs_key search_key;
>                 struct inode *inode;
> -               u64 ino;

Why are the 'ino' and 'fs_info' declarations moved to the outer scope?
They are only used inside the while loop's scope, so I don't see a
reason to move them.

Thanks.

>                 int ret = 0;
>
>                 btrfs_release_path(path);
>
>                 ino = found_key.offset;
> -
> -               search_key.objectid = found_key.offset;
> -               search_key.type = BTRFS_INODE_ITEM_KEY;
> -               search_key.offset = 0;
>                 inode = btrfs_iget(fs_info->sb, ino, root);
>                 if (IS_ERR(inode))
>                         return PTR_ERR(inode);
> @@ -5966,29 +5965,14 @@ static int log_new_ancestors(struct btrfs_trans_handle *trans,
>                 if (ret)
>                         return ret;
>
> -               if (search_key.objectid == BTRFS_FIRST_FREE_OBJECTID)
> +               if (ino == BTRFS_FIRST_FREE_OBJECTID)
>                         break;
>
> -               search_key.type = BTRFS_INODE_REF_KEY;
> -               ret = btrfs_search_slot(NULL, root, &search_key, path, 0, 0);
> +               ret = btrfs_find_item(root, path, ino, BTRFS_INODE_REF_KEY, 0,
> +                                       &found_key);
>                 if (ret < 0)
>                         return ret;
> -
> -               leaf = path->nodes[0];
> -               slot = path->slots[0];
> -               if (slot >= btrfs_header_nritems(leaf)) {
> -                       ret = btrfs_next_leaf(root, path);
> -                       if (ret < 0)
> -                               return ret;
> -                       else if (ret > 0)
> -                               return -ENOENT;
> -                       leaf = path->nodes[0];
> -                       slot = path->slots[0];
> -               }
> -
> -               btrfs_item_key_to_cpu(leaf, &found_key, slot);
> -               if (found_key.objectid != search_key.objectid ||
> -                   found_key.type != BTRFS_INODE_REF_KEY)
> +               else if (ret > 0)
>                         return -ENOENT;
>         }
>         return 0;
> --
> 2.31.1
>
Marcos Paulo de Souza Aug. 5, 2021, 12:41 p.m. UTC | #2 
On Thu, 2021-08-05 at 10:00 +0100, Filipe Manana wrote:
> On Wed, Aug 4, 2021 at 10:07 PM Marcos Paulo de Souza
> <mpdesouza@suse.com> wrote:
> > The search_key variable was being used only as argument of
> > btrfs_search_slot. This can be simplified by calling
> > btrfs_find_item,
> > which also handles the next leaf condition as well.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> >  fs/btrfs/tree-log.c | 40 ++++++++++++----------------------------
> >  1 file changed, 12 insertions(+), 28 deletions(-)
> > 
> > diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
> > index 567adc3de11a..22417cd32347 100644
> > --- a/fs/btrfs/tree-log.c
> > +++ b/fs/btrfs/tree-log.c
> > @@ -5929,31 +5929,30 @@ static int btrfs_log_all_parents(struct
> > btrfs_trans_handle *trans,
> >         return ret;
> >  }
> > 
> > +/*
> > + * Iterate over the given and all it's parent directories, logging
> > them if
> > + * needed.
> > + *
> > + * Return 0 if we reach the toplevel directory, or < 0 if error.
> > + */
> >  static int log_new_ancestors(struct btrfs_trans_handle *trans,
> >                              struct btrfs_root *root,
> >                              struct btrfs_path *path,
> >                              struct btrfs_log_ctx *ctx)
> >  {
> > +       struct btrfs_fs_info *fs_info = root->fs_info;
> >         struct btrfs_key found_key;
> > +       u64 ino;
> > 
> >         btrfs_item_key_to_cpu(path->nodes[0], &found_key, path-
> > >slots[0]);
> > 
> >         while (true) {
> > -               struct btrfs_fs_info *fs_info = root->fs_info;
> > -               struct extent_buffer *leaf = path->nodes[0];
> > -               int slot = path->slots[0];
> > -               struct btrfs_key search_key;
> >                 struct inode *inode;
> > -               u64 ino;
> 
> Why are the 'ino' and 'fs_info' declarations moved to the outer
> scope?
> They are only used inside the while loop's scope, so I don't see a
> reason to move them.

You're right, I will keep them in place for v2. Thanks!

> 
> Thanks.
> 
> >                 int ret = 0;
> > 
> >                 btrfs_release_path(path);
> > 
> >                 ino = found_key.offset;
> > -
> > -               search_key.objectid = found_key.offset;
> > -               search_key.type = BTRFS_INODE_ITEM_KEY;
> > -               search_key.offset = 0;
> >                 inode = btrfs_iget(fs_info->sb, ino, root);
> >                 if (IS_ERR(inode))
> >                         return PTR_ERR(inode);
> > @@ -5966,29 +5965,14 @@ static int log_new_ancestors(struct
> > btrfs_trans_handle *trans,
> >                 if (ret)
> >                         return ret;
> > 
> > -               if (search_key.objectid ==
> > BTRFS_FIRST_FREE_OBJECTID)
> > +               if (ino == BTRFS_FIRST_FREE_OBJECTID)
> >                         break;
> > 
> > -               search_key.type = BTRFS_INODE_REF_KEY;
> > -               ret = btrfs_search_slot(NULL, root, &search_key,
> > path, 0, 0);
> > +               ret = btrfs_find_item(root, path, ino,
> > BTRFS_INODE_REF_KEY, 0,
> > +                                       &found_key);
> >                 if (ret < 0)
> >                         return ret;
> > -
> > -               leaf = path->nodes[0];
> > -               slot = path->slots[0];
> > -               if (slot >= btrfs_header_nritems(leaf)) {
> > -                       ret = btrfs_next_leaf(root, path);
> > -                       if (ret < 0)
> > -                               return ret;
> > -                       else if (ret > 0)
> > -                               return -ENOENT;
> > -                       leaf = path->nodes[0];
> > -                       slot = path->slots[0];
> > -               }
> > -
> > -               btrfs_item_key_to_cpu(leaf, &found_key, slot);
> > -               if (found_key.objectid != search_key.objectid ||
> > -                   found_key.type != BTRFS_INODE_REF_KEY)
> > +               else if (ret > 0)
> >                         return -ENOENT;
> >         }
> >         return 0;
> > --
> > 2.31.1
> > 
> 
>
diff mbox series

Patch

diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 567adc3de11a..22417cd32347 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -5929,31 +5929,30 @@  static int btrfs_log_all_parents(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
+/*
+ * Iterate over the given and all it's parent directories, logging them if
+ * needed.
+ *
+ * Return 0 if we reach the toplevel directory, or < 0 if error.
+ */
 static int log_new_ancestors(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root,
 			     struct btrfs_path *path,
 			     struct btrfs_log_ctx *ctx)
 {
+	struct btrfs_fs_info *fs_info = root->fs_info;
 	struct btrfs_key found_key;
+	u64 ino;
 
 	btrfs_item_key_to_cpu(path->nodes[0], &found_key, path->slots[0]);
 
 	while (true) {
-		struct btrfs_fs_info *fs_info = root->fs_info;
-		struct extent_buffer *leaf = path->nodes[0];
-		int slot = path->slots[0];
-		struct btrfs_key search_key;
 		struct inode *inode;
-		u64 ino;
 		int ret = 0;
 
 		btrfs_release_path(path);
 
 		ino = found_key.offset;
-
-		search_key.objectid = found_key.offset;
-		search_key.type = BTRFS_INODE_ITEM_KEY;
-		search_key.offset = 0;
 		inode = btrfs_iget(fs_info->sb, ino, root);
 		if (IS_ERR(inode))
 			return PTR_ERR(inode);
@@ -5966,29 +5965,14 @@  static int log_new_ancestors(struct btrfs_trans_handle *trans,
 		if (ret)
 			return ret;
 
-		if (search_key.objectid == BTRFS_FIRST_FREE_OBJECTID)
+		if (ino == BTRFS_FIRST_FREE_OBJECTID)
 			break;
 
-		search_key.type = BTRFS_INODE_REF_KEY;
-		ret = btrfs_search_slot(NULL, root, &search_key, path, 0, 0);
+		ret = btrfs_find_item(root, path, ino, BTRFS_INODE_REF_KEY, 0,
+					&found_key);
 		if (ret < 0)
 			return ret;
-
-		leaf = path->nodes[0];
-		slot = path->slots[0];
-		if (slot >= btrfs_header_nritems(leaf)) {
-			ret = btrfs_next_leaf(root, path);
-			if (ret < 0)
-				return ret;
-			else if (ret > 0)
-				return -ENOENT;
-			leaf = path->nodes[0];
-			slot = path->slots[0];
-		}
-
-		btrfs_item_key_to_cpu(leaf, &found_key, slot);
-		if (found_key.objectid != search_key.objectid ||
-		    found_key.type != BTRFS_INODE_REF_KEY)
+		else if (ret > 0)
 			return -ENOENT;
 	}
 	return 0;