On Thu, Jul 28, 2016 at 11:40:29AM +0100, Chris Wilson wrote: > On Thu, Jul 28, 2016 at 12:32:42PM +0200, Daniel Vetter wrote: > > On Wed, Jul 27, 2016 at 12:15:00PM +0100, Chris Wilson wrote: > > > If the GEM objects being rendered with in this request have been > > > exported via dma-buf to a third party, hook ourselves into the dma-buf > > > reservation object so that the third party can serialise with our > > > rendering via the dma-buf fences. > > > > > > Testcase: igt/prime_busy > > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > > > Style nit: I prefer ww_mutex_lock(&resv->lock, NULL); over > > mutex_lock(&resv->lock.base). The former makes it clear it's a ww mutex, > > but we don't bother with the multi-lock dance. The latter needles around > > in implemenation details, which it really shouldn't. Please change. > > Passing NULL as ww_acquite_ctx is illegal. Hm, where exactly do you see that? kerneldoc for ww_mutex_lock clearly says that it can be NULL, and the static inline has a check for it and calls mutex_lock in the else path. Which means it /should/ boil down to the exact same code after gcc has pondered it enough. > > The other wonky bit is that changing reservations on multiple objects > > without the full ww mutex dance is deadlock-risky. But only when you both > > add and wait/stall on fences. > > Note that it is only so when trying to lock multiple objects simultaneously, > which we do not require. With deadlocks I didn't mean locking deadlocks, but loops in the fences. Sure we eventually recover when you have a good gpu driver with hang check, but other drivers might be less fortunate. And if you lock objects individually and update their fences individually it's fairly simple to race command submission (on different drivers) against each another with objects listed in reverse and end up with a ABBA fence depency. The fix for that is to: 1. grab all the reservation locks 2. assemeble the full list of fences you need to stall on 3. update the reservation with the new fence for the current rendering job 4. only then release all locks Without that drama ensues ;-) And the current patch is still pretty far away from the above sequence (but it does roughly match to our overall execbuf logic, with s/lock/pin/). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
