Date:   Mon, 13 Mar 2023 07:43:42 +0530
Message-Id: <87pm9dflex.fsf@doe.com>
From:   Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To:     Kemeng Shi <shikemeng@huaweicloud.com>, tytso@mit.edu,
        adilger.kernel@dilger.ca
Cc:     linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
        shikemeng@huaweicloud.com
Subject: Re: [PATCH 2/2] ext4: avoid unnecessary pointer dereference in ext4_mb_normalize_request
In-Reply-To: <20230311170949.1047958-3-shikemeng@huaweicloud.com>
Precedence: bulk

Kemeng Shi <shikemeng@huaweicloud.com> writes:

> Result of EXT4_SB(ac->ac_sb) is already stored in sbi at beginning of
> ext4_mb_normalize_request. Use sbi instead of EXT4_SB(ac->ac_sb) to
> remove unnecessary pointer dereference.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  fs/ext4/mballoc.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>

Looks good to me. Please feel free to add -

Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 13dce6f07fa4..d94eb52dda17 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4049,7 +4049,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>  		size = 8 * 1024 * 1024;
>  	} else {
>  		start_off = (loff_t) ac->ac_o_ex.fe_logical << bsbits;
> -		size	  = (loff_t) EXT4_C2B(EXT4_SB(ac->ac_sb),
> +		size	  = (loff_t) EXT4_C2B(sbi,
>  					      ac->ac_o_ex.fe_len) << bsbits;
>  	}
>  	size = size >> bsbits;
> @@ -4094,8 +4094,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>  			continue;
>  		}
>
> -		pa_end = pa->pa_lstart + EXT4_C2B(EXT4_SB(ac->ac_sb),
> -						  pa->pa_len);
> +		pa_end = pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
>
>  		/* PA must not overlap original request */
>  		BUG_ON(!(ac->ac_o_ex.fe_logical >= pa_end ||
> @@ -4128,8 +4127,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>
>  		spin_lock(&pa->pa_lock);
>  		if (pa->pa_deleted == 0) {
> -			pa_end = pa->pa_lstart + EXT4_C2B(EXT4_SB(ac->ac_sb),
> -							  pa->pa_len);
> +			pa_end = pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
>  			BUG_ON(!(start >= pa_end || end <= pa->pa_lstart));
>  		}
>  		spin_unlock(&pa->pa_lock);
> --
> 2.30.0