All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zi Yan <ziy@nvidia.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: akpm@linux-foundation.org, david@kernel.org,
	lorenzo.stoakes@oracle.com, baolin.wang@linux.alibaba.com,
	Liam.Howlett@oracle.com, npache@redhat.com, ryan.roberts@arm.com,
	dev.jain@arm.com, baohua@kernel.org, lance.yang@linux.dev,
	linux-mm@kvack.org
Subject: Re: [Patch v3 2/2] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported()
Date: Mon, 17 Nov 2025 10:56:39 -0500	[thread overview]
Message-ID: <337CD281-F5B3-47FE-82C3-ECB236450F60@nvidia.com> (raw)
In-Reply-To: <20251117012239.lqm33uu4vl4y5zqc@master>

On 16 Nov 2025, at 20:22, Wei Yang wrote:

> On Thu, Nov 06, 2025 at 03:41:55AM +0000, 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>
>>
> [...]
>> -/* See comments in non_uniform_split_supported() */
>> -bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>> -		bool warns)
>> -{
>> -	if (folio_test_anon(folio)) {
>> -		VM_WARN_ONCE(warns && new_order == 1,
>> -				"Cannot split to order-1 folio");
>> -		if (new_order == 1)
>> -			return false;
>> -	} else  if (new_order) {
>> +	} 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)) {
>
> After re-scan the code, I found we may have a NULL pointer dereference here.
>
> We bail out if folio->mapping == NULL in __folio_split(), which means it is
> possible to be NULL. But we access mapping->flags here.
>
> Looks there is no bug report yet, so I am not sure it worth a separate fix to
> original code.

Probably because the race is small, but a fix is still needed.
Likely commit 6a50c9b512f7 ("mm: huge_memory: fix misused
mapping_large_folio_support() for anon folios") introduced it, but please
double check.

>
>> +			/*
>> +			 * 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.
>> +			 */
>> 			VM_WARN_ONCE(warns,
>> 				"Cannot split file folio to non-0 order");
>> 			return false;
>> 		}
>> 	}
>>
>> -	if (new_order && folio_test_swapcache(folio)) {
>> +	/*
>> +	 * swapcache folio could only be split to order 0
>> +	 *
>> +	 * non-uniform split creates after-split folios with orders from
>> +	 * folio_order(folio) - 1 to new_order, making it not suitable for any
>> +	 * swapcache folio split. Only uniform split to order-0 can be used
>> +	 * here.
>> +	 */
>> +	if ((split_type == SPLIT_TYPE_NON_UNIFORM || new_order) && folio_test_swapcache(folio)) {
>> 		VM_WARN_ONCE(warns,
>> 			"Cannot split swapcache folio to non-0 order");
>> 		return false;
>> @@ -3794,11 +3787,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>> 	if (new_order >= old_order)
>> 		return -EINVAL;
>>
>> -	if (split_type == SPLIT_TYPE_UNIFORM && !uniform_split_supported(folio, new_order, true))
>> -		return -EINVAL;
>> -
>> -	if (split_type == SPLIT_TYPE_NON_UNIFORM &&
>> -	    !non_uniform_split_supported(folio, new_order, true))
>> +	if (!folio_split_supported(folio, new_order, split_type, /* warn = */ true))
>> 		return -EINVAL;
>>
>> 	is_hzp = is_huge_zero_folio(folio);
>> -- 
>> 2.34.1
>
> -- 
> Wei Yang
> Help you, Help me


Best Regards,
Yan, Zi


  reply	other threads:[~2025-11-17 15:56 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-06  3:41 [Patch v3 0/2] mm/huge_memory: Define split_type and consolidate split support checks Wei Yang
2025-11-06  3:41 ` [Patch v3 1/2] mm/huge_memory: introduce enum split_type for clarity Wei Yang
2025-11-06 10:17   ` David Hildenbrand (Red Hat)
2025-11-06 14:57     ` Wei Yang
2025-11-07  0:44   ` Zi Yan
2025-11-06  3:41 ` [Patch v3 2/2] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported() Wei Yang
2025-11-06 10:20   ` David Hildenbrand (Red Hat)
2025-11-07  0:46   ` Zi Yan
2025-11-07  1:17     ` Wei Yang
2025-11-07  2:07       ` Zi Yan
2025-11-07  2:49         ` Wei Yang
2025-11-07  3:21           ` Zi Yan
2025-11-07  7:29             ` Wei Yang
2025-11-14  3:03               ` Wei Yang
2025-11-17  1:22   ` Wei Yang
2025-11-17 15:56     ` Zi Yan [this message]
2025-11-18  2:10       ` Wei Yang
2025-11-18  3:33       ` Wei Yang
2025-11-18  4:10         ` Zi Yan
2025-11-18 18:32           ` Andrew Morton
2025-11-18 18:55             ` Zi Yan
2025-11-18 22:06               ` Andrew Morton
2025-11-19  0:52                 ` Wei Yang
2025-11-20 21:16                   ` Andrew Morton
2025-11-21  0:55                     ` Zi Yan
2025-11-21  9:00                     ` Wei Yang
2025-11-21 14:59                       ` Zi Yan
2025-11-21 16:50                         ` Andrew Morton
2025-11-21 17:00                           ` Zi Yan
2025-11-21 18:39                             ` Andrew Morton
2025-11-21 19:09                               ` Zi Yan
2025-11-21 19:15                                 ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=337CD281-F5B3-47FE-82C3-ECB236450F60@nvidia.com \
    --to=ziy@nvidia.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=npache@redhat.com \
    --cc=richard.weiyang@gmail.com \
    --cc=ryan.roberts@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.