-----Original Message----- From: Chris Wilson [mailto:chris@chris-wilson.co.uk] Sent: Thursday, October 3, 2019 10:19 AM To: Ruhl, Michael J michael.j.ruhl@intel.com; intel- gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org Subject: RE: [Intel-gfx] [PATCH v3] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
Quoting Ruhl, Michael J (2019-10-03 15:12:38)
-----Original Message----- From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
Of
Chris Wilson Sent: Thursday, October 3, 2019 9:24 AM To: intel-gfx@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org 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@chris-wilson.co.uk
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.
Ah got it.
In the future, I will think a bit outside the patch. (sorry for the pun...)
Thanks,
Mike
-Chris