Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Jul 2023 21:01:58 +0530
Message-Id: <87cz0ll1qp.fsf@doe.com>
From:   Ritesh Harjani (IBM) <ritesh.list@gmail.com>
To:     Kemeng Shi <shikemeng@huaweicloud.com>, tytso@mit.edu,
        adilger.kernel@dilger.ca, ojaswin@linux.ibm.com,
        linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
Cc:     shikemeng@huaweicloud.com
Subject: Re: [PATCH 03/10] ext4: return found group directly in ext4_mb_choose_next_group_p2_aligned
In-Reply-To: <20230721171007.2065423-4-shikemeng@huaweicloud.com>
Precedence: bulk

Kemeng Shi <shikemeng@huaweicloud.com> writes:

> Return good group when it's found in loop to remove unnecessary NULL
> initialization of grp and futher check if good group is found after loop.
>
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> ---
>  fs/ext4/mballoc.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)

Makes it simpler. Thanks for the cleanup. 

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

-ritesh

>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 33ee3991f62c..4031f8e2a660 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -875,7 +875,7 @@ static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context
>  			enum criteria *new_cr, ext4_group_t *group, ext4_group_t ngroups)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
> -	struct ext4_group_info *iter, *grp;
> +	struct ext4_group_info *iter;
>  	int i;
>  
>  	if (ac->ac_status == AC_STATUS_FOUND)
> @@ -884,7 +884,6 @@ static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context
>  	if (unlikely(sbi->s_mb_stats && ac->ac_flags & EXT4_MB_CR_POWER2_ALIGNED_OPTIMIZED))
>  		atomic_inc(&sbi->s_bal_p2_aligned_bad_suggestions);
>  
> -	grp = NULL;
>  	for (i = ac->ac_2order; i < MB_NUM_ORDERS(ac->ac_sb); i++) {
>  		if (list_empty(&sbi->s_mb_largest_free_orders[i]))
>  			continue;
> @@ -893,28 +892,22 @@ static void ext4_mb_choose_next_group_p2_aligned(struct ext4_allocation_context
>  			read_unlock(&sbi->s_mb_largest_free_orders_locks[i]);
>  			continue;
>  		}
> -		grp = NULL;
>  		list_for_each_entry(iter, &sbi->s_mb_largest_free_orders[i],
>  				    bb_largest_free_order_node) {
>  			if (sbi->s_mb_stats)
>  				atomic64_inc(&sbi->s_bal_cX_groups_considered[CR_POWER2_ALIGNED]);
>  			if (likely(ext4_mb_good_group(ac, iter->bb_group, CR_POWER2_ALIGNED))) {
> -				grp = iter;
> -				break;
> +				*group = iter->bb_group;
> +				ac->ac_flags |= EXT4_MB_CR_POWER2_ALIGNED_OPTIMIZED;
> +				read_unlock(&sbi->s_mb_largest_free_orders_locks[i]);
> +				return;
>  			}
>  		}
>  		read_unlock(&sbi->s_mb_largest_free_orders_locks[i]);
> -		if (grp)
> -			break;
>  	}
>  
> -	if (!grp) {
> -		/* Increment cr and search again */
> -		*new_cr = CR_GOAL_LEN_FAST;
> -	} else {
> -		*group = grp->bb_group;
> -		ac->ac_flags |= EXT4_MB_CR_POWER2_ALIGNED_OPTIMIZED;
> -	}
> +	/* Increment cr and search again if no group is found */
> +	*new_cr = CR_GOAL_LEN_FAST;
>  }
>  
>  /*
> -- 
> 2.30.0