[Intel-gfx] [PATCH v3] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)
tvrtko.ursulin at linux.intel.com
Thu Oct 3 14:43:51 UTC 2019
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 at lists.freedesktop.org] On Behalf Of
>>> Chris Wilson
>>> Sent: Thursday, October 3, 2019 9:24 AM
>>> To: intel-gfx at lists.freedesktop.org
>>> Cc: dri-devel at lists.freedesktop.org
>>> Subject: [Intel-gfx] [PATCH v3] dma-fence: Serialise signal enabling
>>> 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 at chris-wilson.co.uk>
>>> Return false for "could not _enable_ signaling as it was already
>>> 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)
>>> +static bool __dma_fence_enable_signaling(struct dma_fence *fence)
>>> + bool was_set;
>>> + lockdep_assert_held(fence->lock);
>> With this held...
>>> + was_set =
>>> + &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. :)
More information about the dri-devel