[Intel-gfx] [PATCH 16/42] drm/i915: Refactor object page API

[Chris Wilson]

On Thu, Oct 13, 2016 at 02:04:18PM +0300, Joonas Lahtinen wrote:
> On pe, 2016-10-07 at 10:46 +0100, Chris Wilson wrote:
> > @@ -2483,24 +2474,25 @@ i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
> >  
> >  	lockdep_assert_held(&obj->base.dev->struct_mutex);
> >  
> > -	if (obj->pages)
> > +	if (obj->mm.pages)
> >  		return 0;
> >  
> > -	if (obj->madv != I915_MADV_WILLNEED) {
> > +	if (obj->mm.madv != I915_MADV_WILLNEED) {
> >  		DRM_DEBUG("Attempting to obtain a purgeable object\n");
> > +		__i915_gem_object_unpin_pages(obj);
> 
> Confusing to have teardown of another function in here.
> 
> >  		return -EFAULT;
> >  	}
> >  
> > -	BUG_ON(obj->pages_pin_count);
> > -
> >  	ret = ops->get_pages(obj);
> > -	if (ret)
> > +	if (ret) {
> > +		__i915_gem_object_unpin_pages(obj);
> 
> And if you like *really* have to, at least try not to duplicate code.
> Bonus points form moving this to be proper teardown path where it has a
> counter-part.

It is here, so that the unlikely error handling is not inside the
inlined function, but where we expect the code to grow to handle the
more complex locking requirements.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre