[PATCH 2/2] dma-fence: Simply wrap dma_fence_signal_locked with dma_fence_signal

[Chris Wilson]

Quoting Daniel Vetter (2019-08-16 20:07:24)
> On Fri, Aug 16, 2019 at 9:02 PM Koenig, Christian
> <Christian.Koenig at amd.com> 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 at chris-wilson.co.uk>
> > > Cc: Christian König <christian.koenig at amd.com>
> > > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > > ---
> > >   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