Re: [PATCH] mm/huge_memory.c: introduce split_unmapped_folio_to_order
From: David Hildenbrand (Red Hat)
Date: Tue Nov 18 2025 - 15:17:19 EST
On 14.11.25 01:23, Zi Yan wrote:
On 13 Nov 2025, at 16:56, Balbir Singh wrote:
On 11/14/25 08:45, Zi Yan wrote:
On 13 Nov 2025, at 16:39, Balbir Singh wrote:
On 11/13/25 10:49, Balbir Singh wrote:
On 11/12/25 22:34, David Hildenbrand (Red Hat) wrote:
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.
Ack
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.
I came up with a helper, but that ends up with another boolean do_lru.
Zi, David, any opinions on the approach below?
Looks good to me. We might want a better name instead of
__folio_split_unmapped(). Or __split_unmapped_folio() should
be renamed, since these two function names are too similar.
Maybe __folio_split_unmapped() -> __freeze_and_split_unmapped_folio().
Feel free to come up with a better name. :)
I had __folio_split_unfreeze() to indicate we split the folio and unfreeze at the end, but
it does not reflect that we freeze it as well. Looks like we are trending towards folio_split
as the prefix (IIUC Dave correctly), I like your name, but if folio_split is going to be
required then may be __folio_split_unmapped_unfreeze()?
OK, if __folio prefix is a convention,
Yes, let's start cleaning all this up.
how about
__folio_freeze_and_split_unmapped()? __folio_split_unmapped_unfreeze() sounds
like folio is frozen when the function is called. Of course, a more accurate
one would be __folio_freeze_split_unfreeze_unmapped(). It can work if
it is not too long. :)
Let me take a look at v2.
--
Cheers
David