linux-kernel - Re: [PATCH] btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_truncate_inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAPjX3Ff8BQr+Wh0WtsvKcBa1qxiyPCm=0tLW=OKFJCqtLOqkPw@mail.gmail.com>
Date: Mon, 14 Apr 2025 16:06:40 +0200
From: Daniel Vacek <neelx@...e.com>
To: 李扬韬 <frank.li@...o.com>
Cc: "clm@...com" <clm@...com>, "josef@...icpanda.com" <josef@...icpanda.com>, 
	"dsterba@...e.com" <dsterba@...e.com>, 
	"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_truncate_inode_items()

On Mon, 14 Apr 2025 at 15:34, 李扬韬 <frank.li@...o.com> wrote:
>
> Hi Daniel,
>
> > And what about the other functions in that file? We could even get rid of two allocations passing the path from ..._inode_ref() to ..._inode_extref().
>
> I made the following changes, is this what you meant?
> I will do the rest if that's ok.

Yeah, quite almost.

> Thx,
> Yangtao
>
> diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
> index 3530de0618c8..e082d7e27c29 100644
> --- a/fs/btrfs/inode-item.c
> +++ b/fs/btrfs/inode-item.c
> @@ -105,11 +105,11 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans,
>
>  static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>                                   struct btrfs_root *root,
> +                                 struct btrfs_path *path,
>                                   const struct fscrypt_str *name,
>                                   u64 inode_objectid, u64 ref_objectid,
>                                   u64 *index)
>  {
> -       struct btrfs_path *path;
>         struct btrfs_key key;
>         struct btrfs_inode_extref *extref;
>         struct extent_buffer *leaf;

I think you missed this:

@@ -123,10 +123,6 @@ static int btrfs_del_inode_extref(struct
btrfs_trans_handle *trans,
        key.type = BTRFS_INODE_EXTREF_KEY;
        key.offset = btrfs_extref_hash(ref_objectid, name->name, name->len);

-       path = btrfs_alloc_path();
-       if (!path)
-               return -ENOMEM;
-
        ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
        if (ret > 0)
                ret = -ENOENT;

Which was ironically the whole point ;-)

> @@ -131,7 +131,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>         if (ret > 0)
>                 ret = -ENOENT;

You can also directly return -ENOENT; here.

>         if (ret < 0)
> -               goto out;
> +               return ret;
>
>         /*
>          * Sanity check - did we find the right item for this name?
> @@ -142,8 +142,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>                                                 ref_objectid, name);
>         if (!extref) {
>                 btrfs_abort_transaction(trans, -ENOENT);
> -               ret = -ENOENT;
> -               goto out;
> +               return -ENOENT;
>         }
>
>         leaf = path->nodes[0];
> @@ -156,8 +155,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>                  * Common case only one ref in the item, remove the
>                  * whole item.
>                  */
> -               ret = btrfs_del_item(trans, root, path);
> -               goto out;
> +               return btrfs_del_item(trans, root, path);
>         }
>
>         ptr = (unsigned long)extref;
> @@ -168,9 +166,6 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans,
>
>         btrfs_truncate_item(trans, path, item_size - del_len, 1);
>
> -out:
> -       btrfs_free_path(path);
> -
>         return ret;
>  }
>
> @@ -178,7 +173,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
>                         struct btrfs_root *root, const struct fscrypt_str *name,
>                         u64 inode_objectid, u64 ref_objectid, u64 *index)
>  {
> -       struct btrfs_path *path;
> +       BTRFS_PATH_AUTO_FREE(path);
>         struct btrfs_key key;
>         struct btrfs_inode_ref *ref;
>         struct extent_buffer *leaf;
> @@ -230,7 +225,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
>                               item_size - (ptr + sub_item_len - item_start));
>         btrfs_truncate_item(trans, path, item_size - sub_item_len, 1);
>  out:
> -       btrfs_free_path(path);
> +       btrfs_release_path(path);
>
>         if (search_ext_refs) {
>                 /*
> @@ -238,7 +233,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans,
>                  * name in our ref array. Find and remove the extended
>                  * inode ref then.
>                  */
> -               return btrfs_del_inode_extref(trans, root, name,
> +               return btrfs_del_inode_extref(trans, root, path, name,
>                                               inode_objectid, ref_objectid, index);
>         }