[PATCH 7/9] dma-buf/dma-fence: Signal all callbacks from dma_fence_release()

[Chris Wilson]

Quoting Gustavo Padovan (2018-01-31 12:32:21)
> Hi,
> 
> 2017-08-11 Jason Ekstrand <jason at jlekstrand.net>:
> 
> > From: Chris Wilson <chris at chris-wilson.co.uk>
> > 
> > This is an illegal scenario, to free the fence whilst there are pending
> > callbacks. Currently, we emit a WARN and then cast aside the callbacks
> > leaving them dangling. Alternatively, we could set an error on the fence
> > and then signal fence so that any dependency chains from the fence can
> > be tidied up, and if they care they can check for the error.
> > 
> > The question is whether or not the cure is worse than the disease
> > (premature fence signaling is never pretty).
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/dma-buf/dma-fence.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 0cac367..4062708 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -172,7 +172,19 @@ void dma_fence_release(struct kref *kref)
> >  
> >       trace_dma_fence_destroy(fence);
> >  
> > -     WARN_ON(!list_empty(&fence->cb_list));
> > +     if (WARN_ON(!list_empty(&fence->cb_list))) {
> > +             unsigned long flags;
> > +
> > +             /*
> > +              * This should never happen, but if it does make sure that we
> > +              * don't leave chains dangling. We set the error flag first
> > +              * so that the callbacks know this signal is due to an error.
> > +              */
> > +             spin_lock_irqsave(fence->lock, flags);
> > +             fence->error = -EDEADLK;
> > +             dma_fence_signal_locked(fence);
> > +             spin_unlock_irqrestore(fence->lock, flags);
> > +     }
> 
> Thanks for the patch!
> 
> Reviewed-by: Gustavo Padovan <gustavo.padovan at collabora.com>

If you are picking up this version, please also include
a Reviewed-by: Christian König <christian.koenig at amd.com> given to an
earlier posting, which included a discussion on the pitiful lack of
EHUMAN.
-Chris