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

["David Hildenbrand (Red Hat)" <david@kernel.org>]

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?

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?

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