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

[David Hildenbrand (Arm)]

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.

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?

>>
>>> +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.
> 
> 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"

-- 
Cheers,

David