Re: [PATCH] dma-buf/fence-array: fix deadlock in fence-array

[Gustavo Padovan <gustavo@padovan.org>]

2016-10-17 Rob Clark <robdclark@gmail.com>:

> On Mon, Oct 17, 2016 at 2:52 PM, Gustavo Padovan <gustavo@padovan.org> wrote:
> > 2016-10-17 Rob Clark <robdclark@gmail.com>:
> >
> >> Currently with fence-array, we have a potential deadlock situation.  If we
> >> fence_add_callback() on an array-fence, the array-fence's lock is acquired
> >> first, and in it's ->enable_signaling() callback, it will install cb's on
> >> it's array-member fences, so the array-member's lock is acquired second.
> >>
> >> But in the signal path, the array-member's lock is acquired first, and the
> >> array-fence's lock acquired second.
> >>
> >> To solve that, always enabling signaling up-front (in the fence_array
> >> constructor) without the fence_array's lock held.
> >
> > Do we always want to enable signaling for arrays? One of the things we
> > removed from the Sync Framework was the need to enable signalling at
> > creation time.
> >
> > Just merging fencing doesn't mean you want signaling, that is supposed
> > to happen only when poll() is called on the sync file.
> 
> It was something Maarten suggested, as an alternative to introducing a
> wq into the mix or worse hacks..
> https://lists.freedesktop.org/archives/dri-devel/2016-October/120868.html
> 
> I think I agree with him that it is an optimization that is unlikely
> to be useful in the case of fence-arrays.  If you need to wait on
> multiple fences from different timelines, you probably aren't doing
> that in hw.

Fair enough, that makes sense to me. 

If no one else has concerns with this, I'd add my r-b to it:

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Gustavo

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel