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

[Zi Yan <ziy@nvidia.com>]

On 4 Nov 2025, at 2:53, Wei Yang wrote:

> 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.
>

LGTM.

>>>
>>>>>
>>>>> 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?

Right. If you mean uniform split, then write uniform split instead.

>
>>>
>>>    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.

Exactly.

>
> 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.

LGTM.
>
>
>>>>>  			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.

LGTM.

Thank you for updating the comments. Looking forward to your updated patch.
>
>>>
>>>>> +	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


Best Regards,
Yan, Zi