Re: [PATCH v3] dma-contiguous: setup default numa cma area if not configured explicitly

[David Hildenbrand (Arm)]

On 4/28/26 08:05, Feng Tang wrote:
> There was a report on a multi-numa-nodes ARM server that when IOMMU is
> disabled, the dma_alloc_coherent() function always returns memory from
> node 0 even for devices attaching to other nodes, while they can get
> local dma memory when IOMMU is on with the same API.
> 
> The reason is, when IOMMU is disabled, the dma_alloc_coherent() will
> go the direct way and call dma_alloc_contiguous(). The system doesn't
> have any explicit cma setting (like per-numa cma), and only has a
> default 64MB cma reserved area (on node 0), where kernel will try
> first to allocate memory from.
> 
> Robin Murphy suggested to setup pernuma cma or disable cma, which did
> solve the issue. 

That sounds like the obvious approach to me.

> While there is still concern that for customers
> which don't have much kernel knowledge, they could still suffer from
> this silently as some architectures enable cma area by default (not
> an issue for X86 though, which set CONFIG_CMA_SIZE_MBYTES to 0 by
> default) for most Linux distributions.

Okay, so on x86 it is not silent, because they don't even have a default CMA area?

> 
> One thought is to follow the current cma reserving policy for platform
> with 'CONFIG_DMA_NUMA_CMA=y', that if the numa cma (either the 'numa cma'
> or 'cma pernuma' method) is not explicitly configured, set it up
> according to size of default 'dma_contiguous_default_area', while
> skipping the numa node where the 'dma_contiguous_default_area' lies
> in, this way the default behavior of platform with one NUMA node is
> kept unchanged.

So, the kernel is configured to have a certain CONFIG_CMA_SIZE_MBYTES size, but
you go ahead and multiply that by the number of nodes? Sounds wrong.

The whole proposal here looks rather hacky.

Wouldn't a default for e.g., pernuma_size_bytes make more sense, that users can
then overwrite on the cmdline?

> 
> To get the node info of cma area, add some helpr funciton and setup
> in cma code.
> 
> Reported-by: Changrong Chen <chenchangrong.ccr@xxxxxxxxxxxxxxx>
> Suggested-by: Ying Huang <ying.huang@xxxxxxxxxxxxxxxxx>
> Suggested-by: Robin Murphy <robin.murphy@xxxxxxx>
> Signed-off-by: Feng Tang <feng.tang@xxxxxxxxxxxxxxxxx>
> ---
> Changelog:
> 
>   since v2:
>     * setup the numa cma are following default cma, while
> 	  skipping the node holds the default cma (Robin Murphy)
> 	* add cma_get_node() help and related code
> 	* add reporter info 
> 
>   since v1:
> 	* don't use the original way of adding alloc_pages_node()
> 	  before trying default cma node (Robin Murphy)
> 	* setup default numa cma area if not configured (Ying Huang)
> 
>    v2: https://lore.kernel.org/lkml/20260423095243.14239-1-feng.tang@xxxxxxxxxxxxxxxxx/
>    v1: https://lore.kernel.org/lkml/20260414090310.92055-1-feng.tang@xxxxxxxxxxxxxxxxx/
> 
>  include/linux/cma.h     |  1 +
>  kernel/dma/contiguous.c | 14 ++++++++++++--
>  mm/cma.c                | 11 ++++++++++-
>  3 files changed, 23 insertions(+), 3 deletions(-)

[...]

>  		if (numa_cma_size[nid]) {
>  
>  			cma = &dma_contiguous_numa_area[nid];
> @@ -255,8 +265,6 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  	phys_addr_t selected_limit = limit;
>  	bool fixed = false;
>  
> -	dma_numa_cma_reserve();
> -
>  	pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
>  
>  	if (size_cmdline != -1) {
> @@ -312,6 +320,8 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
>  		if (ret)
>  			pr_warn("Couldn't queue default CMA region for heap creation.");
>  	}
> +
> +	dma_numa_cma_reserve();
>  }
>  
>  void __weak
> diff --git a/mm/cma.c b/mm/cma.c
> index c7ca567f4c5c..3bbfafeaf6c1 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -54,6 +54,11 @@ const char *cma_get_name(const struct cma *cma)
>  }
>  EXPORT_SYMBOL_GPL(cma_get_name);
>  
> +extern int cma_get_nid(const struct cma *cma)
> +{
> +	return cma->nid;
> +}

Why do you have to store the nid instead of just looking it up from the base_pfn
in here?

Also, what is the expectation when the ranges would span different NIDs? (is
that possible?)

-- 
Cheers,

David