Re: [PATCH v4 5/6] mm: cma: add cma_alloc_frozen{_compound}()

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2025/12/18 23:52, Zi Yan wrote:
> On 18 Dec 2025, at 7:54, Kefeng Wang wrote:
> 
>> On 2025/12/18 3:38, Zi Yan wrote:
>>> On 17 Dec 2025, at 3:02, Kefeng Wang wrote:
>>>
>>>> On 2025/12/17 2:40, Zi Yan wrote:
>>>>> On 16 Dec 2025, at 6:48, Kefeng Wang wrote:
>>>>>
>>>>>> Introduce cma_alloc_frozen{_compound}() helper to alloc pages without
>>>>>> incrementing their refcount, then convert hugetlb cma to use the
>>>>>> cma_alloc_frozen_compound() and cma_release_frozen() and remove the
>>>>>> unused cma_{alloc,free}_folio(), also move the cma_validate_zones()
>>>>>> into mm/internal.h since no outside user.
>>>>>>
>>>>>> The set_pages_refcounted() is only called to set non-compound pages
>>>>>> after above changes, so remove the processing about PageHead.
>>>>>>
>>>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>>>> ---
>>>>>>     include/linux/cma.h | 26 ++++++------------------
>>>>>>     mm/cma.c            | 48 +++++++++++++++++++++++++--------------------
>>>>>>     mm/hugetlb_cma.c    | 24 +++++++++++++----------
>>>>>>     mm/internal.h       | 10 +++++-----
>>>>>>     4 files changed, 52 insertions(+), 56 deletions(-)
>>>>>>
>>>>
>>>> ...
>>>>
>>>>>>     static bool __cma_release(struct cma *cma, const struct page *pages,
>>>>>> -			  unsigned long count, bool compound)
>>>>>> +			  unsigned long count, bool frozen)
>>>>>>     {
>>>>>>     	unsigned long pfn, end;
>>>>>>     	int r;
>>>>>> @@ -974,8 +982,8 @@ static bool __cma_release(struct cma *cma, const struct page *pages,
>>>>>>     		return false;
>>>>>>     	}
>>>>>>
>>>>>> -	if (compound)
>>>>>> -		__free_pages((struct page *)pages, compound_order(pages));
>>>>>> +	if (frozen)
>>>>>> +		free_contig_frozen_range(pfn, count);
>>>>>>     	else
>>>>>>     		free_contig_range(pfn, count);
>>>>>
>>>>> Can we get rid of free_contig_range() branch by making cma_release() put
>>>>> each page’s refcount? Then, __cma_relase() becomes cma_release_frozen()
>>>>> and the release pattern matches allocation pattern:
>>>>> 1. cma_alloc() calls cma_alloc_frozen() and manipulates page refcount.
>>>>> 2. cma_release() manipulates page refcount and calls cma_release_frozen().
>>>>>
>>>>
>>>> Have considered similar things before, but we need manipulates page
>>>> refcount only find the correct cma memrange from cma/pages, it seems
>>>> that no big improvement, any more comments?
>>>>
>>>> 1) for cma_release:
>>>>      a. cma find memrange
>>>>      b. manipulates page refcount when cmr found
>>>>      c. free page and release cma resource
>>>> 2) for cma_release_frozen
>>>>      a. cma find memrange
>>>>      b. free page and release cma resource whne cmr found
>>>
>>> Right, I think it makes code simpler.
>>>
>>> Basically add a helper function:
>>> struct cma_memrange* find_cma_memrange(struct cma *cma,
>>> 		const struct page *pages, unsigned long count);
>>>
>>> Then
>>>
>>> __cma_release_frozen()
>>> {
>>> 	free_contig_frozen_range(pfn, count);
>>> 	cma_clear_bitmap(cma, cmr, pfn, count);
>>> 	cma_sysfs_account_release_pages(cma, count);
>>> 	trace_cma_release(cma->name, pfn, pages, count);
>>> }
>>>
>>>
>>> cma_release()
>>> {
>>> 	cmr = find_cma_memrange();
>>>
>>> 	if (!cmr)
>>> 		return false;
>>> 	
>>> 	for (; count--; pages++)
>>> 		VM_WARN_ON(!put_page_testzero(pages);
>>>
>>> 	__cma_release_frozen();
>>> }
>>>
>>> cma_release_frozen()
>>> {
>>> 	cmr = find_cma_memrange();
>>>
>>> 	if (!cmr)
>>> 		return false;
>>>
>>> 	__cma_release_frozen();
>>>
>>> }
>>>
>>> Let me know your thoughts.
>>
>> Yes, this is exactly what I described above that needs to be done, but I
>> think it will add more codes :)
>>
>> Our goal is that convert all cma_{alloc,release} caller to
>> cma_frozen_{alloc,release}, and complete remove free_contig_range in cma, Maybe no changes? But if you prefer above way, I can also update
>> it.
> 
> If the goal is to replace all cma_{alloc,release}() calls with the frozen version,
> there is no need to make the change as I suggested. Are you planning to send
> another patchset to do the replacement after this one?

There are few callers, the following can be straightforwardly converted
to a frozen version.

   mm/cma_debug.c
   drivers/dma-buf/heaps/cma_heap.c
   drivers/s390/char/vmcp.c
   arch/powerpc/kvm/book3s_hv_builtin.c

For the DMA part, we suppose there is no driver using the page refcount, 
as too many drivers are involved, maybe there is a very special usage in 
the driver，I can't be sure.

   kernel/dma/contiguous.c