Re: [PATCH 13/16] drm/i915: Remove pinned check from madvise ioctl

Joonas Lahtinen
2016-08-04T11:47:56Z

On to, 2016-08-04 at 11:42 +0100, Chris Wilson wrote:
> On Thu, Aug 04, 2016 at 01:36:24PM +0300, Joonas Lahtinen wrote:
> >
> > On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> > >
> > > We don't need to incur the overhead of checking whether the object is
> > > pinned prior to changing its madvise. If the object is pinned, the
> > > madvise will not take effect until it is unpinned and so we cannot free
> > > the pages being pointed at by hardware. Marking a pinned object with
> > > allocated pages as DONTNEED will not trigger any undue warnings. The check
> > > is therefore superfluous, and by removing it we can remove a linear walk
> > > over all the vma the object has.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem.c | 6 ------
> > >  1 file changed, 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index 51ec5cd1c6ca..4b8a391912bc 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -3853,11 +3853,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
> > >   goto unlock;
> > >   }
> > >
> > > - if (i915_gem_obj_is_pinned(obj)) {
> > > - ret = -EINVAL;
> > > - goto out;
> > > - }
> > > -
> > Does not this change our ABI too?
> Yes. It relaxes an immediate failure condition and enforces it later.
>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

But might want to also ping for A-b from Daniel?

Regards, Joonas

> Anyone who tried to purge the scanout object now has their BUG hidden.
> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation