On 03/10/2019 15:18, Chris Wilson wrote:
Quoting Ruhl, Michael J (2019-10-03 15:12:38)
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf Of
Chris Wilson
Sent: Thursday, October 3, 2019 9:24 AM
To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
Subject: [Intel-gfx] [PATCH v3] dma-fence: Serialise signal enabling
(dma_fence_enable_sw_signaling)
Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.
See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").
v2: Refactor all 3 enable_signaling paths to use a common function.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
Return false for "could not _enable_ signaling as it was already
signaled"
---
drivers/dma-buf/dma-fence.c | 78 +++++++++++++++++--------------------
1 file changed, 35 insertions(+), 43 deletions(-)
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..b58528c1cc9d 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
}
EXPORT_SYMBOL(dma_fence_free);
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)
+{
+ bool was_set;
+
+ lockdep_assert_held(fence->lock);
With this held...
+ was_set =
test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+ &fence->flags);
+
+ if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+ return false;
Would making these the non-atomic versions be useful (and/or reasonable)?
That depends on all modifications to the dword (not just the bit) being
serialised by the same lock (or otherwise guaranteed to be serial and
coherent), which as Tvrtko rediscovered is not the case. dma_fence.flags
is also home to a number of user flags.
Michael, for completeness of the reference - I fell into the same trap -
https://patchwork.freedesktop.org/series/67532/. Check the results. :)
Regards,
Tvrtko
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
