Re: [PATCH v2 4/4] drm/i915: Move ioremap_wc tracking onto VMA

Chris Wilson
2016-04-18T11:56:42Z

On Mon, Apr 18, 2016 at 12:14:02PM +0100, Chris Wilson wrote:
> By tracking the iomapping on the VMA itself, we can share that area
> between multiple users. Also by only revoking the iomapping upon
> unbinding from the mappable portion of the GGTT, we can keep that iomap
> across multiple invocations (e.g. execlists context pinning).
>
> Note that by moving the iounnmap tracking to the VMA, we actually end up
> fixing a leak of the iomapping in intel_fbdev.
>
> v1.5: Rebase prompted by Tvrtko
> v2: Drop dev_priv parameter, we can recover the i915_ggtt from the vma.
> v3: Move handling of ioremap space exhaustion to vmap_purge and also
> allow vmallocs to recover old iomaps. Add Tvrtko's kerneldoc.
> v4: Fix a use-after-free in shrinker and rearrange i915_vma_iomap
> v5: Back to i915_vm_to_ggtt
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v4
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 2 ++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 23 +++++++++++++++++++
> drivers/gpu/drm/i915/i915_gem_gtt.h | 38 ++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 28 ++++++++++++++++++-----
> drivers/gpu/drm/i915/intel_fbdev.c | 22 +++++++++---------
> 5 files changed, 97 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6ce2c31b9a81..82d0af3f1692 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3378,6 +3378,8 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
> ret = i915_gem_object_put_fence(obj);
> if (ret)
> return ret;
> +
> + i915_vma_iounmap(vma);
> }
>
> trace_i915_vma_unbind(vma);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index b3af2e808b49..c894af7e0ea1 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3634,3 +3634,26 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
> return obj->base.size;
> }
> }
> +
> +void *i915_vma_iomap(struct i915_vma *vma)

I've been arguing with myself as to whether to make this
i915_vma_pin_iomap (and so acquire the vma->pin_count as well).

There will still be the requirement that the vma is already usuable, but
it will mean that the returned iomap cannot be reaped for the critical
section of its user.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre