Hi John,

On Sat, Jul 25, 2020 at 03:26:33AM +0000, John Stultz wrote:
> Add proper refcounting on the dma_heap structure.
> While existing heaps are built-in, we may eventually
> have heaps loaded from modules, and we'll need to be
> able to properly handle the references to the heaps

I'm not 100% clear on the intention here. What would take/drop a
reference on a heap?

In the case of modules I think the bigger problem is how to prevent
the module getting removed while there's still something using it.

Cheers,
-Brian

> 
> Cc: Sumit Semwal <sumit.sem...@linaro.org>
> Cc: Andrew F. Davis <a...@ti.com>
> Cc: Benjamin Gaignard <benjamin.gaign...@linaro.org>
> Cc: Liam Mark <lm...@codeaurora.org>
> Cc: Laura Abbott <labb...@kernel.org>
> Cc: Brian Starkey <brian.star...@arm.com>
> Cc: linux-me...@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: John Stultz <john.stu...@linaro.org>
> ---
>  drivers/dma-buf/dma-heap.c | 31 +++++++++++++++++++++++++++----
>  include/linux/dma-heap.h   |  6 ++++++
>  2 files changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c
> index afd22c9dbdcf..90c3720acc1c 100644
> --- a/drivers/dma-buf/dma-heap.c
> +++ b/drivers/dma-buf/dma-heap.c
> @@ -40,6 +40,8 @@ struct dma_heap {
>       dev_t heap_devt;
>       struct list_head list;
>       struct cdev heap_cdev;
> +     int minor;
> +     struct kref refcount;
>  };
>  
>  static LIST_HEAD(heap_list);
> @@ -190,11 +192,31 @@ void *dma_heap_get_drvdata(struct dma_heap *heap)
>       return heap->priv;
>  }
>  
> +static void dma_heap_release(struct kref *ref)
> +{
> +     struct dma_heap *heap = container_of(ref, struct dma_heap, refcount);
> +
> +     /* Remove heap from the list */
> +     mutex_lock(&heap_list_lock);
> +     list_del(&heap->list);
> +     mutex_unlock(&heap_list_lock);
> +
> +     device_destroy(dma_heap_class, heap->heap_devt);
> +     cdev_del(&heap->heap_cdev);
> +     xa_erase(&dma_heap_minors, heap->minor);
> +
> +     kfree(heap);
> +}
> +
> +void dma_heap_put(struct dma_heap *h)
> +{
> +     kref_put(&h->refcount, dma_heap_release);
> +}
> +
>  struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info)
>  {
>       struct dma_heap *heap, *h, *err_ret;
>       struct device *dev_ret;
> -     unsigned int minor;
>       int ret;
>  
>       if (!exp_info->name || !strcmp(exp_info->name, "")) {
> @@ -223,12 +245,13 @@ struct dma_heap *dma_heap_add(const struct 
> dma_heap_export_info *exp_info)
>       if (!heap)
>               return ERR_PTR(-ENOMEM);
>  
> +     kref_init(&heap->refcount);
>       heap->name = exp_info->name;
>       heap->ops = exp_info->ops;
>       heap->priv = exp_info->priv;
>  
>       /* Find unused minor number */
> -     ret = xa_alloc(&dma_heap_minors, &minor, heap,
> +     ret = xa_alloc(&dma_heap_minors, &heap->minor, heap,
>                      XA_LIMIT(0, NUM_HEAP_MINORS - 1), GFP_KERNEL);
>       if (ret < 0) {
>               pr_err("dma_heap: Unable to get minor number for heap\n");
> @@ -237,7 +260,7 @@ struct dma_heap *dma_heap_add(const struct 
> dma_heap_export_info *exp_info)
>       }
>  
>       /* Create device */
> -     heap->heap_devt = MKDEV(MAJOR(dma_heap_devt), minor);
> +     heap->heap_devt = MKDEV(MAJOR(dma_heap_devt), heap->minor);
>  
>       cdev_init(&heap->heap_cdev, &dma_heap_fops);
>       ret = cdev_add(&heap->heap_cdev, heap->heap_devt, 1);
> @@ -267,7 +290,7 @@ struct dma_heap *dma_heap_add(const struct 
> dma_heap_export_info *exp_info)
>  err2:
>       cdev_del(&heap->heap_cdev);
>  err1:
> -     xa_erase(&dma_heap_minors, minor);
> +     xa_erase(&dma_heap_minors, heap->minor);
>  err0:
>       kfree(heap);
>       return err_ret;
> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
> index 454e354d1ffb..c1572f29cfac 100644
> --- a/include/linux/dma-heap.h
> +++ b/include/linux/dma-heap.h
> @@ -56,4 +56,10 @@ void *dma_heap_get_drvdata(struct dma_heap *heap);
>   */
>  struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info);
>  
> +/**
> + * dma_heap_put - drops a reference to a dmabuf heaps, potentially freeing it
> + * @heap:            heap pointer
> + */
> +void dma_heap_put(struct dma_heap *heap);
> +
>  #endif /* _DMA_HEAPS_H */
> -- 
> 2.17.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to