Quoting Mika Kuoppala (2018-03-05 13:59:43)
> Chris Wilson <ch...@chris-wilson.co.uk> 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 <ch...@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuopp...@intel.com>
> > ---
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
