On Tue, Oct 18, 2016 at 3:41 AM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Mon, Oct 17, 2016 at 05:44:48PM -0200, Gustavo Padovan wrote: >> 2016-10-17 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: >> >> > 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@xxxxxxxxxxx> wrote: >> > > > 2016-10-17 Rob Clark <robdclark@xxxxxxxxx>: >> > > > >> > > >> 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. Well, in all my cases, if you end up with a fence-array, it means you are waiting on cpu, so enabling signalling now vs later doesn't matter. But Chris mentioned on IRC that he does actually have a case where he can wait in hw on fences from multiple contexts (ie. meaning MERGE would create a fence-array).. so in that case it does still make sense to defer enable-signalling. So I guess back to the wq approach.. BR, -R > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
