[Intel-gfx] [PATCH] drm/i915: Make i915_gem_object_phys_attach() use obj->mm.lock more appropriately

[Chris Wilson]

Quoting Ville Syrjälä (2017-06-12 14:46:40)
> On Thu, Jun 08, 2017 at 11:53:44AM +0100, Chris Wilson wrote:
> > Actually transferring from shmemfs to the physically contiguous set of
> > pages should be wholly guarded by its obj->mm.lock!
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem.c | 43 +++++++++++++++++++++++++++--------------
> >  1 file changed, 28 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index aff449807399..db168e395e8e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -574,7 +574,8 @@ int
> >  i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
> >                           int align)
> >  {
> > -     int ret;
> > +     struct sg_table *pages;
> > +     int err;
> >  
> >       if (align > obj->base.size)
> >               return -EINVAL;
> > @@ -582,32 +583,44 @@ i915_gem_object_attach_phys(struct drm_i915_gem_object *obj,
> >       if (obj->ops == &i915_gem_phys_ops)
> >               return 0;
> >  
> > -     if (obj->mm.madv != I915_MADV_WILLNEED)
> > -             return -EFAULT;
> > -
> > -     if (obj->base.filp == NULL)
> > +     if (obj->ops != &i915_gem_object_ops)
> >               return -EINVAL;
> >  
> > -     ret = i915_gem_object_unbind(obj);
> > -     if (ret)
> > -             return ret;
> > +     err = i915_gem_object_unbind(obj);
> > +     if (err)
> > +             return err;
> > +
> > +     mutex_lock(&obj->mm.lock);
> > +
> > +     if (obj->mm.quirked) {
> > +             err = -EFAULT;
> > +             goto err_unlock;
> > +     }
> >  
> > -     __i915_gem_object_put_pages(obj, I915_MM_NORMAL);
> 
> Are we now leaking the old mm.pages?

Hmm, I thought I had replace it with a direct call to free them on
completion. All that is needed is a call to
i915_gem_object_put_pages_gtt() which we can rely since we know this is
i915_gem_object_ops. Make that i915_gem_object_ops.put_pages().
-Chris