On 18/04/16 12:56, Chris Wilson wrote:
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@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> #v4
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
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.
I don't have a strong feeling either way. vma_pin_iomap would make the
name (and usage model) more consistent with obj_pin_map and taking the
pin count could potentially make it a little bit safer for the users
where it would leak rather than crash. So maybe on the basis of those it
would be a tiny bit better.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
