Skip to content

Commit cd06ab2

Browse files
committed
drm/locking: add backtrace for locking contended locks without backoff
If drm_modeset_lock() returns -EDEADLK, the caller is supposed to drop all currently held locks using drm_modeset_backoff(). Failing to do so will result in warnings and backtraces on the paths trying to lock a contended lock. Add support for optionally printing the backtrace on the path that hit the deadlock and didn't gracefully handle the situation. For example, the patch [1] inadvertently dropped the return value check and error return on replacing calc_watermark_data() with intel_compute_global_watermarks(). The backtraces on the subsequent locking paths hitting WARN_ON(ctx->contended) were unhelpful, but adding the backtrace to the deadlock path produced this helpful printout: <7> [98.002465] drm_modeset_lock attempting to lock a contended lock without backoff: drm_modeset_lock+0x107/0x130 drm_atomic_get_plane_state+0x76/0x150 skl_compute_wm+0x251d/0x2b20 [i915] intel_atomic_check+0x1942/0x29e0 [i915] drm_atomic_check_only+0x554/0x910 drm_atomic_nonblocking_commit+0xe/0x50 drm_mode_atomic_ioctl+0x8c2/0xab0 drm_ioctl_kernel+0xac/0x140 Add new CONFIG_DRM_DEBUG_MODESET_LOCK to enable modeset lock debugging with stack depot and trace. [1] https://lore.kernel.org/r/[email protected] v2: - default y if DEBUG_WW_MUTEX_SLOWPATH (Daniel) - depends on DEBUG_KERNEL Cc: Daniel Vetter <[email protected]> Cc: Dave Airlie <[email protected]> Reviewed-by: Daniel Vetter <[email protected]> Signed-off-by: Jani Nikula <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 91302d6 commit cd06ab2

File tree

3 files changed

+70
-2
lines changed

3 files changed

+70
-2
lines changed

drivers/gpu/drm/Kconfig

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,21 @@ config DRM_DEBUG_DP_MST_TOPOLOGY_REFS
100100
This has the potential to use a lot of memory and print some very
101101
large kernel messages. If in doubt, say "N".
102102

103+
config DRM_DEBUG_MODESET_LOCK
104+
bool "Enable backtrace history for lock contention"
105+
depends on STACKTRACE_SUPPORT
106+
depends on DEBUG_KERNEL
107+
depends on EXPERT
108+
select STACKDEPOT
109+
default y if DEBUG_WW_MUTEX_SLOWPATH
110+
help
111+
Enable debug tracing of failures to gracefully handle drm modeset lock
112+
contention. A history of each drm modeset lock path hitting -EDEADLK
113+
will be saved until gracefully handled, and the backtrace will be
114+
printed when attempting to lock a contended lock.
115+
116+
If in doubt, say "N".
117+
103118
config DRM_FBDEV_EMULATION
104119
bool "Enable legacy fbdev support for your modesetting driver"
105120
depends on DRM

drivers/gpu/drm/drm_modeset_lock.c

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <drm/drm_crtc.h>
2626
#include <drm/drm_device.h>
2727
#include <drm/drm_modeset_lock.h>
28+
#include <drm/drm_print.h>
2829

2930
/**
3031
* DOC: kms locking
@@ -77,6 +78,45 @@
7778

7879
static DEFINE_WW_CLASS(crtc_ww_class);
7980

81+
#if IS_ENABLED(CONFIG_DRM_DEBUG_MODESET_LOCK)
82+
static noinline depot_stack_handle_t __stack_depot_save(void)
83+
{
84+
unsigned long entries[8];
85+
unsigned int n;
86+
87+
n = stack_trace_save(entries, ARRAY_SIZE(entries), 1);
88+
89+
return stack_depot_save(entries, n, GFP_NOWAIT | __GFP_NOWARN);
90+
}
91+
92+
static void __stack_depot_print(depot_stack_handle_t stack_depot)
93+
{
94+
struct drm_printer p = drm_debug_printer("drm_modeset_lock");
95+
unsigned long *entries;
96+
unsigned int nr_entries;
97+
char *buf;
98+
99+
buf = kmalloc(PAGE_SIZE, GFP_NOWAIT | __GFP_NOWARN);
100+
if (!buf)
101+
return;
102+
103+
nr_entries = stack_depot_fetch(stack_depot, &entries);
104+
stack_trace_snprint(buf, PAGE_SIZE, entries, nr_entries, 2);
105+
106+
drm_printf(&p, "attempting to lock a contended lock without backoff:\n%s", buf);
107+
108+
kfree(buf);
109+
}
110+
#else /* CONFIG_DRM_DEBUG_MODESET_LOCK */
111+
static depot_stack_handle_t __stack_depot_save(void)
112+
{
113+
return 0;
114+
}
115+
static void __stack_depot_print(depot_stack_handle_t stack_depot)
116+
{
117+
}
118+
#endif /* CONFIG_DRM_DEBUG_MODESET_LOCK */
119+
80120
/**
81121
* drm_modeset_lock_all - take all modeset locks
82122
* @dev: DRM device
@@ -225,7 +265,9 @@ EXPORT_SYMBOL(drm_modeset_acquire_fini);
225265
*/
226266
void drm_modeset_drop_locks(struct drm_modeset_acquire_ctx *ctx)
227267
{
228-
WARN_ON(ctx->contended);
268+
if (WARN_ON(ctx->contended))
269+
__stack_depot_print(ctx->stack_depot);
270+
229271
while (!list_empty(&ctx->locked)) {
230272
struct drm_modeset_lock *lock;
231273

@@ -243,7 +285,8 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
243285
{
244286
int ret;
245287

246-
WARN_ON(ctx->contended);
288+
if (WARN_ON(ctx->contended))
289+
__stack_depot_print(ctx->stack_depot);
247290

248291
if (ctx->trylock_only) {
249292
lockdep_assert_held(&ctx->ww_ctx);
@@ -274,6 +317,7 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
274317
ret = 0;
275318
} else if (ret == -EDEADLK) {
276319
ctx->contended = lock;
320+
ctx->stack_depot = __stack_depot_save();
277321
}
278322

279323
return ret;
@@ -296,6 +340,7 @@ int drm_modeset_backoff(struct drm_modeset_acquire_ctx *ctx)
296340
struct drm_modeset_lock *contended = ctx->contended;
297341

298342
ctx->contended = NULL;
343+
ctx->stack_depot = 0;
299344

300345
if (WARN_ON(!contended))
301346
return 0;

include/drm/drm_modeset_lock.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
#ifndef DRM_MODESET_LOCK_H_
2525
#define DRM_MODESET_LOCK_H_
2626

27+
#include <linux/types.h> /* stackdepot.h is not self-contained */
28+
#include <linux/stackdepot.h>
2729
#include <linux/ww_mutex.h>
2830

2931
struct drm_modeset_lock;
@@ -51,6 +53,12 @@ struct drm_modeset_acquire_ctx {
5153
*/
5254
struct drm_modeset_lock *contended;
5355

56+
/*
57+
* Stack depot for debugging when a contended lock was not backed off
58+
* from.
59+
*/
60+
depot_stack_handle_t stack_depot;
61+
5462
/*
5563
* list of held locks (drm_modeset_lock)
5664
*/

0 commit comments

Comments
 (0)