Re: [RESEND][PATCH v16 3/5] dma-buf: heaps: Add system heap to dmabuf heaps
On Thu, Dec 12, 2019 at 8:20 AM Andrew F. Davis wrote: > On 12/3/19 12:26 PM, John Stultz wrote: > > +static int system_heap_create(void) > > +{ > > + struct dma_heap_export_info exp_info; > > + int ret = 0; > > + > > + exp_info.name = "system_heap"; > > > nit: Would prefer the name just be "system", the heap part is redundant > given it will be in a "heaps" directory, other heaps don't have that. As > the heap will be accessed by users using this name: > (/sys/dma_heap/system_heap) we need to think of it like an ABI and get > it right the first time. The directory name should probably also be > plural "heaps" as it is a collection of heaps.. Wish this thought had come up earlier. Again, not bad suggestions, just a bit late as the patches have been queued in drm-misc-next. Can you submit a tack-on patch to address this, and we can see about pulling it in so it lands before the code heads to Linus? thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND][PATCH v16 3/5] dma-buf: heaps: Add system heap to dmabuf heaps
On 12/3/19 12:26 PM, John Stultz wrote: > This patch adds system heap to the dma-buf heaps framework. > > This allows applications to get a page-allocator backed dma-buf > for non-contiguous memory. > > This code is an evolution of the Android ION implementation, so > thanks to its original authors and maintainters: > Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others! > > Cc: Laura Abbott > Cc: Benjamin Gaignard > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Pratik Patel > Cc: Brian Starkey > Cc: Vincent Donnefort > Cc: Sudipto Paul > Cc: Andrew F. Davis > Cc: Christoph Hellwig > Cc: Chenbo Feng > Cc: Alistair Strachan > Cc: Hridya Valsaraju > Cc: Sandeep Patil > Cc: Hillf Danton > Cc: Dave Airlie > Cc: dri-devel@lists.freedesktop.org > Reviewed-by: Benjamin Gaignard > Reviewed-by: Brian Starkey > Acked-by: Sandeep Patil > Acked-by: Laura Abbott > Tested-by: Ayan Kumar Halder > Signed-off-by: John Stultz > --- > v2: > * Switch allocate to return dmabuf fd > * Simplify init code > * Checkpatch fixups > * Droped dead system-contig code > v3: > * Whitespace fixups from Benjamin > * Make sure we're zeroing the allocated pages (from Liam) > * Use PAGE_ALIGN() consistently (suggested by Brian) > * Fold in new registration style from Andrew > * Avoid needless dynamic allocation of sys_heap (suggested by > Christoph) > * Minor cleanups > * Folded in changes from Andrew to use simplified page list > from the heap helpers > v4: > * Optimization to allocate pages in chunks, similar to old > pagepool code > * Use fd_flags when creating dmabuf fd (Suggested by Benjamin) > v5: > * Back out large order page allocations (was leaking memory, > as the page array didn't properly track order size) > v6: > * Minor whitespace change suggested by Brian > * Remove unused variable > v7: > * Use newly lower-cased init_heap_helper_buffer helper > * Add system heap DOS avoidance suggested by Laura from ION code > * Use new dmabuf export helper > v8: > * Make struct dma_heap_ops consts (suggested by Christoph) > * Get rid of needless struct system_heap (suggested by Christoph) > * Condense dma_heap_buffer and heap_helper_buffer (suggested by > Christoph) > * Add forgotten include file to fix build issue on x86 > v12: > * Minor tweaks to prep loading heap from module > v14: > * Fix "redundant assignment to variable ret" issue reported > by Colin King and fixed by Andrew Davis > v15: > * Drop unused heap flag from heap_helper_buffer as suggested > by Sandeep Patil > --- > drivers/dma-buf/Kconfig | 2 + > drivers/dma-buf/heaps/Kconfig | 6 ++ > drivers/dma-buf/heaps/Makefile | 1 + > drivers/dma-buf/heaps/system_heap.c | 123 > 4 files changed, 132 insertions(+) > create mode 100644 drivers/dma-buf/heaps/Kconfig > create mode 100644 drivers/dma-buf/heaps/system_heap.c > > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index bffa58fc3e6e..0613bb7770f5 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -53,4 +53,6 @@ menuconfig DMABUF_HEAPS > allows userspace to allocate dma-bufs that can be shared > between drivers. > > +source "drivers/dma-buf/heaps/Kconfig" > + > endmenu > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig > new file mode 100644 > index ..205052744169 > --- /dev/null > +++ b/drivers/dma-buf/heaps/Kconfig > @@ -0,0 +1,6 @@ > +config DMABUF_HEAPS_SYSTEM > + bool "DMA-BUF System Heap" > + depends on DMABUF_HEAPS > + help > + Choose this option to enable the system dmabuf heap. The system heap > + is backed by pages from the buddy allocator. If in doubt, say Y. > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile > index de49898112db..d1808eca2581 100644 > --- a/drivers/dma-buf/heaps/Makefile > +++ b/drivers/dma-buf/heaps/Makefile > @@ -1,2 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-y+= heap-helpers.o > +obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o > diff --git a/drivers/dma-buf/heaps/system_heap.c > b/drivers/dma-buf/heaps/system_heap.c > new file mode 100644 > index ..1aa01e98c595 > --- /dev/null > +++ b/drivers/dma-buf/heaps/system_heap.c > @@ -0,0 +1,123 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DMABUF System heap exporter > + * > + * Copyright (C) 2011 Google, Inc. > + * Copyright (C) 2019 Linaro Ltd. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "heap-helpers.h" > + > +struct dma_heap *sys_heap; > + > +static void system_heap_free(struct heap_helper_buffer *buffer) > +{ > + pgoff_t pg; > + > + for (pg = 0; pg < buffer->pagecount; pg++) > + __free_page(buffer->pages[pg]); > + kfree(buffer->pages); > + kfree(buffer); > +} > + > +static i
[RESEND][PATCH v16 3/5] dma-buf: heaps: Add system heap to dmabuf heaps
This patch adds system heap to the dma-buf heaps framework. This allows applications to get a page-allocator backed dma-buf for non-contiguous memory. This code is an evolution of the Android ION implementation, so thanks to its original authors and maintainters: Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others! Cc: Laura Abbott Cc: Benjamin Gaignard Cc: Sumit Semwal Cc: Liam Mark Cc: Pratik Patel Cc: Brian Starkey Cc: Vincent Donnefort Cc: Sudipto Paul Cc: Andrew F. Davis Cc: Christoph Hellwig Cc: Chenbo Feng Cc: Alistair Strachan Cc: Hridya Valsaraju Cc: Sandeep Patil Cc: Hillf Danton Cc: Dave Airlie Cc: dri-devel@lists.freedesktop.org Reviewed-by: Benjamin Gaignard Reviewed-by: Brian Starkey Acked-by: Sandeep Patil Acked-by: Laura Abbott Tested-by: Ayan Kumar Halder Signed-off-by: John Stultz --- v2: * Switch allocate to return dmabuf fd * Simplify init code * Checkpatch fixups * Droped dead system-contig code v3: * Whitespace fixups from Benjamin * Make sure we're zeroing the allocated pages (from Liam) * Use PAGE_ALIGN() consistently (suggested by Brian) * Fold in new registration style from Andrew * Avoid needless dynamic allocation of sys_heap (suggested by Christoph) * Minor cleanups * Folded in changes from Andrew to use simplified page list from the heap helpers v4: * Optimization to allocate pages in chunks, similar to old pagepool code * Use fd_flags when creating dmabuf fd (Suggested by Benjamin) v5: * Back out large order page allocations (was leaking memory, as the page array didn't properly track order size) v6: * Minor whitespace change suggested by Brian * Remove unused variable v7: * Use newly lower-cased init_heap_helper_buffer helper * Add system heap DOS avoidance suggested by Laura from ION code * Use new dmabuf export helper v8: * Make struct dma_heap_ops consts (suggested by Christoph) * Get rid of needless struct system_heap (suggested by Christoph) * Condense dma_heap_buffer and heap_helper_buffer (suggested by Christoph) * Add forgotten include file to fix build issue on x86 v12: * Minor tweaks to prep loading heap from module v14: * Fix "redundant assignment to variable ret" issue reported by Colin King and fixed by Andrew Davis v15: * Drop unused heap flag from heap_helper_buffer as suggested by Sandeep Patil --- drivers/dma-buf/Kconfig | 2 + drivers/dma-buf/heaps/Kconfig | 6 ++ drivers/dma-buf/heaps/Makefile | 1 + drivers/dma-buf/heaps/system_heap.c | 123 4 files changed, 132 insertions(+) create mode 100644 drivers/dma-buf/heaps/Kconfig create mode 100644 drivers/dma-buf/heaps/system_heap.c diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig index bffa58fc3e6e..0613bb7770f5 100644 --- a/drivers/dma-buf/Kconfig +++ b/drivers/dma-buf/Kconfig @@ -53,4 +53,6 @@ menuconfig DMABUF_HEAPS allows userspace to allocate dma-bufs that can be shared between drivers. +source "drivers/dma-buf/heaps/Kconfig" + endmenu diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig new file mode 100644 index ..205052744169 --- /dev/null +++ b/drivers/dma-buf/heaps/Kconfig @@ -0,0 +1,6 @@ +config DMABUF_HEAPS_SYSTEM + bool "DMA-BUF System Heap" + depends on DMABUF_HEAPS + help + Choose this option to enable the system dmabuf heap. The system heap + is backed by pages from the buddy allocator. If in doubt, say Y. diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile index de49898112db..d1808eca2581 100644 --- a/drivers/dma-buf/heaps/Makefile +++ b/drivers/dma-buf/heaps/Makefile @@ -1,2 +1,3 @@ # SPDX-License-Identifier: GPL-2.0 obj-y += heap-helpers.o +obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c new file mode 100644 index ..1aa01e98c595 --- /dev/null +++ b/drivers/dma-buf/heaps/system_heap.c @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * DMABUF System heap exporter + * + * Copyright (C) 2011 Google, Inc. + * Copyright (C) 2019 Linaro Ltd. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "heap-helpers.h" + +struct dma_heap *sys_heap; + +static void system_heap_free(struct heap_helper_buffer *buffer) +{ + pgoff_t pg; + + for (pg = 0; pg < buffer->pagecount; pg++) + __free_page(buffer->pages[pg]); + kfree(buffer->pages); + kfree(buffer); +} + +static int system_heap_allocate(struct dma_heap *heap, + unsigned long len, + unsigned long fd_flags, + unsigned long heap_flags) +{ + struct heap_helper_buffer *helper_buffer; + struct dma_buf *dmabuf; + int ret = -ENOMEM; + pgof