Am 14.09.21 um 14:47 schrieb Tvrtko Ursulin:

On 14/09/2021 13:32, Christian König wrote:
Am 14.09.21 um 14:27 schrieb Tvrtko Ursulin:

On 13/09/2021 14:16, Christian König wrote:
This is maybe even a fix since the RCU usage here looks incorrect.

What you think is incorrect? Pointless extra rcu locking?

Yeah, exactly that. I also wondered for a second if rcu_read_lock() can nest or not. But obviously it either works or lockdep hasn't complained yet.

But I've made a mistake here and at a couple of other places to remove to many rcu_read_lock() calls. Thanks for pointing that out, going to fix it as well.

Ack.

Also, FWIW, I submitted a patch to remove this function altogether since its IMO pretty useless, just failed in getting anyone to ack it so far.

I was on the edge of suggesting that as well since it's only debugfs usage looked quite pointless to me.

Feel free to CC me on the patch and you can have my acked-by.

Patch is here https://patchwork.freedesktop.org/patch/451864/?series=94202&rev=1, thanks!

Feel free to add an Acked-by: Christian König <christian.koe...@amd.com> to that one.

Regards,
Christian.


Regards,

Tvrtko

Thanks,
Christian.


Regards,

Tvrtko

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/gpu/drm/i915/gem/i915_gem_object.h | 15 +++++++--------
  1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index e9eecebf5c9d..3343922af4d6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -500,16 +500,15 @@ static inline struct intel_engine_cs *
  i915_gem_object_last_write_engine(struct drm_i915_gem_object *obj)
  {
      struct intel_engine_cs *engine = NULL;
+    struct dma_resv_cursor cursor;
      struct dma_fence *fence;
  -    rcu_read_lock();
-    fence = dma_resv_get_excl_unlocked(obj->base.resv);
-    rcu_read_unlock();
-
-    if (fence && dma_fence_is_i915(fence) && !dma_fence_is_signaled(fence))
-        engine = to_request(fence)->engine;
-    dma_fence_put(fence);
-
+    dma_resv_for_each_fence_unlocked(obj->base.resv, &cursor, false,
+                     fence) {
+        if (fence && dma_fence_is_i915(fence) &&
+            !dma_fence_is_signaled(fence))
+            engine = to_request(fence)->engine;
+    }
      return engine;
  }



Reply via email to