On 22 Oct 2025, at 3:16, Balbir Singh wrote:

> On 10/22/25 13:59, Zi Yan wrote:
>> On 21 Oct 2025, at 17:34, Balbir Singh wrote:
>>
>>> On 10/20/25 09:59, Zi Yan wrote:
>>>> On 19 Oct 2025, at 18:49, Balbir Singh wrote:
>>>>
>>>>> On 10/19/25 19:19, Wei Yang wrote:
>>>>>> On Wed, Oct 01, 2025 at 04:57:02PM +1000, Balbir Singh wrote:
>>>>>> [...]
>>>>>>> static int __folio_split(struct folio *folio, unsigned int new_order,
>>>>>>>                 struct page *split_at, struct page *lock_at,
>>>>>>> -               struct list_head *list, bool uniform_split)
>>>>>>> +               struct list_head *list, bool uniform_split, bool 
>>>>>>> unmapped)
>>>>>>> {
>>>>>>>         struct deferred_split *ds_queue = 
>>>>>>> get_deferred_split_queue(folio);
>>>>>>>         XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>>>>>>> @@ -3765,13 +3757,15 @@ static int __folio_split(struct folio *folio, 
>>>>>>> unsigned int new_order,
>>>>>>>                  * is taken to serialise against parallel split or 
>>>>>>> collapse
>>>>>>>                  * operations.
>>>>>>>                  */
>>>>>>> -               anon_vma = folio_get_anon_vma(folio);
>>>>>>> -               if (!anon_vma) {
>>>>>>> -                       ret = -EBUSY;
>>>>>>> -                       goto out;
>>>>>>> +               if (!unmapped) {
>>>>>>> +                       anon_vma = folio_get_anon_vma(folio);
>>>>>>> +                       if (!anon_vma) {
>>>>>>> +                               ret = -EBUSY;
>>>>>>> +                               goto out;
>>>>>>> +                       }
>>>>>>> +                       anon_vma_lock_write(anon_vma);
>>>>>>>                 }
>>>>>>>                 mapping = NULL;
>>>>>>> -               anon_vma_lock_write(anon_vma);
>>>>>>>         } else {
>>>>>>>                 unsigned int min_order;
>>>>>>>                 gfp_t gfp;
>>>>>>> @@ -3838,7 +3832,8 @@ static int __folio_split(struct folio *folio, 
>>>>>>> unsigned int new_order,
>>>>>>>                 goto out_unlock;
>>>>>>>         }
>>>>>>>
>>>>>>> -       unmap_folio(folio);
>>>>>>> +       if (!unmapped)
>>>>>>> +               unmap_folio(folio);
>>>>>>>
>>>>>>>         /* block interrupt reentry in xa_lock and spinlock */
>>>>>>>         local_irq_disable();
>>>>>>> @@ -3925,10 +3920,13 @@ static int __folio_split(struct folio *folio, 
>>>>>>> unsigned int new_order,
>>>>>>>
>>>>>>>                         next = folio_next(new_folio);
>>>>>>>
>>>>>>> +                       zone_device_private_split_cb(folio, new_folio);
>>>>>>> +
>>>>>>>                         expected_refs = 
>>>>>>> folio_expected_ref_count(new_folio) + 1;
>>>>>>>                         folio_ref_unfreeze(new_folio, expected_refs);
>>>>>>>
>>>>>>> -                       lru_add_split_folio(folio, new_folio, lruvec, 
>>>>>>> list);
>>>>>>> +                       if (!unmapped)
>>>>>>> +                               lru_add_split_folio(folio, new_folio, 
>>>>>>> lruvec, list);
>>>>>>>
>>>>>>>                         /*
>>>>>>>                          * Anonymous folio with swap cache.
>>>>>>> @@ -3959,6 +3957,8 @@ static int __folio_split(struct folio *folio, 
>>>>>>> unsigned int new_order,
>>>>>>>                         __filemap_remove_folio(new_folio, NULL);
>>>>>>>                         folio_put_refs(new_folio, nr_pages);
>>>>>>>                 }
>>>>>>> +
>>>>>>> +               zone_device_private_split_cb(folio, NULL);
>>>>>>>                 /*
>>>>>>>                  * Unfreeze @folio only after all page cache entries, 
>>>>>>> which
>>>>>>>                  * used to point to it, have been updated with new 
>>>>>>> folios.
>>>>>>> @@ -3982,6 +3982,9 @@ static int __folio_split(struct folio *folio, 
>>>>>>> unsigned int new_order,
>>>>>>>
>>>>>>>         local_irq_enable();
>>>>>>>
>>>>>>> +       if (unmapped)
>>>>>>> +               return ret;
>>>>>>
>>>>>> As the comment of __folio_split() and 
>>>>>> __split_huge_page_to_list_to_order()
>>>>>> mentioned:
>>>>>>
>>>>>>   * The large folio must be locked
>>>>>>   * After splitting, the after-split folio containing @lock_at remains 
>>>>>> locked
>>>>>>
>>>>>> But here we seems to change the prerequisites.
>>>>>>
>>>>>> Hmm.. I am not sure this is correct.
>>>>>>
>>>>>
>>>>> The code is correct, but you are right in that the documentation needs to 
>>>>> be updated.
>>>>> When "unmapped", we do want to leave the folios locked after the split.
>>>>
>>>> Sigh, this "unmapped" code needs so many special branches and a different 
>>>> locking
>>>> requirement. It should be a separate function to avoid confusions.
>>>>
>>>
>>> Yep, I have a patch for it, I am also waiting on Matthew's feedback, FYI, 
>>> here is
>>> a WIP patch that can be applied on top of the series
>>
>> Nice cleanup! Thanks.
>>
>>>
>>> ---
>>>  include/linux/huge_mm.h |   5 +-
>>>  mm/huge_memory.c        | 137 ++++++++++++++++++++++++++++++++++------
>>>  mm/migrate_device.c     |   3 +-
>>>  3 files changed, 120 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>>> index c4a811958cda..86e1cefaf391 100644
>>> --- a/include/linux/huge_mm.h
>>> +++ b/include/linux/huge_mm.h
>>> @@ -366,7 +366,8 @@ unsigned long thp_get_unmapped_area_vmflags(struct file 
>>> *filp, unsigned long add
>>>
>>>  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 uniform_split_supported(struct folio *folio, unsigned int new_order,
>>> @@ -379,7 +380,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);
>>>  }
>>>
>>>  /*
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 8c82a0ac6e69..e20cbf68d037 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -3711,7 +3711,6 @@ bool uniform_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
>>>   * @uniform_split: 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
>>> @@ -3727,7 +3726,7 @@ bool uniform_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, bool uniform_split, bool unmapped)
>>> +           struct list_head *list, bool uniform_split)
>>>  {
>>>     struct deferred_split *ds_queue;
>>>     XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>>> @@ -3777,14 +3776,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;
>>> @@ -3852,8 +3849,7 @@ static int __folio_split(struct folio *folio, 
>>> unsigned int new_order,
>>>             goto out_unlock;
>>>     }
>>>
>>> -   if (!unmapped)
>>> -           unmap_folio(folio);
>>> +   unmap_folio(folio);
>>>
>>>     /* block interrupt reentry in xa_lock and spinlock */
>>>     local_irq_disable();
>>> @@ -3954,8 +3950,7 @@ static int __folio_split(struct folio *folio, 
>>> unsigned int new_order,
>>>                     expected_refs = folio_expected_ref_count(new_folio) + 1;
>>>                     folio_ref_unfreeze(new_folio, expected_refs);
>>>
>>> -                   if (!unmapped)
>>> -                           lru_add_split_folio(folio, new_folio, lruvec, 
>>> list);
>>> +                   lru_add_split_folio(folio, new_folio, lruvec, list);
>>>
>>>                     /*
>>>                      * Anonymous folio with swap cache.
>>> @@ -4011,9 +4006,6 @@ static int __folio_split(struct folio *folio, 
>>> unsigned int new_order,
>>>
>>>     local_irq_enable();
>>>
>>> -   if (unmapped)
>>> -           return ret;
>>> -
>>>     if (nr_shmem_dropped)
>>>             shmem_uncharge(mapping->host, nr_shmem_dropped);
>>>
>>> @@ -4057,6 +4049,111 @@ static int __folio_split(struct folio *folio, 
>>> unsigned int new_order,
>>>     return ret;
>>>  }
>>>
>>> +/*
>>> + * 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.
>>
>> I wonder if it makes sense to remove CPU side folio from both deferred_split 
>> queue
>> and swap cache before migration to further simplify 
>> split_unmapped_folio_to_order().
>> Basically require that device private folios cannot be on deferred_split 
>> queue nor
>> swap cache.
>>
>
> This API can be called for non-device private folios as well. Device private 
> folios are
> already not on the deferred queue. The use case is
>
> 1. Migrate a large folio page from CPU to Device
> 2. SRC - CPU has a THP (large folio page)
> 3. DST - Device cannot allocate a large page, hence split the SRC page

Right. That is what I am talking about, sorry I was not clear.
I mean when migrating a large folio from CPU to device, the CPU large folio
can be first removed from deferred_split queue and swap cache, if it is there,
then the migration process begins, so that the CPU large folio will always
be out of deferred_split queue and not in swap cache. As a result, this split
function does not need to handle these two situations.

--
Best Regards,
Yan, Zi

Reply via email to