Re: [PATCH v3 2/5] drm/i915: Combine tasklet_kill and tasklet_disable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Chris Wilson (2018-05-09 15:02:12)
> Quoting Mika Kuoppala (2018-05-09 14:54:04)
> > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> > 
> > > Ideally, we want to atomically flush and disable the tasklet before
> > > resetting the GPU. At present, we rely on being the only part to touch
> > > our tasklet and serialisation of the reset process to ensure that we can
> > > suspend the tasklet from the mix of reset/wedge pathways. In this patch,
> > > we move the tasklet abuse into its own function and tweak it such that
> > > we only do a synchronous operation the first time it is disabled around
> > > the reset. This allows us to avoid the sync inside a softirq context in
> > > subsequent patches.
> > >
> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > > Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > > CC: Michel Thierry <michel.thierry@xxxxxxxxx>
> > > Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c        |  2 --
> > >  drivers/gpu/drm/i915/i915_tasklet.h    | 14 ++++++++++++++
> > >  drivers/gpu/drm/i915/intel_engine_cs.c |  4 ++--
> > >  3 files changed, 16 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 98481e150e61..8d27e78b052c 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3043,8 +3043,6 @@ i915_gem_reset_prepare_engine(struct intel_engine_cs *engine)
> > >        * common case of recursively being called from set-wedged from inside
> > >        * i915_reset.
> > >        */
> > > -     if (i915_tasklet_is_enabled(&engine->execlists.tasklet))
> > > -             i915_tasklet_flush(&engine->execlists.tasklet);
> > >       i915_tasklet_disable(&engine->execlists.tasklet);
> > >  
> > >       /*
> > > diff --git a/drivers/gpu/drm/i915/i915_tasklet.h b/drivers/gpu/drm/i915/i915_tasklet.h
> > > index c9f41a5ebb91..d8263892f385 100644
> > > --- a/drivers/gpu/drm/i915/i915_tasklet.h
> > > +++ b/drivers/gpu/drm/i915/i915_tasklet.h
> > > @@ -59,10 +59,24 @@ static inline void i915_tasklet_enable(struct i915_tasklet *t)
> > >  }
> > >  
> > >  static inline void i915_tasklet_disable(struct i915_tasklet *t)
> > > +{
> > > +     if (i915_tasklet_is_enabled(t))
> > > +             i915_tasklet_flush(t);
> > 
> > This needs a comment to explain how we can get away with
> > the race.
> 
> It doesn't, we rely on exclusivity that is provided by it is only us
> that controls the tasklet. The rationale comes from the caller. That we
> call flush here at all, is just that it helps avoid spinning, it is not
> required. If the race is too much, we just need the next line.

Rather than ever worry, let's just kill it.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux