ACK/cmnt: [SRU][Bionic][PATCH 1/1] UBUNTU: SAUCE: drm/nouveau: Fix deadlock in nv50_mstm_register_connector()
Kleber Souza
kleber.souza at canonical.com
Wed May 16 10:45:45 UTC 2018
On 05/11/18 18:00, Joseph Salisbury wrote:
> From: Lyude <lyude at redhat.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1763189
>
> Currently; we're grabbing all of the modesetting locks before adding MST
> connectors to fbdev. This isn't actually necessary, and causes a
> deadlock as well:
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.17.0-rc3Lyude-Test+ #1 Tainted: G O
> ------------------------------------------------------
> kworker/1:0/18 is trying to acquire lock:
> 00000000c832f62d (&helper->lock){+.+.}, at: drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
>
> but task is already holding lock:
> 00000000942e28e2 (crtc_ww_class_mutex){+.+.}, at: drm_modeset_backoff+0x8e/0x1c0 [drm]
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (crtc_ww_class_mutex){+.+.}:
> ww_mutex_lock+0x43/0x80
> drm_modeset_lock+0x71/0x130 [drm]
> drm_helper_probe_single_connector_modes+0x7d/0x6b0 [drm_kms_helper]
> drm_setup_crtcs+0x15e/0xc90 [drm_kms_helper]
> __drm_fb_helper_initial_config_and_unlock+0x29/0x480 [drm_kms_helper]
> nouveau_fbcon_init+0x138/0x1a0 [nouveau]
> nouveau_drm_load+0x173/0x7e0 [nouveau]
> drm_dev_register+0x134/0x1c0 [drm]
> drm_get_pci_dev+0x8e/0x160 [drm]
> nouveau_drm_probe+0x1a9/0x230 [nouveau]
> pci_device_probe+0xcd/0x150
> driver_probe_device+0x30b/0x480
> __driver_attach+0xbc/0xe0
> bus_for_each_dev+0x67/0x90
> bus_add_driver+0x164/0x260
> driver_register+0x57/0xc0
> do_one_initcall+0x4d/0x323
> do_init_module+0x5b/0x1f8
> load_module+0x20e5/0x2ac0
> __do_sys_finit_module+0xb7/0xd0
> do_syscall_64+0x60/0x1b0
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> -> #2 (crtc_ww_class_acquire){+.+.}:
> drm_helper_probe_single_connector_modes+0x58/0x6b0 [drm_kms_helper]
> drm_setup_crtcs+0x15e/0xc90 [drm_kms_helper]
> __drm_fb_helper_initial_config_and_unlock+0x29/0x480 [drm_kms_helper]
> nouveau_fbcon_init+0x138/0x1a0 [nouveau]
> nouveau_drm_load+0x173/0x7e0 [nouveau]
> drm_dev_register+0x134/0x1c0 [drm]
> drm_get_pci_dev+0x8e/0x160 [drm]
> nouveau_drm_probe+0x1a9/0x230 [nouveau]
> pci_device_probe+0xcd/0x150
> driver_probe_device+0x30b/0x480
> __driver_attach+0xbc/0xe0
> bus_for_each_dev+0x67/0x90
> bus_add_driver+0x164/0x260
> driver_register+0x57/0xc0
> do_one_initcall+0x4d/0x323
> do_init_module+0x5b/0x1f8
> load_module+0x20e5/0x2ac0
> __do_sys_finit_module+0xb7/0xd0
> do_syscall_64+0x60/0x1b0
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> -> #1 (&dev->mode_config.mutex){+.+.}:
> drm_setup_crtcs+0x10c/0xc90 [drm_kms_helper]
> __drm_fb_helper_initial_config_and_unlock+0x29/0x480 [drm_kms_helper]
> nouveau_fbcon_init+0x138/0x1a0 [nouveau]
> nouveau_drm_load+0x173/0x7e0 [nouveau]
> drm_dev_register+0x134/0x1c0 [drm]
> drm_get_pci_dev+0x8e/0x160 [drm]
> nouveau_drm_probe+0x1a9/0x230 [nouveau]
> pci_device_probe+0xcd/0x150
> driver_probe_device+0x30b/0x480
> __driver_attach+0xbc/0xe0
> bus_for_each_dev+0x67/0x90
> bus_add_driver+0x164/0x260
> driver_register+0x57/0xc0
> do_one_initcall+0x4d/0x323
> do_init_module+0x5b/0x1f8
> load_module+0x20e5/0x2ac0
> __do_sys_finit_module+0xb7/0xd0
> do_syscall_64+0x60/0x1b0
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> -> #0 (&helper->lock){+.+.}:
> __mutex_lock+0x70/0x9d0
> drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> nv50_mstm_register_connector+0x2c/0x50 [nouveau]
> drm_dp_add_port+0x2f5/0x420 [drm_kms_helper]
> drm_dp_send_link_address+0x155/0x1e0 [drm_kms_helper]
> drm_dp_add_port+0x33f/0x420 [drm_kms_helper]
> drm_dp_send_link_address+0x155/0x1e0 [drm_kms_helper]
> drm_dp_check_and_send_link_address+0x87/0xd0 [drm_kms_helper]
> drm_dp_mst_link_probe_work+0x4d/0x80 [drm_kms_helper]
> process_one_work+0x20d/0x650
> worker_thread+0x3a/0x390
> kthread+0x11e/0x140
> ret_from_fork+0x3a/0x50
>
> other info that might help us debug this:
> Chain exists of:
> &helper->lock --> crtc_ww_class_acquire --> crtc_ww_class_mutex
> Possible unsafe locking scenario:
> CPU0 CPU1
> ---- ----
> lock(crtc_ww_class_mutex);
> lock(crtc_ww_class_acquire);
> lock(crtc_ww_class_mutex);
> lock(&helper->lock);
>
> *** DEADLOCK ***
> 5 locks held by kworker/1:0/18:
> #0: 000000004a05cd50 ((wq_completion)"events_long"){+.+.}, at: process_one_work+0x187/0x650
> #1: 00000000601c11d1 ((work_completion)(&mgr->work)){+.+.}, at: process_one_work+0x187/0x650
> #2: 00000000586ca0df (&dev->mode_config.mutex){+.+.}, at: drm_modeset_lock_all+0x3a/0x1b0 [drm]
> #3: 00000000d3ca0ffa (crtc_ww_class_acquire){+.+.}, at: drm_modeset_lock_all+0x44/0x1b0 [drm]
> #4: 00000000942e28e2 (crtc_ww_class_mutex){+.+.}, at: drm_modeset_backoff+0x8e/0x1c0 [drm]
>
> stack backtrace:
> CPU: 1 PID: 18 Comm: kworker/1:0 Tainted: G O 4.17.0-rc3Lyude-Test+ #1
> Hardware name: Gateway FX6840/FX6840, BIOS P01-A3 05/17/2010
> Workqueue: events_long drm_dp_mst_link_probe_work [drm_kms_helper]
> Call Trace:
> dump_stack+0x85/0xcb
> print_circular_bug.isra.38+0x1ce/0x1db
> __lock_acquire+0x128f/0x1350
> ? lock_acquire+0x9f/0x200
> ? lock_acquire+0x9f/0x200
> ? __ww_mutex_lock.constprop.13+0x8f/0x1000
> lock_acquire+0x9f/0x200
> ? drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> ? drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> __mutex_lock+0x70/0x9d0
> ? drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> ? ww_mutex_lock+0x43/0x80
> ? _cond_resched+0x15/0x30
> ? ww_mutex_lock+0x43/0x80
> ? drm_modeset_lock+0xb2/0x130 [drm]
> ? drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> drm_fb_helper_add_one_connector+0x2a/0x60 [drm_kms_helper]
> nv50_mstm_register_connector+0x2c/0x50 [nouveau]
> drm_dp_add_port+0x2f5/0x420 [drm_kms_helper]
> ? mark_held_locks+0x50/0x80
> ? kfree+0xcf/0x2a0
> ? drm_dp_check_mstb_guid+0xd6/0x120 [drm_kms_helper]
> ? trace_hardirqs_on_caller+0xed/0x180
> ? drm_dp_check_mstb_guid+0xd6/0x120 [drm_kms_helper]
> drm_dp_send_link_address+0x155/0x1e0 [drm_kms_helper]
> drm_dp_add_port+0x33f/0x420 [drm_kms_helper]
> ? nouveau_connector_aux_xfer+0x7c/0xb0 [nouveau]
> ? find_held_lock+0x2d/0x90
> ? drm_dp_dpcd_access+0xd9/0xf0 [drm_kms_helper]
> ? __mutex_unlock_slowpath+0x3b/0x280
> ? drm_dp_dpcd_access+0xd9/0xf0 [drm_kms_helper]
> drm_dp_send_link_address+0x155/0x1e0 [drm_kms_helper]
> drm_dp_check_and_send_link_address+0x87/0xd0 [drm_kms_helper]
> drm_dp_mst_link_probe_work+0x4d/0x80 [drm_kms_helper]
> process_one_work+0x20d/0x650
> worker_thread+0x3a/0x390
> ? process_one_work+0x650/0x650
> kthread+0x11e/0x140
> ? kthread_create_worker_on_cpu+0x50/0x50
> ret_from_fork+0x3a/0x50
>
> Taking example from i915, the only time we need to hold any modesetting
> locks is when changing the port on the mstc, and in that case we only
> need to hold the connection mutex.
>
> Signed-off-by: Lyude Paul <lyude at redhat.com>
> Cc: Karol Herbst <kherbst at redhat.com>
> Cc: stable at vger.kernel.org
> Signed-off-by: Lyude Paul <lyude at redhat.com>
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
This patch has landed upstream as 352672db857290ab5b0e2b6a99c414f92bee024c.
We can remove the "UBUNTU: SAUCE:" part and fix the SOB block when
applying the patch.
Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
>
> ---
> drivers/gpu/drm/nouveau/nv50_display.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 325bff4..7c9ecd3 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -3216,10 +3216,11 @@ nv50_mstm_destroy_connector(struct drm_dp_mst_topology_mgr *mgr,
>
> drm_connector_unregister(&mstc->connector);
>
> - drm_modeset_lock_all(drm->dev);
> drm_fb_helper_remove_one_connector(&drm->fbcon->helper, &mstc->connector);
> +
> + drm_modeset_lock(&drm->dev->mode_config.connection_mutex, NULL);
> mstc->port = NULL;
> - drm_modeset_unlock_all(drm->dev);
> + drm_modeset_unlock(&drm->dev->mode_config.connection_mutex);
>
> drm_connector_unreference(&mstc->connector);
> }
> @@ -3229,9 +3230,7 @@ nv50_mstm_register_connector(struct drm_connector *connector)
> {
> struct nouveau_drm *drm = nouveau_drm(connector->dev);
>
> - drm_modeset_lock_all(drm->dev);
> drm_fb_helper_add_one_connector(&drm->fbcon->helper, connector);
> - drm_modeset_unlock_all(drm->dev);
>
> drm_connector_register(connector);
> }
>
More information about the kernel-team
mailing list