[PATCH 2/2] drm/i915/active: Reorder nested locking
Janusz Krzysztofik
janusz.krzysztofik at linux.intel.com
Tue Jul 18 08:53:39 UTC 2023
Pre-merge results indicate locking dependency issue. Try with the
order of locks reverted.
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
drivers/gpu/drm/i915/i915_active.c | 39 ++++++++++++++++++------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 53c2c669ad003..38333831a360a 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -1059,14 +1059,13 @@ __i915_active_fence_set(struct i915_active_fence *active,
* Both A and B have got a reference to C or NULL, depending on the
* timing of the interrupt handler. Let's assume that if A has got C
* then it has locked C first (before B).
- *
- * Note the strong ordering of the timeline also provides consistent
- * nesting rules for the fence->lock; the inner lock is always the
- * older lock.
*/
- spin_lock_irqsave(fence->lock, flags);
- if (prev)
- spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
+ if (prev) {
+ spin_lock_irqsave(prev->lock, flags);
+ spin_lock_nested(fence->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ spin_lock_irqsave(fence->lock, flags);
+ }
/*
* A does the cmpxchg first, and so it sees C or NULL, as before, or
@@ -1080,17 +1079,22 @@ __i915_active_fence_set(struct i915_active_fence *active,
*/
while (!try_cmpxchg(__active_fence_slot(active), &prev, fence)) {
if (prev) {
- spin_unlock(prev->lock);
+ spin_unlock(fence->lock);
+ spin_unlock_irqrestore(prev->lock, flags);
dma_fence_put(prev);
+ } else {
+ spin_unlock_irqrestore(fence->lock, flags);
}
- spin_unlock_irqrestore(fence->lock, flags);
prev = i915_active_fence_get(active);
GEM_BUG_ON(prev == fence);
- spin_lock_irqsave(fence->lock, flags);
- if (prev)
- spin_lock_nested(prev->lock, SINGLE_DEPTH_NESTING);
+ if (prev) {
+ spin_lock_irqsave(prev->lock, flags);
+ spin_lock_nested(fence->lock, SINGLE_DEPTH_NESTING);
+ } else {
+ spin_lock_irqsave(fence->lock, flags);
+ }
}
/*
@@ -1105,12 +1109,15 @@ __i915_active_fence_set(struct i915_active_fence *active,
* it to complete its transition and takes over the occupancy for
* itself -- remembering that it needs to wait on A before executing.
*/
- if (prev) {
+ if (prev)
__list_del_entry(&active->cb.node);
- spin_unlock(prev->lock); /* serialise with prev->cb_list */
- }
list_add_tail(&active->cb.node, &fence->cb_list);
- spin_unlock_irqrestore(fence->lock, flags);
+ if (prev) {
+ spin_unlock(fence->lock);
+ spin_unlock_irqrestore(prev->lock, flags);
+ } else {
+ spin_unlock_irqrestore(fence->lock, flags);
+ }
return prev;
}
--
2.41.0
More information about the Intel-gfx-trybot
mailing list