[PATCH] drm: Reverse lock order in pan_display_legacy()

[Thomas Zimmermann <tzimmermann@suse.de>]

Acquiring drm_client_dev.modeset_mutex after the locks in drm_fb_helper.dev
creates a deadlock with drm_setup_crtcs() as shown below:

  [    4.959319] fbcon: radeondrmfb (fb0) is primary device
  [    4.993952] Console: switching to colour frame buffer device 240x67
  [    4.994040]
  [    4.994041] ======================================================
  [    4.994041] WARNING: possible circular locking dependency detected
  [    4.994042] 5.2.0-rc4-1-default+ #39 Tainted: G            E
  [    4.994043] ------------------------------------------------------
  [    4.994043] systemd-udevd/369 is trying to acquire lock:
  [    4.994044] 00000000fb622acb (&client->modeset_mutex){+.+.}, at: drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
  [    4.994055]
  [    4.994055] but task is already holding lock:
  [    4.994055] 0000000028767ae4 (crtc_ww_class_mutex){+.+.}, at: drm_modeset_lock+0x42/0xf0 [drm]
  [    4.994072]
  [    4.994072] which lock already depends on the new lock.
  [    4.994072]
  [    4.994072]
  [    4.994072] the existing dependency chain (in reverse order) is:
  [    4.994073]
  [    4.994073] -> #3 (crtc_ww_class_mutex){+.+.}:
  [    4.994076]        lock_acquire+0x9e/0x170
  [    4.994079]        __ww_mutex_lock.constprop.18+0x97/0xf40
  [    4.994080]        ww_mutex_lock+0x30/0x90
  [    4.994091]        drm_modeset_lock+0x42/0xf0 [drm]
  [    4.994102]        drm_modeset_lock_all_ctx+0x1f/0xe0 [drm]
  [    4.994113]        drm_modeset_lock_all+0x5e/0x1a0 [drm]
  [    4.994163]        intel_modeset_init+0x60b/0xda0 [i915]
  ..
  [    4.994253]
  [    4.994253] -> #2 (crtc_ww_class_acquire){+.+.}:
  [    4.994255]        lock_acquire+0x9e/0x170
  [    4.994270]        drm_modeset_acquire_init+0xcc/0x100 [drm]
  [    4.994280]        drm_modeset_lock_all+0x44/0x1a0 [drm]
  [    4.994320]        intel_modeset_init+0x60b/0xda0 [i915]
  ..
  [    4.994403]
  [    4.994403] -> #1 (&dev->mode_config.mutex){+.+.}:
  [    4.994405]        lock_acquire+0x9e/0x170
  [    4.994408]        __mutex_lock+0x62/0x8c0
  [    4.994413]        drm_setup_crtcs+0x17c/0xc50 [drm_kms_helper]
  [    4.994418]        __drm_fb_helper_initial_config_and_unlock+0x34/0x530 [drm_kms_helper]
  [    4.994450]        radeon_fbdev_init+0x110/0x130 [radeon]
  ..
  [    4.994535]
  [    4.994535] -> #0 (&client->modeset_mutex){+.+.}:
  [    4.994537]        __lock_acquire+0xa85/0xe90
  [    4.994538]        lock_acquire+0x9e/0x170
  [    4.994540]        __mutex_lock+0x62/0x8c0
  [    4.994545]        drm_fb_helper_pan_display+0x103/0x1f0 [drm_kms_helper]
  [    4.994547]        fb_pan_display+0x92/0x120
  [    4.994549]        bit_update_start+0x1a/0x40
  [    4.994550]        fbcon_switch+0x392/0x580
  [    4.994552]        redraw_screen+0x12c/0x220
  [    4.994553]        do_bind_con_driver.cold.30+0xe1/0x10d
  [    4.994554]        do_take_over_console+0x113/0x190
  [    4.994555]        do_fbcon_takeover+0x58/0xb0
  [    4.994557]        notifier_call_chain+0x47/0x70
  [    4.994558]        blocking_notifier_call_chain+0x44/0x60
  [    4.994559]        register_framebuffer+0x231/0x310
  [    4.994564]        __drm_fb_helper_initial_config_and_unlock+0x2fd/0x530 [drm_kms_helper]
  [    4.994590]        radeon_fbdev_init+0x110/0x130 [radeon]
  ..

This problem was introduced in

  d81294afe	drm/fb-helper: Remove drm_fb_helper_crtc

Reversing the lock ordering in pan_display_legacy() fixes the issue. The fix
was suggested by Daniel Vetter.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Fixes: d81294afeecdacc8d84804ba0bcb3d39e64d0f27
---
 drivers/gpu/drm/drm_fb_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7b388674a456..d6991f07cb17 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1586,8 +1586,8 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 	struct drm_mode_set *modeset;
 	int ret = 0;
 
-	drm_modeset_lock_all(fb_helper->dev);
 	mutex_lock(&client->modeset_mutex);
+	drm_modeset_lock_all(fb_helper->dev);
 	drm_client_for_each_modeset(modeset, client) {
 		modeset->x = var->xoffset;
 		modeset->y = var->yoffset;
@@ -1600,8 +1600,8 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
 			}
 		}
 	}
-	mutex_unlock(&client->modeset_mutex);
 	drm_modeset_unlock_all(fb_helper->dev);
+	mutex_unlock(&client->modeset_mutex);
 
 	return ret;
 }
-- 
2.21.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel