diff options
author | Gu Zheng <guz.fnst@cn.fujitsu.com> | 2013-11-05 05:00:57 -0500 |
---|---|---|
committer | Tomi Valkeinen <tomi.valkeinen@ti.com> | 2013-11-11 08:52:59 -0500 |
commit | 3a41c5dbe8bc396a7fb16ca8739e945bb003342e (patch) | |
tree | 637757f9d320f138aa062b959b741e44fcce90ac /drivers/video/sh_mobile_lcdcfb.c | |
parent | deccd24f9077ccabc6b34f2e6d2f75c98b528fa1 (diff) |
fb: reorder the lock sequence to fix potential dead lock
Following commits:
50e244cc79 fb: rework locking to fix lock ordering on takeover
e93a9a8687 fb: Yet another band-aid for fixing lockdep mess
054430e773 fbcon: fix locking harder
reworked locking to fix related lock ordering on takeover, and introduced console_lock
into fbmem, but it seems that the new lock sequence(fb_info->lock ---> console_lock)
is against with the one in console_callback(console_lock ---> fb_info->lock), and leads to
a potential dead lock as following:
[ 601.079000] ======================================================
[ 601.079000] [ INFO: possible circular locking dependency detected ]
[ 601.079000] 3.11.0 #189 Not tainted
[ 601.079000] -------------------------------------------------------
[ 601.079000] kworker/0:3/619 is trying to acquire lock:
[ 601.079000] (&fb_info->lock){+.+.+.}, at: [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000]
but task is already holding lock:
[ 601.079000] (console_lock){+.+.+.}, at: [<ffffffff8141aae3>] console_callback+0x13/0x160
[ 601.079000]
which lock already depends on the new lock.
[ 601.079000]
the existing dependency chain (in reverse order) is:
[ 601.079000]
-> #1 (console_lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff810c6267>] console_lock+0x77/0x80
[ 601.079000] [<ffffffff81399448>] register_framebuffer+0x1d8/0x320
[ 601.079000] [<ffffffff81cfb4c8>] efifb_probe+0x408/0x48f
[ 601.079000] [<ffffffff8144a963>] platform_drv_probe+0x43/0x80
[ 601.079000] [<ffffffff8144853b>] driver_probe_device+0x8b/0x390
[ 601.079000] [<ffffffff814488eb>] __driver_attach+0xab/0xb0
[ 601.079000] [<ffffffff814463bd>] bus_for_each_dev+0x5d/0xa0
[ 601.079000] [<ffffffff81447e6e>] driver_attach+0x1e/0x20
[ 601.079000] [<ffffffff81447a07>] bus_add_driver+0x117/0x290
[ 601.079000] [<ffffffff81448fea>] driver_register+0x7a/0x170
[ 601.079000] [<ffffffff8144a10a>] __platform_driver_register+0x4a/0x50
[ 601.079000] [<ffffffff8144a12d>] platform_driver_probe+0x1d/0xb0
[ 601.079000] [<ffffffff81cfb0a1>] efifb_init+0x273/0x292
[ 601.079000] [<ffffffff81002132>] do_one_initcall+0x102/0x1c0
[ 601.079000] [<ffffffff81cb80a6>] kernel_init_freeable+0x15d/0x1ef
[ 601.079000] [<ffffffff8166d2de>] kernel_init+0xe/0xf0
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
-> #0 (&fb_info->lock){+.+.+.}:
[ 601.079000] [<ffffffff810dc1d8>] __lock_acquire+0x1e18/0x1f10
[ 601.079000] [<ffffffff810dc971>] lock_acquire+0xa1/0x140
[ 601.079000] [<ffffffff816835ca>] mutex_lock_nested+0x7a/0x3b0
[ 601.079000] [<ffffffff81397566>] lock_fb_info+0x26/0x60
[ 601.079000] [<ffffffff813a4aeb>] fbcon_blank+0x29b/0x2e0
[ 601.079000] [<ffffffff81418658>] do_blank_screen+0x1d8/0x280
[ 601.079000] [<ffffffff8141ab34>] console_callback+0x64/0x160
[ 601.079000] [<ffffffff8108d855>] process_one_work+0x1f5/0x540
[ 601.079000] [<ffffffff8108e04c>] worker_thread+0x11c/0x370
[ 601.079000] [<ffffffff81095fbd>] kthread+0xed/0x100
[ 601.079000] [<ffffffff816914ec>] ret_from_fork+0x7c/0xb0
[ 601.079000]
other info that might help us debug this:
[ 601.079000] Possible unsafe locking scenario:
[ 601.079000] CPU0 CPU1
[ 601.079000] ---- ----
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000] lock(console_lock);
[ 601.079000] lock(&fb_info->lock);
[ 601.079000]
*** DEADLOCK ***
so we reorder the lock sequence the same as it in console_callback() to
avoid this issue. And following Tomi's suggestion, fix these similar
issues all in fb subsystem.
Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Diffstat (limited to 'drivers/video/sh_mobile_lcdcfb.c')
-rw-r--r-- | drivers/video/sh_mobile_lcdcfb.c | 10 |
1 files changed, 6 insertions, 4 deletions
diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c index eaeae0fd0962..ab85ad6c25ec 100644 --- a/drivers/video/sh_mobile_lcdcfb.c +++ b/drivers/video/sh_mobile_lcdcfb.c | |||
@@ -574,8 +574,9 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch, | |||
574 | switch (event) { | 574 | switch (event) { |
575 | case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT: | 575 | case SH_MOBILE_LCDC_EVENT_DISPLAY_CONNECT: |
576 | /* HDMI plug in */ | 576 | /* HDMI plug in */ |
577 | console_lock(); | ||
577 | if (lock_fb_info(info)) { | 578 | if (lock_fb_info(info)) { |
578 | console_lock(); | 579 | |
579 | 580 | ||
580 | ch->display.width = monspec->max_x * 10; | 581 | ch->display.width = monspec->max_x * 10; |
581 | ch->display.height = monspec->max_y * 10; | 582 | ch->display.height = monspec->max_y * 10; |
@@ -594,19 +595,20 @@ static int sh_mobile_lcdc_display_notify(struct sh_mobile_lcdc_chan *ch, | |||
594 | fb_set_suspend(info, 0); | 595 | fb_set_suspend(info, 0); |
595 | } | 596 | } |
596 | 597 | ||
597 | console_unlock(); | 598 | |
598 | unlock_fb_info(info); | 599 | unlock_fb_info(info); |
599 | } | 600 | } |
601 | console_unlock(); | ||
600 | break; | 602 | break; |
601 | 603 | ||
602 | case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT: | 604 | case SH_MOBILE_LCDC_EVENT_DISPLAY_DISCONNECT: |
603 | /* HDMI disconnect */ | 605 | /* HDMI disconnect */ |
606 | console_lock(); | ||
604 | if (lock_fb_info(info)) { | 607 | if (lock_fb_info(info)) { |
605 | console_lock(); | ||
606 | fb_set_suspend(info, 1); | 608 | fb_set_suspend(info, 1); |
607 | console_unlock(); | ||
608 | unlock_fb_info(info); | 609 | unlock_fb_info(info); |
609 | } | 610 | } |
611 | console_unlock(); | ||
610 | break; | 612 | break; |
611 | 613 | ||
612 | case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: | 614 | case SH_MOBILE_LCDC_EVENT_DISPLAY_MODE: |