Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210727080708.3708814-1-guoqing.jiang@linux.dev>
In-Reply-To: <20210727080708.3708814-1-guoqing.jiang@linux.dev>
From:   harshad shirwadkar <harshadshirwadkar@gmail.com>
Date:   Tue, 27 Jul 2021 10:36:58 -0700
Message-ID: <CAD+ocbyXcJ4YwEhsV4YO1B9m_K8S-OT1S6aoTp-u4UPYndoc=g@mail.gmail.com>
Subject: Re: [PATCH] ext4: reduce arguments of ext4_fc_add_dentry_tlv
To:     Guoqing Jiang <guoqing.jiang@linux.dev>
Cc:     "Theodore Y. Ts'o" <tytso@mit.edu>,
        Andreas Dilger <adilger.kernel@dilger.ca>,
        Ext4 Developers List <linux-ext4@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Thanks for the cleanup! Looks good.

Reviewed-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>


On Tue, Jul 27, 2021 at 1:11 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> From: Guoqing Jiang <jiangguoqing@kylinos.cn>
>
> Let's pass fc_dentry directly since those arguments (tag, parent_ino and
> ino etc) can be deferenced from it.
>
> Signed-off-by: Guoqing Jiang <jiangguoqing@kylinos.cn>
> ---
>  fs/ext4/fast_commit.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 5106c9fe2e19..797105adcabf 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -775,28 +775,27 @@ static bool ext4_fc_add_tlv(struct super_block *sb, u16 tag, u16 len, u8 *val,
>  }
>
>  /* Same as above, but adds dentry tlv. */
> -static  bool ext4_fc_add_dentry_tlv(struct super_block *sb, u16 tag,
> -                                       int parent_ino, int ino, int dlen,
> -                                       const unsigned char *dname,
> -                                       u32 *crc)
> +static bool ext4_fc_add_dentry_tlv(struct super_block *sb, u32 *crc,
> +                                  struct ext4_fc_dentry_update *fc_dentry)
>  {
>         struct ext4_fc_dentry_info fcd;
>         struct ext4_fc_tl tl;
> +       int dlen = fc_dentry->fcd_name.len;
>         u8 *dst = ext4_fc_reserve_space(sb, sizeof(tl) + sizeof(fcd) + dlen,
>                                         crc);
>
>         if (!dst)
>                 return false;
>
> -       fcd.fc_parent_ino = cpu_to_le32(parent_ino);
> -       fcd.fc_ino = cpu_to_le32(ino);
> -       tl.fc_tag = cpu_to_le16(tag);
> +       fcd.fc_parent_ino = cpu_to_le32(fc_dentry->fcd_parent);
> +       fcd.fc_ino = cpu_to_le32(fc_dentry->fcd_ino);
> +       tl.fc_tag = cpu_to_le16(fc_dentry->fcd_op);
>         tl.fc_len = cpu_to_le16(sizeof(fcd) + dlen);
>         ext4_fc_memcpy(sb, dst, &tl, sizeof(tl), crc);
>         dst += sizeof(tl);
>         ext4_fc_memcpy(sb, dst, &fcd, sizeof(fcd), crc);
>         dst += sizeof(fcd);
> -       ext4_fc_memcpy(sb, dst, dname, dlen, crc);
> +       ext4_fc_memcpy(sb, dst, fc_dentry->fcd_name.name, dlen, crc);
>         dst += dlen;
>
>         return true;
> @@ -991,11 +990,7 @@ __acquires(&sbi->s_fc_lock)
>                                  &sbi->s_fc_dentry_q[FC_Q_MAIN], fcd_list) {
>                 if (fc_dentry->fcd_op != EXT4_FC_TAG_CREAT) {
>                         spin_unlock(&sbi->s_fc_lock);
> -                       if (!ext4_fc_add_dentry_tlv(
> -                               sb, fc_dentry->fcd_op,
> -                               fc_dentry->fcd_parent, fc_dentry->fcd_ino,
> -                               fc_dentry->fcd_name.len,
> -                               fc_dentry->fcd_name.name, crc)) {
> +                       if (!ext4_fc_add_dentry_tlv(sb, crc, fc_dentry)) {
>                                 ret = -ENOSPC;
>                                 goto lock_and_exit;
>                         }
> @@ -1034,11 +1029,7 @@ __acquires(&sbi->s_fc_lock)
>                 if (ret)
>                         goto lock_and_exit;
>
> -               if (!ext4_fc_add_dentry_tlv(
> -                       sb, fc_dentry->fcd_op,
> -                       fc_dentry->fcd_parent, fc_dentry->fcd_ino,
> -                       fc_dentry->fcd_name.len,
> -                       fc_dentry->fcd_name.name, crc)) {
> +               if (!ext4_fc_add_dentry_tlv(sb, crc, fc_dentry)) {
>                         ret = -ENOSPC;
>                         goto lock_and_exit;
>                 }
> --
> 2.25.1
>