Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
3028 discussions

[PATCH] dma-buf: fix locking in sync_print_obj()

by Dan Carpenter

This is always called with IRQs disabled and we don't actually want to enable IRQs at the end. Fixes: a6aa8fca4d79 ("dma-buf/sw-sync: Reduce irqsave/irqrestore from known context") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- drivers/dma-buf/sync_debug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/sync_debug.c b/drivers/dma-buf/sync_debug.c index 101394f16930..952331344b1c 100644 --- a/drivers/dma-buf/sync_debug.c +++ b/drivers/dma-buf/sync_debug.c @@ -107,15 +107,16 @@ static void sync_print_fence(struct seq_file *s, static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj) { struct list_head *pos; + unsigned long flags; seq_printf(s, "%s: %d\n", obj->name, obj->value); - spin_lock_irq(&obj->lock); + spin_lock_irqsave(&obj->lock, flags); list_for_each(pos, &obj->pt_list) { struct sync_pt *pt = container_of(pos, struct sync_pt, link); sync_print_fence(s, &pt->base, false); } - spin_unlock_irq(&obj->lock); + spin_unlock_irqrestore(&obj->lock, flags); } static void sync_print_sync_file(struct seq_file *s, -- 2.11.0

5 years, 7 months

[PATCH] dma-heap: Make the symbol 'dma_heap_ioctl_cmds' static

by Sumit Semwal

From: zhong jiang <zhongjiang(a)huawei.com> Fix the following sparse warning. drivers/dma-buf/dma-heap.c:109:14: warning: symbol 'dma_heap_ioctl_cmds' was not declared. Should it be static? Acked-by: Andrew F. Davis <afd(a)ti.com> Acked-by: John Stultz <john.stultz(a)linaro.org> Signed-off-by: zhong jiang <zhongjiang(a)huawei.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> [sumits: rebased over IOCTL rename patches] --- drivers/dma-buf/dma-heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 1886aee46131..afd22c9dbdcf 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -106,7 +106,7 @@ static long dma_heap_ioctl_allocate(struct file *file, void *data) return 0; } -unsigned int dma_heap_ioctl_cmds[] = { +static unsigned int dma_heap_ioctl_cmds[] = { DMA_HEAP_IOCTL_ALLOC, }; -- 2.18.0

5 years, 9 months

[PATCH][next] dma-buf: fix resource leak on -ENOTTY error return path

by Colin King

From: Colin Ian King <colin.king(a)canonical.com> The -ENOTTY error return path does not free the allocated kdata as it returns directly. Fix this by returning via the error handling label err. Addresses-Coverity: ("Resource leak") Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework") Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- drivers/dma-buf/dma-heap.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 4f04d104ae61..80f2f5eac1e4 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -157,7 +157,8 @@ static long dma_heap_ioctl(struct file *file, unsigned int ucmd, ret = dma_heap_ioctl_allocate(file, kdata); break; default: - return -ENOTTY; + ret = -ENOTTY; + goto err; } if (copy_to_user((void __user *)arg, kdata, out_size) != 0) -- 2.24.0

5 years, 9 months

[PATCH] MAINTAINERS: Match on dma_buf|fence|resv anywhere

by Daniel Vetter

I've spent a bit too much time reviewing all kinds of users all over the kernel for this buffer sharing infrastructure. And some of it is at least questionable. Make sure we at least see when this stuff flies by. Acked-by: Dave Airlie <airlied(a)gmail.com> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Mauro Carvalho Chehab <mchehab+samsung(a)kernel.org> Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> Cc: Rob Herring <robh(a)kernel.org> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 375dbea8bc88..c1e3da2c1947 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4941,6 +4941,7 @@ F: include/linux/dma-buf* F: include/linux/reservation.h F: include/linux/*fence.h F: Documentation/driver-api/dma-buf.rst +K: dma_(buf|fence|resv) T: git git://anongit.freedesktop.org/drm/drm-misc DMA GENERIC OFFLOAD ENGINE SUBSYSTEM -- 2.24.0

5 years, 9 months

[PATCH] udmabuf: Remove deleted map/unmap handlers.

by Maarten Lankhorst

Commit 7f0de8d80816 ("dma-buf: Drop dma_buf_k(un)map") removed map/unmap handlers, but they still existed in udmabuf. Remove them there as well Signed-off-by: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Fixes: 7f0de8d80816 ("dma-buf: Drop dma_buf_k(un)map") Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Daniel Vetter <daniel.vetter(a)intel.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: dri-devel(a)lists.freedesktop.org --- drivers/dma-buf/udmabuf.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9635897458a0..9de539c1def4 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -93,26 +93,10 @@ static void release_udmabuf(struct dma_buf *buf) kfree(ubuf); } -static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num) -{ - struct udmabuf *ubuf = buf->priv; - struct page *page = ubuf->pages[page_num]; - - return kmap(page); -} - -static void kunmap_udmabuf(struct dma_buf *buf, unsigned long page_num, - void *vaddr) -{ - kunmap(vaddr); -} - static const struct dma_buf_ops udmabuf_ops = { .map_dma_buf = map_udmabuf, .unmap_dma_buf = unmap_udmabuf, .release = release_udmabuf, - .map = kmap_udmabuf, - .unmap = kunmap_udmabuf, .mmap = mmap_udmabuf, }; -- 2.24.0

