[Intel-gfx] [PATCH]gem: fix a regression

[Shaohua Li]

On Wed, Apr 08, 2009 at 01:57:42AM +0800, Ian Romanick wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Shaohua Li wrote:
> > regression caused by commit 5e118f4139feafe97e913df67b1f7c1e5083e535
> > i915_gem_object_move_to_inactive() should be called in task context, as
> > it calls fput()
> > 
> > Signed-off-by: Shaohua Li<shaohua.li at intel.com>
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 1449b45..54a258c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1494,8 +1494,17 @@ i915_gem_retire_request(struct drm_device *dev,
> >  
> >  		if (obj->write_domain != 0)
> >  			i915_gem_object_move_to_flushing(obj);
> > -		else
> > +		else {
> > +			/*
> > +			 * the object might be freed, take reference first so
> > +			 * it will not be freed in atomic environment
> > +			 **/
> > +			drm_gem_object_reference(obj);
> >  			i915_gem_object_move_to_inactive(obj);
> > +			spin_unlock(&dev_priv->mm.active_list_lock);
> > +			drm_gem_object_unreference(obj);
> > +			spin_lock(&dev_priv->mm.active_list_lock);
> > +		}
> >  	}
> 
> I don't see the relationship between this code change and the commit
> message.  I assume that calling drm_gem_object_reference doesn't change
> up to task context.  Am I missing something?
i915_gem_object_move_to_inactive() might call into fput(), this API
might do schedule(), so it shouldn't be called between
spin_lock()/spin_unlock(). The patch adds reference for the obj, so the
obj will not be freed (so will not call fput()). Then the patch release
the lock and unreference the obj, which will be freed there. The
suspected commit adds spin lock and causes the regression.

Thanks,
Shaohua