Re: [Intel-gfx] [PATCH 15/28] drm/i915/gt: Protect context lifetime with RCU

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 17/11/2020 11:30, 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.
> v4: Drop the unrequired change to remove the temporary reference around
> dropping the active context, and add back some more missing ctor
> operations.
> v5: The ctor is back. Tvrtko spotted that ce->signal_lock [introduced
> later] maybe accessed under RCU and so needs special care not to be
> reinitialised.
> v6: Don't mix SLAB_TYPESAFE_BY_RCU and RCU list iteration.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_context.c       | 12 +++++++++---
>   drivers/gpu/drm/i915/gt/intel_context_types.h | 11 ++++++++++-
>   2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
> index 92a3f25c4006..d3a835212167 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context.c
> +++ b/drivers/gpu/drm/i915/gt/intel_context.c
> @@ -25,11 +25,18 @@ static struct intel_context *intel_context_alloc(void)
>   	return kmem_cache_zalloc(global.slab_ce, GFP_KERNEL);
>   }
>   
> -void intel_context_free(struct intel_context *ce)
> +static void rcu_context_free(struct rcu_head *rcu)
>   {
> +	struct intel_context *ce = container_of(rcu, typeof(*ce), rcu);
> +
>   	kmem_cache_free(global.slab_ce, ce);
>   }
>   
> +void intel_context_free(struct intel_context *ce)
> +{
> +	call_rcu(&ce->rcu, rcu_context_free);
> +}
> +
>   struct intel_context *
>   intel_context_create(struct intel_engine_cs *engine)
>   {
> @@ -356,8 +363,7 @@ static int __intel_context_active(struct i915_active *active)
>   }
>   
>   void
> -intel_context_init(struct intel_context *ce,
> -		   struct intel_engine_cs *engine)
> +intel_context_init(struct intel_context *ce, struct intel_engine_cs *engine)
>   {
>   	GEM_BUG_ON(!engine->cops);
>   	GEM_BUG_ON(!engine->gt->vm);
> diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
> index 552cb57a2e8c..20cb5835d1c3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
> @@ -44,7 +44,16 @@ struct intel_context_ops {
>   };
>   
>   struct intel_context {
> -	struct kref ref;
> +	/*
> +	 * Note: Some fields may be accessed under RCU.
> +	 *
> +	 * Unless otherwise noted a field can safely be assumed to be protected
> +	 * by strong reference counting.
> +	 */
> +	union {
> +		struct kref ref; /* no kref_get_unless_zero()! */
> +		struct rcu_head rcu;
> +	};
>   
>   	struct intel_engine_cs *engine;
>   	struct intel_engine_cs *inflight;
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx