Re: [PATCH] mm/huge_memory.c: introduce split_unmapped_folio_to_order
From: David Hildenbrand (Red Hat)
Date: Wed Nov 12 2025 - 06:34:29 EST
On 12.11.25 11:17, Balbir Singh wrote:
On 11/12/25 21:00, David Hildenbrand (Red Hat) wrote:
On 12.11.25 05:46, Balbir Singh wrote:
Unmapped was added as a parameter to __folio_split() and related
call sites to support splitting of folios already in the midst
of a migration. This special case arose for device private folio
migration since during migration there could be a disconnect between
source and destination on the folio size.
Introduce split_unmapped_folio_to_order() to handle this special case.
This in turn removes the special casing introduced by the unmapped
parameter in __folio_split().
As raised recently, I would hope that we can find a way to make all these splitting functions look more similar in the long term, ideally starting with "folio_split" / "folio_try_split".
What about
folio_split_unmapped()
Do we really have to spell out the "to order" part in the function name?
And if it's more a mostly-internal helper, maybe
__folio_split_unmapped()
subject: "mm/huge_memory: introduce ..."
I can rename it, but currently it confirms to the split_folio with order in the name
The order is there in the name because in the future with mTHP we will want to
support splitting to various orders.
I think we should start naming them more consistently regarding folio_split() immediately and cleanup the other ones later.
I don't understand why "_to_order" must be in the name right now. You can add another variant and start using longer names when really required.
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Zi Yan <ziy@xxxxxxxxxx>
Cc: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
Cc: Rakie Kim <rakie.kim@xxxxxx>
Cc: Byungchul Park <byungchul@xxxxxx>
Cc: Gregory Price <gourry@xxxxxxxxxx>
Cc: Ying Huang <ying.huang@xxxxxxxxxxxxxxxxx>
Cc: Alistair Popple <apopple@xxxxxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxx>
Cc: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
Cc: "Liam R. Howlett" <Liam.Howlett@xxxxxxxxxx>
Cc: Nico Pache <npache@xxxxxxxxxx>
Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
Cc: Dev Jain <dev.jain@xxxxxxx>
Cc: Barry Song <baohua@xxxxxxxxxx>
Cc: Lyude Paul <lyude@xxxxxxxxxx>
Cc: Danilo Krummrich <dakr@xxxxxxxxxx>
Cc: David Airlie <airlied@xxxxxxxxx>
Cc: Simona Vetter <simona@xxxxxxxx>
Cc: Ralph Campbell <rcampbell@xxxxxxxxxx>
Cc: Mika Penttilä <mpenttil@xxxxxxxxxx>
Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
Cc: Francois Dugast <francois.dugast@xxxxxxxxx>
Suggested-by: Zi Yan <ziy@xxxxxxxxxx>
Signed-off-by: Balbir Singh <balbirs@xxxxxxxxxx>
---
include/linux/huge_mm.h | 5 +-
mm/huge_memory.c | 135 ++++++++++++++++++++++++++++++++++------
mm/migrate_device.c | 3 +-
3 files changed, 120 insertions(+), 23 deletions(-)
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index e2e91aa1a042..9155e683c08a 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -371,7 +371,8 @@ enum split_type {
bool can_split_folio(struct folio *folio, int caller_pins, int *pextra_pins);
int __split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
- unsigned int new_order, bool unmapped);
+ unsigned int new_order);
+int split_unmapped_folio_to_order(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,
@@ -382,7 +383,7 @@ int folio_split(struct folio *folio, unsigned int new_order, struct page *page,
static inline int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
unsigned int new_order)
{
- return __split_huge_page_to_list_to_order(page, list, new_order, false);
+ return __split_huge_page_to_list_to_order(page, list, new_order);
}
static inline int split_huge_page_to_order(struct page *page, unsigned int new_order)
{
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 0184cd915f44..942bd8410c54 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3747,7 +3747,6 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
* @lock_at: a page within @folio to be left locked to caller
* @list: after-split folios will be put on it if non NULL
* @split_type: perform uniform split or not (non-uniform split)
- * @unmapped: The pages are already unmapped, they are migration entries.
*
* It calls __split_unmapped_folio() to perform uniform and non-uniform split.
* It is in charge of checking whether the split is supported or not and
@@ -3763,7 +3762,7 @@ bool folio_split_supported(struct folio *folio, unsigned int new_order,
*/
static int __folio_split(struct folio *folio, unsigned int new_order,
struct page *split_at, struct page *lock_at,
- struct list_head *list, enum split_type split_type, bool unmapped)
+ struct list_head *list, enum split_type split_type)
Yeah, nice to see that go.
{
struct deferred_split *ds_queue;
XA_STATE(xas, &folio->mapping->i_pages, folio->index);
@@ -3809,14 +3808,12 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
* is taken to serialise against parallel split or collapse
* operations.
*/
- if (!unmapped) {
- anon_vma = folio_get_anon_vma(folio);
- if (!anon_vma) {
- ret = -EBUSY;
- goto out;
- }
- anon_vma_lock_write(anon_vma);
+ anon_vma = folio_get_anon_vma(folio);
+ if (!anon_vma) {
+ ret = -EBUSY;
+ goto out;
}
+ anon_vma_lock_write(anon_vma);
mapping = NULL;
} else {
unsigned int min_order;
@@ -3882,8 +3879,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
goto out_unlock;
}
- if (!unmapped)
- unmap_folio(folio);
+ unmap_folio(folio);
Hm, I would have hoped that we could factor out the core logic and reuse it for the new helper, instead of duplicating code.
Did you look into that?
I did, I ended up with larger spaghetti, I was hoping to look it as a follow up
after the series with the mTHP changes and support (that is to be designed and
prototyped).
Looking at it in more detail, the code duplication is not desired.
We have to find a way to factor the existing code out and reuse it from any new function.
--
Cheers
David