RE: [Intel-gfx] [PATCH v3] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux