Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

14 participants
3706 discussions

[PATCH] dma-buf: make returning the exclusive fence optional

by Christian König

Change reservation_object_get_fences_rcu to make the exclusive fence pointer optional. If not specified the exclusive fence is put into the fence array as well. This is helpful for a couple of cases where we need all fences in a single array. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/reservation.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b759a569b7b8..461afa9febd4 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -374,8 +374,9 @@ EXPORT_SYMBOL(reservation_object_copy_fences); * @pshared: the array of shared fence ptrs returned (array is krealloc'd to * the required size, and must be freed by caller) * - * RETURNS - * Zero or -errno + * Retrieve all fences from the reservation object. If the pointer for the + * exclusive fence is not specified the fence is put into the array of the + * shared fences as well. Returns either zero or -ENOMEM. */ int reservation_object_get_fences_rcu(struct reservation_object *obj, struct dma_fence **pfence_excl, @@ -389,8 +390,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, do { struct reservation_object_list *fobj; - unsigned seq; - unsigned int i; + unsigned int i, seq; + size_t sz = 0; shared_count = i = 0; @@ -402,9 +403,14 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, goto unlock; fobj = rcu_dereference(obj->fence); - if (fobj) { + if (fobj) + sz += sizeof(*shared) * fobj->shared_max; + + if (!pfence_excl && fence_excl) + sz += sizeof(*shared); + + if (sz) { struct dma_fence **nshared; - size_t sz = sizeof(*shared) * fobj->shared_max; nshared = krealloc(shared, sz, GFP_NOWAIT | __GFP_NOWARN); @@ -420,13 +426,19 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, break; } shared = nshared; - shared_count = fobj->shared_count; - + shared_count = fobj ? fobj->shared_count : 0; for (i = 0; i < shared_count; ++i) { shared[i] = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(shared[i])) break; } + + if (!pfence_excl && fence_excl) { + shared[i] = fence_excl; + fence_excl = NULL; + ++i; + ++shared_count; + } } if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) { @@ -448,7 +460,8 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, *pshared_count = shared_count; *pshared = shared; - *pfence_excl = fence_excl; + if (pfence_excl) + *pfence_excl = fence_excl; return ret; } -- 2.14.1

8 years, 10 months

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Fix dma direction for dma_sync_sg_for_cpu/device

by Laura Abbott

On 12/15/2017 12:59 PM, Sushmita Susheelendra wrote: > Use the direction argument passed into begin_cpu_access > and end_cpu_access when calling the dma_sync_sg_for_cpu/device. > The actual cache primitive called depends on the direction > passed in. > Acked-by: Laura Abbott <labbott(a)redhat.com> > Signed-off-by: Sushmita Susheelendra <ssusheel(a)codeaurora.org> > --- > drivers/staging/android/ion/ion.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index a7d9b0e..f480885 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -346,7 +346,7 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, > mutex_lock(&buffer->lock); > list_for_each_entry(a, &buffer->attachments, list) { > dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents, > - DMA_BIDIRECTIONAL); > + direction); > } > mutex_unlock(&buffer->lock); > > @@ -368,7 +368,7 @@ static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf, > mutex_lock(&buffer->lock); > list_for_each_entry(a, &buffer->attachments, list) { > dma_sync_sg_for_device(a->dev, a->table->sgl, a->table->nents, > - DMA_BIDIRECTIONAL); > + direction); > } > mutex_unlock(&buffer->lock); > >

8 years, 10 months

Re: [Linaro-mm-sig] [PATCH 0/4] Backported amdgpu ttm deadlock fixes for 4.14

by Christian König

Am 01.12.2017 um 01:23 schrieb Lyude Paul: > I haven't gone to see where it started, but as of late a good number of > pretty nasty deadlock issues have appeared with the kernel. Easy > reproduction recipe on a laptop with i915/amdgpu prime with lockdep enabled: > > DRI_PRIME=1 glxinfo Acked-by: Christian König <christian.koenig(a)amd.com> Thanks for taking care of this, Christian. > > Additionally, some more race conditions exist that I've managed to > trigger with piglit and lockdep enabled after applying these patches: > > ============================= > WARNING: suspicious RCU usage > 4.14.3Lyude-Test+ #2 Not tainted > ----------------------------- > ./include/linux/reservation.h:216 suspicious rcu_dereference_protected() usage! > > other info that might help us debug this: > > rcu_scheduler_active = 2, debug_locks = 1 > 1 lock held by ext_image_dma_b/27451: > #0: (reservation_ww_class_mutex){+.+.}, at: [<ffffffffa034f2ff>] ttm_bo_unref+0x9f/0x3c0 [ttm] > > stack backtrace: > CPU: 0 PID: 27451 Comm: ext_image_dma_b Not tainted 4.14.3Lyude-Test+ #2 > Hardware name: HP HP ZBook 15 G4/8275, BIOS P70 Ver. 01.02 06/09/2017 > Call Trace: > dump_stack+0x8e/0xce > lockdep_rcu_suspicious+0xc5/0x100 > reservation_object_copy_fences+0x292/0x2b0 > ? ttm_bo_unref+0x9f/0x3c0 [ttm] > ttm_bo_unref+0xbd/0x3c0 [ttm] > amdgpu_bo_unref+0x2a/0x50 [amdgpu] > amdgpu_gem_object_free+0x4b/0x50 [amdgpu] > drm_gem_object_free+0x1f/0x40 [drm] > drm_gem_object_put_unlocked+0x40/0xb0 [drm] > drm_gem_object_handle_put_unlocked+0x6c/0xb0 [drm] > drm_gem_object_release_handle+0x51/0x90 [drm] > drm_gem_handle_delete+0x5e/0x90 [drm] > ? drm_gem_handle_create+0x40/0x40 [drm] > drm_gem_close_ioctl+0x20/0x30 [drm] > drm_ioctl_kernel+0x5d/0xb0 [drm] > drm_ioctl+0x2f7/0x3b0 [drm] > ? drm_gem_handle_create+0x40/0x40 [drm] > ? trace_hardirqs_on_caller+0xf4/0x190 > ? trace_hardirqs_on+0xd/0x10 > amdgpu_drm_ioctl+0x4f/0x90 [amdgpu] > do_vfs_ioctl+0x93/0x670 > ? __fget+0x108/0x1f0 > SyS_ioctl+0x79/0x90 > entry_SYSCALL_64_fastpath+0x23/0xc2 > > I've also added the relevant fixes for the issue mentioned above. > > Christian König (3): > drm/ttm: fix ttm_bo_cleanup_refs_or_queue once more > dma-buf: make reservation_object_copy_fences rcu save > drm/amdgpu: reserve root PD while releasing it > > Michel Dänzer (1): > drm/ttm: Always and only destroy bo->ttm_resv in ttm_bo_release_list > > drivers/dma-buf/reservation.c | 56 +++++++++++++++++++++++++--------- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 13 ++++++-- > drivers/gpu/drm/ttm/ttm_bo.c | 43 +++++++++++++------------- > 3 files changed, 74 insertions(+), 38 deletions(-) > > -- > 2.14.3 > > _______________________________________________ > amd-gfx mailing list > amd-gfx(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx

8 years, 11 months

Re: [Linaro-mm-sig] [PATCH] dma-buf: Fix ifnullfree.cocci warnings

by Sumit Semwal

Hello Vasyl, On 22 November 2017 at 20:52, Vasyl Gomonovych <gomonovych(a)gmail.com> wrote: > NULL check before some freeing functions is not needed. > drivers/dma-buf/dma-buf.c:1183:2-26: WARNING: NULL check before freeing debugfs_remove_recursive > Generated by: scripts/coccinelle/free/ifnullfree.cocci Thank you for your patch; applied to drm-misc-next. > > Signed-off-by: Vasyl Gomonovych <gomonovych(a)gmail.com> > --- > drivers/dma-buf/dma-buf.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 4a038dcf5361..048827b06c03 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -1179,8 +1179,7 @@ static int dma_buf_init_debugfs(void) > > static void dma_buf_uninit_debugfs(void) > { > - if (dma_buf_debugfs_dir) > - debugfs_remove_recursive(dma_buf_debugfs_dir); > + debugfs_remove_recursive(dma_buf_debugfs_dir); > } > #else > static inline int dma_buf_init_debugfs(void) > -- > 1.9.1 > -- Thanks and regards, Sumit Semwal Linaro Mobile Group - Kernel Team Lead Linaro.org │ Open source software for ARM SoCs

8 years, 11 months

[PATCH] reservation: don't wait when timeout=0

by Rob Clark

If we are testing if a reservation object's fences have been signaled with timeout=0 (non-blocking), we need to pass 0 for timeout to dma_fence_wait_timeout(). Plus bonus spelling correction. Signed-off-by: Rob Clark <robdclark(a)gmail.com> --- drivers/dma-buf/reservation.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index dec3a815455d..71f51140a9ad 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -420,7 +420,7 @@ EXPORT_SYMBOL_GPL(reservation_object_get_fences_rcu); * * RETURNS * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or - * greater than zer on success. + * greater than zero on success. */ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, bool wait_all, bool intr, @@ -483,7 +483,14 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, goto retry; } - ret = dma_fence_wait_timeout(fence, intr, ret); + /* + * Note that dma_fence_wait_timeout() will return 1 if + * the fence is already signaled, so in the wait_all + * case when we go through the retry loop again, ret + * will be greater than 0 and we don't want this to + * cause _wait_timeout() to block + */ + ret = dma_fence_wait_timeout(fence, intr, timeout ? ret : 0); dma_fence_put(fence); if (ret > 0 && wait_all && (i + 1 < shared_count)) goto retry; -- 2.13.6

8 years, 11 months

[PATCH 0/4] dma-buf: Silence dma_fence __rcu sparse warnings

by Ville Syrjala

From: Ville Syrjälä <ville.syrjala(a)linux.intel.com> When building drm+i915 I get around 150 lines of sparse noise from dma_fence __rcu warnings. This series eliminates all of that. The first two patches were already posted by Chris, but there wasn't any real reaction, so I figured I'd repost with a wider Cc list. As for the other two patches, I'm no expert on dma_fence and I didn't spend a lot of time looking at it so I can't be sure I annotated all the accesses correctly. But I figured someone will scream at me if I got it wrong ;) Cc: Dave Airlie <airlied(a)redhat.com> Cc: Jason Ekstrand <jason(a)jlekstrand.net> Cc: linaro-mm-sig(a)lists.linaro.org Cc: linux-media(a)vger.kernel.org Cc: Alex Deucher <alexander.deucher(a)amd.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Chris Wilson (2): drm/syncobj: Mark up the fence as an RCU protected pointer dma-buf/fence: Sparse wants __rcu on the object itself Ville Syrjälä (2): drm/syncobj: Use proper methods for accessing rcu protected pointers dma-buf: Use rcu_assign_pointer() to set rcu protected pointers drivers/dma-buf/reservation.c | 2 +- drivers/gpu/drm/drm_syncobj.c | 11 +++++++---- include/drm/drm_syncobj.h | 2 +- include/linux/dma-fence.h | 2 +- 4 files changed, 10 insertions(+), 7 deletions(-) -- 2.13.6

9 years

Re: [Linaro-mm-sig] [PATCH] dma-fence: fix dma_fence_get_rcu_safe

by Chris Wilson

Quoting Christian König (2017-09-04 14:27:33) > From: Christian König <christian.koenig(a)amd.com> > > The logic is buggy and unnecessary complex. When dma_fence_get_rcu() fails to > acquire a reference it doesn't necessary mean that there is no fence at all. > > It usually mean that the fence was replaced by a new one and in this situation > we certainly want to have the new one as result and *NOT* NULL. Which is not guaranteed by the code you wrote either. The point of the comment is that the mb is only inside the successful kref_atomic_inc_unless_zero, and that only after that mb do you know whether or not you have the current fence. You can argue that you want to replace the if (!dma_fence_get_rcu()) return NULL with if (!dma_fence_get_rcu() continue; but it would be incorrect to say that by simply ignoring the post-condition check that you do have the right fence. -Chris

9 years, 1 month

[PATCH] dma-fence: fix dma_fence_get_rcu_safe v2

by Christian König

From: Christian König <christian.koenig(a)amd.com> When dma_fence_get_rcu() fails to acquire a reference it doesn't necessary mean that there is no fence at all. It usually mean that the fence was replaced by a new one and in this situation we certainly want to have the new one as result and *NOT* NULL. v2: Keep extra check after dma_fence_get_rcu(). Signed-off-by: Christian König <christian.koenig(a)amd.com> Cc: Chris Wilson <chris(a)chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org --- include/linux/dma-fence.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 0a186c4..f4f23cb 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -248,9 +248,12 @@ dma_fence_get_rcu_safe(struct dma_fence * __rcu *fencep) struct dma_fence *fence; fence = rcu_dereference(*fencep); - if (!fence || !dma_fence_get_rcu(fence)) + if (!fence) return NULL; + if (!dma_fence_get_rcu(fence)) + continue; + /* The atomic_inc_not_zero() inside dma_fence_get_rcu() * provides a full memory barrier upon success (such as now). * This is paired with the write barrier from assigning -- 2.7.4

