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

[Daniel Vetter]

On Mon, Oct 17, 2016 at 05:44:48PM -0200, Gustavo Padovan wrote:
> 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.

The thing creating the array might not be aware of the fences contained
therein, e.g. SYNC_FILE_MERGE. Imo we really need to keep the lazy
signalling enabling properties of fences.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch