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

[Gustavo Padovan]

2016-10-17 Chris Wilson <chris at chris-wilson.co.uk>:

> On Mon, Oct 17, 2016 at 02:59:52PM -0400, Rob Clark wrote:
> > On Mon, Oct 17, 2016 at 2:52 PM, Gustavo Padovan <gustavo at padovan.org> wrote:
> > > 2016-10-17 Rob Clark <robdclark at 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.
> 
> For 2 i915 fences, I definitely do not want signaling enabled at
> creation time.

Should we add arg flags for fence_array_create()? We already have
signal_on_any flag there. We can convert that arg to a bitfield.

Gustavo