Re: [PATCH 2/7] drm/i915: Check context status before looking up our obj/vma

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

 



Quoting Mika Kuoppala (2017-08-17 15:10:02)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> 
> > Since we keep the context around across the slow lookup where we may
> > drop the struct_mutex, we should double check that the context is still
> > valid upon reacquisition.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index 359d5dc6d8df..044fb1205554 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -679,13 +679,6 @@ static int eb_select_context(struct i915_execbuffer *eb)
> >       if (unlikely(!ctx))
> >               return -ENOENT;
> >  
> > -     if (unlikely(i915_gem_context_is_banned(ctx))) {
> > -             DRM_DEBUG("Context %u tried to submit while banned\n",
> > -                       ctx->user_handle);
> > -             i915_gem_context_put(ctx);
> > -             return -EIO;
> > -     }
> > -
> >       eb->ctx = ctx;
> >       eb->vm = ctx->ppgtt ? &ctx->ppgtt->base : &eb->i915->ggtt.base;
> >  
> > @@ -707,6 +700,12 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> >       int slow_pass = -1;
> >       int err;
> >  
> > +     if (unlikely(i915_gem_context_is_closed(eb->ctx)))
> > +             return -ENOENT;
> > +
> > +     if (unlikely(i915_gem_context_is_banned(eb->ctx)))
> > +             return -EIO;
> > +
> 
> We are referring the lut before the context has been validated.
> Not that it matters but for style, please consider assigning
> the lut after the context check.

~o~ Not feeling it. If it was checking for an invalid pointer, then yes
rearranging the code to avoid the appearance of the early dereference
makes sense, but as it is we are just creating an alias for part of the
ctx. Should look at whether gcc likes a few more locals...
-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