BUG: circular locking dependency detected

BUG: circular locking dependency detected

[Russell King]

This looks like a bug in the framebuffer/console layers.  Looks like
we have one path where we call the notifier list, and a called
function takes the console lock, and another path where we hold the
console lock while calling the notifier list.

===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc4+ #656 Not tainted
-------------------------------------------------------
kworker/0:1/442 is trying to acquire lock:
 ((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
       [<c006f36c>] __lock_acquire+0x1d20/0x1e80
       [<c006fa04>] lock_acquire+0x68/0x7c
       [<c002a894>] console_lock+0x5c/0x70
       [<c01c0adc>] register_con_driver+0x44/0x150
       [<c01c1158>] take_over_console+0x24/0x3b4
       [<c019d778>] fbcon_takeover+0x70/0xd4
       [<c01a3108>] fbcon_event_notify+0x7c8/0x818
       [<c004e538>] notifier_call_chain+0x4c/0x8c
       [<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
       [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
       [<c0196e70>] fb_notifier_call_chain+0x20/0x28
       [<c0198194>] register_framebuffer+0x18c/0x238
       [<c01a6148>] clcdfb_probe+0x2b0/0x3c0
       [<c01a6da4>] amba_probe+0x88/0xa0
       [<c01d1730>] driver_probe_device+0x84/0x218
       [<c01d1960>] __driver_attach+0x9c/0xa0
       [<c01cfe5c>] bus_for_each_dev+0x5c/0x88
       [<c01d1398>] driver_attach+0x20/0x28
       [<c01d0ddc>] bus_add_driver+0xa4/0x244
       [<c01d1ed4>] driver_register+0x80/0x14c
       [<c01a6848>] amba_driver_register+0x48/0x5c
       [<c043aae0>] amba_clcdfb_init+0x28/0x3c
       [<c000868c>] do_one_initcall+0x44/0x1ac
       [<c0425928>] kernel_init_freeable+0x104/0x1c8
       [<c03242ec>] kernel_init+0x10/0xec
       [<c0014510>] ret_from_fork+0x14/0x24

-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
       [<c006bc44>] print_circular_bug+0x84/0x2f0
       [<c006f458>] __lock_acquire+0x1e0c/0x1e80
       [<c006fa04>] lock_acquire+0x68/0x7c
       [<c032b3a0>] down_read+0x34/0x44
       [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
       [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
       [<c0196e70>] fb_notifier_call_chain+0x20/0x28
       [<c0197690>] fb_blank+0x40/0xac
       [<c019f874>] fbcon_blank+0x1f4/0x29c
       [<c01c09e0>] do_blank_screen+0x1b8/0x270
       [<c01c2ba8>] console_callback+0x74/0x138
       [<c00408c8>] process_one_work+0x1b4/0x4ec
       [<c0043610>] worker_thread+0x17c/0x4bc
       [<c004891c>] kthread+0xb0/0xbc
       [<c0014510>] ret_from_fork+0x14/0x24

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(console_lock);
                               lock((fb_notifier_list).rwsem);
                               lock(console_lock);
  lock((fb_notifier_list).rwsem);

 *** DEADLOCK ***

3 locks held by kworker/0:1/442:
 #0:  (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
 #1:  (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
 #2:  (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138

stack backtrace:
Backtrace:
[<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
 r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
[<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
[<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
[<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
[<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
 r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
[<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
 r5:ffffffff r4:c04715cc
[<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
 r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
[<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
[<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
[<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
 r6:00000001 r5:cf80a000 r4:cfb5f800
[<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
[<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
 r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
[<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
 r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
[<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
[<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
[<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
 r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8

--
Russell King

BUG: circular locking dependency detected

[Russell King]

This looks like a bug in the framebuffer/console layers.  Looks like
we have one path where we call the notifier list, and a called
function takes the console lock, and another path where we hold the
console lock while calling the notifier list.

===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc4+ #656 Not tainted
-------------------------------------------------------
kworker/0:1/442 is trying to acquire lock:
 ((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
       [<c006f36c>] __lock_acquire+0x1d20/0x1e80
       [<c006fa04>] lock_acquire+0x68/0x7c
       [<c002a894>] console_lock+0x5c/0x70
       [<c01c0adc>] register_con_driver+0x44/0x150
       [<c01c1158>] take_over_console+0x24/0x3b4
       [<c019d778>] fbcon_takeover+0x70/0xd4
       [<c01a3108>] fbcon_event_notify+0x7c8/0x818
       [<c004e538>] notifier_call_chain+0x4c/0x8c
       [<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
       [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
       [<c0196e70>] fb_notifier_call_chain+0x20/0x28
       [<c0198194>] register_framebuffer+0x18c/0x238
       [<c01a6148>] clcdfb_probe+0x2b0/0x3c0
       [<c01a6da4>] amba_probe+0x88/0xa0
       [<c01d1730>] driver_probe_device+0x84/0x218
       [<c01d1960>] __driver_attach+0x9c/0xa0
       [<c01cfe5c>] bus_for_each_dev+0x5c/0x88
       [<c01d1398>] driver_attach+0x20/0x28
       [<c01d0ddc>] bus_add_driver+0xa4/0x244
       [<c01d1ed4>] driver_register+0x80/0x14c
       [<c01a6848>] amba_driver_register+0x48/0x5c
       [<c043aae0>] amba_clcdfb_init+0x28/0x3c
       [<c000868c>] do_one_initcall+0x44/0x1ac
       [<c0425928>] kernel_init_freeable+0x104/0x1c8
       [<c03242ec>] kernel_init+0x10/0xec
       [<c0014510>] ret_from_fork+0x14/0x24

-> #0 ((fb_notifier_list).rwsem){.+.+.+}:
       [<c006bc44>] print_circular_bug+0x84/0x2f0
       [<c006f458>] __lock_acquire+0x1e0c/0x1e80
       [<c006fa04>] lock_acquire+0x68/0x7c
       [<c032b3a0>] down_read+0x34/0x44
       [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
       [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
       [<c0196e70>] fb_notifier_call_chain+0x20/0x28
       [<c0197690>] fb_blank+0x40/0xac
       [<c019f874>] fbcon_blank+0x1f4/0x29c
       [<c01c09e0>] do_blank_screen+0x1b8/0x270
       [<c01c2ba8>] console_callback+0x74/0x138
       [<c00408c8>] process_one_work+0x1b4/0x4ec
       [<c0043610>] worker_thread+0x17c/0x4bc
       [<c004891c>] kthread+0xb0/0xbc
       [<c0014510>] ret_from_fork+0x14/0x24

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(console_lock);
                               lock((fb_notifier_list).rwsem);
                               lock(console_lock);
  lock((fb_notifier_list).rwsem);

 *** DEADLOCK ***

3 locks held by kworker/0:1/442:
 #0:  (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
 #1:  (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
 #2:  (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138

stack backtrace:
Backtrace:
[<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
 r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
[<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
[<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
[<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
[<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
 r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
[<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
 r5:ffffffff r4:c04715cc
[<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
 r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
[<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
[<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
[<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
 r6:00000001 r5:cf80a000 r4:cfb5f800
[<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
[<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
 r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
[<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
 r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
[<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
[<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
[<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
 r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Russell King]

Also adding Greg and Daniel to this as Daniel introduced the lockdep
checking.

This looks extremely horrid to be to solve - the paths are rather deep
where the dependency occurs.  The two paths between the locks are:

console_lock+0x5c/0x70
register_con_driver+0x44/0x150
take_over_console+0x24/0x3b4
fbcon_takeover+0x70/0xd4
fbcon_event_notify+0x7c8/0x818
notifier_call_chain+0x4c/0x8c
__blocking_notifier_call_chain+0x50/0x68
blocking_notifier_call_chain+0x20/0x28

and

__blocking_notifier_call_chain+0x34/0x68
blocking_notifier_call_chain+0x20/0x28
fb_notifier_call_chain+0x20/0x28
fb_blank+0x40/0xac
fbcon_blank+0x1f4/0x29c
do_blank_screen+0x1b8/0x270
console_callback+0x74/0x138

On Wed, Jan 30, 2013 at 08:06:48PM +0000, Russell King wrote:
> This looks like a bug in the framebuffer/console layers.  Looks like
> we have one path where we call the notifier list, and a called
> function takes the console lock, and another path where we hold the
> console lock while calling the notifier list.
> 
> ===========================
> [ INFO: possible circular locking dependency detected ]
> 3.8.0-rc4+ #656 Not tainted
> -------------------------------------------------------
> kworker/0:1/442 is trying to acquire lock:
>  ((fb_notifier_list).rwsem){.+.+.+}, at: [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
> 
> but task is already holding lock:
>  (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (console_lock){+.+.+.}:
>        [<c006f36c>] __lock_acquire+0x1d20/0x1e80
>        [<c006fa04>] lock_acquire+0x68/0x7c
>        [<c002a894>] console_lock+0x5c/0x70
>        [<c01c0adc>] register_con_driver+0x44/0x150
>        [<c01c1158>] take_over_console+0x24/0x3b4
>        [<c019d778>] fbcon_takeover+0x70/0xd4
>        [<c01a3108>] fbcon_event_notify+0x7c8/0x818
>        [<c004e538>] notifier_call_chain+0x4c/0x8c
>        [<c004ea64>] __blocking_notifier_call_chain+0x50/0x68
>        [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
>        [<c0196e70>] fb_notifier_call_chain+0x20/0x28
>        [<c0198194>] register_framebuffer+0x18c/0x238
>        [<c01a6148>] clcdfb_probe+0x2b0/0x3c0
>        [<c01a6da4>] amba_probe+0x88/0xa0
>        [<c01d1730>] driver_probe_device+0x84/0x218
>        [<c01d1960>] __driver_attach+0x9c/0xa0
>        [<c01cfe5c>] bus_for_each_dev+0x5c/0x88
>        [<c01d1398>] driver_attach+0x20/0x28
>        [<c01d0ddc>] bus_add_driver+0xa4/0x244
>        [<c01d1ed4>] driver_register+0x80/0x14c
>        [<c01a6848>] amba_driver_register+0x48/0x5c
>        [<c043aae0>] amba_clcdfb_init+0x28/0x3c
>        [<c000868c>] do_one_initcall+0x44/0x1ac
>        [<c0425928>] kernel_init_freeable+0x104/0x1c8
>        [<c03242ec>] kernel_init+0x10/0xec
>        [<c0014510>] ret_from_fork+0x14/0x24
> 
> -> #0 ((fb_notifier_list).rwsem){.+.+.+}:
>        [<c006bc44>] print_circular_bug+0x84/0x2f0
>        [<c006f458>] __lock_acquire+0x1e0c/0x1e80
>        [<c006fa04>] lock_acquire+0x68/0x7c
>        [<c032b3a0>] down_read+0x34/0x44
>        [<c004ea48>] __blocking_notifier_call_chain+0x34/0x68
>        [<c004ea9c>] blocking_notifier_call_chain+0x20/0x28
>        [<c0196e70>] fb_notifier_call_chain+0x20/0x28
>        [<c0197690>] fb_blank+0x40/0xac
>        [<c019f874>] fbcon_blank+0x1f4/0x29c
>        [<c01c09e0>] do_blank_screen+0x1b8/0x270
>        [<c01c2ba8>] console_callback+0x74/0x138
>        [<c00408c8>] process_one_work+0x1b4/0x4ec
>        [<c0043610>] worker_thread+0x17c/0x4bc
>        [<c004891c>] kthread+0xb0/0xbc
>        [<c0014510>] ret_from_fork+0x14/0x24
> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(console_lock);
>                                lock((fb_notifier_list).rwsem);
>                                lock(console_lock);
>   lock((fb_notifier_list).rwsem);
> 
>  *** DEADLOCK ***
> 
> 3 locks held by kworker/0:1/442:
>  #0:  (events){.+.+..}, at: [<c0040854>] process_one_work+0x140/0x4ec
>  #1:  (console_work){+.+...}, at: [<c0040854>] process_one_work+0x140/0x4ec
>  #2:  (console_lock){+.+.+.}, at: [<c01c2b48>] console_callback+0x14/0x138
> 
> stack backtrace:
> Backtrace:
> [<c00185d8>] (dump_backtrace+0x0/0x10c) from [<c03294c8>] (dump_stack+0x18/0x1c)
>  r6:c05323f0 r5:c0524800 r4:c05323f0 r3:cf8f6b80
> [<c03294b0>] (dump_stack+0x0/0x1c) from [<c006bda4>] (print_circular_bug+0x1e4/0x2f0)
> [<c006bbc0>] (print_circular_bug+0x0/0x2f0) from [<c006f458>] (__lock_acquire+0x1e0c/0x1e80)
> [<c006d64c>] (__lock_acquire+0x0/0x1e80) from [<c006fa04>] (lock_acquire+0x68/0x7c)
> [<c006f99c>] (lock_acquire+0x0/0x7c) from [<c032b3a0>] (down_read+0x34/0x44)
>  r7:00000010 r6:cfb7dd20 r5:00000002 r4:c04715cc
> [<c032b36c>] (down_read+0x0/0x44) from [<c004ea48>] (__blocking_notifier_call_chain+0x34/0x68)
>  r5:ffffffff r4:c04715cc
> [<c004ea14>] (__blocking_notifier_call_chain+0x0/0x68) from [<c004ea9c>] (blocking_notifier_call_chain+0x20/0x28)
>  r7:cf0ffc00 r6:00000001 r5:cfb7dd20 r4:cfb5f800
> [<c004ea7c>] (blocking_notifier_call_chain+0x0/0x28) from [<c0196e70>] (fb_notifier_call_chain+0x20/0x28)
> [<c0196e50>] (fb_notifier_call_chain+0x0/0x28) from [<c0197690>] (fb_blank+0x40/0xac)
> [<c0197650>] (fb_blank+0x0/0xac) from [<c019f874>] (fbcon_blank+0x1f4/0x29c)
>  r6:00000001 r5:cf80a000 r4:cfb5f800
> [<c019f680>] (fbcon_blank+0x0/0x29c) from [<c01c09e0>] (do_blank_screen+0x1b8/0x270)
> [<c01c0828>] (do_blank_screen+0x0/0x270) from [<c01c2ba8>] (console_callback+0x74/0x138)
>  r7:c0ba8640 r6:c0bac300 r5:c099a51c r4:c099a51c
> [<c01c2b34>] (console_callback+0x0/0x138) from [<c00408c8>] (process_one_work+0x1b4/0x4ec)
>  r6:c0bac300 r5:cf9489c0 r4:c0472e3c r3:c01c2b34
> [<c0040714>] (process_one_work+0x0/0x4ec) from [<c0043610>] (worker_thread+0x17c/0x4bc)
> [<c0043494>] (worker_thread+0x0/0x4bc) from [<c004891c>] (kthread+0xb0/0xbc)
> [<c004886c>] (kthread+0x0/0xbc) from [<c0014510>] (ret_from_fork+0x14/0x24)
>  r8:00000000 r7:00000000 r6:00000000 r5:c004886c r4:cf84dde8
> 
> -- 
> Russell King
>  Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
>  maintainer of:

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Daniel Vetter]

On Wed, Jan 30, 2013 at 10:52 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> Also adding Greg and Daniel to this as Daniel introduced the lockdep
> checking.
>
> This looks extremely horrid to be to solve - the paths are rather deep
> where the dependency occurs.  The two paths between the locks are:
>
> console_lock+0x5c/0x70
> register_con_driver+0x44/0x150
> take_over_console+0x24/0x3b4
> fbcon_takeover+0x70/0xd4
> fbcon_event_notify+0x7c8/0x818
> notifier_call_chain+0x4c/0x8c
> __blocking_notifier_call_chain+0x50/0x68
> blocking_notifier_call_chain+0x20/0x28
>
> and
>
> __blocking_notifier_call_chain+0x34/0x68
> blocking_notifier_call_chain+0x20/0x28
> fb_notifier_call_chain+0x20/0x28
> fb_blank+0x40/0xac
> fbcon_blank+0x1f4/0x29c
> do_blank_screen+0x1b8/0x270
> console_callback+0x74/0x138

You want Dave Airlie's pile of locking reworks, which fixes all
currently known offenders around console_lock and fb_notifier. Patches
won't go into 3.9 since it took a few rounds until they did not cause
regression by making these deadlocks easier to hit.

http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes

Long term solution would be to abolish the fb_notifier, at least for
the purpose of linking fbdevs up with the fbcon and just replace those
with direct function calls. But that requires that we no longer allow
fbdev drivers and the fbcon to be loaded in any arbitrary order. Or
just force fbcon to be built-in if enabled, imo the sane choice (no
one's bothering with config_vt=m either, after all).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: BUG: circular locking dependency detected

[Russell King]

On Wed, Jan 30, 2013 at 11:07:16PM +0100, Daniel Vetter wrote:
> On Wed, Jan 30, 2013 at 10:52 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> > Also adding Greg and Daniel to this as Daniel introduced the lockdep
> > checking.
> >
> > This looks extremely horrid to be to solve - the paths are rather deep
> > where the dependency occurs.  The two paths between the locks are:
> >
> > console_lock+0x5c/0x70
> > register_con_driver+0x44/0x150
> > take_over_console+0x24/0x3b4
> > fbcon_takeover+0x70/0xd4
> > fbcon_event_notify+0x7c8/0x818
> > notifier_call_chain+0x4c/0x8c
> > __blocking_notifier_call_chain+0x50/0x68
> > blocking_notifier_call_chain+0x20/0x28
> >
> > and
> >
> > __blocking_notifier_call_chain+0x34/0x68
> > blocking_notifier_call_chain+0x20/0x28
> > fb_notifier_call_chain+0x20/0x28
> > fb_blank+0x40/0xac
> > fbcon_blank+0x1f4/0x29c
> > do_blank_screen+0x1b8/0x270
> > console_callback+0x74/0x138
> 
> You want Dave Airlie's pile of locking reworks, which fixes all
> currently known offenders around console_lock and fb_notifier. Patches
> won't go into 3.9 since it took a few rounds until they did not cause
> regression by making these deadlocks easier to hit.
> 
> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
> 
> Long term solution would be to abolish the fb_notifier, at least for
> the purpose of linking fbdevs up with the fbcon and just replace those
> with direct function calls. But that requires that we no longer allow
> fbdev drivers and the fbcon to be loaded in any arbitrary order. Or
> just force fbcon to be built-in if enabled, imo the sane choice (no
> one's bothering with config_vt=m either, after all).

So... what you seem to be telling me is that 3.9 is going to be a
release which issues lockdep complaints when the console blanks, and
you think that's acceptable?

Adding Linus and Andrew so they're aware of this issue...

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Daniel Vetter]

On Wed, Jan 30, 2013 at 11:19 PM, Russell King <rmk@arm.linux.org.uk> wrote:
> So... what you seem to be telling me is that 3.9 is going to be a
> release which issues lockdep complaints when the console blanks, and
> you think that's acceptable?
>
> Adding Linus and Andrew so they're aware of this issue...

Linus was the guy who shot down the patches for 3.9 since one of the
earlier iterations caused instant deadlocks - I've been pushing them
to merge them ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: BUG: circular locking dependency detected

[Linus Torvalds]

On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>
> So... what you seem to be telling me is that 3.9 is going to be a
> release which issues lockdep complaints when the console blanks, and
> you think that's acceptable?
>
> Adding Linus and Andrew so they're aware of this issue...

Oh, we're extremely aware of it. And it's not a new issue, the locking
problem have apparently been around forever, although I'm not sure why
the lockdep splat itself started happening only recently.

They'll make it into 3.9, it's 3.8 that won't have them. The patches
initially caused way *worse* behavior than just a lockdep splat - they
caused actual hard lockups (and that was *after* the initial series of
fixes). That got fixed (hopefully for the last case!) fairly recently,
and I'm not willing to take the scary patch-series that has had
several problem cases.

              LInus

Re: BUG: circular locking dependency detected

[Dave Airlie]

On Thu, Jan 31, 2013 at 9:52 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>>
>> So... what you seem to be telling me is that 3.9 is going to be a
>> release which issues lockdep complaints when the console blanks, and
>> you think that's acceptable?
>>
>> Adding Linus and Andrew so they're aware of this issue...
>
> Oh, we're extremely aware of it. And it's not a new issue, the locking
> problem have apparently been around forever, although I'm not sure why
> the lockdep splat itself started happening only recently.
>
> They'll make it into 3.9, it's 3.8 that won't have them. The patches
> initially caused way *worse* behavior than just a lockdep splat - they
> caused actual hard lockups (and that was *after* the initial series of
> fixes). That got fixed (hopefully for the last case!) fairly recently,
> and I'm not willing to take the scary patch-series that has had
> several problem cases.

Well we didn't have any lock validation support before Daniel added it
a couple of kernels back,
so instead of hidden locking problems we've had from time began, we now have
lockdep detectable locking problems.

Dave.

Re: BUG: circular locking dependency detected

[Russell King]

On Thu, Jan 31, 2013 at 10:04:05AM +1000, Dave Airlie wrote:
> On Thu, Jan 31, 2013 at 9:52 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >>
> >> So... what you seem to be telling me is that 3.9 is going to be a
> >> release which issues lockdep complaints when the console blanks, and
> >> you think that's acceptable?
> >>
> >> Adding Linus and Andrew so they're aware of this issue...
> >
> > Oh, we're extremely aware of it. And it's not a new issue, the locking
> > problem have apparently been around forever, although I'm not sure why
> > the lockdep splat itself started happening only recently.
> >
> > They'll make it into 3.9, it's 3.8 that won't have them. The patches
> > initially caused way *worse* behavior than just a lockdep splat - they
> > caused actual hard lockups (and that was *after* the initial series of
> > fixes). That got fixed (hopefully for the last case!) fairly recently,
> > and I'm not willing to take the scary patch-series that has had
> > several problem cases.
> 
> Well we didn't have any lock validation support before Daniel added it
> a couple of kernels back,
> so instead of hidden locking problems we've had from time began, we now have
> lockdep detectable locking problems.

Which may or may not be a good thing depending how you look at it; it
means that once your kernel blanks, you get a lockdep dump.  At that
point you lose lockdep checking for everything else because lockdep
disables itself after the first dump.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Linus Torvalds]

On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>
> Which may or may not be a good thing depending how you look at it; it
> means that once your kernel blanks, you get a lockdep dump.  At that
> point you lose lockdep checking for everything else because lockdep
> disables itself after the first dump.

Fair enough, we may want to revert the lockdep checking for
console_lock, and make re-enabling it part of the patch-series that
fixes the locking.

Daniel/Dave? Does that sound reasonable?

              Linus

Re: BUG: circular locking dependency detected

[Greg Kroah-Hartman]

On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >
> > Which may or may not be a good thing depending how you look at it; it
> > means that once your kernel blanks, you get a lockdep dump.  At that
> > point you lose lockdep checking for everything else because lockdep
> > disables itself after the first dump.
> 
> Fair enough, we may want to revert the lockdep checking for
> console_lock, and make re-enabling it part of the patch-series that
> fixes the locking.
> 
> Daniel/Dave? Does that sound reasonable?

Reverting the patch is fine with me.  Just let me know so I can queue it
up again for 3.9.

thanks,

greg k-h

Re: BUG: circular locking dependency detected

[Daniel Vetter]

On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
>> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>> >
>> > Which may or may not be a good thing depending how you look at it; it
>> > means that once your kernel blanks, you get a lockdep dump.  At that
>> > point you lose lockdep checking for everything else because lockdep
>> > disables itself after the first dump.
>>
>> Fair enough, we may want to revert the lockdep checking for
>> console_lock, and make re-enabling it part of the patch-series that
>> fixes the locking.
>>
>> Daniel/Dave? Does that sound reasonable?

Yeah, sounds good.

> Reverting the patch is fine with me.  Just let me know so I can queue it
> up again for 3.9.

Can you please also pick up the (currently) three locking fixups
around fbcon? Just so that we don't repeat the same fun where people
complain about lockdep splats, but the fixes are stuck somewhere. And
I guess Dave would be happy to not end up as fbcon maintainer ;-) He
has a git branch with them at
http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
though I have a small bikeshed on his last patch pending.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: BUG: circular locking dependency detected

[Greg Kroah-Hartman]

On Thu, Jan 31, 2013 at 09:21:16AM +0100, Daniel Vetter wrote:
> On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
> >> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >> >
> >> > Which may or may not be a good thing depending how you look at it; it
> >> > means that once your kernel blanks, you get a lockdep dump.  At that
> >> > point you lose lockdep checking for everything else because lockdep
> >> > disables itself after the first dump.
> >>
> >> Fair enough, we may want to revert the lockdep checking for
> >> console_lock, and make re-enabling it part of the patch-series that
> >> fixes the locking.
> >>
> >> Daniel/Dave? Does that sound reasonable?
> 
> Yeah, sounds good.
> 
> > Reverting the patch is fine with me.  Just let me know so I can queue it
> > up again for 3.9.
> 
> Can you please also pick up the (currently) three locking fixups
> around fbcon? Just so that we don't repeat the same fun where people
> complain about lockdep splats, but the fixes are stuck somewhere. And
> I guess Dave would be happy to not end up as fbcon maintainer ;-) He
> has a git branch with them at
> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
> though I have a small bikeshed on his last patch pending.

Care to just send me the patches through email when you all get done
bikesheding?  And for some reason I thought Andrew was going to handle
these fbcon patches, but if not, I'll be glad to take them.

thanks,

greg k-h

Re: BUG: circular locking dependency detected

[Daniel Vetter]

On Thu, Jan 31, 2013 at 10:21 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>> Can you please also pick up the (currently) three locking fixups
>> around fbcon? Just so that we don't repeat the same fun where people
>> complain about lockdep splats, but the fixes are stuck somewhere. And
>> I guess Dave would be happy to not end up as fbcon maintainer ;-) He
>> has a git branch with them at
>> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
>> though I have a small bikeshed on his last patch pending.
>
> Care to just send me the patches through email when you all get done
> bikesheding?  And for some reason I thought Andrew was going to handle
> these fbcon patches, but if not, I'll be glad to take them.

Yeah, I'll annoy people again with patches until they're merged ;-)
Iirc Andrew picked them due to lack of an fbcon maintainer, and
everyone else likes to pass the bucket. Having looked through the code
a bit, imo understandable ...

btw, I've started to look into how we could fix the locking madness
around fbcon for good instead of with duct-tape [1]. I'll try to
discuss this with a few fbdev guys at fosdem (some at least should be
there). Certainly a long-term thing, but comments from whomever gets
volunteered to shepherd fbcon would be great.

Cheers, Daniel

1: http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg33535.html
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: BUG: circular locking dependency detected

[Dave Airlie]

On Thu, Jan 31, 2013 at 8:21 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 31, 2013 at 09:21:16AM +0100, Daniel Vetter wrote:
>> On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
>> >> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>> >> >
>> >> > Which may or may not be a good thing depending how you look at it; it
>> >> > means that once your kernel blanks, you get a lockdep dump.  At that
>> >> > point you lose lockdep checking for everything else because lockdep
>> >> > disables itself after the first dump.
>> >>
>> >> Fair enough, we may want to revert the lockdep checking for
>> >> console_lock, and make re-enabling it part of the patch-series that
>> >> fixes the locking.
>> >>
>> >> Daniel/Dave? Does that sound reasonable?
>>
>> Yeah, sounds good.
>>
>> > Reverting the patch is fine with me.  Just let me know so I can queue it
>> > up again for 3.9.
>>
>> Can you please also pick up the (currently) three locking fixups
>> around fbcon? Just so that we don't repeat the same fun where people
>> complain about lockdep splats, but the fixes are stuck somewhere. And
>> I guess Dave would be happy to not end up as fbcon maintainer ;-) He
>> has a git branch with them at
>> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
>> though I have a small bikeshed on his last patch pending.
>
> Care to just send me the patches through email when you all get done
> bikesheding?  And for some reason I thought Andrew was going to handle
> these fbcon patches, but if not, I'll be glad to take them.

I'll ship them via my tree at this point I think, since I now need to
queue a revert of the revert on top.

I have a few vgacon/fbcon fixes that I need to go in this cycle.

Dave.

Re: BUG: circular locking dependency detected

[Greg Kroah-Hartman]

On Thu, Jan 31, 2013 at 10:51:27PM +1100, Dave Airlie wrote:
> On Thu, Jan 31, 2013 at 8:21 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Thu, Jan 31, 2013 at 09:21:16AM +0100, Daniel Vetter wrote:
> >> On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
> >> <gregkh@linuxfoundation.org> wrote:
> >> > On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
> >> >> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >> >> >
> >> >> > Which may or may not be a good thing depending how you look at it; it
> >> >> > means that once your kernel blanks, you get a lockdep dump.  At that
> >> >> > point you lose lockdep checking for everything else because lockdep
> >> >> > disables itself after the first dump.
> >> >>
> >> >> Fair enough, we may want to revert the lockdep checking for
> >> >> console_lock, and make re-enabling it part of the patch-series that
> >> >> fixes the locking.
> >> >>
> >> >> Daniel/Dave? Does that sound reasonable?
> >>
> >> Yeah, sounds good.
> >>
> >> > Reverting the patch is fine with me.  Just let me know so I can queue it
> >> > up again for 3.9.
> >>
> >> Can you please also pick up the (currently) three locking fixups
> >> around fbcon? Just so that we don't repeat the same fun where people
> >> complain about lockdep splats, but the fixes are stuck somewhere. And
> >> I guess Dave would be happy to not end up as fbcon maintainer ;-) He
> >> has a git branch with them at
> >> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
> >> though I have a small bikeshed on his last patch pending.
> >
> > Care to just send me the patches through email when you all get done
> > bikesheding?  And for some reason I thought Andrew was going to handle
> > these fbcon patches, but if not, I'll be glad to take them.
> 
> I'll ship them via my tree at this point I think, since I now need to
> queue a revert of the revert on top.
> 
> I have a few vgacon/fbcon fixes that I need to go in this cycle.

Ok, I'll gladly let you handle this :)

thanks,

greg k-h

Re: BUG: circular locking dependency detected

[Russell King]

On Thu, Jan 31, 2013 at 10:51:27PM +1100, Dave Airlie wrote:
> I'll ship them via my tree at this point I think, since I now need to
> queue a revert of the revert on top.
> 
> I have a few vgacon/fbcon fixes that I need to go in this cycle.

Great, thanks.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Russell King]

On Thu, Jan 31, 2013 at 10:52:51AM +1100, Linus Torvalds wrote:
> On Thu, Jan 31, 2013 at 9:19 AM, Russell King <rmk@arm.linux.org.uk> wrote:
> >
> > So... what you seem to be telling me is that 3.9 is going to be a
> > release which issues lockdep complaints when the console blanks, and
> > you think that's acceptable?
> >
> > Adding Linus and Andrew so they're aware of this issue...
> 
> Oh, we're extremely aware of it. And it's not a new issue, the locking
> problem have apparently been around forever, although I'm not sure why
> the lockdep splat itself started happening only recently.

Well, the reason the splat started happening recently is because of:

commit daee779718a319ff9f83e1ba3339334ac650bb22
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Sat Sep 22 19:52:11 2012 +0200

    console: implement lockdep support for console_lock
    
    Dave Airlie recently discovered a locking bug in the fbcon layer,
    where a timer_del_sync (for the blinking cursor) deadlocks with the
    timer itself, since both (want to) hold the console_lock:
    
    https://lkml.org/lkml/2012/8/21/36

which, if I'm looking at the git history right, appears to have come
in during the last merge window?

Yes, the locking may be wrong, but we've lived with that locking for
a long time without problem.

Can we at least silence these warnings by temporarily disabling the
lockdep tracking added by the above commit for this lock, until the
fixes for this are merged during the next merge window?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: BUG: circular locking dependency detected

[Sedat Dilek]

[ CCing Linux-Next and MMOTM folks ]

Original posting from Daniel see [0]

[ QUOTE ]
On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
>> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>> >
>> > Which may or may not be a good thing depending how you look at it; it
>> > means that once your kernel blanks, you get a lockdep dump.  At that
>> > point you lose lockdep checking for everything else because lockdep
>> > disables itself after the first dump.
>>
>> Fair enough, we may want to revert the lockdep checking for
>> console_lock, and make re-enabling it part of the patch-series that
>> fixes the locking.
>>
>> Daniel/Dave? Does that sound reasonable?

Yeah, sounds good.

> Reverting the patch is fine with me.  Just let me know so I can queue it
> up again for 3.9.

Can you please also pick up the (currently) three locking fixups
around fbcon? Just so that we don't repeat the same fun where people
complain about lockdep splats, but the fixes are stuck somewhere. And
I guess Dave would be happy to not end up as fbcon maintainer ;-) He
has a git branch with them at
http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
though I have a small bikeshed on his last patch pending.
-Daniel
[ /QUOTE ]

Did the 3rd patch go also to mmotm tree and got marked for Linux-Next inclusion?
Best would be to have it in mainline, finally.
Please, fix that for-3.8!

Thanks to all volunteers (Alan, Andrew, Takashi Iwai (Sorry, dunno
which is 1st and last name), Daniel and finally Dave) trying to get
this incredible pain-in-the-a** upstream :-).

- Sedat -

[0] http://marc.info/?l=dri-devel&m\x135962051326601&w=2
[1] http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
[2] http://cgit.freedesktop.org/~airlied/linux/commit/?hûcon-locking-fixes&id˜dfe36b5532576dedf41408d5bbd45fa31ec62d

Re: BUG: circular locking dependency detected

[Sedat Dilek]

On Thu, Jan 31, 2013 at 11:12 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> [ CCing Linux-Next and MMOTM folks ]
>
> Original posting from Daniel see [0]
>
> [ QUOTE ]
> On Thu, Jan 31, 2013 at 6:40 AM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
>> On Thu, Jan 31, 2013 at 11:26:53AM +1100, Linus Torvalds wrote:
>>> On Thu, Jan 31, 2013 at 11:13 AM, Russell King <rmk@arm.linux.org.uk> wrote:
>>> >
>>> > Which may or may not be a good thing depending how you look at it; it
>>> > means that once your kernel blanks, you get a lockdep dump.  At that
>>> > point you lose lockdep checking for everything else because lockdep
>>> > disables itself after the first dump.
>>>
>>> Fair enough, we may want to revert the lockdep checking for
>>> console_lock, and make re-enabling it part of the patch-series that
>>> fixes the locking.
>>>
>>> Daniel/Dave? Does that sound reasonable?
>
> Yeah, sounds good.
>
>> Reverting the patch is fine with me.  Just let me know so I can queue it
>> up again for 3.9.
>
> Can you please also pick up the (currently) three locking fixups
> around fbcon? Just so that we don't repeat the same fun where people
> complain about lockdep splats, but the fixes are stuck somewhere. And
> I guess Dave would be happy to not end up as fbcon maintainer ;-) He
> has a git branch with them at
> http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
> though I have a small bikeshed on his last patch pending.
> -Daniel
> [ /QUOTE ]
>
> Did the 3rd patch go also to mmotm tree and got marked for Linux-Next inclusion?
> Best would be to have it in mainline, finally.
> Please, fix that for-3.8!
>
> Thanks to all volunteers (Alan, Andrew, Takashi Iwai (Sorry, dunno
> which is 1st and last name), Daniel and finally Dave) trying to get
> this incredible pain-in-the-a** upstream :-).
>
> - Sedat -
>
> [0] http://marc.info/?l=dri-devel&m\x135962051326601&w=2
> [1] http://cgit.freedesktop.org/~airlied/linux/log/?hûcon-locking-fixes
> [2] http://cgit.freedesktop.org/~airlied/linux/commit/?hûcon-locking-fixes&id˜dfe36b5532576dedf41408d5bbd45fa31ec62d

[ Adjusting outdated email-adresses, CC Borislav ]

What's with the patch from [3] in mmotm? For-3.8, no more needed?

- Sedat -

[3] http://ozlabs.org/~akpm/mmots/broken-out/drm-fb-helper-dont-sleep-for-screen-unblank-when-an-oopps-is-in-progress.patch