| Message ID | 20210119204508.9256-3-john.stultz@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,1/3] dma-buf: system_heap: Make sure to return an error if we abort | expand |
Hi John, On Wed, 20 Jan 2021 at 02:15, John Stultz <john.stultz@linaro.org> 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 <sumit.semwal@linaro.org> > Cc: Liam Mark <lmark@codeaurora.org> > Cc: Laura Abbott <labbott@kernel.org> > Cc: Brian Starkey <Brian.Starkey@arm.com> > Cc: Hridya Valsaraju <hridya@google.com> > Cc: Suren Baghdasaryan <surenb@google.com> > Cc: Sandeep Patil <sspatil@google.com> > Cc: Daniel Mentz <danielmentz@google.com> > Cc: Chris Goldsworthy <cgoldswo@codeaurora.org> > Cc: Ørjan Eide <orjan.eide@arm.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Ezequiel Garcia <ezequiel@collabora.com> > Cc: Simon Ser <contact@emersion.fr> > Cc: James Jones <jajones@nvidia.com> > Cc: linux-media@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: John Stultz <john.stultz@linaro.org> > --- > 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(&buffer->attachments); > mutex_init(&buffer->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); > if (!buffer) > - return -ENOMEM; > + return ERR_PTR(-ENOMEM); > > INIT_LIST_HEAD(&buffer->attachments); > mutex_init(&buffer->lock); > @@ -400,14 +400,7 @@ static int system_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: > for_each_sgtable_sg(table, sg, i) { > @@ -421,7 +414,7 @@ static int system_heap_allocate(struct dma_heap *heap, > __free_pages(page, compound_order(page)); > kfree(buffer); > > - return ret; > + return ERR_PTR(ret); > } > > static const struct dma_heap_ops system_heap_ops = { > diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h > index 454e354d1ffb..5bc5c946af58 100644 > --- a/include/linux/dma-heap.h > +++ b/include/linux/dma-heap.h > @@ -16,15 +16,15 @@ struct dma_heap; > > /** > * struct dma_heap_ops - ops to operate on a given heap > - * @allocate: allocate dmabuf and return fd > + * @allocate: allocate dmabuf and return struct dma_buf ptr > * > - * allocate returns dmabuf fd on success, -errno on error. > + * allocate returns dmabuf on success, ERR_PTR(-errno) on error. > */ > struct dma_heap_ops { > - int (*allocate)(struct dma_heap *heap, > - unsigned long len, > - unsigned long fd_flags, > - unsigned long heap_flags); > + struct dma_buf *(*allocate)(struct dma_heap *heap, > + unsigned long len, > + unsigned long fd_flags, > + unsigned long heap_flags); > }; > > /** > -- > 2.17.1 > Best, Sumit.
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(&buffer->attachments); mutex_init(&buffer->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); if (!buffer) - return -ENOMEM; + return ERR_PTR(-ENOMEM); INIT_LIST_HEAD(&buffer->attachments); mutex_init(&buffer->lock); @@ -400,14 +400,7 @@ static int system_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: for_each_sgtable_sg(table, sg, i) { @@ -421,7 +414,7 @@ static int system_heap_allocate(struct dma_heap *heap, __free_pages(page, compound_order(page)); kfree(buffer); - return ret; + return ERR_PTR(ret); } static const struct dma_heap_ops system_heap_ops = { diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h index 454e354d1ffb..5bc5c946af58 100644 --- a/include/linux/dma-heap.h +++ b/include/linux/dma-heap.h @@ -16,15 +16,15 @@ struct dma_heap; /** * struct dma_heap_ops - ops to operate on a given heap - * @allocate: allocate dmabuf and return fd + * @allocate: allocate dmabuf and return struct dma_buf ptr * - * allocate returns dmabuf fd on success, -errno on error. + * allocate returns dmabuf on success, ERR_PTR(-errno) on error. */ struct dma_heap_ops { - int (*allocate)(struct dma_heap *heap, - unsigned long len, - unsigned long fd_flags, - unsigned long heap_flags); + struct dma_buf *(*allocate)(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags); }; /**
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 <sumit.semwal@linaro.org> Cc: Liam Mark <lmark@codeaurora.org> Cc: Laura Abbott <labbott@kernel.org> Cc: Brian Starkey <Brian.Starkey@arm.com> Cc: Hridya Valsaraju <hridya@google.com> Cc: Suren Baghdasaryan <surenb@google.com> Cc: Sandeep Patil <sspatil@google.com> Cc: Daniel Mentz <danielmentz@google.com> Cc: Chris Goldsworthy <cgoldswo@codeaurora.org> Cc: Ørjan Eide <orjan.eide@arm.com> Cc: Robin Murphy <robin.murphy@arm.com> Cc: Ezequiel Garcia <ezequiel@collabora.com> Cc: Simon Ser <contact@emersion.fr> Cc: James Jones <jajones@nvidia.com> Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Signed-off-by: John Stultz <john.stultz@linaro.org> --- 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(-)