All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Yang <richard.weiyang@gmail.com>
To: Zi Yan <ziy@nvidia.com>
Cc: Wei Yang <richard.weiyang@gmail.com>,
	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: Tue, 18 Nov 2025 03:33:39 +0000	[thread overview]
Message-ID: <20251118033339.7rukw7vvjvlbj5jb@master> (raw)
In-Reply-To: <337CD281-F5B3-47FE-82C3-ECB236450F60@nvidia.com>

On Mon, Nov 17, 2025 at 10:56:39AM -0500, Zi Yan wrote:
>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.
>

After searching the history, it has four related commits.

I listed here in timeline.

[1] commit c010d47f107f609b9f4d6a103b6dfc53889049e9
    Author: Zi Yan <ziy@nvidia.com>
    Date:   Mon Feb 26 15:55:33 2024 -0500
    
        mm: thp: split huge page to any lower order pages
    
[2] commit 6a50c9b512f7734bc356f4bd47885a6f7c98491a (HEAD -> tmp)
    Author: Ran Xiaokai <ran.xiaokai@zte.com.cn>
    Date:   Fri Jun 7 17:40:48 2024 +0800
    
        mm: huge_memory: fix misused mapping_large_folio_support() for anon folios
    
[3] commit 9b2f764933eb5e3ac9ebba26e3341529219c4401 (refs/bisect/bad)
    Author: Zi Yan <ziy@nvidia.com>
    Date:   Wed Jan 22 11:19:27 2025 -0500
    
        mm/huge_memory: allow split shmem large folio to any lower order

[4] commit 58729c04cf1092b87aeef0bf0998c9e2e4771133 (HEAD -> tmp)
    Author: Zi Yan <ziy@nvidia.com>
    Date:   Fri Mar 7 12:39:57 2025 -0500
    
        mm/huge_memory: add buddy allocator like (non-uniform) folio_split()

So I think the fix tag should be [1], right?

And do we need cc stable?

-- 
Wei Yang
Help you, Help me


  parent reply	other threads:[~2025-11-18  3:33 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
2025-11-18  2:10       ` Wei Yang
2025-11-18  3:33       ` Wei Yang [this message]
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=20251118033339.7rukw7vvjvlbj5jb@master \
    --to=richard.weiyang@gmail.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=ryan.roberts@arm.com \
    --cc=ziy@nvidia.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.