Re: [Intel-gfx] [PATCH 31/31] drm/i915: Drop some RCU usage around context VMs

From: Daniel Vetter <daniel@ffwll.ch>
To: Jason Ekstrand <jason@jlekstrand.net>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 31/31] drm/i915: Drop some RCU usage around context VMs
Date: Wed, 9 Jun 2021 13:41:24 +0200	[thread overview]
Message-ID: <YMCo5N8cn902qLgc@phenom.ffwll.local> (raw)
In-Reply-To: <20210609043613.102962-32-jason@jlekstrand.net>

On Tue, Jun 08, 2021 at 11:36:13PM -0500, Jason Ekstrand wrote:
> This instance now only happens during context creation so there's no way
> we can race with a context close/destroy.  We don't need to bother with
> the RCU and can access the pointer directly.
> 
> Signed-off-by: Jason Ekstrand <jason@jlekstrand.net>

There's another one in execbuf.c, and the real crux is that we need to
fully audit the lifetimes of everything. I think it's better to do this
all together in a seperate patch series, which entirely removes the rcu
barrier on the i915_address_space cleanup, and also removes the rcu
protection from everywhere else.

Otherwise we just have an inconsistent mess that happens to work,
sometimes.
-Daniel

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_context.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index 5312142daa0c0..ffdfed536ce9a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -787,15 +787,12 @@ static int intel_context_set_gem(struct intel_context *ce,
>  
>  	ce->ring_size = SZ_16K;
>  
> -	if (rcu_access_pointer(ctx->vm)) {
> -		struct i915_address_space *vm;
> -
> -		rcu_read_lock();
> -		vm = context_get_vm_rcu(ctx); /* hmm */
> -		rcu_read_unlock();
> -
> +	if (ctx->vm) {
> +		/* This only happens during context creation so no need to
> +		 * bother with any RCU nonsense.
> +		 */
>  		i915_vm_put(ce->vm);
> -		ce->vm = vm;
> +		ce->vm = i915_vm_get(ctx->vm);
>  	}
>  
>  	if (ctx->sched.priority >= I915_PRIORITY_NORMAL &&
> -- 
> 2.31.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch