Hi John,
I spotted one thing below which might be harmless, but best to check.
On Fri, Sep 06, 2019 at 06:47:11PM +0000, John Stultz wrote:
This adds a CMA heap, which allows userspace to allocate a dma-buf of contiguous memory out of a CMA region.
This code is an evolution of the Android ION implementation, so thanks to its original author and maintainters: Benjamin Gaignard, Laura Abbott, and others!
Cc: Laura Abbott labbott@redhat.com Cc: Benjamin Gaignard benjamin.gaignard@linaro.org Cc: Sumit Semwal sumit.semwal@linaro.org Cc: Liam Mark lmark@codeaurora.org Cc: Pratik Patel pratikp@codeaurora.org Cc: Brian Starkey Brian.Starkey@arm.com Cc: Vincent Donnefort Vincent.Donnefort@arm.com Cc: Sudipto Paul Sudipto.Paul@arm.com Cc: Andrew F. Davis afd@ti.com Cc: Christoph Hellwig hch@infradead.org Cc: Chenbo Feng fengc@google.com Cc: Alistair Strachan astrachan@google.com Cc: Hridya Valsaraju hridya@google.com Cc: dri-devel@lists.freedesktop.org Reviewed-by: Benjamin Gaignard benjamin.gaignard@linaro.org Signed-off-by: John Stultz john.stultz@linaro.org
v2:
- Switch allocate to return dmabuf fd
- Simplify init code
- Checkpatch fixups
v3:
- Switch to inline function for to_cma_heap()
- Minor cleanups suggested by Brian
- Fold in new registration style from Andrew
- Folded in changes from Andrew to use simplified page list from the heap helpers
v4:
- Use the fd_flags when creating dmabuf fd (Suggested by Benjamin)
- Use precalculated pagecount (Suggested by Andrew)
v6:
- Changed variable names to improve clarity, as suggested by Brian
v7:
- Use newly lower-cased init_heap_helper_buffer helper
- Use new dmabuf export helper
v8:
- Make struct dma_heap_ops const (Suggested by Christoph)
- Condense dma_heap_buffer and heap_helper_buffer (suggested by Christoph)
- Checkpatch whitespace fixups
...
+/* dmabuf heap CMA operations functions */ +static int 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_data(heap);
- struct heap_helper_buffer *helper_buffer;
- struct page *cma_pages;
- size_t size = PAGE_ALIGN(len);
- unsigned long nr_pages = size >> PAGE_SHIFT;
- unsigned long align = get_order(size);
- struct dma_buf *dmabuf;
- int ret = -ENOMEM;
- pgoff_t pg;
- if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;- helper_buffer = kzalloc(sizeof(*helper_buffer), GFP_KERNEL);
- if (!helper_buffer)
return -ENOMEM;- init_heap_helper_buffer(helper_buffer, cma_heap_free);
- helper_buffer->flags = heap_flags;
- helper_buffer->heap = heap;
- helper_buffer->size = len;
- cma_pages = cma_alloc(cma_heap->cma, nr_pages, align, false);
- if (!cma_pages)
goto free_buf;- if (PageHighMem(cma_pages)) {
unsigned long nr_clear_pages = nr_pages;struct page *page = cma_pages;while (nr_clear_pages > 0) {void *vaddr = kmap_atomic(page);memset(vaddr, 0, PAGE_SIZE);kunmap_atomic(vaddr);page++;nr_clear_pages--;}- } else {
memset(page_address(cma_pages), 0, size);- }
- helper_buffer->pagecount = nr_pages;
- helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
sizeof(*helper_buffer->pages),GFP_KERNEL);- if (!helper_buffer->pages) {
ret = -ENOMEM;goto free_cma;- }
- for (pg = 0; pg < helper_buffer->pagecount; pg++) {
helper_buffer->pages[pg] = &cma_pages[pg];if (!helper_buffer->pages[pg])
Is this ever really possible? If cma_pages is non-NULL (which you check earlier), then only if the pointer arithmetic overflows right?
If it's just redundant, then you could remove it (and in that case add my r-b). But maybe you meant to check something else?
Cheers, -Brian