On Wed, Mar 27, 2024 at 05:38:23AM +0800, Kemeng Shi wrote:
> Expand next_linear_group to remove repat check for linear scan.
>
> Signed-off-by: Kemeng Shi <shikemeng@xxxxxxxxxxxxxxx>
> ---
> fs/ext4/mballoc.c | 37 ++++++-------------------------------
> 1 file changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 0f8a34513bf6..561780a274cd 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -1075,31 +1075,6 @@ static inline int should_optimize_scan(struct ext4_allocation_context *ac)
> return 1;
> }
>
> -/*
> - * Return next linear group for allocation. If linear traversal should not be
> - * performed, this function just returns the same group
> - */
> -static ext4_group_t
> -next_linear_group(struct ext4_allocation_context *ac, ext4_group_t group,
> - ext4_group_t ngroups)
> -{
> - if (!should_optimize_scan(ac))
> - goto inc_and_return;
> -
> - if (ac->ac_groups_linear_remaining) {
> - ac->ac_groups_linear_remaining--;
> - goto inc_and_return;
> - }
> -
> - return group;
> -inc_and_return:
> - /*
> - * Artificially restricted ngroups for non-extent
> - * files makes group > ngroups possible on first loop.
> - */
> - return group + 1 >= ngroups ? 0 : group + 1;
> -}
> -
> /*
> * ext4_mb_choose_next_group: choose next group for allocation.
> *
> @@ -1118,12 +1093,12 @@ static void ext4_mb_choose_next_group(struct ext4_allocation_context *ac,
> {
> *new_cr = ac->ac_criteria;
>
> - if (!should_optimize_scan(ac) || ac->ac_groups_linear_remaining) {
> - *group = next_linear_group(ac, *group, ngroups);
> - return;
> - }
> -
> - if (*new_cr == CR_POWER2_ALIGNED) {
> + if (!should_optimize_scan(ac))
> + *group = *group + 1 >= ngroups ? 0 : *group + 1;
> + else if (ac->ac_groups_linear_remaining) {
> + ac->ac_groups_linear_remaining--;
> + *group = *group + 1 >= ngroups ? 0 : *group + 1;
> + } else if (*new_cr == CR_POWER2_ALIGNED) {
Hi Kemeng, thanks for the cleanups
I feel that open coding this logic and having a single if for linear scan and
non linear scan cases is making the code a bit more harder to follow and we are
losing some comments as well.
Since our main aim is to avoid the double checking, maybe we can keep
next_linear_group() strictly for getting the next linear group correctly and
rest of the checks outside. So something like:
static ext4_group_t
next_linear_group(ext4_group_t group, ext4_group_t ngroups)
{
/*
* Artificially restricted ngroups for non-extent
* files makes group > ngroups possible on first loop.
*/
return group + 1 >= ngroups ? 0 : group + 1;
}
static void ext4_mb_choose_next_group(...)
{
...
/*
* Fallback to linear scan when optimized scanning is disabled
*/
if (!should_optimize_scan(ac)) {
*group = next_linear_group(*group, ngroups);
return;
}
/*
* Optimized scanning can return non adjacent groups which can cause
* seek overhead for rotational disks. So try few linear groups before
* trying optimized scan.
*/
if (ac->ac_groups_linear_remaining) {
*group = next_linear_group(*group, ngroups);
ac->ac_groups_linear_remaining--;
return;
}
...
}
Let me know your thought.
Regards,
ojaswin
> ext4_mb_choose_next_group_p2_aligned(ac, new_cr, group);
> } else if (*new_cr == CR_GOAL_LEN_FAST) {
> ext4_mb_choose_next_group_goal_fast(ac, new_cr, group);
> --
> 2.30.0
>
