From 5c6c201ccbaf9d3901f829441d457293f7ca8ef4 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 4 Aug 2016 18:35:48 +0200 Subject: [PATCH] drm: Paper over locking inversion after registration rework drm_connector_register_all requires a few too many locks because our connector_list locking is busted. Add another FIXME+hack to work around this. This should address the below lockdep splat: ====================================================== [ INFO: possible circular locking dependency detected ] 4.7.0-rc5+ #524 Tainted: G O ------------------------------------------------------- kworker/u8:0/6 is trying to acquire lock: (&dev->mode_config.mutex){+.+.+.}, at: [] drm_modeset_lock_all+0x40/0x120 but task is already holding lock: ((fb_notifier_list).rwsem){++++.+}, at: [] __blocking_notifier_call_chain+0x35/0x70 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 ((fb_notifier_list).rwsem){++++.+}: [] lock_acquire+0xb1/0x200 [] down_write+0x44/0x80 [] blocking_notifier_chain_register+0x21/0xb0 [] fb_register_client+0x18/0x20 [] backlight_device_register+0x136/0x260 [] intel_backlight_device_register+0xa2/0x160 [i915] [] intel_connector_register+0xe/0x10 [i915] [] intel_dp_connector_register+0x1b/0x80 [i915] [] drm_connector_register+0x4a/0x80 [] drm_connector_register_all+0x64/0xf0 [] drm_modeset_register_all+0x174/0x1c0 [] drm_dev_register+0xc2/0xd0 [] i915_driver_load+0x1547/0x2200 [i915] [] i915_pci_probe+0x4f/0x70 [i915] [] local_pci_probe+0x45/0xa0 [] pci_device_probe+0xdb/0x130 [] driver_probe_device+0x223/0x440 [] __driver_attach+0xd5/0x100 [] bus_for_each_dev+0x66/0xa0 [] driver_attach+0x1e/0x20 [] bus_add_driver+0x1ee/0x280 [] driver_register+0x60/0xe0 [] __pci_register_driver+0x60/0x70 [] i915_init+0x5b/0x62 [i915] [] do_one_initcall+0x3d/0x150 [] do_init_module+0x5f/0x1d9 [] load_module+0x20e6/0x27e0 [] SYSC_finit_module+0xc3/0xf0 [] SyS_finit_module+0xe/0x10 [] entry_SYSCALL_64_fastpath+0x1c/0xac -> #0 (&dev->mode_config.mutex){+.+.+.}: [] __lock_acquire+0x10fc/0x1260 [] lock_acquire+0xb1/0x200 [] mutex_lock_nested+0x67/0x3c0 [] drm_modeset_lock_all+0x40/0x120 [] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80 [] drm_fb_helper_set_par+0x2d/0x50 [] intel_fbdev_set_par+0x1a/0x60 [i915] [] fbcon_init+0x586/0x610 [] visual_init+0xca/0x130 [] do_bind_con_driver+0x1c1/0x3a0 [] do_take_over_console+0x116/0x180 [] do_fbcon_takeover+0x57/0xb0 [] fbcon_event_notify+0x658/0x750 [] notifier_call_chain+0x3e/0xb0 [] __blocking_notifier_call_chain+0x4d/0x70 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x251/0x330 [] drm_fb_helper_initial_config+0x25f/0x3f0 [] intel_fbdev_initial_config+0x18/0x30 [i915] [] async_run_entry_fn+0x48/0x150 [] process_one_work+0x1e7/0x750 [] worker_thread+0x4b/0x4f0 [] kthread+0xef/0x110 [] ret_from_fork+0x1f/0x40 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock((fb_notifier_list).rwsem); lock(&dev->mode_config.mutex); lock((fb_notifier_list).rwsem); lock(&dev->mode_config.mutex); *** DEADLOCK *** 6 locks held by kworker/u8:0/6: #0: ("events_unbound"){.+.+.+}, at: [] process_one_work+0x169/0x750 #1: ((&entry->work)){+.+.+.}, at: [] process_one_work+0x169/0x750 #2: (registration_lock){+.+.+.}, at: [] register_framebuffer+0x27/0x330 #3: (console_lock){+.+.+.}, at: [] register_framebuffer+0x26e/0x330 #4: (&fb_info->lock){+.+.+.}, at: [] lock_fb_info+0x1d/0x40 #5: ((fb_notifier_list).rwsem){++++.+}, at: [] __blocking_notifier_call_chain+0x35/0x70 stack backtrace: CPU: 2 PID: 6 Comm: kworker/u8:0 Tainted: G O 4.7.0-rc5+ #524 Hardware name: Intel Corp. Broxton P/NOTEBOOK, BIOS APLKRVPA.X64.0138.B33.1606250842 06/25/2016 Workqueue: events_unbound async_run_entry_fn 0000000000000000 ffff8800758577f0 ffffffff814507a5 ffffffff828b9900 ffffffff828b9900 ffff880075857830 ffffffff810dc6fa ffff880075857880 ffff88007584d688 0000000000000005 0000000000000006 ffff88007584d6b0 Call Trace: [] dump_stack+0x67/0x92 [] print_circular_bug+0x1aa/0x200 [] __lock_acquire+0x10fc/0x1260 [] lock_acquire+0xb1/0x200 [] ? drm_modeset_lock_all+0x40/0x120 [] ? drm_modeset_lock_all+0x40/0x120 [] mutex_lock_nested+0x67/0x3c0 [] ? drm_modeset_lock_all+0x40/0x120 [] ? rcu_read_lock_sched_held+0x7f/0x90 [] ? kmem_cache_alloc_trace+0x248/0x2b0 [] ? drm_modeset_lock_all+0x25/0x120 [] drm_modeset_lock_all+0x40/0x120 [] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80 [] drm_fb_helper_set_par+0x2d/0x50 [] intel_fbdev_set_par+0x1a/0x60 [i915] [] fbcon_init+0x586/0x610 [] visual_init+0xca/0x130 [] do_bind_con_driver+0x1c1/0x3a0 [] do_take_over_console+0x116/0x180 [] do_fbcon_takeover+0x57/0xb0 [] fbcon_event_notify+0x658/0x750 [] notifier_call_chain+0x3e/0xb0 [] __blocking_notifier_call_chain+0x4d/0x70 [] blocking_notifier_call_chain+0x16/0x20 [] fb_notifier_call_chain+0x1b/0x20 [] register_framebuffer+0x251/0x330 [] ? vga_switcheroo_client_fb_set+0x5d/0x70 [] drm_fb_helper_initial_config+0x25f/0x3f0 [] intel_fbdev_initial_config+0x18/0x30 [i915] [] async_run_entry_fn+0x48/0x150 [] process_one_work+0x1e7/0x750 [] ? process_one_work+0x169/0x750 [] worker_thread+0x4b/0x4f0 [] ? process_one_work+0x750/0x750 [] kthread+0xef/0x110 [] ret_from_fork+0x1f/0x40 [] ? kthread_stop+0x2e0/0x2e0 v2: Rebase onto the right branch (hand-editing patches ftw) and add more reporters. Reported-by: Imre Deak Cc: Imre Deak Cc: Chris Wilson Acked-by: Chris Wilson Reported-by: Jiri Kosina Cc: Jiri Kosina Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_crtc.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index f1d9f0569d7f..b1dbb60af99f 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1121,16 +1121,14 @@ static int drm_connector_register_all(struct drm_device *dev) struct drm_connector *connector; int ret; - mutex_lock(&dev->mode_config.mutex); - - drm_for_each_connector(connector, dev) { + /* FIXME: taking the mode config mutex ends up in a clash with + * fbcon/backlight registration */ + list_for_each_entry(connector, &dev->mode_config.connector_list, head) { ret = drm_connector_register(connector); if (ret) goto err; } - mutex_unlock(&dev->mode_config.mutex); - return 0; err: -- 2.20.1