Re: [PATCH] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported()

[Wei Yang <richard.weiyang@gmail.com>]

On Mon, Nov 03, 2025 at 09:30:03PM -0500, Zi Yan wrote:
>On 3 Nov 2025, at 19:36, Wei Yang wrote:
>
>> On Mon, Nov 03, 2025 at 11:34:47AM -0500, Zi Yan wrote:
>>> On 31 Oct 2025, at 22:11, Wei Yang wrote:
>>>
>>>> The functions uniform_split_supported() and
>>>> non_uniform_split_supported() share significantly similar logic.
>>>>
>>>> The only functional difference is that uniform_split_supported()
>>>> includes an additional check on the requested @new_order before
>>>
>>> Please elaborate on what the check is for.
>>>
>>>> proceeding with further validation.
>>
>> How about this:
>>
>> The only functional difference is that uniform_split_supported() includes an
>> additional check on the requested @new_order and split type to confirm support
>> from file system or swap cache.
>
>You are describing what the code does instead of its actual meaning.
>You need to describe:
>1. what is the difference between uniform split and non-uniform split?
>2. what order does what file systems support? Only order-0.
>3. what order does swap cache support? Only order-0.
>4. why can uniform split be used to split large folios from 2 or 3 to
>   order-0?
>5. why can non uniform split not be used to split large folios from 2
>   or 3 to order-0?
>6. The logic similarity between uniform_split_supported() and
>   non_uniform_split_supported() and they can be combined with detailed
>   comment.
>

Here is the updated version:

The only functional difference is that uniform_split_supported() includes an
additional check on the requested @new_order.

The reason for this check comes from the following two aspects:

  * some file system or swap cache just supports order-0 folio
  * the behavioral difference between uniform/non-uniform split

The behavioral difference between uniform split and non-uniform:

  * uniform split splits folio directly to @new_order
  * non-uniform split creates after-split folios with orders from
    folio_order(folio) - 1 to new_order.

This means for non-uniform split or !new_order split we should check the file
system and swap cache respectively.

>>
>>>>
>>>> This commit unifies the logic by introducing a new variable,
>>>> @need_check, which is conditionally set based on whether a uniform
>>>> split is requested. This allows us to merge the two functions into
>>>> a single, combined helper, removing redundant code and simplifying
>>>> the split support checking mechanism.
>>>>
>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>> Cc: Zi Yan <ziy@nvidia.com>
>>>> ---
>>>>  include/linux/huge_mm.h |  8 +++---
>>>>  mm/huge_memory.c        | 55 +++++++++++------------------------------
>>>>  2 files changed, 18 insertions(+), 45 deletions(-)
>>>>
>>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>>> index cbb2243f8e56..79343809a7be 100644
>>>> --- a/include/linux/huge_mm.h
>>>> +++ b/include/linux/huge_mm.h
>>>> @@ -369,10 +369,8 @@ int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list
>>>>  		unsigned int new_order, bool unmapped);
>>>>  int min_order_for_split(struct folio *folio);
>>>>  int split_folio_to_list(struct folio *folio, struct list_head *list);
>>>> -bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>> -		bool warns);
>>>> -bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>> -		bool warns);
>>>> +bool folio_split_supported(struct folio *folio, unsigned int new_order,
>>>> +		bool uniform_split, bool warns);
>>>>  int folio_split(struct folio *folio, unsigned int new_order, struct page *page,
>>>>  		struct list_head *list);
>>>>
>>>> @@ -403,7 +401,7 @@ static inline int split_huge_page_to_order(struct page *page, unsigned int new_o
>>>>  static inline int try_folio_split_to_order(struct folio *folio,
>>>>  		struct page *page, unsigned int new_order)
>>>>  {
>>>> -	if (!non_uniform_split_supported(folio, new_order, /* warns= */ false))
>>>> +	if (!folio_split_supported(folio, new_order, /* uniform_split = */ false, /* warns= */ false))
>>>>  		return split_huge_page_to_order(&folio->page, new_order);
>>>>  	return folio_split(folio, new_order, page, NULL);
>>>>  }
>>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>>> index d1fa0d2d9b44..f6d2cb2a5ca0 100644
>>>> --- a/mm/huge_memory.c
>>>> +++ b/mm/huge_memory.c
>>>> @@ -3673,55 +3673,34 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>>>>  	return 0;
>>>>  }
>>>>
>>>> -bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>> -		bool warns)
>>>> +bool folio_split_supported(struct folio *folio, unsigned int new_order,
>>>> +		bool uniform_split, bool warns)
>>>>  {
>>>> -	if (folio_test_anon(folio)) {
>>>> -		/* order-1 is not supported for anonymous THP. */
>>>> -		VM_WARN_ONCE(warns && new_order == 1,
>>>> -				"Cannot split to order-1 folio");
>>>> -		return new_order != 1;
>>>> -	} else if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>>> -	    !mapping_large_folio_support(folio->mapping)) {
>>>> -		/*
>>>> -		 * No split if the file system does not support large folio.
>>>> -		 * Note that we might still have THPs in such mappings due to
>>>> -		 * CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping
>>>> -		 * does not actually support large folios properly.
>>>> -		 */
>>>> -		VM_WARN_ONCE(warns,
>>>> -			"Cannot split file folio to non-0 order");
>>>> -		return false;
>>>> -	}
>>>> -
>>>> -	/* Only swapping a whole PMD-mapped folio is supported */
>>>> -	if (folio_test_swapcache(folio)) {
>>>> -		VM_WARN_ONCE(warns,
>>>> -			"Cannot split swapcache folio to non-0 order");
>>>> -		return false;
>>>> -	}
>>>> +	bool need_check = uniform_split ? new_order : true;
>>>>
>>>> -	return true;
>>>> -}
>>>> -
>>>> -/* See comments in non_uniform_split_supported() */
>>>> -bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>>>> -		bool warns)
>>>> -{
>>>>  	if (folio_test_anon(folio)) {
>>>> +		/* order-1 is not supported for anonymous THP. */
>>>>  		VM_WARN_ONCE(warns && new_order == 1,
>>>>  				"Cannot split to order-1 folio");
>>>>  		return new_order != 1;
>>>> -	} else  if (new_order) {
>>>> +	} else if (need_check) {
>>>>  		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>>>>  		    !mapping_large_folio_support(folio->mapping)) {
>>>> +			/*
>>>> +			 * No split if the file system does not support large
>>>> +			 * folio.  Note that we might still have THPs in such
>>>> +			 * mappings due to CONFIG_READ_ONLY_THP_FOR_FS. But in
>>>> +			 * that case, the mapping does not actually support
>>>> +			 * large folios properly.
>>>> +			 */
>>>
>>> Blindly copying the comment here causes fusion. The checks for
>>> uniform and non uniform look similar but this comment is specific
>>> for non uniform split. The “No split” only applies to non uniform
>>> split, but for uniform split as long as order is 0, the folio
>>> can be split.
>>>
>>
>> Per my understanding, "no split" applies to both uniform/non uniform split
>> when new_order is not 0.
>
>Not exactly. For non uniform split, any new_order value is not allowed.
>
>>
>> So the logic here is:
>>
>>   * uniform split && !new_order: no more check
>>   * non uniform split: do the check regardless of the new_order
>>
>> But I am lack of some background knowledge, if it is wrong, please correct me.
>
>You are changing the code, please do your homework first. Or you can
>ask. After go through the above 6 bullet points, you should get the
>background knowledge.
>
>>
>>> Please rewrite this comment to clarify both uniform and non uniform
>>> cases.
>>
>> Not sure this one would be better?
>>
>>    We can always split a folio down to a single page (new_order == 0) directly.
>
>Not always, the exceptions are listed below.
>

I mean uniform split to order-0, maybe above line misleading to non-uniform
split?

>>
>>    For any other scenario
>>       * uniform split targeting a large folio (new_order > 0)
>>       * any non-uniform split
>>    we must confirm that the file system supports large folios.
>>
>>    Note that we might still have THPs in such mappings due to
>>    CONFIG_READ_ONLY_THP_FOR_FS. But in that case, the mapping does not actually
>>    support large folios properly.
>
>These filesystems do not support large folios except THPs created from khugepaged when CONFIG_READ_ONLY_THP_FOR_FS is enabled.
>

Want to confirm to see whether I understand correctly.

We have two kinds of file system:

  a) support large folio
  b) not support large folio

