[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7E76988F-A17B-46A5-8121-9D348821AC3E@nvidia.com>
Date: Wed, 26 Nov 2025 11:55:50 -0500
From: Zi Yan <ziy@...dia.com>
To: Balbir Singh <balbirs@...dia.com>
Cc: David Hildenbrand <david@...nel.org>,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>, Nico Pache <npache@...hat.com>,
Ryan Roberts <ryan.roberts@....com>, Dev Jain <dev.jain@....com>,
Barry Song <baohua@...nel.org>, Lance Yang <lance.yang@...ux.dev>,
Miaohe Lin <linmiaohe@...wei.com>, Naoya Horiguchi <nao.horiguchi@...il.com>,
Wei Yang <richard.weiyang@...il.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] mm/huge_memory: change folio_split_supported() to
folio_check_splittable()
On 25 Nov 2025, at 23:14, Balbir Singh wrote:
> On 11/26/25 14:50, Zi Yan wrote:
>> folio_split_supported() used in try_folio_split_to_order() requires
>> folio->mapping to be non NULL, but current try_folio_split_to_order() does
>> not check it. There is no issue in the current code, since
>> try_folio_split_to_order() is only used in truncate_inode_partial_folio(),
>> where folio->mapping is not NULL.
>>
>> To prevent future misuse, move folio->mapping NULL check (i.e., folio is
>> truncated) into folio_split_supported(). Since folio->mapping NULL check
>> returns -EBUSY and folio_split_supported() == false means -EINVAL, change
>> folio_split_supported() return type from bool to int and return error
>> numbers accordingly. Rename folio_split_supported() to
>> folio_check_splittable() to match the return type change.
>>
>> While at it, move is_huge_zero_folio() check and folio_test_writeback()
>> check into folio_check_splittable() and add kernel-doc.
>>
>> Remove all warnings inside folio_check_splittable() and give warnings
>> in __folio_split() instead, so that bool warns parameter can be removed.
>>
>> Signed-off-by: Zi Yan <ziy@...dia.com>
>> Reviewed-by: Wei Yang <richard.weiyang@...il.com>
>> ---
>> include/linux/huge_mm.h | 6 ++--
>> mm/huge_memory.c | 76 +++++++++++++++++++++++------------------
>> 2 files changed, 46 insertions(+), 36 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 1d439de1ca2c..66105a90b4c3 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -375,8 +375,8 @@ int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list
>> int folio_split_unmapped(struct folio *folio, unsigned int new_order);
>> int min_order_for_split(struct folio *folio);
>> int split_folio_to_list(struct folio *folio, struct list_head *list);
>> -bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> - enum split_type split_type, bool warns);
>> +int folio_check_splittable(struct folio *folio, unsigned int new_order,
>> + enum split_type split_type);
>> int folio_split(struct folio *folio, unsigned int new_order, struct page *page,
>> struct list_head *list);
>>
>> @@ -407,7 +407,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 (!folio_split_supported(folio, new_order, SPLIT_TYPE_NON_UNIFORM, /* warns= */ false))
>> + if (folio_check_splittable(folio, new_order, SPLIT_TYPE_NON_UNIFORM))
>> 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 041b554c7115..771df0c02a4a 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3688,15 +3688,40 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>> return 0;
>> }
>>
>> -bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> - enum split_type split_type, bool warns)
>> +/**
>> + * folio_check_splittable() - check if a folio can be split to a given order
>> + * @folio: folio to be split
>> + * @new_order: the smallest order of the after split folios (since buddy
>> + * allocator like split generates folios with orders from @folio's
>> + * order - 1 to new_order).
>> + * @split_type: uniform or non-uniform split
>> + *
>> + * folio_check_splittable() checks if @folio can be split to @new_order using
>> + * @split_type method. The truncated folio check must come first.
>> + *
>> + * Context: folio must be locked.
>> + *
>> + * Return: 0 - @folio can be split to @new_order, otherwise an error number is
>> + * returned.
>> + */
>> +int folio_check_splittable(struct folio *folio, unsigned int new_order,
>> + enum split_type split_type)
>> {
>> + VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
>> + /*
>> + * Folios that just got truncated cannot get split. Signal to the
>> + * caller that there was a race.
>> + *
>> + * TODO: this will also currently refuse folios without a mapping in the
>> + * swapcache (shmem or to-be-anon folios).
>> + */
>> + if (!folio_test_anon(folio) && !folio->mapping)
>> + return -EBUSY;
>> +
>
> Nit: Shouldn't the order of check be
>
> if (!folio->mapping && !folio_test_anon(folio))
>
> works better if folio->mapping is NULL
It does not matter, since folio_test_anon() checks folio->mapping too.
I can revert the order in the next version.
>
>
>> 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");
>> if (new_order == 1)
>> - return false;
>> + 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)) {
>> @@ -3717,9 +3742,7 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> * case, the mapping does not actually support large
>> * folios properly.
>> */
>> - VM_WARN_ONCE(warns,
>> - "Cannot split file folio to non-0 order");
>> - return false;
>> + return -EINVAL;
>> }
>> }
>>
>> @@ -3732,12 +3755,16 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
>> * 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;
>> + return -EINVAL;
>> }
>>
>> - return true;
>> + if (is_huge_zero_folio(folio))
>> + return -EINVAL;
>> +
>> + if (folio_test_writeback(folio))
>> + return -EBUSY;
>> +
>> + return 0;
>> }
>>
>> static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int new_order,
>> @@ -3922,7 +3949,6 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>> int remap_flags = 0;
>> int extra_pins, ret;
>> pgoff_t end = 0;
>> - bool is_hzp;
>>
>> VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>> VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>> @@ -3930,31 +3956,15 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>> if (folio != page_folio(split_at) || folio != page_folio(lock_at))
>> return -EINVAL;
>>
>> - /*
>> - * Folios that just got truncated cannot get split. Signal to the
>> - * caller that there was a race.
>> - *
>> - * TODO: this will also currently refuse shmem folios that are in the
>> - * swapcache.
>> - */
>> - if (!is_anon && !folio->mapping)
>> - return -EBUSY;
>> -
>> if (new_order >= old_order)
>> return -EINVAL;
>>
>> - if (!folio_split_supported(folio, new_order, split_type, /* warn = */ true))
>> - return -EINVAL;
>> -
>> - is_hzp = is_huge_zero_folio(folio);
>> - if (is_hzp) {
>> - pr_warn_ratelimited("Called split_huge_page for huge zero page\n");
>> - return -EBUSY;
>> + ret = folio_check_splittable(folio, new_order, split_type);
>> + if (ret) {
>> + VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
>> + return ret;
>> }
>>
>> - if (folio_test_writeback(folio))
>> - return -EBUSY;
>> -
>> if (is_anon) {
>> /*
>> * The caller does not necessarily hold an mmap_lock that would
>
> Otherwise,looks good!
>
> Acked-by: Balbir Singh <balbirs@...dia.com>
Thanks.
Best Regards,
Yan, Zi
Powered by blists - more mailing lists