| Message ID | 1376979255-17887-1-git-send-email-benjamin.widawsky@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
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@chris-wilson.co.uk> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 3 +++ 2 files changed, 4 insertions(+)