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

[Qi Zheng]

On 11/12/26 6:26 AM, Frank van der Linden wrote:
> 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.

Agree.

> 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>

Thanks for your review and confirmation!