5 years, 9 months

[PATCH 15/15] dma-buf: Remove kernel map/unmap hooks

by Daniel Vetter

All implementations are gone now. Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- include/linux/dma-buf.h | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h index 7feb9c3805ae..abf5459a5b9d 100644 --- a/include/linux/dma-buf.h +++ b/include/linux/dma-buf.h @@ -249,31 +249,6 @@ struct dma_buf_ops { */ int (*mmap)(struct dma_buf *, struct vm_area_struct *vma); - /** - * @map: - * - * Maps a page from the buffer into kernel address space. The page is - * specified by offset into the buffer in PAGE_SIZE units. - * - * This callback is optional. - * - * Returns: - * - * Virtual address pointer where requested page can be accessed. NULL - * on error or when this function is unimplemented by the exporter. - */ - void *(*map)(struct dma_buf *, unsigned long); - - /** - * @unmap: - * - * Unmaps a page from the buffer. Page offset and address pointer should - * be the same as the one passed to and returned by matching call to map. - * - * This callback is optional. - */ - void (*unmap)(struct dma_buf *, unsigned long, void *); - void *(*vmap)(struct dma_buf *); void (*vunmap)(struct dma_buf *, void *vaddr); }; -- 2.24.0

5 years, 9 months

Re: [Linaro-mm-sig] [PATCH] dma-buf: Fix memory leak in sync_file_merge()

by Daniel Vetter

On Fri, Nov 22, 2019 at 04:09:55PM -0600, Navid Emamdoost wrote: > In the implementation of sync_file_merge() the allocated sync_file is > leaked if number of fences overflows. Release sync_file by goto err. > > Fixes: a02b9dc90d84 ("dma-buf/sync_file: refactor fence storage in struct sync_file") > Signed-off-by: Navid Emamdoost <navid.emamdoost(a)gmail.com> Thanks for your patch, applied with a cc:stable. -Daniel > --- > drivers/dma-buf/sync_file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c > index 25c5c071645b..91185db9a952 100644 > --- a/drivers/dma-buf/sync_file.c > +++ b/drivers/dma-buf/sync_file.c > @@ -221,7 +221,7 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, > a_fences = get_fences(a, &a_num_fences); > b_fences = get_fences(b, &b_num_fences); > if (a_num_fences > INT_MAX - b_num_fences) > - return NULL; > + goto err; > > num_fences = a_num_fences + b_num_fences; > > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch

5 years, 9 months

Re: [Linaro-mm-sig] 2989f64510 ("dma-buf: Add selftests for dma-fence"): WARNING: CPU: 0 PID: 1 at lib/debugobjects.c:524 __debug_object_init

by Chris Wilson

Quoting kernel test robot (2019-11-21 07:19:43) > Greetings, > > 0day kernel testing robot got the below dmesg and the first bad commit is > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > commit 2989f6451084aed3f8cc9992477f7a9bf57a3716 > Author: Chris Wilson <chris(a)chris-wilson.co.uk> > AuthorDate: Mon Aug 19 10:59:27 2019 +0100 > Commit: Chris Wilson <chris(a)chris-wilson.co.uk> > CommitDate: Mon Aug 19 18:09:46 2019 +0100 That's a belated report, fixed by commit 6ac3a0ebfcc2f0c75ca0ca6974389ce421aa5cbd Author: Chris Wilson <chris(a)chris-wilson.co.uk> Date: Tue Aug 20 13:21:18 2019 +0100 dmabuf: Mark up onstack timer for selftests No? -Chris

