Re: [PATCH v2] btrfs: don't pass tree_id in btrfs_search_path_in_tree()

Filipe Manana <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel
Message-ID <CAL3q7H4nBTXCMZh3ZvO8+LUDDKcs8ZS3jXj6PggV9Px+kYTbKw@mail.gmail.com>
On Mon, Jun 29, 2026 at 10:02 PM Miquel Sabaté Solà <[email protected]> wrote:
>
> The 'tree_id' parameter in btrfs_search_path_in_tree() was only being
> used in order to fetch the root tree to be considered for the
> search. For this same reason this function was also requiring a 'struct
> btrfs_fs_info' parameter. This commit replaces these two parameters with
> a single 'struct btrfs_root' one, which identifies from which root tree
> the search should happen.
>
> This function only has one caller, the inode lookup ioctl, which knows
> how to provide the root tree for each case. In fact, if args->treeid ==
> 0, then we don't even have to allocate a new root tree object, and we
> can reuse the one provided by the ioctl system call, thus avoiding an
> extra allocation.
>
> Signed-off-by: Miquel Sabaté Solà <[email protected]>

Reviewed-by: Filipe Manana <[email protected]>

Adding it to for-next, thanks.

> ---
> Changes in v2:
> - Addressed comments by Filipe Manana
> - Link to v1: https://lore.kernel.org/all/[email protected]/
>
>  fs/btrfs/ioctl.c | 49 +++++++++++++++++++++---------------------------
>  1 file changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 1ed16ff4007b..d19867f7eb34 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1664,13 +1664,11 @@ static noinline int btrfs_ioctl_tree_search_v2(struct btrfs_root *root,
>  }
>
>  /*
> - * Search INODE_REFs to identify path name of 'dirid' directory
> - * in a 'tree_id' tree. and sets path name to 'name'.
> + * Search for an INODE_REF in a 'root' tree which identifies the path name of
> + * 'dirid'. When found, it sets 'name' with the path name.
>   */
> -static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info,
> -                               u64 tree_id, u64 dirid, char *name)
> +static noinline int btrfs_search_path_in_tree(struct btrfs_root *root, u64 dirid, char *name)
>  {
> -       struct btrfs_root *root;
>         struct btrfs_key key;
>         char *ptr;
>         int ret = -1;
> @@ -1692,13 +1690,6 @@ static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info,
>
>         ptr = &name[BTRFS_INO_LOOKUP_PATH_MAX - 1];
>
> -       root = btrfs_get_fs_root(info, tree_id, true);
> -       if (IS_ERR(root)) {
> -               ret = PTR_ERR(root);
> -               root = NULL;
> -               goto out;
> -       }
> -
>         key.objectid = dirid;
>         key.type = BTRFS_INODE_REF_KEY;
>         key.offset = (u64)-1;
> @@ -1706,11 +1697,9 @@ static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info,
>         while (1) {
>                 ret = btrfs_search_backwards(root, &key, path);
>                 if (ret < 0)
> -                       goto out;
> -               else if (ret > 0) {
> -                       ret = -ENOENT;
> -                       goto out;
> -               }
> +                       return ret;
> +               else if (ret > 0)
> +                       return -ENOENT;
>
>                 l = path->nodes[0];
>                 slot = path->slots[0];
> @@ -1719,10 +1708,8 @@ static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info,
>                 len = btrfs_inode_ref_name_len(l, iref);
>                 ptr -= len + 1;
>                 total_len += len + 1;
> -               if (ptr < name) {
> -                       ret = -ENAMETOOLONG;
> -                       goto out;
> -               }
> +               if (ptr < name)
> +                       return -ENAMETOOLONG;
>
>                 *(ptr + len) = '/';
>                 read_extent_buffer(l, ptr, (unsigned long)(iref + 1), len);
> @@ -1737,10 +1724,8 @@ static noinline int btrfs_search_path_in_tree(struct btrfs_fs_info *info,
>         }
>         memmove(name, ptr, total_len);
>         name[total_len] = '\0';
> -       ret = 0;
> -out:
> -       btrfs_put_root(root);
> -       return ret;
> +
> +       return 0;
>  }
>
>  static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
> @@ -1884,6 +1869,7 @@ static int btrfs_search_path_in_tree_user(struct mnt_idmap *idmap,
>  static noinline int btrfs_ioctl_ino_lookup(struct btrfs_root *root,
>                                            void __user *argp)
>  {
> +       bool new_root = false;
>         struct btrfs_ioctl_ino_lookup_args AUTO_KFREE(args);
>         int ret = 0;
>
> @@ -1897,6 +1883,8 @@ static noinline int btrfs_ioctl_ino_lookup(struct btrfs_root *root,
>          */
>         if (args->treeid == 0)
>                 args->treeid = btrfs_root_id(root);
> +       else
> +               new_root = true;
>
>         if (args->objectid == BTRFS_FIRST_FREE_OBJECTID) {
>                 args->name[0] = 0;
> @@ -1908,9 +1896,14 @@ static noinline int btrfs_ioctl_ino_lookup(struct btrfs_root *root,
>                 goto out;
>         }
>
> -       ret = btrfs_search_path_in_tree(root->fs_info,
> -                                       args->treeid, args->objectid,
> -                                       args->name);
> +       if (new_root) {
> +               root = btrfs_get_fs_root(root->fs_info, args->treeid, true);
> +               if (IS_ERR(root))
> +                       return PTR_ERR(root);
> +       }
> +       ret = btrfs_search_path_in_tree(root, args->objectid, args->name);
> +       if (new_root)
> +               btrfs_put_root(root);
>
>  out:
>         if (ret == 0 && copy_to_user(argp, args, sizeof(*args)))
> --
> 2.54.0
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.