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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
