Quoting Daniel Vetter (2019-08-16 20:07:24)
> On Fri, Aug 16, 2019 at 9:02 PM Koenig, Christian
> <Christian.Koenig@xxxxxxx> wrote:
> >
> > Am 16.08.19 um 17:21 schrieb Chris Wilson:
> > > Currently dma_fence_signal() tries to avoid the spinlock and only takes
> > > it if absolutely required to walk the callback list. However, to allow
> > > for some users to surreptitiously insert lazy signal callbacks that
> > > do not depend on enabling the signaling mechanism around every fence,
> > > we always need to notify the callbacks on signaling. As such, we will
> > > always need to take the spinlock and dma_fence_signal() effectively
> > > becomes a clone of dma_fence_signal_locked().
> > >
> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Cc: Christian König <christian.koenig@xxxxxxx>
> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> > > ---
> > > drivers/dma-buf/dma-fence.c | 19 +++++--------------
> > > 1 file changed, 5 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > > index ff0cd6eae766..f23eb9f19b4e 100644
> > > --- a/drivers/dma-buf/dma-fence.c
> > > +++ b/drivers/dma-buf/dma-fence.c
> > > @@ -176,6 +176,7 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
> > > int dma_fence_signal(struct dma_fence *fence)
> > > {
> > > unsigned long flags;
> > > + int ret;
> > >
> > > if (!fence)
> > > return -EINVAL;
> > > @@ -183,21 +184,11 @@ int dma_fence_signal(struct dma_fence *fence)
> > > if (test_and_set_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
> > > return -EINVAL;
> >
> > I need to take my review back. You also need to drop this
> > test_and_set_bit here or your completely break drivers using this.
First time we were here, it was just a plain test_bit(). Skipping a
patch, so had to start from scratch... (I blame glancing at the original
outcome and glossing over the test_bit vs test_and_set_bit.)
> Time for a pile of dma_fence selftests? Maybe with a bit of dma_resv
> thrown in for good? This stuff is tricky, and it feels like we had a
> few oopsies in review recently ...
dma_fence_signal vs dma_fence_add_callback
Something like:
while (!timeout)
f1 = mock_fence_create()
push f1 to other thread
pull f2 from other thread
dma_fence_signal(f2);
dma_fence_add_callback(f1)
dma_fence_signal(f1);
check cb
Can't see an obvious way to make the test_and_set_bit window larger.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
