Re: [PATCH 1/2] drm/i915: Check activity on i915_vma after confirming pin_count==0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 23/01/2020 22:44, Chris Wilson wrote:
Only assert that the i915_vma is now idle if and only if no other pins
are present. If another user has the i915_vma pinned, they may submit
more work to the i915_vma skipping the vm->mutex used to serialise the
unbind. We need to wait again, if we want to continue and unbind this
vma.

However, if we own the i915_vma (we hold the vm->mutex for the unbind
and the pin_count is 0), we can assert that the vma remains idle as we
unbind.

Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")
Closes: https://gitlab.freedesktop.org/drm/intel/issues/530
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_vma.c | 14 ++++++++++++--
  1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 306b951831fe..4999882fbceb 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1202,16 +1202,26 @@ int __i915_vma_unbind(struct i915_vma *vma)
  	if (ret)
  		return ret;
- GEM_BUG_ON(i915_vma_is_active(vma));
  	if (i915_vma_is_pinned(vma)) {
  		vma_print_allocator(vma, "is pinned");
  		return -EAGAIN;
  	}
- GEM_BUG_ON(i915_vma_is_active(vma));
+	/*
+	 * After confirming that no one else is pinning this vma, wait for
+	 * any laggards who may have crept in during the wait (through
+	 * a residual pin skipping the vm->mutex) to complete.
+	 */
+	ret = i915_vma_sync(vma);
+	if (ret)
+		return ret;
+
  	if (!drm_mm_node_allocated(&vma->node))
  		return 0;
+ GEM_BUG_ON(i915_vma_is_pinned(vma));
+	GEM_BUG_ON(i915_vma_is_active(vma));
+
  	if (i915_vma_is_map_and_fenceable(vma)) {
  		/*
  		 * Check that we have flushed all writes through the GGTT


GEM_BUG_ON in a sandwich of two syncs, which is similar to the while loop from an earlier version. Are you sure there are no races with this one? If pinned check is the main gate then wouldn't one sync after the pinned check be enough? Especially since in 2/2 you add another sync before taking the mutex.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux