Re: [Patch v3 2/2] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported()

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

On Thu, Nov 06, 2025 at 10:21:21PM -0500, Zi Yan wrote:
>On 6 Nov 2025, at 21:49, Wei Yang wrote:
>
>> On Thu, Nov 06, 2025 at 09:07:22PM -0500, Zi Yan wrote:
>>> On 6 Nov 2025, at 20:17, Wei Yang wrote:
>>>
>>>> On Thu, Nov 06, 2025 at 07:46:14PM -0500, Zi Yan wrote:
>>>>> On 5 Nov 2025, at 22:41, 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.
>>>>>>
>>>>>> 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 and 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>
>>>>>> Cc: "David Hildenbrand (Red Hat)" <david@kernel.org>
>>>>>>
>>>>>> ---
>>>>>> v3:
>>>>>>   * adjust to use split_type
>>>>>>   * rebase on Zi Yan fix lkml.kernel.org/r/20251105162910.752266-1-ziy@nvidia.com
>>>>>> v2:
>>>>>>   * remove need_check
>>>>>>   * update comment
>>>>>>   * add more explanation in change log
>>>>>> ---
>>>>>>  include/linux/huge_mm.h |  8 ++---
>>>>>>  mm/huge_memory.c        | 71 +++++++++++++++++------------------------
>>>>>>  2 files changed, 33 insertions(+), 46 deletions(-)
>>>>>>
>>>>> LGTM. Thanks. Reviewed-by: Zi Yan <ziy@nvidia.com>
>>>>
>>>> Hi, Zi
>>>>
>>>> I am thinking whether it is proper to move the check (new_order < min_order)
>>>> from __folio_split() to folio_split_supported(). So that we could bail out
>>>> early if file system couldn't split to new_order.
>>>>
>>>> Not sure you like it or not.
>>>
>>> It sounds reasonable. My only concern is that that might add another
>>> indentation to the else branch in folio_split_supported().
>>>
>>> You can send a patch, so we can see how it looks.
>>>
>>
>> Here is what come up my mind.
>>
>> If !CONFIG_READ_ONLY_THP_FOR_FS, we directly compare new_order and min_order.
>>
>> If CONFIG_READ_ONLY_THP_FOR_FS, one thing I am not sure is for the khugepaged
>> collapsed THP. If its min_order is 0, it looks we can cover it with following
>> check.
>
>1. mapping_large_folio_support() checks if mapping_max_folio_order() > 0, meaning
>   !mapping_large_folio_support() is mapping_max_folio_order() == 0,
>2. mapping_max_folio_order() >= mapping_min_folio_order(),
>3. combining 1) and 2) means
>   mapping_min_folio_order() <= mapping_max_folio_order() == 0,
>   meaning mapping_min_folio_order() == 0.
>
>so a FS without large folio support always has min_order == 0.
>
>>
>> Look forward your insight.
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index dee416b3f6ed..ef05f246df73 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3704,8 +3704,8 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
>>  		if (new_order == 1)
>>  			return false;
>>  	} else if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order) {
>> -		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>> -		    !mapping_large_folio_support(folio->mapping)) {
>> +		unsigned int min_order = mapping_min_folio_order(folio->mapping);
>> +		if (new_order < min_order) {
>
>This check is good for !CONFIG_READ_ONLY_THP_FOR_FS, but
>for CONFIG_READ_ONLY_THP_FOR_FS and !mapping_large_folio_support(),
>min_order is always 0, how can new_order be smaller than min_order
>to trigger the warning below? You will need to check new_order against
>mapping_max_folio_order().
>
>OK, basically the check should be:
>

Thanks for your analysis.

>if (new_order < mapping_min_folio_order() || new_order > mapping_max_folio_order()).
>

This reminds me one thing, we don't check on max_order now.

For example, the supported split order is [3, 5]. But new_order is set to 6.

In current kernel, we don't do this. try_folio_split_or_unmap() pass
min_order. But selftest will split from pmd_order - 1.

>Then, you might want to add a helper function mapping_folio_order_supported()
>instead and change the warning message below to "Cannot split file folio to
>unsupported order [%d, %d]", min_order, max_order (showing min/max order
>is optional since it kinda defeat the purpose of having the helper function).
>Of course, the comment needs to be changed.
>
>Hmm, but still how could the above check to trigger the warning when
>split_type == SPLIT_TYPE_NON_UNIFORM and new_order is 0? It will not
>trigger, since new_order (as 0) is supported by the mapping.
>
>I guess the min_order check code has to be in the else branch along
>with the existing "if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order)".
>

I am trying to think another way.

For uniform split, after-split folio order is new_order.
For non-uniform split, after-split folio order is [new_order, old_order - 1].

So I come up following draft change.

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index dee416b3f6ed..873680ab4cbb 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3703,28 +3703,18 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
 				"Cannot split to order-1 folio");
 		if (new_order == 1)
 			return false;
-	} else if (split_type == SPLIT_TYPE_NON_UNIFORM || new_order) {
-		if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
-		    !mapping_large_folio_support(folio->mapping)) {
-			/*
-			 * We can always split a folio down to a single page
-			 * (new_order == 0) uniformly.
-			 *
-			 * For any other scenario
-			 *   a) uniform split targeting a large folio
-			 *      (new_order > 0)
-			 *   b) 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.
-			 */
+	} else {
+		/*
+		 * Some explanation here.
+		 */
+		if (new_order && !mapping_folio_order_supported(new_order)) {
+			VM_WARN_ONCE(warns,
+				"Cannot split file folio to unsupported order: %d", new_order);
+			return false;
+		}
+		if (split_type == SPLIT_TYPE_NON_UNIFORM && !mapping_folio_order_supported(old_order - 1)) {
 			VM_WARN_ONCE(warns,
-				"Cannot split file folio to non-0 order");
+				"Cannot split file folio to unsupported order: %d", old_order - 1);
 			return false;
 		}
 	}
-- 
Wei Yang
Help you, Help me