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


[...]


Thanks for the review!
Balbir

Reply via email to