Re: [PATCH] mm/huge_memory: consolidate order-related checks into folio_split_supported()

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

On Fri, Nov 14, 2025 at 07:43:38AM -0500, Zi Yan wrote:
>On 14 Nov 2025, at 3:49, David Hildenbrand (Red Hat) wrote:
>
[...]
>>> +
>>> +	if (new_order >= old_order)
>>> +		return -EINVAL;
>>> +
>>>   	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");
>>>   		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 {
>>> +		const struct address_space *mapping = NULL;
>>> +
>>> +		mapping = folio->mapping;
>>
>> const struct address_space *mapping = folio->mapping;
>>
>>> +
>>> +		/* Truncated ? */
>>> +		/*
>>> +		 * TODO: add support for large shmem folio in swap cache.
>>> +		 * When shmem is in swap cache, mapping is NULL and
>>> +		 * folio_test_swapcache() is true.
>>> +		 */
>>> +		if (!mapping)
>>> +			return false;
>>> +
>>> +		/*
>>> +		 * We have two types of split:
>>> +		 *
>>> +		 *   a) uniform split: split folio directly to new_order.
>>> +		 *   b) non-uniform split: create after-split folios with
>>> +		 *      orders from (old_order - 1) to new_order.
>>> +		 *
>>> +		 * For file system, we encodes it supported folio order in
>>> +		 * mapping->flags, which could be checked by
>>> +		 * mapping_folio_order_supported().
>>> +		 *
>>> +		 * With these knowledge, we can know whether folio support
>>> +		 * split to new_order by:
>>> +		 *
>>> +		 *   1. check new_order is supported first
>>> +		 *   2. check (old_order - 1) is supported if
>>> +		 *      SPLIT_TYPE_NON_UNIFORM
>>> +		 */
>>> +		if (!mapping_folio_order_supported(mapping, new_order)) {
>>> +			VM_WARN_ONCE(warns,
>>> +				"Cannot split file folio to unsupported order: %d", new_order);
>>
>> Is that really worth a VM_WARN_ONCE? We didn't have that previously IIUC, we would only return
>> -EINVAL.
>

Sorry for introducing this unpleasant affair.

Hope I can explain what I have done.

>No, and it causes undesired warning when LBS folio is enabled. I explicitly
>removed this warning one month ago in the LBS related patch[1].
>

Yes, I see you removal of a warning in [1].

While in the discussion in [2], you mentioned:

  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.

I thought you agree to print a warning message here. So I am confused.

>It is so frustrating to see this part of patch. Wei has RB in the aforementioned
>patch and still add this warning blindly. I am not sure if Wei understands
>what he is doing, since he threw the idea to me and I told him to just
>move the code without changing the logic, but he insisted doing it in his
>own way and failed[2]. This retry is still wrong.
>

I think we are still discussing the problem and a patch maybe more convenient
to proceed. I didn't insist anything and actually I am looking forward your
option and always respect your insight. Never thought to offend you.

In discussion [2], you pointed out two concerns:

  1) new_order < min_order is meaning less if min_order is 0
  2) how to do the check if new_order is 0 for non-uniform split

For 1), you suggested to add mapping_folio_order_supported().
For 2), I come up an idea to check (old_order - 1) <= max_order. Originally,
we just check !max_order. I think this could cover it.

So I gather them together here to see whether it is suitable.

If I missed some part, hope you could let me know.

>Wei, please make sure you understand the code before sending any patch.
>
>[1] https://lore.kernel.org/linux-mm/20251017013630.139907-1-ziy@nvidia.com/
>[2] https://lore.kernel.org/linux-mm/20251114030301.hkestzrk534ik7q4@master/
>
>Best Regards,
>Yan, Zi

-- 
Wei Yang
Help you, Help me