[PATCH] drm: Paper over locking inversion after registration rework

[PATCH] drm: Paper over locking inversion after registration rework

[Daniel Vetter]

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: [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120

but task is already holding lock:
 ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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){++++.+}:
       [<ffffffff810df611>] lock_acquire+0xb1/0x200
       [<ffffffff819a55b4>] down_write+0x44/0x80
       [<ffffffff810abf91>] blocking_notifier_chain_register+0x21/0xb0
       [<ffffffff814c7448>] fb_register_client+0x18/0x20
       [<ffffffff814c6c86>] backlight_device_register+0x136/0x260
       [<ffffffffa0127eb2>] intel_backlight_device_register+0xa2/0x160 [i915]
       [<ffffffffa00f46be>] intel_connector_register+0xe/0x10 [i915]
       [<ffffffffa0112bfb>] intel_dp_connector_register+0x1b/0x80 [i915]
       [<ffffffff8159dfea>] drm_connector_register+0x4a/0x80
       [<ffffffff8159fe44>] drm_connector_register_all+0x64/0xf0
       [<ffffffff815a2a64>] drm_modeset_register_all+0x174/0x1c0
       [<ffffffff81599b72>] drm_dev_register+0xc2/0xd0
       [<ffffffffa00621d7>] i915_driver_load+0x1547/0x2200 [i915]
       [<ffffffffa006d80f>] i915_pci_probe+0x4f/0x70 [i915]
       [<ffffffff814a2135>] local_pci_probe+0x45/0xa0
       [<ffffffff814a349b>] pci_device_probe+0xdb/0x130
       [<ffffffff815c07e3>] driver_probe_device+0x223/0x440
       [<ffffffff815c0ad5>] __driver_attach+0xd5/0x100
       [<ffffffff815be386>] bus_for_each_dev+0x66/0xa0
       [<ffffffff815c002e>] driver_attach+0x1e/0x20
       [<ffffffff815bf9be>] bus_add_driver+0x1ee/0x280
       [<ffffffff815c1810>] driver_register+0x60/0xe0
       [<ffffffff814a1a10>] __pci_register_driver+0x60/0x70
       [<ffffffffa01a905b>] i915_init+0x5b/0x62 [i915]
       [<ffffffff8100042d>] do_one_initcall+0x3d/0x150
       [<ffffffff811a935b>] do_init_module+0x5f/0x1d9
       [<ffffffff81124416>] load_module+0x20e6/0x27e0
       [<ffffffff81124d63>] SYSC_finit_module+0xc3/0xf0
       [<ffffffff81124dae>] SyS_finit_module+0xe/0x10
       [<ffffffff819a83a9>] entry_SYSCALL_64_fastpath+0x1c/0xac

