[Intel-gfx] [PATCH 04/54] drm/i915: Mark up protected uses of 'i915_request_completed'

[Chris Wilson]

Quoting Andi Shyti (2020-12-29 12:17:08)
> Hi Chris,
> 
> > When we know that we are inside the timeline mutex, or inside the
> > submission flow (under active.lock or the holder's rcu lock), we know
> > that the rq->hwsp is stable and we can use the simpler direct version.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gem/i915_gem_context.c     |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_breadcrumbs.c     |  4 ++--
> >  drivers/gpu/drm/i915/gt/intel_engine_cs.c       |  2 +-
> >  drivers/gpu/drm/i915/gt/intel_reset.c           |  3 +--
> >  drivers/gpu/drm/i915/gt/intel_ring_submission.c |  4 +++-
> >  drivers/gpu/drm/i915/gt/intel_timeline.c        |  4 ++--
> >  drivers/gpu/drm/i915/i915_request.c             | 15 +++++++--------
> >  7 files changed, 17 insertions(+), 17 deletions(-)
> 
> have you also missed a couple of cases in
> intel_execlists_submission.c and i915_scheduler.c?

Yes. I dealt with those while updating those files. This patch was
originally the fixup of the mainly protected request flow.
 
> [...]
> 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> > index b85b6f3dcd60..e0b4291393ec 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> > @@ -151,8 +151,7 @@ static void mark_innocent(struct i915_request *rq)
> >  void __i915_request_reset(struct i915_request *rq, bool guilty)
> >  {
> >       RQ_TRACE(rq, "guilty? %s\n", yesno(guilty));
> > -
> > -     GEM_BUG_ON(i915_request_completed(rq));
> > +     GEM_BUG_ON(__i915_request_is_complete(rq));
> 
> aren't you outside the lock here?

This is inside the rcu read lock for the reset.
-Chris