[Intel-gfx] [PATCH 3/6] drm/i915/breadcrumbs: Update bottom-half before marking as complete

[Chris Wilson]

On Wed, Mar 15, 2017 at 06:58:27PM +0000, Tvrtko Ursulin wrote:
> 
> On 15/03/2017 14:01, Chris Wilson wrote:
> >When adding a new request to the breadcrumb rbtree, we mark all those
> >requests inside the rbtree that are already completed as complete. This
> >wakes those waiters up and allows them to skip the spinlock before
> >returning to userspace. If one of those is the current bottom-half, it
> >may then overwrite intel_wait as the interrupt handler dereferences it.
> 
> Last sentence sounds suspicious. The interrupts are disabled when
> this runs and locking is in place. And since the fix is to move the
> "completed" block after the "first", I wonder what can get
> overwritten by who?
> 
> Oh.. __intel_breadcrumbs_finish. But how does re-ordering help?
> Shouldn't the fix be to skip the bottom-half assignment if the
> "complete" loop has processed the waiter getting added?

Thread A runs intel_engine_add_wait, marks an earlier waiter complete
and wakes up thread B. Thread C is processing the interrupt and grabs
the irq_wait. However, thread B sees that is is complete and exits
i915_aait_request() invalidating the irq_wait as it is being used by
thread C.

Moving the irq_wait update before we wakeup thread B, ensures that
thread C has a valid irq_wait.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre