Re: [Patch v3 4/4] mm/huge_memory: optimize old_order derivation during folio splitting

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Tue, Oct 21, 2025 at 09:21:42PM +0000, Wei Yang wrote:
> Folio splitting requires both the folio's original order (@old_order)
> and the new target order (@split_order).
>
> In the current implementation, @old_order is repeatedly retrieved using
> folio_order().
>
> However, for every iteration after the first, the folio being split is
> the result of the previous split, meaning its order is already known to
> be equal to the previous iteration's @split_order.
>
> This commit optimizes the logic:
>
>   * Instead of calling folio_order(), we now set @old_order directly to
>     the value of @split_order from the previous iteration.
>
> This change avoids unnecessary function calls and simplifies the loop
> setup.
>
> Also it removes a check for non-existent case, since for uniform
> splitting we only do split when @split_order == @new_order.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>

Thanks the separation makes a HUGE difference, much appreciated. So:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> Cc: Zi Yan <ziy@nvidia.com>
> Cc: wang lian <lianux.mm@gmail.com>
> ---
>  mm/huge_memory.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 093b3ffb180f..a4fa8b0e5b5a 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3596,8 +3596,8 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>  		struct address_space *mapping, bool uniform_split)
>  {
>  	const bool is_anon = folio_test_anon(folio);
> -	int order = folio_order(folio);
> -	int start_order = uniform_split ? new_order : order - 1;
> +	int old_order = folio_order(folio);
> +	int start_order = uniform_split ? new_order : old_order - 1;
>  	int split_order;
>
>  	folio_clear_has_hwpoisoned(folio);
> @@ -3609,14 +3609,11 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>  	for (split_order = start_order;
>  	     split_order >= new_order;

A thought for the future - now things are simplified, it might be nice to just
separate out the core of this loop and have the uniform split just call the
split out function directly, and the non-uniform one do the loop.

As it's a bit gross in the uniform case we just let split_order go to new_order
- 1 to exit the loop.

BUT - let's please save that for another patch :)

This all looks fine.

>  	     split_order--) {
> -		int old_order = folio_order(folio);
>  		int nr_new_folios = 1UL << (old_order - split_order);
>
>  		/* order-1 anonymous folio is not supported */
>  		if (is_anon && split_order == 1)
>  			continue;
> -		if (uniform_split && split_order != new_order)
> -			continue;
>
>  		if (mapping) {
>  			/*
> @@ -3643,7 +3640,13 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>  			mod_mthp_stat(old_order, MTHP_STAT_NR_ANON, -1);
>  			mod_mthp_stat(split_order, MTHP_STAT_NR_ANON, nr_new_folios);
>  		}
> +		/*
> +		 * If uniform split, the process is complete.
> +		 * If non-uniform, continue splitting the folio at @split_at
> +		 * as long as the next @split_order is >= @new_order.
> +		 */
>  		folio = page_folio(split_at);
> +		old_order = split_order;
>  	}
>
>  	return 0;
> --
> 2.34.1
>
>