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

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

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.

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