Re: [PATCH 2/8] dma-heap: Provide accessors so that in-kernel drivers can allocate dmabufs from specific heaps

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue,  5 May 2026 16:05:08 +0200
Ketil Johnsen <ketil.johnsen@arm.com> wrote:

> From: John Stultz <jstultz@google.com>
> 
> This allows drivers who don't want to create their own
> DMA-BUF exporter to be able to allocate DMA-BUFs directly
> from existing DMA-BUF Heaps.
> 
> There is some concern that the premise of DMA-BUF heaps is
> that userland knows better about what type of heap memory
> is needed for a pipeline, so it would likely be best for
> drivers to import and fill DMA-BUFs allocated by userland
> instead of allocating one themselves, but this is still
> up for debate.

I think this commit message needs to be updated with more details
around what it's actually needed for here (driver needing protected
buffer to boot FW and expose a char device, and no clean way to pass
dmabufs around before this cdev is exposed).

> 
> Signed-off-by: John Stultz <jstultz@google.com>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> [Yong: Fix the checkpatch alignment warning]
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
> [Florent: Rebase]
> Signed-off-by: Ketil Johnsen <ketil.johnsen@arm.com>
> [Ketil: Rebase]
> ---
>  drivers/dma-buf/dma-heap.c | 80 ++++++++++++++++++++++++++++++--------
>  include/linux/dma-heap.h   |  6 +++
>  2 files changed, 70 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index 9fd365ddbd517..854d40d789ff2 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -57,12 +57,24 @@ module_param(mem_accounting, bool, 0444);
>  MODULE_PARM_DESC(mem_accounting,
>  		 "Enable cgroup-based memory accounting for dma-buf heap allocations (default=false).");
>  
> -static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> -				 u32 fd_flags,
> -				 u64 heap_flags)
> +/**
> + * dma_heap_buffer_alloc - Allocate dma-buf from a dma_heap
> + * @heap:	DMA-Heap to allocate from
> + * @len:	size to allocate in bytes
> + * @fd_flags:	flags to set on returned dma-buf fd
> + * @heap_flags: flags to pass to the dma heap
> + *
> + * This is for internal dma-buf allocations only. Free returned buffers with dma_buf_put().
> + */
> +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> +				      u32 fd_flags,
> +				      u64 heap_flags)
>  {
> -	struct dma_buf *dmabuf;
> -	int fd;
> +	if (fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> +		return ERR_PTR(-EINVAL);

I'd probably move the flags checks to dma_heap_buffer_alloc() in a
separate patch, to keep the diff easier to read. Same for the
dma_heap_buffer_alloc()/dma_heap_bufferfd_alloc() split, though I'm not
too sure we need dma_heap_bufferfd_alloc(), we could just move the FD
allocation directly in dma_heap_ioctl_allocate().


>  
>  	/*
>  	 * Allocations from all heaps have to begin
> @@ -70,9 +82,20 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
>  	 */
>  	len = PAGE_ALIGN(len);
>  	if (!len)
> -		return -EINVAL;
> +		return ERR_PTR(-EINVAL);
> +
> +	return heap->ops->allocate(heap, len, fd_flags, heap_flags);
> +}
> +EXPORT_SYMBOL_NS_GPL(dma_heap_buffer_alloc, "DMA_BUF_HEAP");
>  
> -	dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags);
> +static int dma_heap_bufferfd_alloc(struct dma_heap *heap, size_t len,
> +				   u32 fd_flags,
> +				   u64 heap_flags)
> +{
> +	struct dma_buf *dmabuf;
> +	int fd;
> +
> +	dmabuf = dma_heap_buffer_alloc(heap, len, fd_flags, heap_flags);
>  	if (IS_ERR(dmabuf))
>  		return PTR_ERR(dmabuf);
>  
> @@ -110,15 +133,9 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data)
>  	if (heap_allocation->fd)
>  		return -EINVAL;
>  
> -	if (heap_allocation->fd_flags & ~DMA_HEAP_VALID_FD_FLAGS)
> -		return -EINVAL;
> -
> -	if (heap_allocation->heap_flags & ~DMA_HEAP_VALID_HEAP_FLAGS)
> -		return -EINVAL;
> -
> -	fd = dma_heap_buffer_alloc(heap, heap_allocation->len,
> -				   heap_allocation->fd_flags,
> -				   heap_allocation->heap_flags);
> +	fd = dma_heap_bufferfd_alloc(heap, heap_allocation->len,
> +				     heap_allocation->fd_flags,
> +				     heap_allocation->heap_flags);
>  	if (fd < 0)
>  		return fd;
>  
> @@ -317,6 +334,36 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
>  }
>  EXPORT_SYMBOL_NS_GPL(dma_heap_add, "DMA_BUF_HEAP");
>  
> +/**
> + * dma_heap_find - get the heap registered with the specified name
> + * @name: Name of the DMA-Heap to find
> + *
> + * Returns:
> + * The DMA-Heap with the provided name.
> + *
> + * NOTE: DMA-Heaps returned from this function MUST be released using
> + * dma_heap_put() when the user is done to enable the heap to be unloaded.
> + */
> +struct dma_heap *dma_heap_find(const char *name)

s/dma_heap_find/dma_heap_find_by_name/gc

> +{
> +	struct dma_heap *h;
> +
> +	mutex_lock(&heap_list_lock);
> +	list_for_each_entry(h, &heap_list, list) {
> +		if (!kref_get_unless_zero(&h->refcount))
> +			continue;
> +
> +		if (!strcmp(h->name, name)) {

I think we should go sysfs_streq(), to make sure we don't return a heap
whose name only starts with the searched name.

> +			mutex_unlock(&heap_list_lock);
> +			return h;
> +		}
> +		dma_heap_put(h);
> +	}
> +	mutex_unlock(&heap_list_lock);
> +	return NULL;

This could be simplified with something like:

	struct dma_heap *h, *ret = NULL;

	guard(mutex)(&heap_list_lock);
	list_for_each_entry(h, &heap_list, list) {
		if (!sysfs_streq(h->name, name))
			continue;

		if (kref_get_unless_zero(&h->refcount))
			ret = h;

		break;
	}

	return ret;

> +}


> +EXPORT_SYMBOL_NS_GPL(dma_heap_find, "DMA_BUF_HEAP");
> +
>  static void dma_heap_release(struct kref *ref)
>  {
>  	struct dma_heap *heap = container_of(ref, struct dma_heap, refcount);
> @@ -341,6 +388,7 @@ void dma_heap_put(struct dma_heap *heap)
>  {
>  	kref_put(&heap->refcount, dma_heap_release);
>  }
> +EXPORT_SYMBOL_NS_GPL(dma_heap_put, "DMA_BUF_HEAP");
>  
>  static char *dma_heap_devnode(const struct device *dev, umode_t *mode)
>  {
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index ff57741700f5f..c3351f8a1f8cf 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -46,8 +46,14 @@ const char *dma_heap_get_name(struct dma_heap *heap);
>  
>  struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
>  
> +struct dma_heap *dma_heap_find(const char *name);
> +
>  void dma_heap_put(struct dma_heap *heap);
>  
> +struct dma_buf *dma_heap_buffer_alloc(struct dma_heap *heap, size_t len,
> +				      u32 fd_flags,
> +				      u64 heap_flags);
> +
>  extern bool mem_accounting;
>  
>  #endif /* _DMA_HEAPS_H */