-> #0 (&dev->mode_config.mutex){+.+.+.}:
       [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
       [<ffffffff810df611>] lock_acquire+0xb1/0x200
       [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
       [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
       [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
       [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
       [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
       [<ffffffff814c13c6>] fbcon_init+0x586/0x610
       [<ffffffff8154d16a>] visual_init+0xca/0x130
       [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
       [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
       [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
       [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
       [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
       [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
       [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
       [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
       [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
       [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
       [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
       [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
       [<ffffffff810a3947>] process_one_work+0x1e7/0x750
       [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
       [<ffffffff810aad4f>] kthread+0xef/0x110
       [<ffffffff819a85ef>] 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: [<ffffffff810a38c9>] process_one_work+0x169/0x750
 #1:  ((&entry->work)){+.+.+.}, at: [<ffffffff810a38c9>] process_one_work+0x169/0x750
 #2:  (registration_lock){+.+.+.}, at: [<ffffffff814c8487>] register_framebuffer+0x27/0x330
 #3:  (console_lock){+.+.+.}, at: [<ffffffff814c86ce>] register_framebuffer+0x26e/0x330
 #4:  (&fb_info->lock){+.+.+.}, at: [<ffffffff814c78dd>] lock_fb_info+0x1d/0x40
 #5:  ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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:
 [<ffffffff814507a5>] dump_stack+0x67/0x92
 [<ffffffff810dc6fa>] print_circular_bug+0x1aa/0x200
 [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
 [<ffffffff810df611>] lock_acquire+0xb1/0x200
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff810fa85f>] ? rcu_read_lock_sched_held+0x7f/0x90
 [<ffffffff81208218>] ? kmem_cache_alloc_trace+0x248/0x2b0
 [<ffffffff815afdc5>] ? drm_modeset_lock_all+0x25/0x120
 [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
 [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
 [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
 [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
 [<ffffffff814c13c6>] fbcon_init+0x586/0x610
 [<ffffffff8154d16a>] visual_init+0xca/0x130
 [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
 [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
 [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
 [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
 [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
 [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
 [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
 [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
 [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
 [<ffffffff815b7e8d>] ? vga_switcheroo_client_fb_set+0x5d/0x70
 [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
 [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
 [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
 [<ffffffff810a3947>] process_one_work+0x1e7/0x750
 [<ffffffff810a38c9>] ? process_one_work+0x169/0x750
 [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
 [<ffffffff810a3eb0>] ? process_one_work+0x750/0x750
 [<ffffffff810aad4f>] kthread+0xef/0x110
 [<ffffffff819a85ef>] ret_from_fork+0x1f/0x40
 [<ffffffff810aac60>] ? kthread_stop+0x2e0/0x2e0

Cc: Imre Deak <imre.deak@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_connector.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ef921fa09a84..d9104d8b3c6b 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -401,16 +401,14 @@ 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.8.1

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

Re: [PATCH] drm: Paper over locking inversion after registration rework

[Jani Nikula]

On Thu, 04 Aug 2016, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> 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: [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
>
> but task is already holding lock:
>  ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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){++++.+}:
>        [<ffffffff810df611>] lock_acquire+0xb1/0x200
>        [<ffffffff819a55b4>] down_write+0x44/0x80
>        [<ffffffff810abf91>] blocking_notifier_chain_register+0x21/0xb0
>        [<ffffffff814c7448>] fb_register_client+0x18/0x20
>        [<ffffffff814c6c86>] backlight_device_register+0x136/0x260
>        [<ffffffffa0127eb2>] intel_backlight_device_register+0xa2/0x160 [i915]
>        [<ffffffffa00f46be>] intel_connector_register+0xe/0x10 [i915]
>        [<ffffffffa0112bfb>] intel_dp_connector_register+0x1b/0x80 [i915]
>        [<ffffffff8159dfea>] drm_connector_register+0x4a/0x80
>        [<ffffffff8159fe44>] drm_connector_register_all+0x64/0xf0
>        [<ffffffff815a2a64>] drm_modeset_register_all+0x174/0x1c0
>        [<ffffffff81599b72>] drm_dev_register+0xc2/0xd0
>        [<ffffffffa00621d7>] i915_driver_load+0x1547/0x2200 [i915]
>        [<ffffffffa006d80f>] i915_pci_probe+0x4f/0x70 [i915]
>        [<ffffffff814a2135>] local_pci_probe+0x45/0xa0
>        [<ffffffff814a349b>] pci_device_probe+0xdb/0x130
>        [<ffffffff815c07e3>] driver_probe_device+0x223/0x440
>        [<ffffffff815c0ad5>] __driver_attach+0xd5/0x100
>        [<ffffffff815be386>] bus_for_each_dev+0x66/0xa0
>        [<ffffffff815c002e>] driver_attach+0x1e/0x20
>        [<ffffffff815bf9be>] bus_add_driver+0x1ee/0x280
>        [<ffffffff815c1810>] driver_register+0x60/0xe0
>        [<ffffffff814a1a10>] __pci_register_driver+0x60/0x70
>        [<ffffffffa01a905b>] i915_init+0x5b/0x62 [i915]
>        [<ffffffff8100042d>] do_one_initcall+0x3d/0x150
>        [<ffffffff811a935b>] do_init_module+0x5f/0x1d9
>        [<ffffffff81124416>] load_module+0x20e6/0x27e0
>        [<ffffffff81124d63>] SYSC_finit_module+0xc3/0xf0
>        [<ffffffff81124dae>] SyS_finit_module+0xe/0x10
>        [<ffffffff819a83a9>] entry_SYSCALL_64_fastpath+0x1c/0xac
>
> -> #0 (&dev->mode_config.mutex){+.+.+.}:
>        [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
>        [<ffffffff810df611>] lock_acquire+0xb1/0x200
>        [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
>        [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
>        [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
>        [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
>        [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
>        [<ffffffff814c13c6>] fbcon_init+0x586/0x610
>        [<ffffffff8154d16a>] visual_init+0xca/0x130
>        [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
>        [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
>        [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
>        [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
>        [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
>        [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
>        [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
>        [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
>        [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
>        [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
>        [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
>        [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
>        [<ffffffff810a3947>] process_one_work+0x1e7/0x750
>        [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
>        [<ffffffff810aad4f>] kthread+0xef/0x110
>        [<ffffffff819a85ef>] 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: [<ffffffff810a38c9>] process_one_work+0x169/0x750
>  #1:  ((&entry->work)){+.+.+.}, at: [<ffffffff810a38c9>] process_one_work+0x169/0x750
>  #2:  (registration_lock){+.+.+.}, at: [<ffffffff814c8487>] register_framebuffer+0x27/0x330
>  #3:  (console_lock){+.+.+.}, at: [<ffffffff814c86ce>] register_framebuffer+0x26e/0x330
>  #4:  (&fb_info->lock){+.+.+.}, at: [<ffffffff814c78dd>] lock_fb_info+0x1d/0x40
>  #5:  ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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:
>  [<ffffffff814507a5>] dump_stack+0x67/0x92
>  [<ffffffff810dc6fa>] print_circular_bug+0x1aa/0x200
>  [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
>  [<ffffffff810df611>] lock_acquire+0xb1/0x200
>  [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
>  [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
>  [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
>  [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
>  [<ffffffff810fa85f>] ? rcu_read_lock_sched_held+0x7f/0x90
>  [<ffffffff81208218>] ? kmem_cache_alloc_trace+0x248/0x2b0
>  [<ffffffff815afdc5>] ? drm_modeset_lock_all+0x25/0x120
>  [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
>  [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
>  [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
>  [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
>  [<ffffffff814c13c6>] fbcon_init+0x586/0x610
>  [<ffffffff8154d16a>] visual_init+0xca/0x130
>  [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
>  [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
>  [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
>  [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
>  [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
>  [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
>  [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
>  [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
>  [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
>  [<ffffffff815b7e8d>] ? vga_switcheroo_client_fb_set+0x5d/0x70
>  [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
>  [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
>  [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
>  [<ffffffff810a3947>] process_one_work+0x1e7/0x750
>  [<ffffffff810a38c9>] ? process_one_work+0x169/0x750
>  [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
>  [<ffffffff810a3eb0>] ? process_one_work+0x750/0x750
>  [<ffffffff810aad4f>] kthread+0xef/0x110
>  [<ffffffff819a85ef>] ret_from_fork+0x1f/0x40
>  [<ffffffff810aac60>] ? kthread_stop+0x2e0/0x2e0
>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Fixes: ?
Cc: drm-intel-fixes@lists.freedesktop.org
Cc: a bunch of people who reported the problem upstream?

BR,
Jani.

> ---
>  drivers/gpu/drm/drm_connector.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ef921fa09a84..d9104d8b3c6b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -401,16 +401,14 @@ 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:

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: failure for drm: Paper over locking inversion after registration rework

[Patchwork]

== Series Details ==

Series: drm: Paper over locking inversion after registration rework
URL   : https://patchwork.freedesktop.org/series/10653/
State : failure

== Summary ==

Applying: drm: Paper over locking inversion after registration rework
fatal: sha1 information is lacking or useless (drivers/gpu/drm/drm_connector.c).
error: could not build fake ancestor
Patch failed at 0001 drm: Paper over locking inversion after registration rework
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Paper over locking inversion after registration rework

[Chris Wilson]

On Thu, Aug 04, 2016 at 12:15:14PM +0200, Daniel Vetter wrote:
> 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:
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ef921fa09a84..d9104d8b3c6b 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -401,16 +401,14 @@ 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) {

Ok, double checked that the only time we delete from this list
(currently) is in cleanup. That's highly unlikely to be running at the
same time as the register, so

Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: Paper over locking inversion after registration rework

[Daniel Vetter]

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: [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120

but task is already holding lock:
 ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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){++++.+}:
       [<ffffffff810df611>] lock_acquire+0xb1/0x200
       [<ffffffff819a55b4>] down_write+0x44/0x80
       [<ffffffff810abf91>] blocking_notifier_chain_register+0x21/0xb0
       [<ffffffff814c7448>] fb_register_client+0x18/0x20
       [<ffffffff814c6c86>] backlight_device_register+0x136/0x260
       [<ffffffffa0127eb2>] intel_backlight_device_register+0xa2/0x160 [i915]
       [<ffffffffa00f46be>] intel_connector_register+0xe/0x10 [i915]
       [<ffffffffa0112bfb>] intel_dp_connector_register+0x1b/0x80 [i915]
       [<ffffffff8159dfea>] drm_connector_register+0x4a/0x80
       [<ffffffff8159fe44>] drm_connector_register_all+0x64/0xf0
       [<ffffffff815a2a64>] drm_modeset_register_all+0x174/0x1c0
       [<ffffffff81599b72>] drm_dev_register+0xc2/0xd0
       [<ffffffffa00621d7>] i915_driver_load+0x1547/0x2200 [i915]
       [<ffffffffa006d80f>] i915_pci_probe+0x4f/0x70 [i915]
       [<ffffffff814a2135>] local_pci_probe+0x45/0xa0
       [<ffffffff814a349b>] pci_device_probe+0xdb/0x130
       [<ffffffff815c07e3>] driver_probe_device+0x223/0x440
       [<ffffffff815c0ad5>] __driver_attach+0xd5/0x100
       [<ffffffff815be386>] bus_for_each_dev+0x66/0xa0
       [<ffffffff815c002e>] driver_attach+0x1e/0x20
       [<ffffffff815bf9be>] bus_add_driver+0x1ee/0x280
       [<ffffffff815c1810>] driver_register+0x60/0xe0
       [<ffffffff814a1a10>] __pci_register_driver+0x60/0x70
       [<ffffffffa01a905b>] i915_init+0x5b/0x62 [i915]
       [<ffffffff8100042d>] do_one_initcall+0x3d/0x150
       [<ffffffff811a935b>] do_init_module+0x5f/0x1d9
       [<ffffffff81124416>] load_module+0x20e6/0x27e0
       [<ffffffff81124d63>] SYSC_finit_module+0xc3/0xf0
       [<ffffffff81124dae>] SyS_finit_module+0xe/0x10
       [<ffffffff819a83a9>] entry_SYSCALL_64_fastpath+0x1c/0xac

-> #0 (&dev->mode_config.mutex){+.+.+.}:
       [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
       [<ffffffff810df611>] lock_acquire+0xb1/0x200
       [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
       [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
       [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
       [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
       [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
       [<ffffffff814c13c6>] fbcon_init+0x586/0x610
       [<ffffffff8154d16a>] visual_init+0xca/0x130
       [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
       [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
       [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
       [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
       [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
       [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
       [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
       [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
       [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
       [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
       [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
       [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
       [<ffffffff810a3947>] process_one_work+0x1e7/0x750
       [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
       [<ffffffff810aad4f>] kthread+0xef/0x110
       [<ffffffff819a85ef>] 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: [<ffffffff810a38c9>] process_one_work+0x169/0x750
 #1:  ((&entry->work)){+.+.+.}, at: [<ffffffff810a38c9>] process_one_work+0x169/0x750
 #2:  (registration_lock){+.+.+.}, at: [<ffffffff814c8487>] register_framebuffer+0x27/0x330
 #3:  (console_lock){+.+.+.}, at: [<ffffffff814c86ce>] register_framebuffer+0x26e/0x330
 #4:  (&fb_info->lock){+.+.+.}, at: [<ffffffff814c78dd>] lock_fb_info+0x1d/0x40
 #5:  ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810ac195>] __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:
 [<ffffffff814507a5>] dump_stack+0x67/0x92
 [<ffffffff810dc6fa>] print_circular_bug+0x1aa/0x200
 [<ffffffff810df0ac>] __lock_acquire+0x10fc/0x1260
 [<ffffffff810df611>] lock_acquire+0xb1/0x200
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff819a3097>] mutex_lock_nested+0x67/0x3c0
 [<ffffffff815afde0>] ? drm_modeset_lock_all+0x40/0x120
 [<ffffffff810fa85f>] ? rcu_read_lock_sched_held+0x7f/0x90
 [<ffffffff81208218>] ? kmem_cache_alloc_trace+0x248/0x2b0
 [<ffffffff815afdc5>] ? drm_modeset_lock_all+0x25/0x120
 [<ffffffff815afde0>] drm_modeset_lock_all+0x40/0x120
 [<ffffffff8158f79b>] drm_fb_helper_restore_fbdev_mode_unlocked+0x2b/0x80
 [<ffffffff8158f81d>] drm_fb_helper_set_par+0x2d/0x50
 [<ffffffffa0105f7a>] intel_fbdev_set_par+0x1a/0x60 [i915]
 [<ffffffff814c13c6>] fbcon_init+0x586/0x610
 [<ffffffff8154d16a>] visual_init+0xca/0x130
 [<ffffffff8154e611>] do_bind_con_driver+0x1c1/0x3a0
 [<ffffffff8154eaf6>] do_take_over_console+0x116/0x180
 [<ffffffff814bd3a7>] do_fbcon_takeover+0x57/0xb0
 [<ffffffff814c1e48>] fbcon_event_notify+0x658/0x750
 [<ffffffff810abcae>] notifier_call_chain+0x3e/0xb0
 [<ffffffff810ac1ad>] __blocking_notifier_call_chain+0x4d/0x70
 [<ffffffff810ac1e6>] blocking_notifier_call_chain+0x16/0x20
 [<ffffffff814c748b>] fb_notifier_call_chain+0x1b/0x20
 [<ffffffff814c86b1>] register_framebuffer+0x251/0x330
 [<ffffffff815b7e8d>] ? vga_switcheroo_client_fb_set+0x5d/0x70
 [<ffffffff8158fa9f>] drm_fb_helper_initial_config+0x25f/0x3f0
 [<ffffffffa0106b48>] intel_fbdev_initial_config+0x18/0x30 [i915]
 [<ffffffff810adfd8>] async_run_entry_fn+0x48/0x150
 [<ffffffff810a3947>] process_one_work+0x1e7/0x750
 [<ffffffff810a38c9>] ? process_one_work+0x169/0x750
 [<ffffffff810a3efb>] worker_thread+0x4b/0x4f0
 [<ffffffff810a3eb0>] ? process_one_work+0x750/0x750
 [<ffffffff810aad4f>] kthread+0xef/0x110
 [<ffffffff819a85ef>] ret_from_fork+0x1f/0x40
 [<ffffffff810aac60>] ? kthread_stop+0x2e0/0x2e0

v2: Rebase onto the right branch (hand-editing patches ftw) and add more
reporters.

Reported-by: Imre Deak <imre.deak@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
Reported-by: Jiri Kosina <jikos@kernel.org>
Cc: Jiri Kosina <jikos@kernel.org>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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 ef921fa09a84..d9104d8b3c6b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -401,16 +401,14 @@ 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.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Paper over locking inversion after registration rework

[Jiri Kosina]

On Thu, 4 Aug 2016, Daniel Vetter wrote:

> 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:
[ ... snip ... ]
> v2: Rebase onto the right branch (hand-editing patches ftw) and add more
> reporters.
> 
> Reported-by: Imre Deak <imre.deak@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reported-by: Jiri Kosina <jikos@kernel.org>

Although I have no idea why this is safe thing to do :) I can confirm that 
it removes the lockdep complaint on my system. So in that regard,

	Tested-by: Jiri Kosina <jkosina@suse.cz>

Thanks,

-- 
Jiri Kosina
SUSE Labs

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm: Paper over locking inversion after registration rework

[Daniel Vetter]

On Fri, Aug 05, 2016 at 10:57:58AM +0200, Jiri Kosina wrote:
> On Thu, 4 Aug 2016, Daniel Vetter wrote:
> 
> > 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:
> [ ... snip ... ]
> > v2: Rebase onto the right branch (hand-editing patches ftw) and add more
> > reporters.
> > 
> > Reported-by: Imre Deak <imre.deak@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Reported-by: Jiri Kosina <jikos@kernel.org>
> 
> Although I have no idea why this is safe thing to do :) I can confirm that 
> it removes the lockdep complaint on my system. So in that regard,
> 
> 	Tested-by: Jiri Kosina <jkosina@suse.cz>

Dave, can you pls pick this up for 4.8?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx