Re: [PATCH] mm/huge_memory.c: introduce split_unmapped_folio_to_order

[Balbir Singh]

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.


>>
>> 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).

Balbir