Quoting Chris Wilson (2018-05-01 10:49:17) > As we reschedule the requests, we do not want the submission tasklet > running until we finish updating the priority chains. (We start > rewriting priorities from the oldest, but the dequeue looks at the most > recent in-flight, so there is a small race condition where dequeue may > decide that preemption is falsely required.) Combine the tasklet kicking > from adding a new request with the set-wedge protection so that we only > have to adjust the preempt-counter once to achieve both goals. > > Reported-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_gem.c | 4 ++-- > drivers/gpu/drm/i915/i915_request.c | 5 +---- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 438a2fc5bba0..a278e4dc5e09 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -577,10 +577,10 @@ static void __fence_set_priority(struct dma_fence *fence, > rq = to_request(fence); > engine = rq->engine; > > - rcu_read_lock(); > + local_bh_disable(); /* RCU serialisation for set-wedged protection */ > if (engine->schedule) > engine->schedule(rq, attr); > - rcu_read_unlock(); > + local_bh_enable(); /* kick the tasklets if queues were reprioritised */ > } > > static void fence_set_priority(struct dma_fence *fence, > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 7bb613c00cc3..eb66579db823 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1111,12 +1111,9 @@ void __i915_request_add(struct i915_request *request, bool flush_caches) > * decide whether to preempt the entire chain so that it is ready to > * run at the earliest possible convenience. > */ > - rcu_read_lock(); > + local_bh_disable(); > if (engine->schedule) > engine->schedule(request, &request->ctx->sched); > - rcu_read_unlock(); > - > - local_bh_disable(); > i915_sw_fence_commit(&request->submit); > local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ It's an improvement in that we don't mix and match rcu/bh anymore, but it doesn't entirely close the race; as local-bh is local to this cpu and the tasklet may be running on another. Bother. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
