From c138782d895d52aa3831aa8c468b33d68fa5c954 Mon Sep 17 00:00:00 2001 From: Liviu Dudau Date: Wed, 1 Nov 2017 14:06:30 +0000 Subject: dma-buf: Cleanup comments on dma_buf_map_attachment() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mappings need to be unmapped by calling dma_buf_unmap_attachment() and not by calling again dma_buf_map_attachment(). Also fix some spelling mistakes. Signed-off-by: Liviu Dudau Reviewed-by: Alex Deucher Reviewed-by: Christian König Signed-off-by: Gustavo Padovan Link: https://patchwork.freedesktop.org/patch/msgid/20171101140630.2884-1-Liviu.Dudau@arm.com --- drivers/dma-buf/dma-buf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index bc1cb284111c..1792385405f0 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -351,13 +351,13 @@ static inline int is_dma_buf_file(struct file *file) * * 2. Userspace passes this file-descriptors to all drivers it wants this buffer * to share with: First the filedescriptor is converted to a &dma_buf using - * dma_buf_get(). The the buffer is attached to the device using + * dma_buf_get(). Then the buffer is attached to the device using * dma_buf_attach(). * * Up to this stage the exporter is still free to migrate or reallocate the * backing storage. * - * 3. Once the buffer is attached to all devices userspace can inniate DMA + * 3. Once the buffer is attached to all devices userspace can initiate DMA * access to the shared buffer. In the kernel this is done by calling * dma_buf_map_attachment() and dma_buf_unmap_attachment(). * @@ -617,7 +617,7 @@ EXPORT_SYMBOL_GPL(dma_buf_detach); * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR * on error. May return -EINTR if it is interrupted by a signal. * - * A mapping must be unmapped again using dma_buf_map_attachment(). Note that + * A mapping must be unmapped by using dma_buf_unmap_attachment(). Note that * the underlying backing storage is pinned for as long as a mapping exists, * therefore users/importers should not hold onto a mapping for undue amounts of * time. -- cgit v1.2.3 From ad46d7b89371c5920abdcad1ed04fbae68c9c8a9 Mon Sep 17 00:00:00 2001 From: Ville Syrjälä Date: Thu, 2 Nov 2017 22:03:36 +0200 Subject: dma-buf: Use rcu_assign_pointer() to set rcu protected pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use rcu_assign_pointer() when setting an rcu protected pointer. This gets rid of another sparse warning. Cc: Dave Airlie Cc: Jason Ekstrand Cc: linaro-mm-sig@lists.linaro.org Cc: linux-media@vger.kernel.org Cc: Alex Deucher Cc: Christian König Cc: Sumit Semwal Cc: Chris Wilson Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20171102200336.23347-5-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter Reviewed-by: Christian König . Acked-by: Sumit Semwal --- drivers/dma-buf/reservation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b44d9d7db347..d90333e0b6d5 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -318,7 +318,7 @@ retry: continue; } - dst_list->shared[dst_list->shared_count++] = fence; + rcu_assign_pointer(dst_list->shared[dst_list->shared_count++], fence); } } else { dst_list = NULL; -- cgit v1.2.3 From 4d9c62e8ce69d0b0a834282a34bff5ce8eeacb1d Mon Sep 17 00:00:00 2001 From: Christian König Date: Tue, 14 Nov 2017 15:24:35 +0100 Subject: dma-buf: keep only not signaled fence in reservation_object_add_shared_replace v3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The amdgpu issue to also need signaled fences in the reservation objects should be fixed by now. Optimize the list by keeping only the not signaled yet fences around. v2: temporary put the signaled fences at the end of the new container v3: put the old fence at the end of the new container as well. Signed-off-by: Christian König Reviewed-by: Chris Wilson Tested-by: Chris Wilson Signed-off-by: Alex Deucher Link: https://patchwork.freedesktop.org/patch/msgid/20171114142436.1360-1-christian.koenig@amd.com --- drivers/dma-buf/reservation.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index d90333e0b6d5..737885e79b74 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -145,8 +145,7 @@ reservation_object_add_shared_replace(struct reservation_object *obj, struct reservation_object_list *fobj, struct dma_fence *fence) { - unsigned i; - struct dma_fence *old_fence = NULL; + unsigned i, j, k; dma_fence_get(fence); @@ -162,24 +161,21 @@ reservation_object_add_shared_replace(struct reservation_object *obj, * references from the old struct are carried over to * the new. */ - fobj->shared_count = old->shared_count; - - for (i = 0; i < old->shared_count; ++i) { + for (i = 0, j = 0, k = fobj->shared_max; i < old->shared_count; ++i) { struct dma_fence *check; check = rcu_dereference_protected(old->shared[i], reservation_object_held(obj)); - if (!old_fence && check->context == fence->context) { - old_fence = check; - RCU_INIT_POINTER(fobj->shared[i], fence); - } else - RCU_INIT_POINTER(fobj->shared[i], check); - } - if (!old_fence) { - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); - fobj->shared_count++; + if (check->context == fence->context || + dma_fence_is_signaled(check)) + RCU_INIT_POINTER(fobj->shared[--k], check); + else + RCU_INIT_POINTER(fobj->shared[j++], check); } + fobj->shared_count = j; + RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); + fobj->shared_count++; done: preempt_disable(); @@ -192,10 +188,18 @@ done: write_seqcount_end(&obj->seq); preempt_enable(); - if (old) - kfree_rcu(old, rcu); + if (!old) + return; - dma_fence_put(old_fence); + /* Drop the references to the signaled fences */ + for (i = k; i < fobj->shared_max; ++i) { + struct dma_fence *f; + + f = rcu_dereference_protected(fobj->shared[i], + reservation_object_held(obj)); + dma_fence_put(f); + } + kfree_rcu(old, rcu); } /** -- cgit v1.2.3 From ca25fe5efe4ab43cc5b4f3117a205c281805a5ca Mon Sep 17 00:00:00 2001 From: Christian König Date: Tue, 14 Nov 2017 15:24:36 +0100 Subject: dma-buf: try to replace a signaled fence in reservation_object_add_shared_inplace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The amdgpu issue to also need signaled fences in the reservation objects should be fixed by now. Optimize the handling by replacing a signaled fence when adding a new shared one. Signed-off-by: Christian König Reviewed-by: Chris Wilson Signed-off-by: Alex Deucher Link: https://patchwork.freedesktop.org/patch/msgid/20171114142436.1360-2-christian.koenig@amd.com --- drivers/dma-buf/reservation.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index 737885e79b74..b759a569b7b8 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -104,7 +104,8 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, struct reservation_object_list *fobj, struct dma_fence *fence) { - u32 i; + struct dma_fence *signaled = NULL; + u32 i, signaled_idx; dma_fence_get(fence); @@ -126,17 +127,28 @@ reservation_object_add_shared_inplace(struct reservation_object *obj, dma_fence_put(old_fence); return; } + + if (!signaled && dma_fence_is_signaled(old_fence)) { + signaled = old_fence; + signaled_idx = i; + } } /* * memory barrier is added by write_seqcount_begin, * fobj->shared_count is protected by this lock too */ - RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); - fobj->shared_count++; + if (signaled) { + RCU_INIT_POINTER(fobj->shared[signaled_idx], fence); + } else { + RCU_INIT_POINTER(fobj->shared[fobj->shared_count], fence); + fobj->shared_count++; + } write_seqcount_end(&obj->seq); preempt_enable(); + + dma_fence_put(signaled); } static void -- cgit v1.2.3 From 03e4e0a9e02cf703da331ff6cfd57d0be9bf5692 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 14 Nov 2017 16:27:19 +0000 Subject: dma-buf/fence: Fix lock inversion within dma-fence-array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ages ago Rob Clark noted, "Currently with fence-array, we have a potential deadlock situation. If we fence_add_callback() on an array-fence, the array-fence's lock is acquired first, and in it's ->enable_signaling() callback, it will install cbs on it's array-member fences, so the array-member's lock is acquired second. But in the signal path, the array-member's lock is acquired first, and the array-fence's lock acquired second." Rob proposed either extensive changes to dma-fence to unnest the fence-array signaling, or to defer the signaling onto a workqueue. This is a more refined version of the later, that should keep the latency of the fence signaling to a minimum by using an irq-work, which is executed asap. Reported-by: Rob Clark Suggested-by: Rob Clark References: 1476635975-21981-1-git-send-email-robdclark@gmail.com Signed-off-by: Chris Wilson Cc: Rob Clark Cc: Gustavo Padovan Cc: Sumit Semwal Cc: Christian König Reviewed-by: Christian König Signed-off-by: Sumit Semwal Link: https://patchwork.freedesktop.org/patch/msgid/20171114162719.30958-1-chris@chris-wilson.co.uk --- drivers/base/Kconfig | 1 + drivers/dma-buf/dma-fence-array.c | 14 ++++++++++++-- include/linux/dma-fence-array.h | 3 +++ 3 files changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 1a5f6a157a57..62b0de06836e 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -244,6 +244,7 @@ config DMA_SHARED_BUFFER bool default n select ANON_INODES + select IRQ_WORK help This option enables the framework for buffer-sharing between multiple drivers. A buffer is associated with a file using driver diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 0350829ba62e..dd1edfb27b61 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -31,6 +31,14 @@ static const char *dma_fence_array_get_timeline_name(struct dma_fence *fence) return "unbound"; } +static void irq_dma_fence_array_work(struct irq_work *wrk) +{ + struct dma_fence_array *array = container_of(wrk, typeof(*array), work); + + dma_fence_signal(&array->base); + dma_fence_put(&array->base); +} + static void dma_fence_array_cb_func(struct dma_fence *f, struct dma_fence_cb *cb) { @@ -39,8 +47,9 @@ static void dma_fence_array_cb_func(struct dma_fence *f, struct dma_fence_array *array = array_cb->array; if (atomic_dec_and_test(&array->num_pending)) - dma_fence_signal(&array->base); - dma_fence_put(&array->base); + irq_work_queue(&array->work); + else + dma_fence_put(&array->base); } static bool dma_fence_array_enable_signaling(struct dma_fence *fence) @@ -136,6 +145,7 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, spin_lock_init(&array->lock); dma_fence_init(&array->base, &dma_fence_array_ops, &array->lock, context, seqno); + init_irq_work(&array->work, irq_dma_fence_array_work); array->num_fences = num_fences; atomic_set(&array->num_pending, signal_on_any ? 1 : num_fences); diff --git a/include/linux/dma-fence-array.h b/include/linux/dma-fence-array.h index 332a5420243c..bc8940ca280d 100644 --- a/include/linux/dma-fence-array.h +++ b/include/linux/dma-fence-array.h @@ -21,6 +21,7 @@ #define __LINUX_DMA_FENCE_ARRAY_H #include +#include /** * struct dma_fence_array_cb - callback helper for fence array @@ -47,6 +48,8 @@ struct dma_fence_array { unsigned num_fences; atomic_t num_pending; struct dma_fence **fences; + + struct irq_work work; }; extern const struct dma_fence_ops dma_fence_array_ops; -- cgit v1.2.3 From 298b6a815bba69c12336114c546104ba9d25bc8f Mon Sep 17 00:00:00 2001 From: Vasyl Gomonovych Date: Wed, 22 Nov 2017 16:22:41 +0100 Subject: dma-buf: Fix ifnullfree.cocci warnings 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 Signed-off-by: Vasyl Gomonovych Signed-off-by: Sumit Semwal Link: https://patchwork.freedesktop.org/patch/msgid/1511364161-6074-1-git-send-email-gomonovych@gmail.com --- drivers/dma-buf/dma-buf.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 1792385405f0..058805b6d164 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) -- cgit v1.2.3 From 5bffee867df7494ecd32c1e6ec4e8fc934c521b7 Mon Sep 17 00:00:00 2001 From: Christian König Date: Mon, 22 Jan 2018 21:00:03 +0100 Subject: dma-buf: fix reservation_object_wait_timeout_rcu once more v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to set shared_count even if we already have a fence to wait for. v2: init i to -1 as well Signed-off-by: Christian König Cc: stable@vger.kernel.org Tested-by: Lyude Paul Reviewed-by: Lyude Paul Reviewed-by: Chris Wilson Signed-off-by: Alex Deucher Link: https://patchwork.freedesktop.org/patch/msgid/20180122200003.6665-1-christian.koenig@amd.com --- drivers/dma-buf/reservation.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index b759a569b7b8..04ebe2204c12 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -471,13 +471,15 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, unsigned long timeout) { struct dma_fence *fence; - unsigned seq, shared_count, i = 0; + unsigned seq, shared_count; long ret = timeout ? timeout : 1; + int i; retry: shared_count = 0; seq = read_seqcount_begin(&obj->seq); rcu_read_lock(); + i = -1; fence = rcu_dereference(obj->fence_excl); if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { @@ -493,14 +495,14 @@ retry: fence = NULL; } - if (!fence && wait_all) { + if (wait_all) { struct reservation_object_list *fobj = rcu_dereference(obj->fence); if (fobj) shared_count = fobj->shared_count; - for (i = 0; i < shared_count; ++i) { + for (i = 0; !fence && i < shared_count; ++i) { struct dma_fence *lfence = rcu_dereference(fobj->shared[i]); if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, -- cgit v1.2.3