From: Sultan Alsawaf sultan@kerneltoast.com
Hi,
There's a mutex lock deadlock in i915 that only affects 5.4, but was fixed in 5.5. Normally, I would send a backport of the fix from 5.5, but the patch set that fixes the deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, I've made a small patch that only addresses the deadlock specifically for 5.4.
Thanks, Sultan
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Sultan Alsawaf (1): drm/i915: Fix ref->mutex deadlock in i915_active_wait()
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com --- drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 48e16ad93bbd..cfc77c08a273 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -120,13 +120,17 @@ static inline void debug_active_assert(struct i915_active *ref) { }
#endif
+#define I915_ACTIVE_RETIRE_NOLOCK BIT(0) + static void __active_retire(struct i915_active *ref) { struct active_node *it, *n; struct rb_root root; bool retire = false; + unsigned long bits;
+ ref = ptr_unpack_bits(ref, &bits, 2); lockdep_assert_held(&ref->mutex);
/* return the unused nodes to our slabcache -- flushing the allocator */ @@ -138,7 +142,8 @@ __active_retire(struct i915_active *ref) retire = true; }
- mutex_unlock(&ref->mutex); + if (!(bits & I915_ACTIVE_RETIRE_NOLOCK)) + mutex_unlock(&ref->mutex); if (!retire) return;
@@ -155,13 +160,18 @@ __active_retire(struct i915_active *ref) static void active_retire(struct i915_active *ref) { + struct i915_active *ref_packed = ref; + unsigned long bits; + + ref = ptr_unpack_bits(ref, &bits, 2); GEM_BUG_ON(!atomic_read(&ref->count)); if (atomic_add_unless(&ref->count, -1, 1)) return;
/* One active may be flushed from inside the acquire of another */ - mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); - __active_retire(ref); + if (!(bits & I915_ACTIVE_RETIRE_NOLOCK)) + mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); + __active_retire(ref_packed); }
static void @@ -170,6 +180,14 @@ node_retire(struct i915_active_request *base, struct i915_request *rq) active_retire(node_from_active(base)->ref); }
+static void +node_retire_nolock(struct i915_active_request *base, struct i915_request *rq) +{ + struct i915_active *ref = node_from_active(base)->ref; + + active_retire(ptr_pack_bits(ref, I915_ACTIVE_RETIRE_NOLOCK, 2)); +} + static struct i915_active_request * active_instance(struct i915_active *ref, struct intel_timeline *tl) { @@ -421,7 +439,8 @@ int i915_active_wait(struct i915_active *ref) break; }
- err = i915_active_request_retire(&it->base, BKL(ref)); + err = i915_active_request_retire(&it->base, BKL(ref), + node_retire_nolock); if (err) break; } diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index f95058f99057..0ad7ef60d15f 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -309,7 +309,7 @@ i915_active_request_isset(const struct i915_active_request *active) */ static inline int __must_check i915_active_request_retire(struct i915_active_request *active, - struct mutex *mutex) + struct mutex *mutex, i915_active_retire_fn retire) { struct i915_request *request; long ret; @@ -327,7 +327,7 @@ i915_active_request_retire(struct i915_active_request *active, list_del_init(&active->link); RCU_INIT_POINTER(active->request, NULL);
- active->retire(active, request); + retire(active, request);
return 0; }
Quoting Sultan Alsawaf (2020-04-07 07:26:22)
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
Incorrect.
You missed that it cannot retire from inside the wait due to the active reference held on the i915_active for the wait.
The only point it can enter retire from inside i915_active_wait() is via the terminal __active_retire() which releases the mutex in doing so. -Chris
On Tue, Apr 14, 2020 at 09:13:28AM +0100, Chris Wilson wrote:
Quoting Sultan Alsawaf (2020-04-07 07:26:22)
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
Incorrect.
You missed that it cannot retire from inside the wait due to the active reference held on the i915_active for the wait.
The only point it can enter retire from inside i915_active_wait() is via the terminal __active_retire() which releases the mutex in doing so. -Chris
The terminal __active_retire() and rbtree_postorder_for_each_entry_safe() loop retire different objects, so this isn't true.
Sultan
On Mon, Apr 06, 2020 at 11:26:21PM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
Hi,
There's a mutex lock deadlock in i915 that only affects 5.4, but was fixed in 5.5. Normally, I would send a backport of the fix from 5.5, but the patch set that fixes the deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, I've made a small patch that only addresses the deadlock specifically for 5.4.
This paragraph needs to go into the patch itself, otherwise just looking at that doesn't make any sense.
And you do not need a cover letter for a single patch.
Please fix up and resend.
thanks,
greg k-h
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com --- drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 48e16ad93bbd..cfc77c08a273 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -120,13 +120,17 @@ static inline void debug_active_assert(struct i915_active *ref) { }
#endif
+#define I915_ACTIVE_RETIRE_NOLOCK BIT(0) + static void __active_retire(struct i915_active *ref) { struct active_node *it, *n; struct rb_root root; bool retire = false; + unsigned long bits;
+ ref = ptr_unpack_bits(ref, &bits, 2); lockdep_assert_held(&ref->mutex);
/* return the unused nodes to our slabcache -- flushing the allocator */ @@ -138,7 +142,8 @@ __active_retire(struct i915_active *ref) retire = true; }
- mutex_unlock(&ref->mutex); + if (!(bits & I915_ACTIVE_RETIRE_NOLOCK)) + mutex_unlock(&ref->mutex); if (!retire) return;
@@ -155,13 +160,18 @@ __active_retire(struct i915_active *ref) static void active_retire(struct i915_active *ref) { + struct i915_active *ref_packed = ref; + unsigned long bits; + + ref = ptr_unpack_bits(ref, &bits, 2); GEM_BUG_ON(!atomic_read(&ref->count)); if (atomic_add_unless(&ref->count, -1, 1)) return;
/* One active may be flushed from inside the acquire of another */ - mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); - __active_retire(ref); + if (!(bits & I915_ACTIVE_RETIRE_NOLOCK)) + mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); + __active_retire(ref_packed); }
static void @@ -170,6 +180,14 @@ node_retire(struct i915_active_request *base, struct i915_request *rq) active_retire(node_from_active(base)->ref); }
+static void +node_retire_nolock(struct i915_active_request *base, struct i915_request *rq) +{ + struct i915_active *ref = node_from_active(base)->ref; + + active_retire(ptr_pack_bits(ref, I915_ACTIVE_RETIRE_NOLOCK, 2)); +} + static struct i915_active_request * active_instance(struct i915_active *ref, struct intel_timeline *tl) { @@ -421,7 +439,8 @@ int i915_active_wait(struct i915_active *ref) break; }
- err = i915_active_request_retire(&it->base, BKL(ref)); + err = i915_active_request_retire(&it->base, BKL(ref), + node_retire_nolock); if (err) break; } diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index f95058f99057..0ad7ef60d15f 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -309,7 +309,7 @@ i915_active_request_isset(const struct i915_active_request *active) */ static inline int __must_check i915_active_request_retire(struct i915_active_request *active, - struct mutex *mutex) + struct mutex *mutex, i915_active_retire_fn retire) { struct i915_request *request; long ret; @@ -327,7 +327,7 @@ i915_active_request_retire(struct i915_active_request *active, list_del_init(&active->link); RCU_INIT_POINTER(active->request, NULL);
- active->retire(active, request); + retire(active, request);
return 0; }
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com --- v3: The `ref` pointer can be unaligned, so we can't pack it. Shucks.
drivers/gpu/drm/i915/i915_active.c | 29 +++++++++++++++++++---------- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 48e16ad93bbd..51dc8753b527 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -121,7 +121,7 @@ static inline void debug_active_assert(struct i915_active *ref) { } #endif
static void -__active_retire(struct i915_active *ref) +__active_retire(struct i915_active *ref, bool lock) { struct active_node *it, *n; struct rb_root root; @@ -138,7 +138,8 @@ __active_retire(struct i915_active *ref) retire = true; }
- mutex_unlock(&ref->mutex); + if (likely(lock)) + mutex_unlock(&ref->mutex); if (!retire) return;
@@ -153,21 +154,28 @@ __active_retire(struct i915_active *ref) }
static void -active_retire(struct i915_active *ref) +active_retire(struct i915_active *ref, bool lock) { GEM_BUG_ON(!atomic_read(&ref->count)); if (atomic_add_unless(&ref->count, -1, 1)) return;
/* One active may be flushed from inside the acquire of another */ - mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); - __active_retire(ref); + if (likely(lock)) + mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); + __active_retire(ref, lock); }
static void node_retire(struct i915_active_request *base, struct i915_request *rq) { - active_retire(node_from_active(base)->ref); + active_retire(node_from_active(base)->ref, true); +} + +static void +node_retire_nolock(struct i915_active_request *base, struct i915_request *rq) +{ + active_retire(node_from_active(base)->ref, false); }
static struct i915_active_request * @@ -364,7 +372,7 @@ int i915_active_acquire(struct i915_active *ref) void i915_active_release(struct i915_active *ref) { debug_active_assert(ref); - active_retire(ref); + active_retire(ref, true); }
static void __active_ungrab(struct i915_active *ref) @@ -391,7 +399,7 @@ void i915_active_ungrab(struct i915_active *ref) { GEM_BUG_ON(!test_bit(I915_ACTIVE_GRAB_BIT, &ref->flags));
- active_retire(ref); + active_retire(ref, true); __active_ungrab(ref); }
@@ -421,12 +429,13 @@ int i915_active_wait(struct i915_active *ref) break; }
- err = i915_active_request_retire(&it->base, BKL(ref)); + err = i915_active_request_retire(&it->base, BKL(ref), + node_retire_nolock); if (err) break; }
- __active_retire(ref); + __active_retire(ref, true); if (err) return err;
diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index f95058f99057..0ad7ef60d15f 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -309,7 +309,7 @@ i915_active_request_isset(const struct i915_active_request *active) */ static inline int __must_check i915_active_request_retire(struct i915_active_request *active, - struct mutex *mutex) + struct mutex *mutex, i915_active_retire_fn retire) { struct i915_request *request; long ret; @@ -327,7 +327,7 @@ i915_active_request_retire(struct i915_active_request *active, list_del_init(&active->link); RCU_INIT_POINTER(active->request, NULL);
- active->retire(active, request); + retire(active, request);
return 0; }
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
Hi Greg,
Thanks for queuing this! However, you queued the v1 version of the patch instead of v2. Please pick the v2 version instead.
Thanks, Sultan
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked into Ubuntu so that's what we're rolling with.
Thanks, Sultan
[1] https://lore.kernel.org/stable/20200407203222.2493-1-sultan@kerneltoast.com
On Fri, Apr 10, 2020 at 07:17:38AM -0700, Sultan Alsawaf wrote:
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked into Ubuntu so that's what we're rolling with.
Ok, thanks, hopefully now I picked upthe right one...
greg k-h
Quoting Greg KH (2020-04-11 12:39:57)
On Fri, Apr 10, 2020 at 07:17:38AM -0700, Sultan Alsawaf wrote:
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked into Ubuntu so that's what we're rolling with.
Ok, thanks, hopefully now I picked upthe right one...
The patch does not fix a deadlock. Greg, this patch is not a backport of a bugfix, why is it in stable? -Chris
On Tue, Apr 14, 2020 at 09:15:07AM +0100, Chris Wilson wrote:
Quoting Greg KH (2020-04-11 12:39:57)
On Fri, Apr 10, 2020 at 07:17:38AM -0700, Sultan Alsawaf wrote:
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked into Ubuntu so that's what we're rolling with.
Ok, thanks, hopefully now I picked upthe right one...
The patch does not fix a deadlock. Greg, this patch is not a backport of a bugfix, why is it in stable?
Because it says it can't be a backport as the 3 above mentioned patches do the same thing instead?
I will be glad to drop this, but I need some kind of direction here, and given that at least one distro is already shipping this, it felt like the correct thing to do.
So, what do you want me to do?
thanks,
greg k-h
Quoting Greg KH (2020-04-14 11:23:44)
On Tue, Apr 14, 2020 at 09:15:07AM +0100, Chris Wilson wrote:
Quoting Greg KH (2020-04-11 12:39:57)
On Fri, Apr 10, 2020 at 07:17:38AM -0700, Sultan Alsawaf wrote:
On Fri, Apr 10, 2020 at 11:08:38AM +0200, Greg KH wrote:
On Tue, Apr 07, 2020 at 12:18:09AM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf sultan@kerneltoast.com
The following deadlock exists in i915_active_wait() due to a double lock on ref->mutex (call chain listed in order from top to bottom): i915_active_wait(); mutex_lock_interruptible(&ref->mutex); <-- ref->mutex first acquired i915_active_request_retire(); node_retire(); active_retire(); mutex_lock_nested(&ref->mutex, SINGLE_DEPTH_NESTING); <-- DEADLOCK
Fix the deadlock by skipping the second ref->mutex lock when active_retire() is called through i915_active_request_retire().
Note that this bug only affects 5.4 and has since been fixed in 5.5. Normally, a backport of the fix from 5.5 would be in order, but the patch set that fixes this deadlock involves massive changes that are neither feasible nor desirable for backporting [1][2][3]. Therefore, this small patch was made to address the deadlock specifically for 5.4.
[1] 274cbf20fd10 ("drm/i915: Push the i915_active.retire into a worker") [2] 093b92287363 ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree") [3] 750bde2fd4ff ("drm/i915: Serialise with remote retirement")
Fixes: 12c255b5dad1 ("drm/i915: Provide an i915_active.acquire callback") Cc: stable@vger.kernel.org # 5.4.x Signed-off-by: Sultan Alsawaf sultan@kerneltoast.com
drivers/gpu/drm/i915/i915_active.c | 27 +++++++++++++++++++++++---- drivers/gpu/drm/i915/i915_active.h | 4 ++-- 2 files changed, 25 insertions(+), 6 deletions(-)
Now queued up, thanks.
greg k-h
I'm sorry, I meant the v3 [1]. Apologies for the confusion. The v3 was picked into Ubuntu so that's what we're rolling with.
Ok, thanks, hopefully now I picked upthe right one...
The patch does not fix a deadlock. Greg, this patch is not a backport of a bugfix, why is it in stable?
Because it says it can't be a backport as the 3 above mentioned patches do the same thing instead?
Apologies for jumping late to the thread.
I will be glad to drop this, but I need some kind of direction here, and given that at least one distro is already shipping this, it felt like the correct thing to do.
So, what do you want me to do?
I think the the patch should be dropped for now before the issue is properly addressed. Either by backporting the mainline fixes or if those are too big and there indeed is a smaller alternative patch that is properly reviewed. But the above patch is not, at least yet.
There is an another similar thread where there's jumping into conclusions and doing ad-hoc patches for already fixed issues:
https://lore.kernel.org/dri-devel/20200414144309.GB2082@sultan-box.localdoma...
I appreciate enthusiasm to provide fixes to i915 but we should continue do the regular due diligence to make sure we're properly fixing bugs in upstream kernels. And when fixing them, to make sure we're not simply papering over them for a single use case.
It would be preferred to file a bug for the seen issues, describing how to reproduce them with vanilla upstream kernels:
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs
That way we have enough information to assess the severity of the bug and to prioritize the work accordingly.
Best Regards, Joonas
On Mon, Apr 20, 2020 at 12:02:39PM +0300, Joonas Lahtinen wrote:
I think the the patch should be dropped for now before the issue is properly addressed. Either by backporting the mainline fixes or if those are too big and there indeed is a smaller alternative patch that is properly reviewed. But the above patch is not, at least yet.
Why should a fix for a bona-fide issue be dropped due to political reasons? This doesn't make sense to me. This just hurts miserable i915 users even more. If my patch is going to be dropped, it should be replaced by a different fix at the same time.
Also, the mainline fixes just *happen* to fix this deadlock by removing the mutex lock from the path in question and creating multiple other bugs in the process that had to be addressed with "Fixes:" commits. The regression potential was too high to include those patches for a "stable" kernel, so I made this patch which fixes the issue in the simplest way possible. We put this patch into Ubuntu now as well, because praying for a response from i915 maintainers while the 20.04 release was on the horizon was not an option.
There is an another similar thread where there's jumping into conclusions and doing ad-hoc patches for already fixed issues:
https://lore.kernel.org/dri-devel/20200414144309.GB2082@sultan-box.localdoma...
Maybe this wouldn't have happened if I had received a proper response for that issue on gitlab from the get-go... Instead I got the run-around from Chris claiming that it wasn't an i915 bug:
https://gitlab.freedesktop.org/drm/intel/issues/1599
I appreciate enthusiasm to provide fixes to i915 but we should continue do the regular due diligence to make sure we're properly fixing bugs in upstream kernels. And when fixing them, to make sure we're not simply papering over them for a single use case.
It would be preferred to file a bug for the seen issues, describing how to reproduce them with vanilla upstream kernels:
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs
gitlab.freedesktop.org/drm/intel is where bugs go to be neglected, as noted above. I really see no reason to send anything there anymore, when the vast majority of community-sourced bug reports go ignored.
Sultan
Quoting Sultan Alsawaf (2020-04-20 18:42:16)
On Mon, Apr 20, 2020 at 12:02:39PM +0300, Joonas Lahtinen wrote:
I think the the patch should be dropped for now before the issue is properly addressed. Either by backporting the mainline fixes or if those are too big and there indeed is a smaller alternative patch that is properly reviewed. But the above patch is not, at least yet.
Why should a fix for a bona-fide issue be dropped due to political reasons? This doesn't make sense to me. This just hurts miserable i915 users even more. If my patch is going to be dropped, it should be replaced by a different fix at the same time.
There's no politics involved. It's all about doing the due diligence that we're fixing upstream bugs, and we're fixing them in a way that does not cause regressions to other users.
Without being able to reproduce a bug against vanilla kernel, there's too high of a risk that the patch that was developed will only work on the downstream kernel it was developed for. That happens for the best of the developers, and that is exactly why the process is in place, to avoid human error. So no politics, just due diligence.
If you could provide bug reproduction instructions by filing a bug, we can make forward progress in solving this issue. After assessing the severity of the bug and the amount of users involved, it will be prioritized accordingly. That is the most efficient way to get attention to a bug.
Also, the mainline fixes just *happen* to fix this deadlock by removing the mutex lock from the path in question and creating multiple other bugs in the process that had to be addressed with "Fixes:" commits. The regression potential was too high to include those patches for a "stable" kernel, so I made this patch which fixes the issue in the simplest way possible.
The thing is that it may be that the patch fixes the exact issue you have at hand in the downstream kernel you are testing against. But in doing so it may as well break other usecases for other users of vanilla kernel. That is what we're trying to avoid.
With the reproduction instructions, it'll be possible to check which kernel versions are affected, and after applying a fix to make sure that the bug is gone from those version. And if the reproduction can be trivialized to a test, we can introduce a regression check to CI.
A patch that claims to fix a deadlock in upstream kernel should include that splat from upstream kernel, not a speculated chain. Again, this is just the regular due diligence, because we have made errors in the past. It is for those self-made errors we know not to merge fixes too quickly before we are able to reproduce the error and make sure it is gone.
It's not about where the patch came from, it's about avoiding errors.
We put this patch into Ubuntu now as well, because praying for a response from i915 maintainers while the 20.04 release was on the horizon was not an option.
There is an another similar thread where there's jumping into conclusions and doing ad-hoc patches for already fixed issues:
https://lore.kernel.org/dri-devel/20200414144309.GB2082@sultan-box.localdoma...
Maybe this wouldn't have happened if I had received a proper response for that issue on gitlab from the get-go... Instead I got the run-around from Chris claiming that it wasn't an i915 bug:
https://gitlab.freedesktop.org/drm/intel/issues/1599
I appreciate enthusiasm to provide fixes to i915 but we should continue do the regular due diligence to make sure we're properly fixing bugs in upstream kernels. And when fixing them, to make sure we're not simply papering over them for a single use case.
It would be preferred to file a bug for the seen issues, describing how to reproduce them with vanilla upstream kernels:
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs
gitlab.freedesktop.org/drm/intel is where bugs go to be neglected, as noted above. I really see no reason to send anything there anymore, when the vast majority of community-sourced bug reports go ignored.
In the above bug, you claim to be booting vanilla kernel but the splat clearly says "5.4.28-00007-g64bb42e80256-dirty", so the developer correctly requested to bisect the error between 5.4.27 and 5.4.28 vanilla kernels, which you seem to have ignored and simply jumped to provide a patch.
Apologies if it feels like the bugs do not get enough attention, but we do our best to act on the reported bugs. You can best guarantee that your bug is getting the attention by providing all the details requested in the above link.
Without that information, it'll be hard to assess the severity of the bug. Above bug is missing critical pieces of information which help us in assessing the severity: 1. Is the bug reproducible on drm-tip? 2. How to reproduce? 3. How often does it reproduce? 4. Which hardware?
If that information is missing, it means that that some of our developers needs to find out all those bits of information before we can even assess the severity of the bug. And as we also have bugs where the information is present, those are often acted on first.
Again, no politics involved and no praying needed. We just have a process to follow to make sure we don't repeat our past mistakes as it's only humans who work on the bugs. At times it may feel rigid and not suited for the specific case where you feel there is a shorter route to produce a fix, but following the bug process helps us understand the problem and avoid trivial mistakes.
Regards, Joonas
On Tue, Apr 21, 2020 at 11:04:13AM +0300, Joonas Lahtinen wrote:
Quoting Sultan Alsawaf (2020-04-20 18:42:16)
On Mon, Apr 20, 2020 at 12:02:39PM +0300, Joonas Lahtinen wrote:
I think the the patch should be dropped for now before the issue is properly addressed. Either by backporting the mainline fixes or if those are too big and there indeed is a smaller alternative patch that is properly reviewed. But the above patch is not, at least yet.
Why should a fix for a bona-fide issue be dropped due to political reasons? This doesn't make sense to me. This just hurts miserable i915 users even more. If my patch is going to be dropped, it should be replaced by a different fix at the same time.
There's no politics involved. It's all about doing the due diligence that we're fixing upstream bugs, and we're fixing them in a way that does not cause regressions to other users.
Due diligence is the furthest thing from what I'd associate with i915. My cherryview laptop doesn't work anymore in 5.6 (it gets many GPU hangs; a RedHat engineer is working on fixing it according to the gitlab issue), my colleagues' laptops with 4k screens still suffer from GPU hangs (with fully documented gitlab reports that have gone completely unanswered), and I still get occasional FIFO underrun messages in dmesg accompanying graphical glitches on my own laptops. On top of that, PSR doesn't work correctly on any of my laptops, and it's enabled by default in i915. But when I reported the issue on gitlab, I got claims that my laptops' panels were bad, and that my reports were "not enough reason to disable PSR by default." [1]
[1] https://gitlab.freedesktop.org/drm/intel/-/issues/425#note_434182
Without being able to reproduce a bug against vanilla kernel, there's too high of a risk that the patch that was developed will only work on the downstream kernel it was developed for. That happens for the best of the developers, and that is exactly why the process is in place, to avoid human error. So no politics, just due diligence.
You could have reviewed the patch in the same amount of time it took you to write this email. It's very simple and obvious. That's why this sounds more like politics to me than "due diligence."
Also, the mainline fixes just *happen* to fix this deadlock by removing the mutex lock from the path in question and creating multiple other bugs in the process that had to be addressed with "Fixes:" commits. The regression potential was too high to include those patches for a "stable" kernel, so I made this patch which fixes the issue in the simplest way possible.
The thing is that it may be that the patch fixes the exact issue you have at hand in the downstream kernel you are testing against. But in doing so it may as well break other usecases for other users of vanilla kernel. That is what we're trying to avoid.
I don't know of any usecase that needs a double mutex lock (except for drm code that uses mutex_trylock_recursive of course), but alright.
A patch that claims to fix a deadlock in upstream kernel should include that splat from upstream kernel, not a speculated chain. Again, this is just the regular due diligence, because we have made errors in the past. It is for those self-made errors we know not to merge fixes too quickly before we are able to reproduce the error and make sure it is gone.
I sent the splat in a previous email. And you lot are all making many errors in the present, far more so than in the past because i915 used to work at some point. The sheer number of Chris' commits with subsequent "Fixes:" for his changes is mind numbing. For Ubuntu, we've had to backport a massive amount of i915 fixes to our 5.4 kernel thus far, and the major bugs still aren't all fixed (probably since those bugs still exist in Linus' tree). And we can't backport all of the relevant fixes we find because they're either contained in massive refactor commits or they only apply to i915 code that's been massively refactored since the last kernel release.
We put this patch into Ubuntu now as well, because praying for a response from i915 maintainers while the 20.04 release was on the horizon was not an option.
There is an another similar thread where there's jumping into conclusions and doing ad-hoc patches for already fixed issues:
https://lore.kernel.org/dri-devel/20200414144309.GB2082@sultan-box.localdoma...
Maybe this wouldn't have happened if I had received a proper response for that issue on gitlab from the get-go... Instead I got the run-around from Chris claiming that it wasn't an i915 bug:
https://gitlab.freedesktop.org/drm/intel/issues/1599
I appreciate enthusiasm to provide fixes to i915 but we should continue do the regular due diligence to make sure we're properly fixing bugs in upstream kernels. And when fixing them, to make sure we're not simply papering over them for a single use case.
It would be preferred to file a bug for the seen issues, describing how to reproduce them with vanilla upstream kernels:
https://gitlab.freedesktop.org/drm/intel/-/wikis/How-to-file-i915-bugs
gitlab.freedesktop.org/drm/intel is where bugs go to be neglected, as noted above. I really see no reason to send anything there anymore, when the vast majority of community-sourced bug reports go ignored.
In the above bug, you claim to be booting vanilla kernel but the splat clearly says "5.4.28-00007-g64bb42e80256-dirty", so the developer correctly requested to bisect the error between 5.4.27 and 5.4.28 vanilla kernels, which you seem to have ignored and simply jumped to provide a patch.
Telling me to bisect between 5.4.27 and 5.4.28 after claiming it's not an i915 bug is a total waste of time. There weren't even any i915 changes in 5.4.28, and the assumption that it's "not i915" is a load of crap too because the bug in question had a very sane crash: `ring->vaddr` was clearly zero, like I said in my description. I "jumped to provide a patch" because we have customers to answer to when i915 doesn't work, and I wasn't going to waste time running in circles to appease the maintainer when I could fix the problem myself.
Apologies if it feels like the bugs do not get enough attention, but we do our best to act on the reported bugs. You can best guarantee that your bug is getting the attention by providing all the details requested in the above link.
Why hasn't this bug got any attention: https://gitlab.freedesktop.org/drm/intel/issues/1412
That seems like a showstopper.
What about this bug? https://gitlab.freedesktop.org/drm/intel/issues/1605
That's the bug that makes my cherryview machine unusable. A RedHat engineer is working on fixing that instead of an i915 maintainer... that doesn't seem right.
Someone else reported that bug here: https://gitlab.freedesktop.org/drm/intel/issues/1585
And Chris gave a very helpful, "Despite all that it is still probably userspace fouling the memory" in response. Even though the problem doesn't occur in older kernels.
Without that information, it'll be hard to assess the severity of the bug. Above bug is missing critical pieces of information which help us in assessing the severity: 1. Is the bug reproducible on drm-tip? 2. How to reproduce? 3. How often does it reproduce? 4. Which hardware?
If that information is missing, it means that that some of our developers needs to find out all those bits of information before we can even assess the severity of the bug. And as we also have bugs where the information is present, those are often acted on first.
Why not ask for that? I was never asked for that information. A splat should've been enough to identify the problem anyway.
Again, no politics involved and no praying needed.
I don't think I'll stop praying for i915 to work based on the current state of affairs, thanks.
Sultan
On Tue, Apr 21, 2020 at 09:38:09AM -0700, Sultan Alsawaf wrote:
Why hasn't this bug got any attention: https://gitlab.freedesktop.org/drm/intel/issues/1412
That seems like a showstopper.
Indeed, pretty shocking. It's worth mentioning that the reporter of said bug, after significant time had elapsed, removed i915 from the kernel entirely and now daily drives the NVIDIA binary blob. Having to fall back to closed source blobs indicates a pretty awful state of affairs. It might otherwise be hard to imagine how that could be preferable, but this situation indicates how.
Joonas - More generally, it seems what's happening now is that i915 driver stability and quality is reaching a real low point. Back when i915 was the only stable driver in town, the ivory tower attitude of the maintainers seemed quasi-justifiable. The drivers kept being awesome, so when they kicked the can back at users or gave them the cold shoulder on reports, there was never much of an issue, since things always got fixed in fairly short order. This is no longer the case. Bugs are piling up. Users are unhappy. So it's only natural that some users will just wind up fixing it themselves, especially with responses from Intel like "not guilty!" in response to i915 bug reports. And these users who try to fix these bugs will do so without access to your proprietary debuggers, top-secret data sheets, and NDA'd hardware engineers down the hall, and thus you'll always be able to accuse them of something or another about "due-diligence", since nobody is better suited than you to find and fix these issues in the best way possible. But it hasn't been happening in a satisfactory way from your end, and users need their laptops to actually work, and so things will gradually get fixed (hopefully, anyhow) through other means. Even on the stable front, if fixes to bugs are intermingled into massive refactoring patches, and your team isn't taking charge to separate them out and send them to stable@, then backporting to stable kernels is going to result in a lot of custom work from other people. In other words, you can shun your users' bug reports and fellow developers and get away with that while your driver quality remains tip-top, but if you let things fall, as they have as of late, then expect your ivory tower to be shaken a bit.
On Tue, Apr 14, 2020 at 09:15:07AM +0100, Chris Wilson wrote:
The patch does not fix a deadlock. Greg, this patch is not a backport of a bugfix, why is it in stable? -Chris
Here's the deadlock this supposedly doesn't fix: INFO: task kswapd0:178 blocked for more than 122 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kswapd0 D 0 178 2 0x80004000 Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 ? i915_request_wait+0x25b/0x370 active_retire+0x26/0x30 i915_active_wait+0xa3/0x1a0 i915_vma_unbind+0xe2/0x1c0 i915_gem_object_unbind+0x111/0x140 i915_gem_shrink+0x21b/0x530 i915_gem_shrinker_scan+0xfd/0x120 do_shrink_slab+0x154/0x2c0 shrink_slab+0xd0/0x2f0 shrink_node+0xdf/0x420 balance_pgdat+0x2e3/0x540 kswapd+0x200/0x3c0 ? __wake_up_common_lock+0xc0/0xc0 kthread+0xfb/0x130 ? balance_pgdat+0x540/0x540 ? __kthread_parkme+0x60/0x60 ret_from_fork+0x1f/0x40 INFO: task kworker/u32:5:222 blocked for more than 122 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u32:5 D 0 222 2 0x80004000 Workqueue: i915 idle_work_handler Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 idle_work_handler+0x34/0x120 process_one_work+0x1ea/0x3a0 worker_thread+0x4d/0x3f0 kthread+0xfb/0x130 ? process_one_work+0x3a0/0x3a0 ? __kthread_parkme+0x60/0x60 ret_from_fork+0x1f/0x40 INFO: task mpv:1535 blocked for more than 122 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. mpv D 0 1535 1 0x00000000 Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 __i915_gem_free_objects+0x68/0x190 i915_gem_create_ioctl+0x18/0x30 ? i915_gem_dumb_create+0xa0/0xa0 drm_ioctl_kernel+0xb2/0x100 drm_ioctl+0x209/0x360 ? i915_gem_dumb_create+0xa0/0xa0 do_vfs_ioctl+0x43f/0x6c0 ksys_ioctl+0x5e/0x90 __x64_sys_ioctl+0x16/0x20 do_syscall_64+0x4e/0x140 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7fb49f1b32eb Code: Bad RIP value. RSP: 002b:00007ffef9eb0948 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00007ffef9eb09c0 RCX: 00007fb49f1b32eb RDX: 00007ffef9eb09c0 RSI: 00000000c010645b RDI: 0000000000000008 RBP: 00000000c010645b R08: 000055fdb80c1370 R09: 000055fdb80c14e0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb4781e56b0 R13: 0000000000000008 R14: 00007fb4781e5560 R15: 00007fb4781e56b0 INFO: task kswapd0:178 blocked for more than 245 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kswapd0 D 0 178 2 0x80004000 Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 ? i915_request_wait+0x25b/0x370 active_retire+0x26/0x30 i915_active_wait+0xa3/0x1a0 i915_vma_unbind+0xe2/0x1c0 i915_gem_object_unbind+0x111/0x140 i915_gem_shrink+0x21b/0x530 i915_gem_shrinker_scan+0xfd/0x120 do_shrink_slab+0x154/0x2c0 shrink_slab+0xd0/0x2f0 shrink_node+0xdf/0x420 balance_pgdat+0x2e3/0x540 kswapd+0x200/0x3c0 ? __wake_up_common_lock+0xc0/0xc0 kthread+0xfb/0x130 ? balance_pgdat+0x540/0x540 ? __kthread_parkme+0x60/0x60 ret_from_fork+0x1f/0x40 INFO: task kworker/u32:5:222 blocked for more than 245 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u32:5 D 0 222 2 0x80004000 Workqueue: i915 idle_work_handler Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 idle_work_handler+0x34/0x120 process_one_work+0x1ea/0x3a0 worker_thread+0x4d/0x3f0 kthread+0xfb/0x130 ? process_one_work+0x3a0/0x3a0 ? __kthread_parkme+0x60/0x60 ret_from_fork+0x1f/0x40 INFO: task mpv:1535 blocked for more than 245 seconds. Tainted: G U 5.4.28-00014-gd1e04f91d2c5 #4 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. mpv D 0 1535 1 0x00000000 Call Trace: ? __schedule+0x2f3/0x750 schedule+0x39/0xa0 schedule_preempt_disabled+0xa/0x10 __mutex_lock.isra.0+0x19b/0x500 __i915_gem_free_objects+0x68/0x190 i915_gem_create_ioctl+0x18/0x30 ? i915_gem_dumb_create+0xa0/0xa0 drm_ioctl_kernel+0xb2/0x100 drm_ioctl+0x209/0x360 ? i915_gem_dumb_create+0xa0/0xa0 do_vfs_ioctl+0x43f/0x6c0 ksys_ioctl+0x5e/0x90 __x64_sys_ioctl+0x16/0x20 do_syscall_64+0x4e/0x140 entry_SYSCALL_64_after_hwframe+0x44/0xa9 RIP: 0033:0x7fb49f1b32eb Code: Bad RIP value. RSP: 002b:00007ffef9eb0948 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 00007ffef9eb09c0 RCX: 00007fb49f1b32eb RDX: 00007ffef9eb09c0 RSI: 00000000c010645b RDI: 0000000000000008 RBP: 00000000c010645b R08: 000055fdb80c1370 R09: 000055fdb80c14e0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb4781e56b0 R13: 0000000000000008 R14: 00007fb4781e5560 R15: 00007fb4781e56b0
Dead inside the shrinker, and very easy to reproduce.
Sultan
linux-stable-mirror@lists.linaro.org
-
Chris Wilson -
Greg KH -
Greg KH -
Jason A. Donenfeld -
Joonas Lahtinen -
Sultan Alsawaf