Quoting Mika Kuoppala (2018-03-05 13:59:43) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > Similar to the staging around handling of engine->submit_request, we > > need to stop adding to the execlists->queue prior to calling > > engine->cancel_requests. cancel_requests will move requests from the > > queue onto the timeline, so if we add a request onto the queue after that > > point, it will be lost. > > > > Fixes: af7a8ffad9c5 ("drm/i915: Use rcu instead of stop_machine in set_wedged") > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 13 +++++++------ > > drivers/gpu/drm/i915/i915_request.c | 2 ++ > > 2 files changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index a5bd07338b46..8d913d833ab9 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -471,10 +471,11 @@ static void __fence_set_priority(struct dma_fence *fence, int prio) > > > > rq = to_request(fence); > > engine = rq->engine; > > - if (!engine->schedule) > > - return; > > > > - engine->schedule(rq, prio); > > + rcu_read_lock(); > > + if (engine->schedule) > > + engine->schedule(rq, prio); > > + rcu_read_unlock(); > > } > > > > static void fence_set_priority(struct dma_fence *fence, int prio) > > @@ -3214,8 +3215,11 @@ void i915_gem_set_wedged(struct drm_i915_private *i915) > > */ > > for_each_engine(engine, i915, id) { > > i915_gem_reset_prepare_engine(engine); > > + > > engine->submit_request = nop_submit_request; > > + engine->schedule = NULL; > > Why we are not using rcu_assign_pointer and rcu_deference pair > in the upper part where we check the schedule? We are not using RCU protection. RCU here is being abused as a free-flowing stop-machine. > Further, is there are risk that we lose sync between the two > assigments. In another words, should we combine both callbaks > behind a single deferensable pointer in the engine struct? They are only tied together by how the backend uses them, not by request flow, so I don't think so. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx