Re: [Intel-gfx] [PATCH v9 25/70] drm/i915: Take reservation lock around i915_vma_pin.

On Tue, Mar 23, 2021 at 04:50:14PM +0100, Maarten Lankhorst wrote:

We previously complained when ww == NULL.

This function is now only used in selftests to pin an object, and ww locking is now fixed.

Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com Reviewed-by: Thomas Hellström thomas.hellstrom@linux.intel.com

---

.../i915/gem/selftests/i915_gem_coherency.c | 12 ++++------- drivers/gpu/drm/i915/i915_gem.c | 6 +++++- drivers/gpu/drm/i915/i915_vma.c | 4 +--- drivers/gpu/drm/i915/i915_vma.h | 20 +++++++++++++++---- 4 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c index b5dbf15570fc..3eec385d43bb 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_coherency.c @@ -218,15 +218,13 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v) u32 *cs; int err;

• vma = i915_gem_object_ggtt_pin(ctx->obj, NULL, 0, 0, 0);
• if (IS_ERR(vma))

• return PTR_ERR(vma);
  

• i915_gem_object_lock(ctx->obj, NULL); i915_gem_object_set_to_gtt_domain(ctx->obj, false);

I have different context here because of

https://lore.kernel.org/intel-gfx/20210203090205.25818-8-chris@chris-wilson....

What's really worrying here is the silent (accidental maybe, commit message doesn't explain anything) change to the argument of set_to_gtt_domain(). I've decided to just go with what we have right now, but please double-check this matches the old version you've had before this landed in drm-tip. Since I haven't pushed out the branch I've pinged you with the pastebin on irc for now. -Daniel

• vma = i915_gem_object_ggtt_pin(ctx->obj, NULL, 0, 0, 0);
• if (IS_ERR(vma)) {

• err = PTR_ERR(vma);
  

• goto out_unlock;
  

• }
• rq = intel_engine_create_kernel_request(ctx->engine); if (IS_ERR(rq)) { err = PTR_ERR(rq);

@@ -265,9 +263,7 @@ static int gpu_set(struct context *ctx, unsigned long offset, u32 v) i915_request_add(rq); out_unpin: i915_vma_unpin(vma); -out_unlock: i915_gem_object_unlock(ctx->obj);

• return err;

}

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3dee4e31fb14..8373662e4b5f 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -920,7 +920,11 @@ i915_gem_object_ggtt_pin_ww(struct drm_i915_gem_object *obj, return ERR_PTR(ret); }

• ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);

• if (ww)

• ret = i915_vma_pin_ww(vma, ww, size, alignment, flags | PIN_GLOBAL);
  

• else

• ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
  

• if (ret) return ERR_PTR(ret);

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c index 1ffda2aaa7a0..265e3a3079e2 100644 --- a/drivers/gpu/drm/i915/i915_vma.c +++ b/drivers/gpu/drm/i915/i915_vma.c @@ -863,9 +863,7 @@ int i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, int err;

#ifdef CONFIG_PROVE_LOCKING

• if (debug_locks && lockdep_is_held(&vma->vm->i915->drm.struct_mutex))

• WARN_ON(!ww);
  

• if (debug_locks && ww && vma->resv)

• if (debug_locks && !WARN_ON(!ww) && vma->resv) assert_vma_held(vma);

#endif

diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h index 6b48f5c42488..8df784a026d2 100644 --- a/drivers/gpu/drm/i915/i915_vma.h +++ b/drivers/gpu/drm/i915/i915_vma.h @@ -246,10 +246,22 @@ i915_vma_pin_ww(struct i915_vma *vma, struct i915_gem_ww_ctx *ww, static inline int __must_check i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags) { -#ifdef CONFIG_LOCKDEP

• WARN_ON_ONCE(vma->resv && dma_resv_held(vma->resv));

-#endif

• return i915_vma_pin_ww(vma, NULL, size, alignment, flags);

• struct i915_gem_ww_ctx ww;
• int err;
• i915_gem_ww_ctx_init(&ww, true);

+retry:

• err = i915_gem_object_lock(vma->obj, &ww);
• if (!err)

• err = i915_vma_pin_ww(vma, &ww, size, alignment, flags);
  

• if (err == -EDEADLK) {

• err = i915_gem_ww_ctx_backoff(&ww);
  

• if (!err)
  

• 	goto retry;
  

• }
• i915_gem_ww_ctx_fini(&ww);
• return err;

}

int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,

2.31.0

---

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