Re: [Patch v2] mm/huge_memory: consolidate order-related checks into folio_check_splittable()

[Zi Yan <ziy@nvidia.com>]

On 6 Jan 2026, at 4:54, Wei Yang wrote:

> On Mon, Jan 05, 2026 at 05:16:45PM +0100, David Hildenbrand (Red Hat) wrote:
>> On 1/4/26 03:37, Wei Yang wrote:
>>> On Tue, Dec 23, 2025 at 12:25:39PM +0000, Wei Yang wrote:
>>>> The primary goal of the folio_check_splittable() function is to validate
>>>> whether a folio is suitable for splitting and to bail out early if it is
>>>> not.
>>>>
>>>> Currently, some order-related checks are scattered throughout the
>>>> calling code rather than being centralized in folio_check_splittable().
>>>>
>>>> This commit moves all remaining order-related validation logic into
>>>> folio_check_splittable(). This consolidation ensures that the function
>>>> serves its intended purpose as a single point of failure and improves
>>>> the clarity and maintainability of the surrounding code.
>>>>
>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>> Cc: Zi Yan <ziy@nvidia.com>
>>>>
>>>> ---
>>> [...]
>>>> @@ -3719,28 +3723,33 @@ int folio_check_splittable(struct folio *folio, unsigned int new_order,
>>>> 		/* order-1 is not supported for anonymous THP. */
>>>> 		if (new_order == 1)
>>>> 			return -EINVAL;
>>>> -	} 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.
>>>> -			 */
>>>> -			return -EINVAL;
>>>> +	} 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.
>>>> +				 */
>>>> +				return -EINVAL;
>>>> +			}
>>>> 		}
>>>
>>> Hi, Happy New Year to all :-)
>>
>> Happy new year to you, too!
>>
>> There was an offlist discussion about some of the text below, because a
>> couple of people wondered whether it was an LLM-generated reply, and whether
>> it is even worth the time to read.
>>
>> So I am curious, did you end up using an LLM to compose this reply, and if
>> so, to which degree? Only to improve your writing or also to come up with an
>> analysis, code etc?
>>
>
> The first three paragraph of the mail is polished by LLM, since once upon a
> time Andrew suggested me to use LLM to refine my text.
>
> Others, including the code change is not LLM-generated.
>
>> Feel free to use an LLM to improve your writing, analysis etc. Just a note
>> that nobody here is interested in getting LLM-slopped, so don't send
>> unfiltered/unchecked LLM output to the list.
>>
>> In general, I think it was raised already in the past, please don't send
>> patches for code you don't fully understand. It consumes quite some bandwidth
>> for us reviewers/maintainers here and it just gets very likely to break
>> things by accident.
>>
>> The comment change suggestion below does not make any sense to fix a warning
>> we trigger. If an LLM wrote it, you should never have sent it. If you wrote
>> it, you should have invested more time to understand the problem and come up
>> with a reasonable solution ... or not worked on it in the first place if you
>> don't understand the details.
>>
>>
>> To the issue at hand: Zi Yan pointed this very thing out in v1 [1], no?
>>
>
> Hmm.. this is not the same thing.
>
> Actually before sending v2, I have talked with Zi Yan off-list and he said it
> looks good.

The off-list discussion was purely on V1 and you never sent me V2.
The last off-list email exchange was:

you: The related cleanup looks merged. Do you think it is proper to send v2 now?
me: Sure, feel free to do so.

No one would interpret it as “V2 looks good”.

In addition, if your patches are solely relying on other’s “it looks good”,
please do not send them. You are responsible for the correctness of your patches.

I am done with wasting time on you.

>
> But after triggering the warning, I re-read the history and found the logic is
> correct but comment is misleading. And current upstream don't report the
> warning if we want to split folio uniformly to 0 when the folio doesn't
> support.
>
> Or more worse, we should split to 0, but we didn't.
>
> Thanks for your patience on taking a look.
>
>> The patch as is cannot work: we cannot return -EINVAL for something that is
>> not supposed to trigger a warning.
>>
>> [1] https://lore.kernel.org/linux-mm/01FABE3A-AD4E-4A09-B971-C89503A848DF@nvidia.com/
>>
>> -- 
>> Cheers
>>
>> David
>
> -- 
> Wei Yang
> Help you, Help me


Best Regards,
Yan, Zi