[Intel-gfx] [PATCH 2/7] drm/i915/gt: Protect context lifetime with RCU

[Chris Wilson]

Quoting Tvrtko Ursulin (2020-08-07 15:10:32)
> 
> On 07/08/2020 13:54, Chris Wilson wrote:
> > Allow a brief period for continued access to a dead intel_context by
> > deferring the release of the struct until after an RCU grace period.
> > As we are using a dedicated slab cache for the contexts, we can defer
> > the release of the slab pages via RCU, with the caveat that individual
> > structs may be reused from the freelist within an RCU grace period. To
> > handle that, we have to avoid clearing members of the zombie struct.
> > 
> > This is required for a later patch to handle locking around virtual
> > requests in the signaler, as those requests may want to move between
> > engines and be destroyed while we are holding b->irq_lock on a physical
> > engine.
> > 
> > v2: Drop mutex_reinit(), if we never mark the mutex as destroyed we
> > don't need to reset the debug code, at the loss of having the mutex
> > debug code spot us attempting to destroy a locked mutex.
> > v3: As the intended use will remain strongly referenced counted, with
> > very little inflight access across reuse, drop the ctor.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_context.c       | 27 ++++++++++++++-----
> >   drivers/gpu/drm/i915/gt/intel_context_types.h |  6 +++++
> >   2 files changed, 26 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> > index 52db2bde44a3..8eebb31c6c28 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_context.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> > @@ -22,7 +22,7 @@ static struct i915_global_context {
> >   
> >   static struct intel_context *intel_context_alloc(void)
> >   {
> > -     return kmem_cache_zalloc(global.slab_ce, GFP_KERNEL);
> > +     return kmem_cache_alloc(global.slab_ce, GFP_KERNEL);
> >   }
> >   
> >   void intel_context_free(struct intel_context *ce)
> > @@ -158,12 +158,12 @@ void intel_context_unpin(struct intel_context *ce)
> >       /*
> >        * Once released, we may asynchronously drop the active reference.
> >        * As that may be the only reference keeping the context alive,
> > -      * take an extra now so that it is not freed before we finish
> > +      * hold onto RCU so that it is not freed before we finish
> >        * dereferencing it.
> >        */
> > -     intel_context_get(ce);
> > +     rcu_read_lock();
> >       intel_context_active_release(ce);
> > -     intel_context_put(ce);
> > +     rcu_read_unlock();
> 
> Strong reference here couldn't stay?

It really shouldn't be here in the first place...

But no, it doesn't have to change in this patch.
-Chris