The code has at least one place currently, and maybe more in the future
where we want to only unbind a VMA, but not destroy it. There are two
reasons for this, correctness and optimization. Using the one actual
case as an example:
1. Correctness: When doing the execbuf sequence we end up traversing the
"exec_list" looking for misaligned VMAs. At this point we'd like to
unbind the VMA, which could lead to it's destruction. In the current
code, this is "fine" because our vma destroy doesn't remove the VMA from
the exec_list. The latter fact was an oversight which was of trivial
consequence in the original PPGTT series - but now actually matters
since we have a multistage execbuffer.
2. Optimization: Unbinding a VMA is a relatively expensive operation
because it has to go and modify the PTEs. At present, therefore, it
isn't terribly interesting to try to optimize this case. However, one
could potentially do the PTE writes lazily (with full PPGTT there is no
security risk, only a correctness one) - where we don't even update PTEs
until later. Destroying and recreating the VMA in this case can be
spared with this busy flag.
"Hey, but isn't that a leak?"
As you might recall from previous patches, VMAs will be destroyed on
object freeing, and can be cheaply looked up later. Therefore the
dangling VMA isn't a problem. In the future if we end up with an
overabundance of dangling VMAs, we can potentially add some sort of
garbage collection for them.
NOTE: This patch doesn't actually fix anything. The explanation above is
the problem, this is the harness, and the fix comes next.
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Ben Widawsky <ben@xxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1eb4d98..28c0e0a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -574,6 +574,7 @@ struct i915_vma {
unsigned long exec_handle;
struct drm_i915_gem_exec_object2 *exec_entry;
+ bool busy;
};
struct i915_ctx_hang_stats {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c7efb60..6d2d7df 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4156,6 +4156,9 @@ i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj,
void i915_gem_vma_destroy(struct i915_vma *vma)
{
+ if (vma->busy)
+ return;
+
WARN_ON(vma->node.allocated);
list_del(&vma->vma_link);
kfree(vma);
--
1.8.3.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
