Re: [PATCH 1/2] drm/i915: Fix assert in i915_ggtt_pin

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

 




On 25/02/2022 17:58, Ville Syrjälä wrote:
On Fri, Feb 25, 2022 at 05:41:17PM +0000, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Use lockdep_assert_not_held to simplify and correct the code. Otherwise
false positive are hit if lock state is uknown like after a previous
taint.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reported-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_vma.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 3558b16a929c..4469b7f52853 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1552,9 +1552,7 @@ int i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
  	if (ww)
  		return __i915_ggtt_pin(vma, ww, align, flags);
-#ifdef CONFIG_LOCKDEP
-	WARN_ON(dma_resv_held(vma->obj->base.resv));
-#endif
+	lockdep_assert_not_held(&vma->obj->base.resv->lock.base);

Should there be a dma_resv wrapper for that? Shrug.

Probably.. it is really ugly being this verbose.

Makes sense to me:
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>

I'll leave the other one to someone how knows how it's actually used.

Yeah I think that one is likely wrong. If lockdep is compiled in but not enabled it would make some asserts fire since state will be 'unknown' then and so it would always say "not locked" to the callers.

Regards,

Tvrtko


for_i915_gem_ww(&_ww, err, true) {
  		err = i915_gem_object_lock(vma->obj, &_ww);
--
2.32.0




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

  Powered by Linux