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

From: Jason Ekstrand <jason@jlekstrand.net>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Intel GFX <intel-gfx@lists.freedesktop.org>,
	Maling list - DRI developers <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 11:07:42 -0500	[thread overview]
Message-ID: <CAOFGe94EOR0A4EH=bKRhAvxemkNvXktBWKhXKbCRZ8BgZ=r2+Q@mail.gmail.com> (raw)
In-Reply-To: <YMCo5N8cn902qLgc@phenom.ffwll.local>

On Wed, Jun 9, 2021 at 6:41 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> 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.

Ok, let's drop this for now.

--Jason

> -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