Message-ID: <7c88d247-e0e0-465a-a3ad-f6d641291977@nvidia.com>
Date: Wed, 19 Nov 2025 17:44:22 +1100
From: Balbir Singh <balbirs@...dia.com>
To: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 dri-devel@...ts.freedesktop.org
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
 Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
 Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
 Ying Huang <ying.huang@...ux.alibaba.com>,
 Alistair Popple <apopple@...dia.com>, Oscar Salvador <osalvador@...e.de>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 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>, Lyude Paul <lyude@...hat.com>,
 Danilo Krummrich <dakr@...nel.org>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Ralph Campbell <rcampbell@...dia.com>,
 Mika Penttilä <mpenttil@...hat.com>,
 Matthew Brost <matthew.brost@...el.com>,
 Francois Dugast <francois.dugast@...el.com>
Subject: Re: [PATCH v2] mm/huge_memory: introduce folio_split_unmapped

[Reforwarding - not sure if the original response made it back]

On 11/19/25 07:36, David Hildenbrand (Red Hat) wrote:
> On 15.11.25 09:40, 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 folio_split_unmapped() to handle this special case. Also
>> refactor code and add __folio_freeze_and_split_unmapped() helper that
>> is common to both __folio_split() and folio_split_unmapped().
>>
>> This in turn removes the special casing introduced by the unmapped
>> parameter in __folio_split().
>>
>
> I was briefly wondering: why can't we just detect at the beginning of the __folio_split() that the folio is unmapped (!folio_mapped()) and then continue assuming the folio is unmapped?
>
> The folio is locked, so it shouldn't just become mapped again.
>
> But then I looked into the details and figured that we will also not try to remap (replace migration entries) and focus on anon folios only.
>
> I think we really have to document this properly. See below.
>
> [...]
>

Your observations are correct.

>> +/*
>
> Can we have proper kerneldoc?
>
>> + * This function is a helper for splitting folios that have already been unmapped.
>> + * The use case is that the device or the CPU can refuse to migrate THP pages in
>> + * the middle of migration, due to allocation issues on either side
>> + *
>> + * The high level code is copied from __folio_split, since the pages are anonymous
>> + * and are already isolated from the LRU, the code has been simplified to not
>> + * burden __folio_split with unmapped sprinkled into the code.
>
> Please drop the history of how this code was obtained 🙂 Focus on ducmenting what the function does, what it expects from the caller, and what the result of the operation will be.
>
>> + *
>> + * None of the split folios are unlocked
>
>
> Looking into the details, I think this function also does not
>
> (a) remap the folio
>
> (b) call things like free_folio_and_swap_cache()
>
> Which locks do have to be held by the caller? I'd assume the anon vma lock and the folio lock?
>
> Would this function currently work for anon folios that are in the swapcache?
>
> And I assume this function works for ZONE_DEVICE and !ZONE_DEVICE?
>
>
> Please carefully document all that (locks held, folio isolated, folio unmapped, will not remap the folio, anon folios only, etc).


Something like this below?

/**
 * folio_split_unmapped() - split a large anon folio that is already unmapped
 * @folio: folio to split
 * @new_order: the order of folios after split
 *
 * This function is a helper for splitting folios that have already been
 * unmapped. The use case is that the device or the CPU can refuse to migrate
 * THP pages in the middle of migration, due to allocation issues on either
 * side.
 *
 * anon_vma_lock is not required to be held, mmap_read_lock() or
 * mmap_write_lock() should be held. @folio is expected to be locked by the
 * caller. device-private and non device-private folios are supported along
 * with folios that are in the swapcache. @folio should also be unmapped and
 * isolated from LRU (if applicable)
 *
 * Upon return, the folio is not remapped, split folios are not added to LRU,
 * free_folio_and_swap_cache() is not called, and new folios remain locked.
 *
 * Return: 0 on success, -EAGAIN if the folio cannot be split (e.g., due to
 *         insufficient reference count or extra pins).
 */

>
>> + */
>> +int folio_split_unmapped(struct folio *folio, unsigned int new_order)
>> +{
>> +    int extra_pins, ret = 0;
>> +
>> +    VM_WARN_ON_ONCE_FOLIO(folio_mapped(folio), folio);
>> +    VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>> +    VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
>> +    VM_WARN_ON_ONCE_FOLIO(!folio_test_anon(folio), folio);
>> +
>> +    if (!can_split_folio(folio, 1, &extra_pins))
>> +        return -EAGAIN;
>> +
>> +    local_irq_disable();
>> +    ret = __folio_freeze_and_split_unmapped(folio, new_order, &folio->page, NULL,
>> +                        NULL, false, NULL, SPLIT_TYPE_UNIFORM,
>> +                        0, NULL, extra_pins);
>> +    local_irq_enable();
>> +    return ret;
>> +}
>> +
>>   /*
>>    * This function splits a large folio into smaller folios of order @new_order.
>>    * @page can point to any page of the large folio to split. The split operation
>> @@ -4127,12 +4171,12 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>>    * with the folio. Splitting to order 0 is compatible with all folios.
>>    */
>>   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)
>>   {
>>       struct folio *folio = page_folio(page);
>>         return __folio_split(folio, new_order, &folio->page, page, list,
>> -                 SPLIT_TYPE_UNIFORM, unmapped);
>> +                 SPLIT_TYPE_UNIFORM);
>>   }
>>     /**
>> @@ -4163,7 +4207,7 @@ int folio_split(struct folio *folio, unsigned int new_order,
>>           struct page *split_at, struct list_head *list)
>>   {
>>       return __folio_split(folio, new_order, split_at, &folio->page, list,
>> -                 SPLIT_TYPE_NON_UNIFORM, false);
>> +                 SPLIT_TYPE_NON_UNIFORM);
>>   }
>>     int min_order_for_split(struct folio *folio)
>> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
>> index 46dd68cfc503..05ce95f43ab9 100644
>> --- a/mm/migrate_device.c
>> +++ b/mm/migrate_device.c
>> @@ -909,8 +909,7 @@ static int migrate_vma_split_unmapped_folio(struct migrate_vma *migrate,
>>         folio_get(folio);
>>       split_huge_pmd_address(migrate->vma, addr, true);
>> -    ret = __split_huge_page_to_list_to_order(folio_page(folio, 0), NULL,
>> -                            0, true);
>> +    ret = folio_split_unmapped(folio, 0);
>>       if (ret)
>>           return ret;
>>       migrate->src[idx] &= ~MIGRATE_PFN_COMPOUND;
>
> This is clearly a win.
>

Thanks for the review!
Balbir

Powered by blists - more mailing lists