[Intel-gfx] [PATCH] drm/i915/execlist: Mark up racy read of execlists->pending[0]

[Chris Wilson]

Quoting Mika Kuoppala (2020-01-29 09:29:43)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > We write to execlists->pending[0] in process_csb() to acknowledge the
> > completion of the ESLP update, outside of the main spinlock. When we
> > check the current status of the previous submission in
> > __execlists_submission_tasklet() we should therefore use READ_ONCE() to
> > reflect and document the unsynchronized read.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_lrc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index cf6c43bd540a..058484958e87 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -2347,7 +2347,7 @@ static void process_csb(struct intel_engine_cs *engine)
> >  static void __execlists_submission_tasklet(struct intel_engine_cs *const engine)
> >  {
> >       lockdep_assert_held(&engine->active.lock);
> > -     if (!engine->execlists.pending[0]) {
> > +     if (!READ_ONCE(engine->execlists.pending[0])) {
> 
> With same token, should we also include assert_pending_invalid()
> read of pending with READ_ONCE?

That happens on the control paths, so the state of pending[] at that
point should be static (and the compiler can be left to its own
devices).
-Chris