On Tue,  5 May 2026 16:05:08 +0200
Ketil Johnsen <[email protected]> wrote:

> From: John Stultz <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: T.J. Mercier <[email protected]>
> Signed-off-by: Yong Wu <[email protected]>
> [Yong: Fix the checkpatch alignment warning]
> Signed-off-by: Yunfei Dong <[email protected]>
> Signed-off-by: Florent Tomasin <[email protected]>
> [Florent: Rebase]
> Signed-off-by: Ketil Johnsen <[email protected]>
> [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 */

Reply via email to