[Intel-xe] [PATCH] drm/xe: Fix engine lookup refcount race

[Thomas Hellström]

On Thu, 2023-05-25 at 11:52 +0100, Matthew Auld wrote:
> On Thu, 25 May 2023 at 09:16, Thomas Hellström
> <thomas.hellstrom at linux.intel.com> wrote:
> > 
> > Fix a race where the engine could disappear between lookup unlock
> > and
> > xe_engine_get().
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_engine.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_engine.c
> > b/drivers/gpu/drm/xe/xe_engine.c
> > index 094ec17d3004..3a400a546dfe 100644
> > --- a/drivers/gpu/drm/xe/xe_engine.c
> > +++ b/drivers/gpu/drm/xe/xe_engine.c
> > @@ -161,10 +161,9 @@ struct xe_engine *xe_engine_lookup(struct
> > xe_file *xef, u32 id)
> > 
> >         mutex_lock(&xef->engine.lock);
> >         e = xa_load(&xef->engine.xa, id);
> > -       mutex_unlock(&xef->engine.lock);
> > -
> >         if (e)
> >                 xe_engine_get(e);
> > +       mutex_unlock(&xef->engine.lock);
> 
> Makes sense to me.
> 
> Reviewed-by: Matthew Auld <matthew.auld at intel.com>
> 
> But looking around a bit, why does engine_get_property_ioctl() not
> use
> this? It also does a raw xa_load() and then touches the engine
> without
> holding the lock or keeping a ref?

Yeah, the lookups / xa_load()s probably needs an audit Also should
probably check if we can reuse the xa_lock for some lookups.

/Thomas


> 
> > 
> >         return e;
> >  }
> 
> > --
> > 2.39.2
> >