For a), we can split large folio to min_order_for_split(), uniform or
non-uniform.

For b), normally there is no large folio. The large folio is collapsed by
khugepaged when CONFIG_READ_ONLY_THP_FOR_FS is enabled. So we can only split
it to order-0 folio for this case.

Not sure this one would be better?

   We can always split a folio down to a single page (new_order == 0)
   uniformly.

   For any other scenario
      * uniform split targeting a large folio (new_order > 0)
      * any non-uniform split
   we must confirm that the file system supports large folios.

   Note that we might still have THPs in such mappings, which is created from
   khugepaged when CONFIG_READ_ONLY_THP_FOR_FS is enabled. But in that case,
   the mapping does not actually support large folios properly.


>>>>  			VM_WARN_ONCE(warns,
>>>>  				"Cannot split file folio to non-0 order");
>>>>  			return false;
>>>>  		}
>>>>  	}
>>>>
>>>> -	if (new_order && folio_test_swapcache(folio)) {
>>>> +	/* Only swapping a whole PMD-mapped folio is supported */
>>>
>>> The same issue like the above one. Please rewrite this comment as well.
>>>
>>
>> How about this one:
>>
>>    swapcache folio could only be split to order 0
>
>This looks good.
>
>>
>>    For non-uniform split or uniform split targeting a large folio, return
>>    false.
>
>You are just describing the code.
>
>non-uniform split creates after-split folios with orders from
>folio_order(folio) - 1 to new_order, making it not suitable for any swapcache
>folio split. Only uniform split to order-0 can be used here.
>

Below is the updated version:

    swapcache folio could only be split to order 0

    non-uniform split creates after-split folios with orders from
    folio_order(folio) - 1 to new_order, making it not suitable for any
    swapcache folio split. Only uniform split to order-0 can be used here.

>>
>>>> +	if (need_check && folio_test_swapcache(folio)) {
>>>>  		VM_WARN_ONCE(warns,
>>>>  			"Cannot split swapcache folio to non-0 order");
>>>>  		return false;
>>>> @@ -3779,11 +3758,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>>>>  	if (new_order >= old_order)
>>>>  		return -EINVAL;
>>>>
>>>> -	if (uniform_split && !uniform_split_supported(folio, new_order, true))
>>>> -		return -EINVAL;
>>>> -
>>>> -	if (!uniform_split &&
>>>> -	    !non_uniform_split_supported(folio, new_order, true))
>>>> +	if (!folio_split_supported(folio, new_order, uniform_split, /* warn = */ true))
>>>>  		return -EINVAL;
>>>>
>>>>  	is_hzp = is_huge_zero_folio(folio);
>>>> -- 
>>>> 2.34.1
>>>
>>>
>>> Best Regards,
>>> Yan, Zi
>>
>> -- 
>> Wei Yang
>> Help you, Help me
>
>
>Best Regards,
>Yan, Zi

-- 
Wei Yang
Help you, Help me