Re: [Intel-gfx] [PATCH 13/16] drm/i915: Remove pinned check from madvise ioctl
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> > > --- > > > 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 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/16] drm/i915: Remove pinned check from madvise ioctl
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> > --- > > 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. Anyone who tried to purge the scanout object now has their BUG hidden. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 13/16] drm/i915: Remove pinned check from madvise ioctl
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> --- > 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? Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 13/16] drm/i915: Remove pinned check from madvise ioctl
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--- 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; - } - if (obj->pages && obj->tiling_mode != I915_TILING_NONE && dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) { @@ -3876,7 +3871,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data, args->retained = obj->madv != __I915_MADV_PURGED; -out: i915_gem_object_put(obj); unlock: mutex_unlock(>struct_mutex); -- 2.8.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx