Re: [PATCH] dma-mapping: don't trace the DMA address when the allocation fails

[Marek Szyprowski]

On 05.09.2026 09:19, Donggeun Yoo wrote:
> dma_alloc_attrs() passes *dma_handle to trace_dma_alloc() and
> debug_dma_alloc_coherent() without checking whether the allocation
> succeeded. No backend writes it on failure: dma_direct_alloc(),
> iommu_dma_alloc() and the dma_map_ops instances assign it only on the
> path that returns a buffer. Callers usually pass an uninitialized
> automatic variable, so with the tracepoint enabled a failed allocation
> records whatever the stack held, next to the virt_addr=(null) that marks
> the record as an error.
>
> The device coherent pool path is the same: a non-zero return from
> dma_alloc_from_dev_coherent() means the request was handled, not that it
> succeeded, so cpu_addr is NULL and dma_handle untouched once the pool
> runs out.
>
> Split both sites on cpu_addr, as dma_alloc_pages() and
> dma_alloc_noncontiguous() do further down the file, and pass 0 for the
> failure case like the two error paths already in this function.
>
> Fixes: 038eb433dc14 ("dma-mapping: add tracing for dma-mapping API calls")
> Fixes: 68b6dbf1f441 ("dma-mapping: trace more error paths")
> Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@xxxxxxxxx>
> ---
> Compile-tested only, with CONFIG_DMA_API_DEBUG=y and CONFIG_TRACEPOINTS=y
> so that both changed calls are built. The claim that no backend writes
> *dma_handle on failure was checked against dma_direct_alloc() and its
> helpers, iommu_dma_alloc(), iommu_dma_alloc_remap() and every in-tree
> dma_map_ops .alloc implementation.
>
>  kernel/dma/mapping.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
> index bf2651a70b7c..098cd57e1157 100644
> --- a/kernel/dma/mapping.c
> +++ b/kernel/dma/mapping.c
> @@ -656,8 +656,12 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
>  		attrs |= __DMA_ATTR_ALLOC_CC_SHARED;
>  
>  	if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr)) {
> -		trace_dma_alloc(dev, cpu_addr, *dma_handle, size,
> -				DMA_BIDIRECTIONAL, flag, attrs);
> +		if (cpu_addr)
> +			trace_dma_alloc(dev, cpu_addr, *dma_handle, size,
> +					DMA_BIDIRECTIONAL, flag, attrs);
> +		else
> +			trace_dma_alloc(dev, NULL, 0, size, DMA_BIDIRECTIONAL,
> +					flag, attrs);
Frankly speaking I would prefer to fix this in include/trace/events/dma.h with
__entry->dma_addr = virt_addr ? dma_addr : 0;


>  		return cpu_addr;
>  	}
>  
> @@ -676,9 +680,15 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
>  		return NULL;
>  	}
>  
> -	trace_dma_alloc(dev, cpu_addr, *dma_handle, size, DMA_BIDIRECTIONAL,
> -			flag, attrs);
> -	debug_dma_alloc_coherent(dev, size, *dma_handle, cpu_addr, attrs);
> +	if (cpu_addr) {
> +		trace_dma_alloc(dev, cpu_addr, *dma_handle, size,
> +				DMA_BIDIRECTIONAL, flag, attrs);
> +		debug_dma_alloc_coherent(dev, size, *dma_handle, cpu_addr,
> +					 attrs);
> +	} else {
> +		trace_dma_alloc(dev, NULL, 0, size, DMA_BIDIRECTIONAL, flag,
> +				attrs);
> +	}
Here You remove the debug_dma_alloc call for the failed case. Right, it
is a noop now for the allocation failure, but it might be extended with
some kind of debugging in the future, so I prefer to keep the current
code here.
>  	return cpu_addr;
>  }
>  EXPORT_SYMBOL(dma_alloc_attrs);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland