From: Wei Yang <richard.weiyang@gmail.com>
To: "David Hildenbrand (Red Hat)" <david@kernel.org>
Cc: Wei Yang <richard.weiyang@gmail.com>,
akpm@linux-foundation.org, lorenzo.stoakes@oracle.com,
ziy@nvidia.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 v2] mm/huge_memory: merge uniform_split_supported() and non_uniform_split_supported()
Date: Wed, 5 Nov 2025 09:15:32 +0000 [thread overview]
Message-ID: <20251105091532.zns677dyv436t2d6@master> (raw)
In-Reply-To: <dc0ecc2c-4089-484f-917f-920fdca4c898@kernel.org>
On Wed, Nov 05, 2025 at 09:05:31AM +0100, David Hildenbrand (Red Hat) wrote:
>On 05.11.25 08:25, 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>
>>
>> ---
>> v2:
>> * remove need_check
>> * update comment
>> * add more explanation in change log
>> * selftests/split_huge_page_test pass
>> ---
>> include/linux/huge_mm.h | 8 ++---
>> mm/huge_memory.c | 70 ++++++++++++++++++-----------------------
>> 2 files changed, 33 insertions(+), 45 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index cbb2243f8e56..79343809a7be 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -369,10 +369,8 @@ int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list
>> unsigned int new_order, bool unmapped);
>> int min_order_for_split(struct folio *folio);
>> int split_folio_to_list(struct folio *folio, struct list_head *list);
>> -bool uniform_split_supported(struct folio *folio, unsigned int new_order,
>> - bool warns);
>> -bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>> - bool warns);
>> +bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> + bool uniform_split, bool warns);
>> int folio_split(struct folio *folio, unsigned int new_order, struct page *page,
>> struct list_head *list);
>> @@ -403,7 +401,7 @@ static inline int split_huge_page_to_order(struct page *page, unsigned int new_o
>> static inline int try_folio_split_to_order(struct folio *folio,
>> struct page *page, unsigned int new_order)
>> {
>> - if (!non_uniform_split_supported(folio, new_order, /* warns= */ false))
>> + if (!folio_split_supported(folio, new_order, /* uniform_split = */ false, /* warns= */ false))
>> return split_huge_page_to_order(&folio->page, new_order);
>> return folio_split(folio, new_order, page, NULL);
>> }
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 381a49c5ac3f..db442e0e3a46 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3666,55 +3666,49 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>> return 0;
>> }
>> -bool non_uniform_split_supported(struct folio *folio, unsigned int new_order,
>> - bool warns)
>> +bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> + bool uniform_split, bool warns)
>> {
>> if (folio_test_anon(folio)) {
>> /* order-1 is not supported for anonymous THP. */
>> VM_WARN_ONCE(warns && new_order == 1,
>> "Cannot split to order-1 folio");
>> return new_order != 1;
>> - } else if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) &&
>> - !mapping_large_folio_support(folio->mapping)) {
>> - /*
>> - * No split if the file system does not support large folio.
>> - * Note that we might still have THPs in such mappings due to
>> - * CONFIG_READ_ONLY_THP_FOR_FS. 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;
>> - }
>> -
>> - /* Only swapping a whole PMD-mapped folio is supported */
>> - if (folio_test_swapcache(folio)) {
>> - VM_WARN_ONCE(warns,
>> - "Cannot split swapcache folio to non-0 order");
>> - return false;
>> - }
>> -
>> - return true;
>> -}
>> -
>> -/* 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");
>> - return new_order != 1;
>> - } else if (new_order) {
>> + } else if (!uniform_split || 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.
>> + */
>> 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 ((!uniform_split || new_order) && folio_test_swapcache(folio)) {
>
>Staring at the existing code, how would we reach the folio_test_swapcache()
>test for anon folios?
>
>At the beginning of the function we have
>
>if (folio_test_anon()) {
> ...
> return new_order != 1;
>}
>
>Aren't we missing a check for anon folios that are in the swapcache?
>
Hmm... haven't noticed this.
I need to do some home work on it.
>--
>Cheers
>
>David
--
Wei Yang
Help you, Help me
next prev parent reply other threads:[~2025-11-05 9:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-05 7:25 Wei Yang
2025-11-05 8:05 ` David Hildenbrand (Red Hat)
2025-11-05 9:15 ` Wei Yang [this message]
2025-11-05 16:28 ` Zi Yan
2025-11-05 16:41 ` Zi Yan
2025-11-06 1:46 ` Wei Yang
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=20251105091532.zns677dyv436t2d6@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox