From e1aaf311dbe82221910cc0e0809c988de210cc3c Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 5 Aug 2016 10:39:34 -0300 Subject: dma-buf/fence-array: add fence_is_array() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add helper to check if fence is array. v2: Comments from Chris Wilson - remove ternary if from ops comparison - add EXPORT_SYMBOL(fence_array_ops) Cc: Chris Wilson Cc: Christian König Signed-off-by: Gustavo Padovan Reviewed-by: Chris Wilson Reviewed-by: Christian König Signed-off-by: Sumit Semwal --- drivers/dma-buf/fence-array.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c index a8731c853da6..ee500226197b 100644 --- a/drivers/dma-buf/fence-array.c +++ b/drivers/dma-buf/fence-array.c @@ -99,6 +99,7 @@ const struct fence_ops fence_array_ops = { .wait = fence_default_wait, .release = fence_array_release, }; +EXPORT_SYMBOL(fence_array_ops); /** * fence_array_create - Create a custom fence array -- cgit v1.2.3 From a02b9dc90d844cc7df7b63264e7920cc425052d9 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 5 Aug 2016 10:39:35 -0300 Subject: dma-buf/sync_file: refactor fence storage in struct sync_file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create sync_file->fence to abstract the type of fence we are using for each sync_file. If only one fence is present we use a normal struct fence but if there is more fences to be added to the sync_file a fence_array is created. This change cleans up sync_file a bit. We don't need to have sync_file_cb array anymore. Instead, as we always have one fence, only one fence callback is registered per sync_file. v2: Comments from Chris Wilson and Christian König - Not using fence_ops anymore - fence_is_array() was created to differentiate fence from fence_array - fence_array_teardown() is now exported and used under fence_is_array() - struct sync_file lost num_fences member v3: Comments from Chris Wilson and Christian König - struct sync_file lost status member in favor of fence_is_signaled() - drop use of fence_array_teardown() - use sizeof(*fence) to allocate only an array on fence pointers v4: Comments from Chris Wilson - use sizeof(*fence) to reallocate array - fix typo in comments - protect num_fences sum against overflows - use array->base instead of casting the to struct fence v5: fixes checkpatch warnings v6: fix case where all fences are signaled. Signed-off-by: Gustavo Padovan Reviewed-by: Chris Wilson Acked-by: Christian König Acked-by: Greg Kroah-Hartman Signed-off-by: Sumit Semwal --- drivers/dma-buf/sync_file.c | 174 +++++++++++++++++++++++++++++--------------- 1 file changed, 115 insertions(+), 59 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 9aaa608dfe01..ac9c250af302 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -28,11 +28,11 @@ static const struct file_operations sync_file_fops; -static struct sync_file *sync_file_alloc(int size) +static struct sync_file *sync_file_alloc(void) { struct sync_file *sync_file; - sync_file = kzalloc(size, GFP_KERNEL); + sync_file = kzalloc(sizeof(*sync_file), GFP_KERNEL); if (!sync_file) return NULL; @@ -45,6 +45,8 @@ static struct sync_file *sync_file_alloc(int size) init_waitqueue_head(&sync_file->wq); + INIT_LIST_HEAD(&sync_file->cb.node); + return sync_file; err: @@ -54,14 +56,11 @@ err: static void fence_check_cb_func(struct fence *f, struct fence_cb *cb) { - struct sync_file_cb *check; struct sync_file *sync_file; - check = container_of(cb, struct sync_file_cb, cb); - sync_file = check->sync_file; + sync_file = container_of(cb, struct sync_file, cb); - if (atomic_dec_and_test(&sync_file->status)) - wake_up_all(&sync_file->wq); + wake_up_all(&sync_file->wq); } /** @@ -76,22 +75,18 @@ struct sync_file *sync_file_create(struct fence *fence) { struct sync_file *sync_file; - sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1])); + sync_file = sync_file_alloc(); if (!sync_file) return NULL; - sync_file->num_fences = 1; - atomic_set(&sync_file->status, 1); + sync_file->fence = fence; + snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d", fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), fence->context, fence->seqno); - sync_file->cbs[0].fence = fence; - sync_file->cbs[0].sync_file = sync_file; - if (fence_add_callback(fence, &sync_file->cbs[0].cb, - fence_check_cb_func)) - atomic_dec(&sync_file->status); + fence_add_callback(fence, &sync_file->cb, fence_check_cb_func); return sync_file; } @@ -121,14 +116,49 @@ err: return NULL; } -static void sync_file_add_pt(struct sync_file *sync_file, int *i, - struct fence *fence) +static int sync_file_set_fence(struct sync_file *sync_file, + struct fence **fences, int num_fences) { - sync_file->cbs[*i].fence = fence; - sync_file->cbs[*i].sync_file = sync_file; + struct fence_array *array; + + /* + * The reference for the fences in the new sync_file and held + * in add_fence() during the merge procedure, so for num_fences == 1 + * we already own a new reference to the fence. For num_fence > 1 + * we own the reference of the fence_array creation. + */ + if (num_fences == 1) { + sync_file->fence = fences[0]; + } else { + array = fence_array_create(num_fences, fences, + fence_context_alloc(1), 1, false); + if (!array) + return -ENOMEM; + + sync_file->fence = &array->base; + } - if (!fence_add_callback(fence, &sync_file->cbs[*i].cb, - fence_check_cb_func)) { + return 0; +} + +static struct fence **get_fences(struct sync_file *sync_file, int *num_fences) +{ + if (fence_is_array(sync_file->fence)) { + struct fence_array *array = to_fence_array(sync_file->fence); + + *num_fences = array->num_fences; + return array->fences; + } + + *num_fences = 1; + return &sync_file->fence; +} + +static void add_fence(struct fence **fences, int *i, struct fence *fence) +{ + fences[*i] = fence; + + if (!fence_is_signaled(fence)) { fence_get(fence); (*i)++; } @@ -147,16 +177,24 @@ static void sync_file_add_pt(struct sync_file *sync_file, int *i, static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, struct sync_file *b) { - int num_fences = a->num_fences + b->num_fences; struct sync_file *sync_file; - int i, i_a, i_b; - unsigned long size = offsetof(struct sync_file, cbs[num_fences]); + struct fence **fences, **nfences, **a_fences, **b_fences; + int i, i_a, i_b, num_fences, a_num_fences, b_num_fences; - sync_file = sync_file_alloc(size); + sync_file = sync_file_alloc(); if (!sync_file) return NULL; - atomic_set(&sync_file->status, num_fences); + 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; + + num_fences = a_num_fences + b_num_fences; + + fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); + if (!fences) + goto err; /* * Assume sync_file a and b are both ordered and have no @@ -165,55 +203,73 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, * If a sync_file can only be created with sync_file_merge * and sync_file_create, this is a reasonable assumption. */ - for (i = i_a = i_b = 0; i_a < a->num_fences && i_b < b->num_fences; ) { - struct fence *pt_a = a->cbs[i_a].fence; - struct fence *pt_b = b->cbs[i_b].fence; + for (i = i_a = i_b = 0; i_a < a_num_fences && i_b < b_num_fences; ) { + struct fence *pt_a = a_fences[i_a]; + struct fence *pt_b = b_fences[i_b]; if (pt_a->context < pt_b->context) { - sync_file_add_pt(sync_file, &i, pt_a); + add_fence(fences, &i, pt_a); i_a++; } else if (pt_a->context > pt_b->context) { - sync_file_add_pt(sync_file, &i, pt_b); + add_fence(fences, &i, pt_b); i_b++; } else { if (pt_a->seqno - pt_b->seqno <= INT_MAX) - sync_file_add_pt(sync_file, &i, pt_a); + add_fence(fences, &i, pt_a); else - sync_file_add_pt(sync_file, &i, pt_b); + add_fence(fences, &i, pt_b); i_a++; i_b++; } } - for (; i_a < a->num_fences; i_a++) - sync_file_add_pt(sync_file, &i, a->cbs[i_a].fence); + for (; i_a < a_num_fences; i_a++) + add_fence(fences, &i, a_fences[i_a]); + + for (; i_b < b_num_fences; i_b++) + add_fence(fences, &i, b_fences[i_b]); + + if (i == 0) { + add_fence(fences, &i, a_fences[0]); + i++; + } - for (; i_b < b->num_fences; i_b++) - sync_file_add_pt(sync_file, &i, b->cbs[i_b].fence); + if (num_fences > i) { + nfences = krealloc(fences, i * sizeof(*fences), + GFP_KERNEL); + if (!nfences) + goto err; + + fences = nfences; + } + + if (sync_file_set_fence(sync_file, fences, i) < 0) { + kfree(fences); + goto err; + } - if (num_fences > i) - atomic_sub(num_fences - i, &sync_file->status); - sync_file->num_fences = i; + fence_add_callback(sync_file->fence, &sync_file->cb, + fence_check_cb_func); strlcpy(sync_file->name, name, sizeof(sync_file->name)); return sync_file; + +err: + fput(sync_file->file); + return NULL; + } static void sync_file_free(struct kref *kref) { struct sync_file *sync_file = container_of(kref, struct sync_file, kref); - int i; - - for (i = 0; i < sync_file->num_fences; ++i) { - fence_remove_callback(sync_file->cbs[i].fence, - &sync_file->cbs[i].cb); - fence_put(sync_file->cbs[i].fence); - } + fence_remove_callback(sync_file->fence, &sync_file->cb); + fence_put(sync_file->fence); kfree(sync_file); } @@ -232,9 +288,9 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait) poll_wait(file, &sync_file->wq, wait); - status = atomic_read(&sync_file->status); + status = fence_is_signaled(sync_file->fence); - if (!status) + if (status) return POLLIN; if (status < 0) return POLLERR; @@ -315,8 +371,9 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, { struct sync_file_info info; struct sync_fence_info *fence_info = NULL; + struct fence **fences; __u32 size; - int ret, i; + int num_fences, ret, i; if (copy_from_user(&info, (void __user *)arg, sizeof(info))) return -EFAULT; @@ -324,6 +381,8 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (info.flags || info.pad) return -EINVAL; + fences = get_fences(sync_file, &num_fences); + /* * Passing num_fences = 0 means that userspace doesn't want to * retrieve any sync_fence_info. If num_fences = 0 we skip filling @@ -333,16 +392,16 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (!info.num_fences) goto no_fences; - if (info.num_fences < sync_file->num_fences) + if (info.num_fences < num_fences) return -EINVAL; - size = sync_file->num_fences * sizeof(*fence_info); + size = num_fences * sizeof(*fence_info); fence_info = kzalloc(size, GFP_KERNEL); if (!fence_info) return -ENOMEM; - for (i = 0; i < sync_file->num_fences; ++i) - sync_fill_fence_info(sync_file->cbs[i].fence, &fence_info[i]); + for (i = 0; i < num_fences; i++) + sync_fill_fence_info(fences[i], &fence_info[i]); if (copy_to_user(u64_to_user_ptr(info.sync_fence_info), fence_info, size)) { @@ -352,11 +411,8 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, no_fences: strlcpy(info.name, sync_file->name, sizeof(info.name)); - info.status = atomic_read(&sync_file->status); - if (info.status >= 0) - info.status = !info.status; - - info.num_fences = sync_file->num_fences; + info.status = fence_is_signaled(sync_file->fence); + info.num_fences = num_fences; if (copy_to_user((void __user *)arg, &info, sizeof(info))) ret = -EFAULT; -- cgit v1.2.3 From 972526a4093243fdaf77dd7c6f8b11fba5b15864 Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 5 Aug 2016 10:39:36 -0300 Subject: dma-buf/sync_file: add sync_file_get_fence() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creates a function that given an sync file descriptor returns a fence containing all fences in the sync_file. v2: Comments by Daniel Vetter - Adapt to new version of fence_collection_init() - Hold a reference for the fence we return v3: - Adapt to use fput() directly - rename to sync_file_get_fence() as we always return one fence v4: Adapt to use fence_array v5: set fence through fence_get() Signed-off-by: Gustavo Padovan Reviewed-by: Chris Wilson Acked-by: Christian König Signed-off-by: Sumit Semwal --- drivers/dma-buf/sync_file.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index ac9c250af302..2873760e02a9 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -116,6 +116,29 @@ err: return NULL; } +/** + * sync_file_get_fence - get the fence related to the sync_file fd + * @fd: sync_file fd to get the fence from + * + * Ensures @fd references a valid sync_file and returns a fence that + * represents all fence in the sync_file. On error NULL is returned. + */ +struct fence *sync_file_get_fence(int fd) +{ + struct sync_file *sync_file; + struct fence *fence; + + sync_file = sync_file_fdget(fd); + if (!sync_file) + return NULL; + + fence = fence_get(sync_file->fence); + fput(sync_file->file); + + return fence; +} +EXPORT_SYMBOL(sync_file_get_fence); + static int sync_file_set_fence(struct sync_file *sync_file, struct fence **fences, int num_fences) { -- cgit v1.2.3 From e24165537312723e2900831dd6e7415b8d85278c Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Fri, 5 Aug 2016 10:39:38 -0300 Subject: dma-buf/sync_file: only enable fence signalling on poll() Signalling doesn't need to be enabled at sync_file creation, it is only required if userspace waiting the fence to signal through poll(). Thus we delay fence_add_callback() until poll is called. It only adds the callback the first time poll() is called. This avoid re-adding the same callback multiple times. v2: rebase and update to work with new fence support for sync_file v3: use atomic operation to set enabled and protect fence_add_callback() v4: use user bit from fence flags (comment from Chris Wilson) v5: use ternary if on poll return (comment from Chris Wilson) Signed-off-by: Gustavo Padovan Reviewed-by: Chris Wilson Signed-off-by: Sumit Semwal [sumits: remove unused var status] Link: http://patchwork.freedesktop.org/patch/msgid/1470404378-27961-1-git-send-email-gustavo@padovan.org --- drivers/dma-buf/sync_file.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 2873760e02a9..486d29c1a830 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -86,8 +86,6 @@ struct sync_file *sync_file_create(struct fence *fence) fence->ops->get_timeline_name(fence), fence->context, fence->seqno); - fence_add_callback(fence, &sync_file->cb, fence_check_cb_func); - return sync_file; } EXPORT_SYMBOL(sync_file_create); @@ -274,9 +272,6 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, goto err; } - fence_add_callback(sync_file->fence, &sync_file->cb, - fence_check_cb_func); - strlcpy(sync_file->name, name, sizeof(sync_file->name)); return sync_file; @@ -291,7 +286,8 @@ static void sync_file_free(struct kref *kref) struct sync_file *sync_file = container_of(kref, struct sync_file, kref); - fence_remove_callback(sync_file->fence, &sync_file->cb); + if (test_bit(POLL_ENABLED, &sync_file->fence->flags)) + fence_remove_callback(sync_file->fence, &sync_file->cb); fence_put(sync_file->fence); kfree(sync_file); } @@ -307,17 +303,16 @@ static int sync_file_release(struct inode *inode, struct file *file) static unsigned int sync_file_poll(struct file *file, poll_table *wait) { struct sync_file *sync_file = file->private_data; - int status; poll_wait(file, &sync_file->wq, wait); - status = fence_is_signaled(sync_file->fence); + if (!test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) { + if (fence_add_callback(sync_file->fence, &sync_file->cb, + fence_check_cb_func) < 0) + wake_up_all(&sync_file->wq); + } - if (status) - return POLLIN; - if (status < 0) - return POLLERR; - return 0; + return fence_is_signaled(sync_file->fence) ? POLLIN : 0; } static long sync_file_ioctl_merge(struct sync_file *sync_file, -- cgit v1.2.3 From ae4e46b14bd7a12fb7908425846be7ceb0853bbc Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 15 Aug 2016 16:42:18 +0100 Subject: dma-buf: Wait on the reservation object when sync'ing before CPU access Rendering operations to the dma-buf are tracked implicitly via the reservation_object (dmabuf->resv). This is used to allow poll() to wait upon outstanding rendering (or just query the current status of rendering). The dma-buf sync ioctl allows userspace to prepare the dma-buf for CPU access, which should include waiting upon rendering. (Some drivers may need to do more work to ensure that the dma-buf mmap is coherent as well as complete.) v2: Always wait upon the reservation object implicitly. We choose to do it after the native handler in case it can do so more efficiently. Testcase: igt/prime_vgem Testcase: igt/gem_concurrent_blit # *vgem* Signed-off-by: Chris Wilson Cc: Sumit Semwal Cc: Daniel Vetter Cc: Eric Anholt Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Cc: linux-kernel@vger.kernel.org Reviewed-by: Daniel Vetter Signed-off-by: Sumit Semwal Link: http://patchwork.freedesktop.org/patch/msgid/1471275738-31994-1-git-send-email-chris@chris-wilson.co.uk --- drivers/dma-buf/dma-buf.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index ddaee60ae52a..cf04d249a6a4 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -586,6 +586,22 @@ void dma_buf_unmap_attachment(struct dma_buf_attachment *attach, } EXPORT_SYMBOL_GPL(dma_buf_unmap_attachment); +static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf, + enum dma_data_direction direction) +{ + bool write = (direction == DMA_BIDIRECTIONAL || + direction == DMA_TO_DEVICE); + struct reservation_object *resv = dmabuf->resv; + long ret; + + /* Wait on any implicit rendering fences */ + ret = reservation_object_wait_timeout_rcu(resv, write, true, + MAX_SCHEDULE_TIMEOUT); + if (ret < 0) + return ret; + + return 0; +} /** * dma_buf_begin_cpu_access - Must be called before accessing a dma_buf from the @@ -608,6 +624,13 @@ int dma_buf_begin_cpu_access(struct dma_buf *dmabuf, if (dmabuf->ops->begin_cpu_access) ret = dmabuf->ops->begin_cpu_access(dmabuf, direction); + /* Ensure that all fences are waited upon - but we first allow + * the native handler the chance to do so more efficiently if it + * chooses. A double invocation here will be reasonably cheap no-op. + */ + if (ret == 0) + ret = __dma_buf_begin_cpu_access(dmabuf, direction); + return ret; } EXPORT_SYMBOL_GPL(dma_buf_begin_cpu_access); -- cgit v1.2.3 From 68acb6afb6f56d8ab92352993425b5472cf79a78 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Tue, 16 Aug 2016 16:31:00 -0700 Subject: dma-buf: fix kernel-doc warning and typos Fix dma-buf kernel-doc warning and 2 minor typos in fence_array_create(). Fixes this warning: ..//drivers/dma-buf/fence-array.c:124: warning: No description found for parameter 'signal_on_any' Signed-off-by: Randy Dunlap Cc: Sumit Semwal Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Signed-off-by: Sumit Semwal Link: http://patchwork.freedesktop.org/patch/msgid/224865a5-947d-9a28-c60a-18fa86bc9329@infradead.org --- drivers/dma-buf/fence-array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/fence-array.c b/drivers/dma-buf/fence-array.c index ee500226197b..f1989fcaf354 100644 --- a/drivers/dma-buf/fence-array.c +++ b/drivers/dma-buf/fence-array.c @@ -107,14 +107,14 @@ EXPORT_SYMBOL(fence_array_ops); * @fences: [in] array containing the fences * @context: [in] fence context to use * @seqno: [in] sequence number to use - * @signal_on_any [in] signal on any fence in the array + * @signal_on_any: [in] signal on any fence in the array * * Allocate a fence_array object and initialize the base fence with fence_init(). * In case of error it returns NULL. * - * The caller should allocte the fences array with num_fences size + * The caller should allocate the fences array with num_fences size * and fill it with the fences it wants to add to the object. Ownership of this - * array is take and fence_put() is used on each fence on release. + * array is taken and fence_put() is used on each fence on release. * * If @signal_on_any is true the fence array signals if any fence in the array * signals, otherwise it signals when all fences in the array signal. -- cgit v1.2.3 From f5bef0b85e5d1586bb2f34035917d5e4c475cea2 Mon Sep 17 00:00:00 2001 From: Rob Clark Date: Fri, 19 Aug 2016 16:55:34 -0400 Subject: reservation: fix small comment typo Signed-off-by: Rob Clark [danvet: Mark up as function for proper cross-linking.] Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1471640134-30888-1-git-send-email-robdclark@gmail.com --- 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 9566a62ad8e3..723d8af988e5 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -205,7 +205,7 @@ done: * @fence: the shared fence to add * * Add a fence to a shared slot, obj->lock must be held, and - * reservation_object_reserve_shared_fence has been called. + * reservation_object_reserve_shared() has been called. */ void reservation_object_add_shared_fence(struct reservation_object *obj, struct fence *fence) -- cgit v1.2.3 From ecebca79f6976ddaddfd054d699272515869ea28 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 29 Aug 2016 19:16:13 +0100 Subject: dma-buf/sync-file: Avoid enable fence signaling if poll(.timeout=0) If we being polled with a timeout of zero, a nonblocking busy query, we don't need to install any fence callbacks as we will not be waiting. As we only install the callback once, the overhead comes from the atomic bit test that also causes serialisation between threads. Signed-off-by: Chris Wilson Cc: Sumit Semwal Cc: Gustavo Padovan Cc: linux-media@vger.kernel.org Cc: dri-devel@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org Reviewed-by: Gustavo Padovan Signed-off-by: Sumit Semwal Link: http://patchwork.freedesktop.org/patch/msgid/20160829181613.30722-1-chris@chris-wilson.co.uk --- drivers/dma-buf/sync_file.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 486d29c1a830..abb5fdab75fd 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -306,7 +306,8 @@ static unsigned int sync_file_poll(struct file *file, poll_table *wait) poll_wait(file, &sync_file->wq, wait); - if (!test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) { + if (!poll_does_not_wait(wait) && + !test_and_set_bit(POLL_ENABLED, &sync_file->fence->flags)) { if (fence_add_callback(sync_file->fence, &sync_file->cb, fence_check_cb_func) < 0) wake_up_all(&sync_file->wq); -- cgit v1.2.3 From 7cec540a724654b6b046200c117bb98ea1bc5d04 Mon Sep 17 00:00:00 2001 From: Rafael Antognolli Date: Thu, 15 Sep 2016 12:14:25 -0700 Subject: dma-buf/sync_file: Increment refcount of fence when all are signaled. When we merge several fences, if all of them are signaled already, we still keep one of them. So instead of using add_fence(), which will not increase the refcount of signaled fences, we should explicitly call fence_get() for the fence we are keeping. This patch fixes a kernel panic that can be triggered by creating a fence that is expired (or increasing the timeline until it expires), then creating a merged fence out of it, and deleting the merged fence. This will make the original expired fence's refcount go to zero. Testcase: igt/sw_sync/sync_expired_merge Signed-off-by: Rafael Antognolli Reviewed-by: Chris Wilson Reviewed-by: Gustavo Padovan Signed-off-by: Sumit Semwal Link: http://patchwork.freedesktop.org/patch/msgid/1473966865-4508-1-git-send-email-rafael.antognolli@intel.com --- drivers/dma-buf/sync_file.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index abb5fdab75fd..0fe7ec2657fd 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -253,10 +253,8 @@ static struct sync_file *sync_file_merge(const char *name, struct sync_file *a, for (; i_b < b_num_fences; i_b++) add_fence(fences, &i, b_fences[i_b]); - if (i == 0) { - add_fence(fences, &i, a_fences[0]); - i++; - } + if (i == 0) + fences[i++] = fence_get(a_fences[0]); if (num_fences > i) { nfences = krealloc(fences, i * sizeof(*fences), -- cgit v1.2.3 From c654dd07522e88bacc546c9ccd81b3ee72838ccf Mon Sep 17 00:00:00 2001 From: Gustavo Padovan Date: Wed, 21 Sep 2016 10:20:19 +0300 Subject: dma-buf/sync_file: free fences array in num_fences is 1 When merging sync_files there is a case when we can end up with only one fence in the merged sync_file: when all fences belong to the same timeline. So for this case a fence_array is not created instead we just assigned the fence to sync_file->fence. Then we do not use the fences array anymore nor does free it. This patch frees the array. Reported-by: Chris Wilson Reviewed-by: Chris Wilson Signed-off-by: Gustavo Padovan Signed-off-by: Sean Paul Link: http://patchwork.freedesktop.org/patch/msgid/1474442419-6040-1-git-send-email-gustavo@padovan.org --- drivers/dma-buf/sync_file.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/dma-buf') diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c index 0fe7ec2657fd..b29a9e817320 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -150,6 +150,7 @@ static int sync_file_set_fence(struct sync_file *sync_file, */ if (num_fences == 1) { sync_file->fence = fences[0]; + kfree(fences); } else { array = fence_array_create(num_fences, fences, fence_context_alloc(1), 1, false); -- cgit v1.2.3