Re: [PATCH v3 2/3] mm: gup: allow long-term pins of CMA-allocated HugeTLB folios

[Frank van der Linden]

On Wed, Nov 11, 2026 at 2:27 AM Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx> wrote:
>
> Hi David,
>
> On 11/11/26 3:43 PM, David Hildenbrand (Arm) wrote:
> > On 11/9/26 04:10, Qi Zheng wrote:
> >>
> >>
> >> On 11/8/26 1:58 AM, David Hildenbrand (Arm) wrote:
> >>> On 11/7/26 11:30, Qi Zheng wrote:
> >>>> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> >>>>
> >>>> Long-term pinning treats explicitly CMA-allocated HugeTLB folios as
> >>>> unpinnable, causing unnecessary migration or pinning failures.
> >>>>
> >>>> These folios already own their CMA bitmap ranges, and memory blocks
> >>>> overlapping CMA areas cannot be offlined. Allow HugeTLB folios with
> >>>> HPG_cma to be pinned directly, including in ZONE_MOVABLE.
> >>>>
> >>>> Fixes: cf11e85fc08c ("mm: hugetlb: optionally allocate gigantic hugepages using cma")
> >>>> Cc: stable@xxxxxxxxxxxxxxx
> >>>> Suggested-by: Muchun Song <muchun.song@xxxxxxxxx>
> >>>> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> >>>> Acked-by: Muchun Song <muchun.song@xxxxxxxxx>
> >>>> Reviewed-by: Oscar Salvador <osalvador@xxxxxxx>
> >>>> Reviewed-by: Jose A. Perez de Azpillaga <azpijr@xxxxxxxxx>
> >>>> ---
> >>>>    include/linux/mm.h | 29 +----------------------------
> >>>>    mm/gup.c           | 31 +++++++++++++++++++++++++++++++
> >>>>    2 files changed, 32 insertions(+), 28 deletions(-)
> >>>>
> >>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
> >>>> index 2d0b8a4c9974f..75dd40265d3f4 100644
> >>>> --- a/include/linux/mm.h
> >>>> +++ b/include/linux/mm.h
> >>>> @@ -2975,35 +2975,8 @@ static inline bool is_zero_folio(const struct folio *folio)
> >>>>            return is_zero_page(&folio->page);
> >>>>    }
> >>>>
> >>>> -/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin folios */
> >>>>    #ifdef CONFIG_MIGRATION
> >>>> -static inline bool folio_is_longterm_pinnable(struct folio *folio)
> >>>> -{
> >>>> -#ifdef CONFIG_CMA
> >>>> -  int mt = folio_migratetype(folio);
> >>>> -
> >>>> -  if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
> >>>> -          return false;
> >>>> -#endif
> >>>> -  /* The zero page can be "pinned" but gets special handling. */
> >>>> -  if (is_zero_folio(folio))
> >>>> -          return true;
> >>>> -
> >>>> -  /* Coherent device memory must always allow eviction. */
> >>>> -  if (folio_is_device_coherent(folio))
> >>>> -          return false;
> >>>> -
> >>>> -  /*
> >>>> -   * Filesystems can only tolerate transient delays to truncate and
> >>>> -   * hole-punch operations
> >>>> -   */
> >>>> -  if (folio_is_fsdax(folio))
> >>>> -          return false;
> >>>> -
> >>>> -  /* Otherwise, non-movable zone folios can be pinned. */
> >>>> -  return !folio_is_zone_movable(folio);
> >>>> -
> >>>> -}
> >>>
> >>> Moving code while adding new stuff to it, please don't.
> >>>
> >>> Move code first, then modify it.
> >>>
> >>> I don't want to play "find the difference between two pictures" on a Saturday
> >>> evening.
> >>
> >
> > You resent your series way too fast.
>
> Got it.
>
> >
> > Now we have discussions in to threads, congratulations ;)
> >
> >> ;), got it.
> >>
> >> I moved the code to resolve a build dependency.
> >>
> >
> > Why can't that be resolved differently? What is the dependency?
>
> The dependency is that the new check wants to use
> `folio_test_hugetlb_cma()`, which is a HugeTLB-specific inline helper
> from `include/linux/hugetlb.h`.
>
> `folio_is_longterm_pinnable()` currently lives in `include/linux/mm.h`,
> while `include/linux/hugetlb.h` already includes `linux/mm.h`. So I
> can't simply use `folio_test_hugetlb_cma()` from `mm.h`
>
> >
> >>>
> >>>> +bool folio_is_longterm_pinnable(struct folio *folio);
> >>>>    #else
> >>>>    static inline bool folio_is_longterm_pinnable(struct folio *folio)
> >>>>    {
> >>>> diff --git a/mm/gup.c b/mm/gup.c
> >>>> index 2b405cce2ca5c..f41701c603500 100644
> >>>> --- a/mm/gup.c
> >>>> +++ b/mm/gup.c
> >>>> @@ -2216,6 +2216,37 @@ struct page *get_dump_page(unsigned long addr, int *locked)
> >>>>
> >>>>    #ifdef CONFIG_MIGRATION
> >>>>
> >>>> +bool folio_is_longterm_pinnable(struct folio *folio)
> >>>> +{
> >>>> +#ifdef CONFIG_CMA
> >>>> +  int mt = folio_migratetype(folio);
> >>>> +
> >>>> +  /* These HugeTLB folios own the CMA memory, so they can be long-term pinned. */
> >>>> +  if (folio_test_hugetlb(folio) && folio_test_hugetlb_cma(folio))
> >>>> +          return true;
> >>> More hugetlb hackery ... don't like that, sorry.
> >>
> >> Agree.
> >>
> >>>
> >>> I think this just goes down to a problem we discussed a while ago that is
> >>> independent of hugetlb: we'd need some way to identify that the the page was
> >>> actually allocated by the CMA owner, and it's not just some other movable
> >>> allcoation that ended up there.
> >>>
> >>> For something generic non-hugetlb thing, can't we call into CMA to ask whether
> >>> the current chunk is marked allocated?
> >>
> >> The existing bitmap is set before allocation completes, so the range
> >> may still contain borrowed movable pages. A set bit alone cannot
> >> establish ownership.
>
> The more accurate code path is as follows.
>
> Assuming the 2M buddy allocation is successful, page P is borrowed from
> the 1G CMA area.
>
> Now, proceed to request a new 1G huge page:
>
> only_alloc_fresh_hugetlb_folio
> --> alloc_gigantic_frozen_folio
>      --> hugetlb_cma_alloc_frozen_folio
>          --> cma_range_alloc
>                  /* set bitmap!! */
>              --> bitmap_set(cmr->bitmap, bitmap_no, bitmap_count);
>
>                  <time A>
>
>                  /* may fail */
>                  alloc_contig_frozen_range
>
>          /* set HPG_cma!! */
>          folio_set_hugetlb_cma(folio);
>
> At time A, for page P, a bitmap was set, but HPG_cma was not set.
>
> If a concurrent FOLL_LONGTERM pin uses the bitmap alone to determine
> ownership, it could incorrectly allow P to remain pinned. If
> alloc_contig_frozen_range() fails, cma_range_alloc() clears the bitmap
> bits, while the long-term pin on P remains.
>
> HPG_cma is set only after explicit CMA allocation succeeds. It records
> the folio's own CMA ownership, whereas a set bitmap bit can represent
> another allocation's in-flight reservation. Therefore, the bitmap
> alone cannot replace HPG_cma in this check.
>
> >>
> >> A generic query may need separate ownership tracking, such as an
> >> owned_bitmap recording successfully allocated pages. Its updates and
> >> lookups would also need to handle concurrency with fast GUP.
> >
> > Or simply two bitmaps? "trying allocation - allocated"
>
> As a fix, perhaps this is a bit too complex? :(
>
> Alternatively, we could adjust the timing for setting the bitmap, though
> this might introduce new concurrency issues.
>
> Thanks,
> Qi

Yeah, ideally there would be a way to identify folios allocated
through cma_alloc. But that would probably require.. dare I say.. a
page flag. Trying to check bitmaps does seem too complex.

And, since we've had a local change that does exactly the same thing
for a few years (which I apparently forgot to send along with other
hugetlb_cma changes), I can confirm that this looks good to me and
works. So,

Reviewed-by: Frank van der Linden <fvdl@xxxxxxxxxx>