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