On 10/23/25 02:26, Zi Yan wrote:
> 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.

It leads to more specialization for LRU and zone device-private folios.
I looked at giving it a go and ended up with some amount of duplication, with
taking the lock, freezing the refs and then prior to that finding the
extra_refs with can_split_folio()

Balbir Singh

Reply via email to