From patchwork Sat Aug 10 15:34:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11088579 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D5F1514D5 for ; Sat, 10 Aug 2019 15:36:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C482B2228E for ; Sat, 10 Aug 2019 15:36:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B863026E1A; Sat, 10 Aug 2019 15:36:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 70D202228E for ; Sat, 10 Aug 2019 15:36:21 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BD8F96E42B; Sat, 10 Aug 2019 15:36:14 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 61DCB6E435; Sat, 10 Aug 2019 15:36:13 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 17954016-1500050 for multiple; Sat, 10 Aug 2019 16:34:32 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH 1/4] dma-fence: Propagate errors to dma-fence-array container Date: Sat, 10 Aug 2019 16:34:27 +0100 Message-Id: <20190810153430.30636-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.23.0.rc1 MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Gustavo Padovan , intel-gfx@lists.freedesktop.org, christian.koenig@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP When one of the array of fences is signaled, propagate its errors to the parent fence-array (keeping the first error to be raised). v2: Opencode cmpxchg_local to avoid compiler freakout. Signed-off-by: Chris Wilson Cc: Sumit Semwal Cc: Gustavo Padovan --- drivers/dma-buf/dma-fence-array.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 12c6f64c0bc2..d90675bb4fcc 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -13,6 +13,12 @@ #include #include +static void fence_set_error_once(struct dma_fence *fence, int error) +{ + if (!fence->error && error) + dma_fence_set_error(fence, error); +} + static const char *dma_fence_array_get_driver_name(struct dma_fence *fence) { return "dma_fence_array"; @@ -38,6 +44,13 @@ static void dma_fence_array_cb_func(struct dma_fence *f, container_of(cb, struct dma_fence_array_cb, cb); struct dma_fence_array *array = array_cb->array; + /* + * Propagate the first error reported by any of our fences, but only + * before we ourselves are signaled. + */ + if (atomic_read(&array->num_pending) > 0) + fence_set_error_once(&array->base, f->error); + if (atomic_dec_and_test(&array->num_pending)) irq_work_queue(&array->work); else @@ -63,6 +76,8 @@ static bool dma_fence_array_enable_signaling(struct dma_fence *fence) dma_fence_get(&array->base); if (dma_fence_add_callback(array->fences[i], &cb[i].cb, dma_fence_array_cb_func)) { + fence_set_error_once(&array->base, + array->fences[i]->error); dma_fence_put(&array->base); if (atomic_dec_and_test(&array->num_pending)) return false; From patchwork Sat Aug 10 15:34:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11088577 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6908A1399 for ; Sat, 10 Aug 2019 15:36:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 591912228E for ; Sat, 10 Aug 2019 15:36:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4D6FB26E1A; Sat, 10 Aug 2019 15:36:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id EBCFF2228E for ; Sat, 10 Aug 2019 15:36:19 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 42B486E437; Sat, 10 Aug 2019 15:36:14 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 75CBA6E42B; Sat, 10 Aug 2019 15:36:12 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 17954017-1500050 for multiple; Sat, 10 Aug 2019 16:34:32 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH 2/4] dma-fence: Report the composite sync_file status Date: Sat, 10 Aug 2019 16:34:28 +0100 Message-Id: <20190810153430.30636-2-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.23.0.rc1 In-Reply-To: <20190810153430.30636-1-chris@chris-wilson.co.uk> References: <20190810153430.30636-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Petri Latvala , Gustavo Padovan , intel-gfx@lists.freedesktop.org, christian.koenig@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Same as for the individual fences, we want to report the actual status of the fence when queried. Reported-by: Petri Latvala Signed-off-by: Chris Wilson Cc: Sumit Semwal Cc: Gustavo Padovan Cc: Petri Latvala Reviewed-by: Matthew Auld --- 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 ee4d1a96d779..25c5c071645b 100644 --- a/drivers/dma-buf/sync_file.c +++ b/drivers/dma-buf/sync_file.c @@ -419,7 +419,7 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, * info->num_fences. */ if (!info.num_fences) { - info.status = dma_fence_is_signaled(sync_file->fence); + info.status = dma_fence_get_status(sync_file->fence); goto no_fences; } else { info.status = 1; From patchwork Sat Aug 10 15:34:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11088583 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 813FD1399 for ; Sat, 10 Aug 2019 15:36:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6DDCB2228E for ; Sat, 10 Aug 2019 15:36:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 61ABF26E1A; Sat, 10 Aug 2019 15:36:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 044182228E for ; Sat, 10 Aug 2019 15:36:24 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 72CE56E441; Sat, 10 Aug 2019 15:36:15 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5D1226E42B; Sat, 10 Aug 2019 15:36:11 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 17954018-1500050 for multiple; Sat, 10 Aug 2019 16:34:33 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH 3/4] dma-fence: Refactor signaling for manual invocation Date: Sat, 10 Aug 2019 16:34:29 +0100 Message-Id: <20190810153430.30636-3-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.23.0.rc1 In-Reply-To: <20190810153430.30636-1-chris@chris-wilson.co.uk> References: <20190810153430.30636-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, Tvrtko Ursulin , christian.koenig@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Move the duplicated code within dma-fence.c into the header for wider reuse. In the process apply a small micro-optimisation to only prune the fence->cb_list once rather than use list_del on every entry. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/dma-buf/Makefile | 10 +- drivers/dma-buf/dma-fence-trace.c | 28 +++ drivers/dma-buf/dma-fence.c | 33 +-- drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 32 +-- include/linux/dma-fence-impl.h | 83 +++++++ include/linux/dma-fence-types.h | 258 ++++++++++++++++++++ include/linux/dma-fence.h | 228 +---------------- 7 files changed, 386 insertions(+), 286 deletions(-) create mode 100644 drivers/dma-buf/dma-fence-trace.c create mode 100644 include/linux/dma-fence-impl.h create mode 100644 include/linux/dma-fence-types.h diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index e8c7310cb800..65c43778e571 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,6 +1,12 @@ # SPDX-License-Identifier: GPL-2.0-only -obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \ - reservation.o seqno-fence.o +obj-y := \ + dma-buf.o \ + dma-fence.o \ + dma-fence-array.o \ + dma-fence-chain.o \ + dma-fence-trace.o \ + reservation.o \ + seqno-fence.o obj-$(CONFIG_SYNC_FILE) += sync_file.o obj-$(CONFIG_SW_SYNC) += sw_sync.o sync_debug.o obj-$(CONFIG_UDMABUF) += udmabuf.o diff --git a/drivers/dma-buf/dma-fence-trace.c b/drivers/dma-buf/dma-fence-trace.c new file mode 100644 index 000000000000..eb6f282be4c0 --- /dev/null +++ b/drivers/dma-buf/dma-fence-trace.c @@ -0,0 +1,28 @@ +/* + * Fence mechanism for dma-buf and to allow for asynchronous dma access + * + * Copyright (C) 2012 Canonical Ltd + * Copyright (C) 2012 Texas Instruments + * + * Authors: + * Rob Clark + * Maarten Lankhorst + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#include + +#define CREATE_TRACE_POINTS +#include + +EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal); +EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled); diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 59ac96ec7ba8..027a6a894abd 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -14,15 +14,9 @@ #include #include #include +#include #include -#define CREATE_TRACE_POINTS -#include - -EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit); -EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal); -EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled); - static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub; @@ -128,7 +122,6 @@ EXPORT_SYMBOL(dma_fence_context_alloc); */ int dma_fence_signal_locked(struct dma_fence *fence) { - struct dma_fence_cb *cur, *tmp; int ret = 0; lockdep_assert_held(fence->lock); @@ -136,7 +129,7 @@ int dma_fence_signal_locked(struct dma_fence *fence) if (WARN_ON(!fence)) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { + if (!__dma_fence_signal(fence)) { ret = -EINVAL; /* @@ -144,15 +137,10 @@ int dma_fence_signal_locked(struct dma_fence *fence) * still run through all callbacks */ } else { - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); - trace_dma_fence_signaled(fence); + __dma_fence_signal__timestamp(fence, ktime_get()); } - list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) { - list_del_init(&cur->node); - cur->func(fence, cur); - } + __dma_fence_signal__notify(fence); return ret; } EXPORT_SYMBOL(dma_fence_signal_locked); @@ -177,21 +165,14 @@ int dma_fence_signal(struct dma_fence *fence) if (!fence) return -EINVAL; - if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) + if (!__dma_fence_signal(fence)) return -EINVAL; - fence->timestamp = ktime_get(); - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); - trace_dma_fence_signaled(fence); + __dma_fence_signal__timestamp(fence, ktime_get()); if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) { - struct dma_fence_cb *cur, *tmp; - spin_lock_irqsave(fence->lock, flags); - list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) { - list_del_init(&cur->node); - cur->func(fence, cur); - } + __dma_fence_signal__notify(fence); spin_unlock_irqrestore(fence->lock, flags); } return 0; diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c index e1bbc9b428cd..65de5c45a233 100644 --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c @@ -22,8 +22,7 @@ * */ -#include -#include +#include #include #include "i915_drv.h" @@ -98,35 +97,6 @@ check_signal_order(struct intel_context *ce, struct i915_request *rq) return true; } -static bool -__dma_fence_signal(struct dma_fence *fence) -{ - return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags); -} - -static void -__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp) -{ - fence->timestamp = timestamp; - set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); - trace_dma_fence_signaled(fence); -} - -static void -__dma_fence_signal__notify(struct dma_fence *fence) -{ - struct dma_fence_cb *cur, *tmp; - - lockdep_assert_held(fence->lock); - lockdep_assert_irqs_disabled(); - - list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) { - INIT_LIST_HEAD(&cur->node); - cur->func(fence, cur); - } - INIT_LIST_HEAD(&fence->cb_list); -} - void intel_engine_breadcrumbs_irq(struct intel_engine_cs *engine) { struct intel_breadcrumbs *b = &engine->breadcrumbs; diff --git a/include/linux/dma-fence-impl.h b/include/linux/dma-fence-impl.h new file mode 100644 index 000000000000..7372021cf082 --- /dev/null +++ b/include/linux/dma-fence-impl.h @@ -0,0 +1,83 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Fence mechanism for dma-buf to allow for asynchronous dma access + * + * Copyright (C) 2012 Canonical Ltd + * Copyright (C) 2012 Texas Instruments + * + * Authors: + * Rob Clark + * Maarten Lankhorst + */ + +#ifndef __LINUX_DMA_FENCE_IMPL_H +#define __LINUX_DMA_FENCE_IMPL_H + +#include +#include +#include +#include + +#include + +/** + * __dma_fence_signal: Mark a fence as signaled + * @fence: the dma fence to mark + * + * The first step of the dma_fence_signal() implementation is to atomically + * mark the fence as signaled. + * + * Returns: true if the fence was not previously signaled, false if it was + * already signaled. + */ +static inline bool +__dma_fence_signal(struct dma_fence *fence) +{ + return !test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags); +} + +/** + * __dma_fence_signal__timestamp: sets the signaling timestamp + * @fence: the dma fence + * @timestamp: the monotonic timestamp (e.g. ktime_get_mono()) + * + * The second step of the dma_fence_signal() implementation it to record + * the siganling timestamp. + * + * The dma-fence stores a timestamp of when it was signaled for inspection + * by userspace. This timestamp is typically the CPU time at which the + * signal was raised, but could be a HW timestamp generated by the event + * itself. Either way, it must be set on the signaled fence before + * callbacks are notified. + */ +static inline void +__dma_fence_signal__timestamp(struct dma_fence *fence, ktime_t timestamp) +{ + fence->timestamp = timestamp; + set_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &fence->flags); + trace_dma_fence_signaled(fence); +} + +/** + * __dma_fence_signal__notify: notify observers of the signal event + * @fence: the dma fence + * + * The final step of the dma_fence_signal() implementation is to notify + * all observers (dma_fence_add_callback()) of the signal event. This must + * be called with the fence->lock already held and irqsoff. + */ +static inline void +__dma_fence_signal__notify(struct dma_fence *fence) +{ + struct dma_fence_cb *cur, *tmp; + + lockdep_assert_held(fence->lock); + + list_for_each_entry_safe(cur, tmp, &fence->cb_list, node) { + INIT_LIST_HEAD(&cur->node); + cur->func(fence, cur); + } + INIT_LIST_HEAD(&fence->cb_list); +} + +#endif /* __LINUX_DMA_FENCE_IMPL_H */ diff --git a/include/linux/dma-fence-types.h b/include/linux/dma-fence-types.h new file mode 100644 index 000000000000..81e22d9ed174 --- /dev/null +++ b/include/linux/dma-fence-types.h @@ -0,0 +1,258 @@ +/* + * Fence mechanism for dma-buf to allow for asynchronous dma access + * + * Copyright (C) 2012 Canonical Ltd + * Copyright (C) 2012 Texas Instruments + * + * Authors: + * Rob Clark + * Maarten Lankhorst + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 as published by + * the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + */ + +#ifndef __LINUX_DMA_FENCE_TYPES_H +#define __LINUX_DMA_FENCE_TYPES_H + +#include +#include +#include +#include +#include +#include + +struct dma_fence; +struct dma_fence_ops; +struct dma_fence_cb; + +/** + * struct dma_fence - software synchronization primitive + * @refcount: refcount for this fence + * @ops: dma_fence_ops associated with this fence + * @rcu: used for releasing fence with kfree_rcu + * @cb_list: list of all callbacks to call + * @lock: spin_lock_irqsave used for locking + * @context: execution context this fence belongs to, returned by + * dma_fence_context_alloc() + * @seqno: the sequence number of this fence inside the execution context, + * can be compared to decide which fence would be signaled later. + * @flags: A mask of DMA_FENCE_FLAG_* defined below + * @timestamp: Timestamp when the fence was signaled. + * @error: Optional, only valid if < 0, must be set before calling + * dma_fence_signal, indicates that the fence has completed with an error. + * + * the flags member must be manipulated and read using the appropriate + * atomic ops (bit_*), so taking the spinlock will not be needed most + * of the time. + * + * DMA_FENCE_FLAG_SIGNALED_BIT - fence is already signaled + * DMA_FENCE_FLAG_TIMESTAMP_BIT - timestamp recorded for fence signaling + * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT - enable_signaling might have been called + * DMA_FENCE_FLAG_USER_BITS - start of the unused bits, can be used by the + * implementer of the fence for its own purposes. Can be used in different + * ways by different fence implementers, so do not rely on this. + * + * Since atomic bitops are used, this is not guaranteed to be the case. + * Particularly, if the bit was set, but dma_fence_signal was called right + * before this bit was set, it would have been able to set the + * DMA_FENCE_FLAG_SIGNALED_BIT, before enable_signaling was called. + * Adding a check for DMA_FENCE_FLAG_SIGNALED_BIT after setting + * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT closes this race, and makes sure that + * after dma_fence_signal was called, any enable_signaling call will have either + * been completed, or never called at all. + */ +struct dma_fence { + struct kref refcount; + const struct dma_fence_ops *ops; + /* We clear the callback list on kref_put so that by the time we + * release the fence it is unused. No one should be adding to the cb_list + * that they don't themselves hold a reference for. + */ + union { + struct rcu_head rcu; + struct list_head cb_list; + }; + spinlock_t *lock; + u64 context; + u64 seqno; + unsigned long flags; + ktime_t timestamp; + int error; +}; + +enum dma_fence_flag_bits { + DMA_FENCE_FLAG_SIGNALED_BIT, + DMA_FENCE_FLAG_TIMESTAMP_BIT, + DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, + DMA_FENCE_FLAG_USER_BITS, /* must always be last member */ +}; + +typedef void (*dma_fence_func_t)(struct dma_fence *fence, + struct dma_fence_cb *cb); + +/** + * struct dma_fence_cb - callback for dma_fence_add_callback() + * @node: used by dma_fence_add_callback() to append this struct to fence::cb_list + * @func: dma_fence_func_t to call + * + * This struct will be initialized by dma_fence_add_callback(), additional + * data can be passed along by embedding dma_fence_cb in another struct. + */ +struct dma_fence_cb { + struct list_head node; + dma_fence_func_t func; +}; + +/** + * struct dma_fence_ops - operations implemented for fence + * + */ +struct dma_fence_ops { + /** + * @use_64bit_seqno: + * + * True if this dma_fence implementation uses 64bit seqno, false + * otherwise. + */ + bool use_64bit_seqno; + + /** + * @get_driver_name: + * + * Returns the driver name. This is a callback to allow drivers to + * compute the name at runtime, without having it to store permanently + * for each fence, or build a cache of some sort. + * + * This callback is mandatory. + */ + const char * (*get_driver_name)(struct dma_fence *fence); + + /** + * @get_timeline_name: + * + * Return the name of the context this fence belongs to. This is a + * callback to allow drivers to compute the name at runtime, without + * having it to store permanently for each fence, or build a cache of + * some sort. + * + * This callback is mandatory. + */ + const char * (*get_timeline_name)(struct dma_fence *fence); + + /** + * @enable_signaling: + * + * Enable software signaling of fence. + * + * For fence implementations that have the capability for hw->hw + * signaling, they can implement this op to enable the necessary + * interrupts, or insert commands into cmdstream, etc, to avoid these + * costly operations for the common case where only hw->hw + * synchronization is required. This is called in the first + * dma_fence_wait() or dma_fence_add_callback() path to let the fence + * implementation know that there is another driver waiting on the + * signal (ie. hw->sw case). + * + * This function can be called from atomic context, but not + * from irq context, so normal spinlocks can be used. + * + * A return value of false indicates the fence already passed, + * or some failure occurred that made it impossible to enable + * signaling. True indicates successful enabling. + * + * &dma_fence.error may be set in enable_signaling, but only when false + * is returned. + * + * Since many implementations can call dma_fence_signal() even before + * @enable_signaling has been called there's a race window, where the + * dma_fence_signal() might result in the final fence reference being + * released and its memory freed. To avoid this, implementations of + * this callback should grab their own reference using dma_fence_get(), + * to be released when the fence is signalled (through e.g. the + * interrupt handler). + * + * This callback is optional. If this callback is not present, then the + * driver must always have signaling enabled. + */ + bool (*enable_signaling)(struct dma_fence *fence); + + /** + * @signaled: + * + * Peek whether the fence is signaled, as a fastpath optimization for + * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this + * callback does not need to make any guarantees beyond that a fence + * once indicates as signalled must always return true from this + * callback. This callback may return false even if the fence has + * completed already, in this case information hasn't propogated through + * the system yet. See also dma_fence_is_signaled(). + * + * May set &dma_fence.error if returning true. + * + * This callback is optional. + */ + bool (*signaled)(struct dma_fence *fence); + + /** + * @wait: + * + * Custom wait implementation, defaults to dma_fence_default_wait() if + * not set. + * + * The dma_fence_default_wait implementation should work for any fence, + * as long as @enable_signaling works correctly. This hook allows + * drivers to have an optimized version for the case where a process + * context is already available, e.g. if @enable_signaling for the + * general case needs to set up a worker thread. + * + * Must return -ERESTARTSYS if the wait is intr = true and the wait was + * interrupted, and remaining jiffies if fence has signaled, or 0 if + * wait timed out. Can also return other error values on custom + * implementations, which should be treated as if the fence is signaled. + * For example a hardware lockup could be reported like that. + * + * This callback is optional. + */ + signed long (*wait)(struct dma_fence *fence, + bool intr, signed long timeout); + + /** + * @release: + * + * Called on destruction of fence to release additional resources. + * Can be called from irq context. This callback is optional. If it is + * NULL, then dma_fence_free() is instead called as the default + * implementation. + */ + void (*release)(struct dma_fence *fence); + + /** + * @fence_value_str: + * + * Callback to fill in free-form debug info specific to this fence, like + * the sequence number. + * + * This callback is optional. + */ + void (*fence_value_str)(struct dma_fence *fence, char *str, int size); + + /** + * @timeline_value_str: + * + * Fills in the current value of the timeline as a string, like the + * sequence number. Note that the specific fence passed to this function + * should not matter, drivers should only use it to look up the + * corresponding timeline structures. + */ + void (*timeline_value_str)(struct dma_fence *fence, + char *str, int size); +}; + +#endif /* __LINUX_DMA_FENCE_TYPES_H */ diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index bea1d05cf51e..1c8dd1fbafae 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -13,6 +13,7 @@ #ifndef __LINUX_DMA_FENCE_H #define __LINUX_DMA_FENCE_H +#include #include #include #include @@ -22,233 +23,6 @@ #include #include -struct dma_fence; -struct dma_fence_ops; -struct dma_fence_cb; - -/** - * struct dma_fence - software synchronization primitive - * @refcount: refcount for this fence - * @ops: dma_fence_ops associated with this fence - * @rcu: used for releasing fence with kfree_rcu - * @cb_list: list of all callbacks to call - * @lock: spin_lock_irqsave used for locking - * @context: execution context this fence belongs to, returned by - * dma_fence_context_alloc() - * @seqno: the sequence number of this fence inside the execution context, - * can be compared to decide which fence would be signaled later. - * @flags: A mask of DMA_FENCE_FLAG_* defined below - * @timestamp: Timestamp when the fence was signaled. - * @error: Optional, only valid if < 0, must be set before calling - * dma_fence_signal, indicates that the fence has completed with an error. - * - * the flags member must be manipulated and read using the appropriate - * atomic ops (bit_*), so taking the spinlock will not be needed most - * of the time. - * - * DMA_FENCE_FLAG_SIGNALED_BIT - fence is already signaled - * DMA_FENCE_FLAG_TIMESTAMP_BIT - timestamp recorded for fence signaling - * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT - enable_signaling might have been called - * DMA_FENCE_FLAG_USER_BITS - start of the unused bits, can be used by the - * implementer of the fence for its own purposes. Can be used in different - * ways by different fence implementers, so do not rely on this. - * - * Since atomic bitops are used, this is not guaranteed to be the case. - * Particularly, if the bit was set, but dma_fence_signal was called right - * before this bit was set, it would have been able to set the - * DMA_FENCE_FLAG_SIGNALED_BIT, before enable_signaling was called. - * Adding a check for DMA_FENCE_FLAG_SIGNALED_BIT after setting - * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT closes this race, and makes sure that - * after dma_fence_signal was called, any enable_signaling call will have either - * been completed, or never called at all. - */ -struct dma_fence { - struct kref refcount; - const struct dma_fence_ops *ops; - /* We clear the callback list on kref_put so that by the time we - * release the fence it is unused. No one should be adding to the cb_list - * that they don't themselves hold a reference for. - */ - union { - struct rcu_head rcu; - struct list_head cb_list; - }; - spinlock_t *lock; - u64 context; - u64 seqno; - unsigned long flags; - ktime_t timestamp; - int error; -}; - -enum dma_fence_flag_bits { - DMA_FENCE_FLAG_SIGNALED_BIT, - DMA_FENCE_FLAG_TIMESTAMP_BIT, - DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - DMA_FENCE_FLAG_USER_BITS, /* must always be last member */ -}; - -typedef void (*dma_fence_func_t)(struct dma_fence *fence, - struct dma_fence_cb *cb); - -/** - * struct dma_fence_cb - callback for dma_fence_add_callback() - * @node: used by dma_fence_add_callback() to append this struct to fence::cb_list - * @func: dma_fence_func_t to call - * - * This struct will be initialized by dma_fence_add_callback(), additional - * data can be passed along by embedding dma_fence_cb in another struct. - */ -struct dma_fence_cb { - struct list_head node; - dma_fence_func_t func; -}; - -/** - * struct dma_fence_ops - operations implemented for fence - * - */ -struct dma_fence_ops { - /** - * @use_64bit_seqno: - * - * True if this dma_fence implementation uses 64bit seqno, false - * otherwise. - */ - bool use_64bit_seqno; - - /** - * @get_driver_name: - * - * Returns the driver name. This is a callback to allow drivers to - * compute the name at runtime, without having it to store permanently - * for each fence, or build a cache of some sort. - * - * This callback is mandatory. - */ - const char * (*get_driver_name)(struct dma_fence *fence); - - /** - * @get_timeline_name: - * - * Return the name of the context this fence belongs to. This is a - * callback to allow drivers to compute the name at runtime, without - * having it to store permanently for each fence, or build a cache of - * some sort. - * - * This callback is mandatory. - */ - const char * (*get_timeline_name)(struct dma_fence *fence); - - /** - * @enable_signaling: - * - * Enable software signaling of fence. - * - * For fence implementations that have the capability for hw->hw - * signaling, they can implement this op to enable the necessary - * interrupts, or insert commands into cmdstream, etc, to avoid these - * costly operations for the common case where only hw->hw - * synchronization is required. This is called in the first - * dma_fence_wait() or dma_fence_add_callback() path to let the fence - * implementation know that there is another driver waiting on the - * signal (ie. hw->sw case). - * - * This function can be called from atomic context, but not - * from irq context, so normal spinlocks can be used. - * - * A return value of false indicates the fence already passed, - * or some failure occurred that made it impossible to enable - * signaling. True indicates successful enabling. - * - * &dma_fence.error may be set in enable_signaling, but only when false - * is returned. - * - * Since many implementations can call dma_fence_signal() even when before - * @enable_signaling has been called there's a race window, where the - * dma_fence_signal() might result in the final fence reference being - * released and its memory freed. To avoid this, implementations of this - * callback should grab their own reference using dma_fence_get(), to be - * released when the fence is signalled (through e.g. the interrupt - * handler). - * - * This callback is optional. If this callback is not present, then the - * driver must always have signaling enabled. - */ - bool (*enable_signaling)(struct dma_fence *fence); - - /** - * @signaled: - * - * Peek whether the fence is signaled, as a fastpath optimization for - * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this - * callback does not need to make any guarantees beyond that a fence - * once indicates as signalled must always return true from this - * callback. This callback may return false even if the fence has - * completed already, in this case information hasn't propogated throug - * the system yet. See also dma_fence_is_signaled(). - * - * May set &dma_fence.error if returning true. - * - * This callback is optional. - */ - bool (*signaled)(struct dma_fence *fence); - - /** - * @wait: - * - * Custom wait implementation, defaults to dma_fence_default_wait() if - * not set. - * - * The dma_fence_default_wait implementation should work for any fence, as long - * as @enable_signaling works correctly. This hook allows drivers to - * have an optimized version for the case where a process context is - * already available, e.g. if @enable_signaling for the general case - * needs to set up a worker thread. - * - * Must return -ERESTARTSYS if the wait is intr = true and the wait was - * interrupted, and remaining jiffies if fence has signaled, or 0 if wait - * timed out. Can also return other error values on custom implementations, - * which should be treated as if the fence is signaled. For example a hardware - * lockup could be reported like that. - * - * This callback is optional. - */ - signed long (*wait)(struct dma_fence *fence, - bool intr, signed long timeout); - - /** - * @release: - * - * Called on destruction of fence to release additional resources. - * Can be called from irq context. This callback is optional. If it is - * NULL, then dma_fence_free() is instead called as the default - * implementation. - */ - void (*release)(struct dma_fence *fence); - - /** - * @fence_value_str: - * - * Callback to fill in free-form debug info specific to this fence, like - * the sequence number. - * - * This callback is optional. - */ - void (*fence_value_str)(struct dma_fence *fence, char *str, int size); - - /** - * @timeline_value_str: - * - * Fills in the current value of the timeline as a string, like the - * sequence number. Note that the specific fence passed to this function - * should not matter, drivers should only use it to look up the - * corresponding timeline structures. - */ - void (*timeline_value_str)(struct dma_fence *fence, - char *str, int size); -}; - void dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops, spinlock_t *lock, u64 context, u64 seqno); From patchwork Sat Aug 10 15:34:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11088569 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 333C31399 for ; Sat, 10 Aug 2019 15:36:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1BE9A2228E for ; Sat, 10 Aug 2019 15:36:15 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0EF2F26E1A; Sat, 10 Aug 2019 15:36:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A83C92228E for ; Sat, 10 Aug 2019 15:36:14 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 08D176E42F; Sat, 10 Aug 2019 15:36:12 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 69D0A6E42B; Sat, 10 Aug 2019 15:36:10 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 17954019-1500050 for multiple; Sat, 10 Aug 2019 16:34:33 +0100 From: Chris Wilson To: dri-devel@lists.freedesktop.org Subject: [PATCH 4/4] dma-fence: Always execute signal callbacks Date: Sat, 10 Aug 2019 16:34:30 +0100 Message-Id: <20190810153430.30636-4-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.23.0.rc1 In-Reply-To: <20190810153430.30636-1-chris@chris-wilson.co.uk> References: <20190810153430.30636-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: intel-gfx@lists.freedesktop.org, christian.koenig@amd.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP Allow for some users to surreptitiously insert lazy signal callbacks that do not depend on enabling the signaling mechanism around every fence. (The cost of interrupts is too darn high, to revive an old meme.) This means that we may have a cb_list even if the signaling bit is not enabled, so always notify the callbacks. The cost is that dma_fence_signal() must always acquire the spinlock to ensure that the cb_list is flushed. Signed-off-by: Chris Wilson --- drivers/dma-buf/dma-fence.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 027a6a894abd..ab4a456bba04 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -170,11 +170,9 @@ int dma_fence_signal(struct dma_fence *fence) __dma_fence_signal__timestamp(fence, ktime_get()); - if (test_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, &fence->flags)) { - spin_lock_irqsave(fence->lock, flags); - __dma_fence_signal__notify(fence); - spin_unlock_irqrestore(fence->lock, flags); - } + spin_lock_irqsave(fence->lock, flags); + __dma_fence_signal__notify(fence); + spin_unlock_irqrestore(fence->lock, flags); return 0; } EXPORT_SYMBOL(dma_fence_signal);