9 years, 1 month

[PATCH] dma-buf: remove redundant initialization of sg_table

by Colin King

From: Colin Ian King <colin.king(a)canonical.com> sg_table is being initialized and is never read before it is updated again later on, hence making the initialization redundant. Remove the initialization. Detected by clang scan-build: "warning: Value stored to 'sg_table' during its initialization is never read" Signed-off-by: Colin Ian King <colin.king(a)canonical.com> --- drivers/dma-buf/dma-buf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 4a038dcf5361..bc1cb284111c 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -625,7 +625,7 @@ EXPORT_SYMBOL_GPL(dma_buf_detach); struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach, enum dma_data_direction direction) { - struct sg_table *sg_table = ERR_PTR(-EINVAL); + struct sg_table *sg_table; might_sleep(); -- 2.14.1

9 years, 1 month

Re: [Linaro-mm-sig] [PATCH] dma-buf: fix reservation_object_wait_timeout_rcu to wait correctly v2

by Deucher, Alexander

> -----Original Message----- > From: Christian König [mailto:deathsimple@vodafone.de] > Sent: Monday, July 31, 2017 10:13 AM > To: linux-media(a)vger.kernel.org; dri-devel(a)lists.freedesktop.org; linaro- > mm-sig(a)lists.linaro.org; Zhou, David(ChunMing); Deucher, Alexander > Subject: Re: [PATCH] dma-buf: fix reservation_object_wait_timeout_rcu to > wait correctly v2 > > Ping, what do you guys think of that? Seems reasonable to me. Reviewed-by: Alex Deucher <alexander.deucher(a)amd.com> > > Am 25.07.2017 um 15:35 schrieb Christian König: > > From: Christian König <christian.koenig(a)amd.com> > > > > With hardware resets in mind it is possible that all shared fences are > > signaled, but the exlusive isn't. Fix waiting for everything in this situation. > > > > v2: make sure we always wait for the exclusive fence > > > > Signed-off-by: Christian König <christian.koenig(a)amd.com> > > --- > > drivers/dma-buf/reservation.c | 33 +++++++++++++++------------------ > > 1 file changed, 15 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > > index 393817e..9d4316d 100644 > > --- a/drivers/dma-buf/reservation.c > > +++ b/drivers/dma-buf/reservation.c > > @@ -373,12 +373,25 @@ long reservation_object_wait_timeout_rcu(struct > reservation_object *obj, > > long ret = timeout ? timeout : 1; > > > > retry: > > - fence = NULL; > > shared_count = 0; > > seq = read_seqcount_begin(&obj->seq); > > rcu_read_lock(); > > > > - if (wait_all) { > > + fence = rcu_dereference(obj->fence_excl); > > + if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence- > >flags)) { > > + if (!dma_fence_get_rcu(fence)) > > + goto unlock_retry; > > + > > + if (dma_fence_is_signaled(fence)) { > > + dma_fence_put(fence); > > + fence = NULL; > > + } > > + > > + } else { > > + fence = NULL; > > + } > > + > > + if (!fence && wait_all) { > > struct reservation_object_list *fobj = > > rcu_dereference(obj- > >fence); > > > > @@ -405,22 +418,6 @@ long reservation_object_wait_timeout_rcu(struct > reservation_object *obj, > > } > > } > > > > - if (!shared_count) { > > - struct dma_fence *fence_excl = rcu_dereference(obj- > >fence_excl); > > - > > - if (fence_excl && > > - !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, > > - &fence_excl->flags)) { > > - if (!dma_fence_get_rcu(fence_excl)) > > - goto unlock_retry; > > - > > - if (dma_fence_is_signaled(fence_excl)) > > - dma_fence_put(fence_excl); > > - else > > - fence = fence_excl; > > - } > > - } > > - > > rcu_read_unlock(); > > if (fence) { > > if (read_seqcount_retry(&obj->seq, seq)) { >

9 years, 3 months

Jump to page:

2026

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig