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@xxxxxxxxxxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
