Re: [RESEND][PATCH 3/3] dma-buf: heaps: Rework heep allocation hooks to return struct dma_buf instead of fd
Hi John, On Wed, 20 Jan 2021 at 02:15, John Stultz wrote: > > Every heap needs to create a dmabuf and then export it to a fd > via dma_buf_fd(), so to consolidate things a bit, have the heaps > just return a struct dmabuf * and let the top level > dma_heap_buffer_alloc() call handle creating the fd via > dma_buf_fd(). Thanks for the patch! LGTM, feels a lot neater now. I'll merge into drm-misc-next. > > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Laura Abbott > Cc: Brian Starkey > Cc: Hridya Valsaraju > Cc: Suren Baghdasaryan > Cc: Sandeep Patil > Cc: Daniel Mentz > Cc: Chris Goldsworthy > Cc: Ørjan Eide > Cc: Robin Murphy > Cc: Ezequiel Garcia > Cc: Simon Ser > Cc: James Jones > Cc: linux-me...@vger.kernel.org > Cc: dri-de...@lists.freedesktop.org > Signed-off-by: John Stultz > --- > drivers/dma-buf/dma-heap.c | 14 +- > drivers/dma-buf/heaps/cma_heap.c| 22 +++--- > drivers/dma-buf/heaps/system_heap.c | 21 +++-- > include/linux/dma-heap.h| 12 ++-- > 4 files changed, 33 insertions(+), 36 deletions(-) > > diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c > index afd22c9dbdcf..6b5db954569f 100644 > --- a/drivers/dma-buf/dma-heap.c > +++ b/drivers/dma-buf/dma-heap.c > @@ -52,6 +52,9 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, > size_t len, > unsigned int fd_flags, > unsigned int heap_flags) > { > + struct dma_buf *dmabuf; > + int fd; > + > /* > * Allocations from all heaps have to begin > * and end on page boundaries. > @@ -60,7 +63,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, > size_t len, > if (!len) > return -EINVAL; > > - return heap->ops->allocate(heap, len, fd_flags, heap_flags); > + dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags); > + if (IS_ERR(dmabuf)) > + return PTR_ERR(dmabuf); > + > + fd = dma_buf_fd(dmabuf, fd_flags); > + if (fd < 0) { > + dma_buf_put(dmabuf); > + /* just return, as put will call release and that will free */ > + } > + return fd; > } > > static int dma_heap_open(struct inode *inode, struct file *file) > diff --git a/drivers/dma-buf/heaps/cma_heap.c > b/drivers/dma-buf/heaps/cma_heap.c > index 0c76cbc3fb11..985c41ffd85b 100644 > --- a/drivers/dma-buf/heaps/cma_heap.c > +++ b/drivers/dma-buf/heaps/cma_heap.c > @@ -272,10 +272,10 @@ static const struct dma_buf_ops cma_heap_buf_ops = { > .release = cma_heap_dma_buf_release, > }; > > -static int cma_heap_allocate(struct dma_heap *heap, > - unsigned long len, > - unsigned long fd_flags, > - unsigned long heap_flags) > +static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, > +unsigned long len, > +unsigned long fd_flags, > +unsigned long heap_flags) > { > struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); > struct cma_heap_buffer *buffer; > @@ -290,7 +290,7 @@ static int cma_heap_allocate(struct dma_heap *heap, > > buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); > if (!buffer) > - return -ENOMEM; > + return ERR_PTR(-ENOMEM); > > INIT_LIST_HEAD(>attachments); > mutex_init(>lock); > @@ -349,15 +349,7 @@ static int cma_heap_allocate(struct dma_heap *heap, > ret = PTR_ERR(dmabuf); > goto free_pages; > } > - > - ret = dma_buf_fd(dmabuf, fd_flags); > - if (ret < 0) { > - dma_buf_put(dmabuf); > - /* just return, as put will call release and that will free */ > - return ret; > - } > - > - return ret; > + return dmabuf; > > free_pages: > kfree(buffer->pages); > @@ -366,7 +358,7 @@ static int cma_heap_allocate(struct dma_heap *heap, > free_buffer: > kfree(buffer); > > - return ret; > + return ERR_PTR(ret); > } > > static const struct dma_heap_ops cma_heap_ops = { > diff --git a/drivers/dma-buf/heaps/system_heap.c > b/drivers/dma-buf/heaps/system_heap.c > index 2321c91891f6..7b154424aeb3 100644 > --- a/drivers/dma-buf/heaps/system_heap.c > +++ b/drivers/dma-buf/heaps/system_heap.c > @@ -332,10 +332,10 @@ static struct page *alloc_largest_available(unsigned > long size, > return NULL; > } > > -static int system_heap_allocate(struct dma_heap *heap, > - unsigned long len, > - unsigned long fd_flags, > - unsigned long heap_flags) > +static struct dma_buf *system_heap_allocate(struct dma_heap *heap, > +
[RESEND][PATCH 3/3] dma-buf: heaps: Rework heep allocation hooks to return struct dma_buf instead of fd
Every heap needs to create a dmabuf and then export it to a fd via dma_buf_fd(), so to consolidate things a bit, have the heaps just return a struct dmabuf * and let the top level dma_heap_buffer_alloc() call handle creating the fd via dma_buf_fd(). Cc: Sumit Semwal Cc: Liam Mark Cc: Laura Abbott Cc: Brian Starkey Cc: Hridya Valsaraju Cc: Suren Baghdasaryan Cc: Sandeep Patil Cc: Daniel Mentz Cc: Chris Goldsworthy Cc: Ørjan Eide Cc: Robin Murphy Cc: Ezequiel Garcia Cc: Simon Ser Cc: James Jones Cc: linux-me...@vger.kernel.org Cc: dri-de...@lists.freedesktop.org Signed-off-by: John Stultz --- drivers/dma-buf/dma-heap.c | 14 +- drivers/dma-buf/heaps/cma_heap.c| 22 +++--- drivers/dma-buf/heaps/system_heap.c | 21 +++-- include/linux/dma-heap.h| 12 ++-- 4 files changed, 33 insertions(+), 36 deletions(-) diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index afd22c9dbdcf..6b5db954569f 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -52,6 +52,9 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len, unsigned int fd_flags, unsigned int heap_flags) { + struct dma_buf *dmabuf; + int fd; + /* * Allocations from all heaps have to begin * and end on page boundaries. @@ -60,7 +63,16 @@ static int dma_heap_buffer_alloc(struct dma_heap *heap, size_t len, if (!len) return -EINVAL; - return heap->ops->allocate(heap, len, fd_flags, heap_flags); + dmabuf = heap->ops->allocate(heap, len, fd_flags, heap_flags); + if (IS_ERR(dmabuf)) + return PTR_ERR(dmabuf); + + fd = dma_buf_fd(dmabuf, fd_flags); + if (fd < 0) { + dma_buf_put(dmabuf); + /* just return, as put will call release and that will free */ + } + return fd; } static int dma_heap_open(struct inode *inode, struct file *file) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 0c76cbc3fb11..985c41ffd85b 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -272,10 +272,10 @@ static const struct dma_buf_ops cma_heap_buf_ops = { .release = cma_heap_dma_buf_release, }; -static int cma_heap_allocate(struct dma_heap *heap, - unsigned long len, - unsigned long fd_flags, - unsigned long heap_flags) +static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, +unsigned long len, +unsigned long fd_flags, +unsigned long heap_flags) { struct cma_heap *cma_heap = dma_heap_get_drvdata(heap); struct cma_heap_buffer *buffer; @@ -290,7 +290,7 @@ static int cma_heap_allocate(struct dma_heap *heap, buffer = kzalloc(sizeof(*buffer), GFP_KERNEL); if (!buffer) - return -ENOMEM; + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(>attachments); mutex_init(>lock); @@ -349,15 +349,7 @@ static int cma_heap_allocate(struct dma_heap *heap, ret = PTR_ERR(dmabuf); goto free_pages; } - - ret = dma_buf_fd(dmabuf, fd_flags); - if (ret < 0) { - dma_buf_put(dmabuf); - /* just return, as put will call release and that will free */ - return ret; - } - - return ret; + return dmabuf; free_pages: kfree(buffer->pages); @@ -366,7 +358,7 @@ static int cma_heap_allocate(struct dma_heap *heap, free_buffer: kfree(buffer); - return ret; + return ERR_PTR(ret); } static const struct dma_heap_ops cma_heap_ops = { diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c index 2321c91891f6..7b154424aeb3 100644 --- a/drivers/dma-buf/heaps/system_heap.c +++ b/drivers/dma-buf/heaps/system_heap.c @@ -332,10 +332,10 @@ static struct page *alloc_largest_available(unsigned long size, return NULL; } -static int system_heap_allocate(struct dma_heap *heap, - unsigned long len, - unsigned long fd_flags, - unsigned long heap_flags) +static struct dma_buf *system_heap_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) { struct system_heap_buffer *buffer; DEFINE_DMA_BUF_EXPORT_INFO(exp_info); @@ -350,7 +350,7 @@ static int system_heap_allocate(struct dma_heap *heap, buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);