5 years, 9 months

[PATCH 2/3] dma-resv: Also prime acquire ctx for lockdep

by Daniel Vetter

Semnatically it really doesn't matter where we grab the ticket. But since the ticket is a fake lockdep lock, it matters for lockdep validation purposes. This means stuff like grabbing a ticket and then doing copy_from/to_user isn't allowed anymore. This is a changed compared to the current ttm fault handler, which doesn't bother with having a full reservation. Since I'm looking into fixing the TODO entry in ttm_mem_evict_wait_busy() I think that'll have to change sooner or later anyway, better get started. A bit more context on why I'm looking into this: For backwards compat with existing i915 gem code I think we'll have to do full slowpath locking in the i915 equivalent of the eviction code. And with dynamic dma-buf that will leak across drivers, so another thing we need to standardize and make sure it's done the same way everyway. Unfortunately this means another full audit of all drivers: - gem helpers: acquire_init is done right before taking locks, so no problem. Same for acquire_fini and unlocking, which means nothing that's not already covered by the dma_resv_lock rules will be caught with this extension here to the acquire_ctx. - etnaviv: An absolute massive amount of code is run between the acquire_init and the first lock acquisition in submit_lock_objects. But nothing that would touch user memory and could cause a fault. Furthermore nothing that uses the ticket, so even if I missed something, it would be easy to fix by pushing the acquire_init right before the first use. Similar on the unlock/acquire_fini side. - i915: Right now (and this will likely change a lot rsn) the acquire ctx and actual locks are right next to each another. No problem. - msm has a problem: submit_create calls acquire_init, but then submit_lookup_objects() has a bunch of copy_from_user to do the object lookups. That's the only thing before submit_lock_objects call dma_resv_lock(). Despite all the copypasta to etnaviv, etnaviv does not have this issue since it copies all the userspace structs earlier. submit_cleanup does not have any such issues. With the prep patch to pull out the acquire_ctx and reorder it msm is going to be safe too. - nouveau: acquire_init is right next to ttm_bo_reserve, so all good. Similar on the acquire_fini/ttm_bo_unreserve side. - ttm execbuf utils: acquire context and locking are even in the same functions here (one function to reserve everything, the other to unreserve), so all good. - vc4: Another case where acquire context and locking are handled in the same functions (one function to lock everything, the other to unlock). Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com> Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Christian König <christian.koenig(a)amd.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Huang Rui <ray.huang(a)amd.com> Cc: Eric Anholt <eric(a)anholt.net> Cc: Ben Skeggs <bskeggs(a)redhat.com> Cc: Alex Deucher <alexander.deucher(a)amd.com> Cc: Rob Herring <robh(a)kernel.org> Cc: Lucas Stach <l.stach(a)pengutronix.de> Cc: Russell King <linux+etnaviv(a)armlinux.org.uk> Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com> Cc: Rob Clark <robdclark(a)gmail.com> Cc: Sean Paul <sean(a)poorly.run> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> --- drivers/dma-buf/dma-resv.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index d3c760e19991..079e38fde33a 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -100,7 +100,9 @@ static void dma_resv_list_free(struct dma_resv_list *list) static void __init dma_resv_lockdep(void) { struct mm_struct *mm = mm_alloc(); + struct ww_acquire_ctx ctx; struct dma_resv obj; + int ret; if (!mm) return; @@ -108,10 +110,14 @@ static void __init dma_resv_lockdep(void) dma_resv_init(&obj); down_read(&mm->mmap_sem); - ww_mutex_lock(&obj.lock, NULL); + ww_acquire_init(&ctx, &reservation_ww_class); + ret = dma_resv_lock(&obj, &ctx); + if (ret == -EDEADLK) + dma_resv_lock_slow(&obj, &ctx); fs_reclaim_acquire(GFP_KERNEL); fs_reclaim_release(GFP_KERNEL); ww_mutex_unlock(&obj.lock); + ww_acquire_fini(&ctx); up_read(&mm->mmap_sem); mmput(mm); -- 2.24.0

5 years, 9 months

Re: [Linaro-mm-sig] [PATCH 15/15] dma-buf: Remove kernel map/unmap hooks

by Daniel Vetter

On Mon, Nov 18, 2019 at 4:23 PM kbuild test robot <lkp(a)intel.com> wrote: > > Hi Daniel, > > I love your patch! Yet something to improve: > > [auto build test ERROR on drm-intel/for-linux-next] > [also build test ERROR on v5.4-rc8 next-20191115] > [if your patch is applied to the wrong git tree, please drop us a note to help > improve the system. BTW, we also suggest to use '--base' option to specify the > base tree in git format-patch, please see https://stackoverflow.com/a/37406982] Too old tree, on latest drm-tip this compiles since udl has lots its own dma-buf implementation. Also, the patch set will start to compile once linux-next is open for 5.6. Cheers, Daniel > > url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/Retire-dma_buf_k-un-… > base: git://anongit.freedesktop.org/drm-intel for-linux-next > config: m68k-allmodconfig (attached as .config) > compiler: m68k-linux-gcc (GCC) 7.4.0 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=7.4.0 make.cross ARCH=m68k > > If you fix the issue, kindly add following tag > Reported-by: kbuild test robot <lkp(a)intel.com> > > All errors (new ones prefixed by >>): > > >> drivers/dma-buf/udmabuf.c:114:3: error: 'const struct dma_buf_ops' has no member named 'map'; did you mean 'mmap'? > .map = kmap_udmabuf, > ^~~ > mmap > >> drivers/dma-buf/udmabuf.c:114:12: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] > .map = kmap_udmabuf, > ^~~~~~~~~~~~ > drivers/dma-buf/udmabuf.c:114:12: note: (near initialization for 'udmabuf_ops.begin_cpu_access') > >> drivers/dma-buf/udmabuf.c:115:3: error: 'const struct dma_buf_ops' has no member named 'unmap'; did you mean 'vunmap'? > .unmap = kunmap_udmabuf, > ^~~~~ > vunmap > drivers/dma-buf/udmabuf.c:115:14: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] > .unmap = kunmap_udmabuf, > ^~~~~~~~~~~~~~ > drivers/dma-buf/udmabuf.c:115:14: note: (near initialization for 'udmabuf_ops.end_cpu_access') > cc1: some warnings being treated as errors > -- > >> drivers/gpu/drm/udl/udl_dmabuf.c:169:3: error: 'const struct dma_buf_ops' has no member named 'map'; did you mean 'mmap'? > .map = udl_dmabuf_kmap, > ^~~ > mmap > >> drivers/gpu/drm/udl/udl_dmabuf.c:169:11: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] > .map = udl_dmabuf_kmap, > ^~~~~~~~~~~~~~~ > drivers/gpu/drm/udl/udl_dmabuf.c:169:11: note: (near initialization for 'udl_dmabuf_ops.release') > >> drivers/gpu/drm/udl/udl_dmabuf.c:170:3: error: 'const struct dma_buf_ops' has no member named 'unmap'; did you mean 'vunmap'? > .unmap = udl_dmabuf_kunmap, > ^~~~~ > vunmap > drivers/gpu/drm/udl/udl_dmabuf.c:170:13: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] > .unmap = udl_dmabuf_kunmap, > ^~~~~~~~~~~~~~~~~ > drivers/gpu/drm/udl/udl_dmabuf.c:170:13: note: (near initialization for 'udl_dmabuf_ops.begin_cpu_access') > cc1: some warnings being treated as errors > > vim +114 drivers/dma-buf/udmabuf.c > > fbb0de79507819 Gerd Hoffmann 2018-08-27 109 > a34852891ba45d Gerd Hoffmann 2018-09-11 110 static const struct dma_buf_ops udmabuf_ops = { > fbb0de79507819 Gerd Hoffmann 2018-08-27 111 .map_dma_buf = map_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 112 .unmap_dma_buf = unmap_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 113 .release = release_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 @114 .map = kmap_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 @115 .unmap = kunmap_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 116 .mmap = mmap_udmabuf, > fbb0de79507819 Gerd Hoffmann 2018-08-27 117 }; > fbb0de79507819 Gerd Hoffmann 2018-08-27 118 > > :::::: The code at line 114 was first introduced by commit > :::::: fbb0de795078190a9834b3409e4b009cfb18a6d4 Add udmabuf misc device > > :::::: TO: Gerd Hoffmann <kraxel(a)redhat.com> > :::::: CC: Gerd Hoffmann <kraxel(a)redhat.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

5 years, 10 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig