3.8-rc2: EFI framebuffer lock inversion...

3.8-rc2: EFI framebuffer lock inversion...

[Daniel J Blueman]

On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
Retina), I see two releated lock inversion issues with the EFI
framebuffer, leading to possible deadlock: when X takes over from the
EFI framebuffer [1] and when nouveau releases the framebuffer when
being vgaswitcherood [2].

Let me know if you'd like any testing or analysis when I can get the time.

Many thanks,
  Daniel

--- [1]

init: lightdm main process (950) terminated with status 1

===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert #1 Not tainted
-------------------------------------------------------
Xorg/1193 is trying to acquire lock:
 ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<ffffffff81263f95>] do_fb_ioctl+0x2e5/0x5f0

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff810407a7>] console_lock+0x77/0x80
    [<ffffffff812c6d84>] register_con_driver+0x34/0x140
    [<ffffffff812c84e9>] take_over_console+0x29/0x60
    [<ffffffff8126e76b>] fbcon_takeover+0x5b/0xb0
    [<ffffffff81272bb5>] fbcon_event_notify+0x715/0x820
    [<ffffffff810693a5>] notifier_call_chain+0x55/0x110
    [<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff81264c1d>] register_framebuffer+0x1bd/0x2f0
    [<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
    [<ffffffff81308dfe>] platform_drv_probe+0x3e/0x70
    [<ffffffff81306dc6>] driver_probe_device+0x76/0x240
    [<ffffffff81307033>] __driver_attach+0xa3/0xb0
    [<ffffffff8130503d>] bus_for_each_dev+0x4d/0x90
    [<ffffffff81306929>] driver_attach+0x19/0x20
    [<ffffffff813064e0>] bus_add_driver+0x1a0/0x270
    [<ffffffff813076c2>] driver_register+0x72/0x170
    [<ffffffff81308671>] platform_driver_register+0x41/0x50
    [<ffffffff81308696>] platform_driver_probe+0x16/0xa0
    [<ffffffff81ac2ece>] efifb_init+0x273/0x292
    [<ffffffff810002da>] do_one_initcall+0x11a/0x170
    [<ffffffff8154187c>] kernel_init+0x11c/0x290
    [<ffffffff8155acac>] ret_from_fork+0x7c/0xb0

-> #0 ((fb_notifier_list).rwsem){++++.+}:
    [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff81557ad7>] down_read+0x47/0x5c
    [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff81263196>] fb_blank+0x36/0xc0
    [<ffffffff81263fa7>] do_fb_ioctl+0x2f7/0x5f0
    [<ffffffff812646e1>] fb_ioctl+0x41/0x50
    [<ffffffff811209d7>] do_vfs_ioctl+0x97/0x580
    [<ffffffff81120f0b>] sys_ioctl+0x4b/0x90
    [<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f

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 ***

2 locks held by Xorg/1193:
 #0: (&fb_info->lock){+.+.+.}, at: [<ffffffff81262ef1>] lock_fb_info+0x21/0x60
 #1: (console_lock){+.+.+.}, at: [<ffffffff81263f95>] do_fb_ioctl+0x2e5/0x5f0

stack backtrace:
Pid: 1193, comm: Xorg Not tainted 3.8.0-rc2-expert #1
Call Trace:
 [<ffffffff8154f6c6>] print_circular_bug+0x28e/0x29f
 [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
 [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
 [<ffffffff8108d3a4>] ? __lock_is_held+0x54/0x80
 [<ffffffff810916ea>] lock_acquire+0x5a/0x70
 [<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff81557ad7>] down_read+0x47/0x5c
 [<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
 [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
 [<ffffffff81263196>] fb_blank+0x36/0xc0
 [<ffffffff81263fa7>] do_fb_ioctl+0x2f7/0x5f0
 [<ffffffff810e8d1a>] ? mmap_region+0x1aa/0x620
 [<ffffffff812646e1>] fb_ioctl+0x41/0x50
 [<ffffffff811209d7>] do_vfs_ioctl+0x97/0x580
 [<ffffffff8112c49a>] ? fget_light+0x3da/0x4d0
 [<ffffffff8155ad7b>] ? sysret_check+0x1b/0x56
 [<ffffffff81120f0b>] sys_ioctl+0x4b/0x90
 [<ffffffff8122c03e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
 [<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
[drm] Enabling RC6 states: RC6 on, RC6p on, RC6pp off

--- [2]

hda-intel 0000:01:00.1: Disabling via VGA-switcheroo
hda-intel 0000:01:00.1: Cannot lock devices!
VGA switcheroo: switched nouveau off
nouveau [   DRM] suspending fbcon...

===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert #1 Not tainted
-------------------------------------------------------
sh/1017 is trying to acquire lock:
 ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<ffffffffa0204d35>]
nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff810407a7>] console_lock+0x77/0x80
    [<ffffffff812c6d84>] register_con_driver+0x34/0x140
    [<ffffffff812c84e9>] take_over_console+0x29/0x60
    [<ffffffff8126e76b>] fbcon_takeover+0x5b/0xb0
    [<ffffffff81272bb5>] fbcon_event_notify+0x715/0x820
    [<ffffffff810693a5>] notifier_call_chain+0x55/0x110
    [<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff81264c1d>] register_framebuffer+0x1bd/0x2f0
    [<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
    [<ffffffff81308dfe>] platform_drv_probe+0x3e/0x70
    [<ffffffff81306dc6>] driver_probe_device+0x76/0x240
    [<ffffffff81307033>] __driver_attach+0xa3/0xb0
    [<ffffffff8130503d>] bus_for_each_dev+0x4d/0x90
    [<ffffffff81306929>] driver_attach+0x19/0x20
    [<ffffffff813064e0>] bus_add_driver+0x1a0/0x270
    [<ffffffff813076c2>] driver_register+0x72/0x170
    [<ffffffff81308671>] platform_driver_register+0x41/0x50
    [<ffffffff81308696>] platform_driver_probe+0x16/0xa0
    [<ffffffff81ac2ece>] efifb_init+0x273/0x292
    [<ffffffff810002da>] do_one_initcall+0x11a/0x170
    [<ffffffff8154187c>] kernel_init+0x11c/0x290
    [<ffffffff8155acac>] ret_from_fork+0x7c/0xb0

-> #0 ((fb_notifier_list).rwsem){++++.+}:
    [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff81557ad7>] down_read+0x47/0x5c
    [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff81263146>] fb_set_suspend+0x46/0x60
    [<ffffffffa0204da2>] nouveau_fbcon_set_suspend+0x92/0xc0 [nouveau]
    [<ffffffffa01f5451>] nouveau_do_suspend+0x51/0x200 [nouveau]
    [<ffffffffa01f564f>] nouveau_pmops_suspend+0x2f/0x80 [nouveau]
    [<ffffffffa01f723c>] nouveau_switcheroo_set_state+0x5c/0xc0 [nouveau]
    [<ffffffff81300877>] vga_switchoff+0x17/0x40
    [<ffffffff81300f1a>] vga_switcheroo_debugfs_write+0xca/0x380
    [<ffffffff8110ec93>] vfs_write+0xa3/0x160
    [<ffffffff8110ef9d>] sys_write+0x4d/0xa0
    [<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f

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 ***

2 locks held by sh/1017:
 #0: (vgasr_mutex){+.+.+.}, at: [<ffffffff81300ea7>]
vga_switcheroo_debugfs_write+0x57/0x380
 #1: (console_lock){+.+.+.}, at: [<ffffffffa0204d35>]
nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]

stack backtrace:
Pid: 1017, comm: sh Not tainted 3.8.0-rc2-expert #1
Call Trace:
 [<ffffffff8154f6c6>] print_circular_bug+0x28e/0x29f
 [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
 [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
 [<ffffffff8108d3a4>] ? __lock_is_held+0x54/0x80
 [<ffffffff810916ea>] lock_acquire+0x5a/0x70
 [<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff81557ad7>] down_read+0x47/0x5c
 [<ffffffff810697c1>] ? __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
 [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
 [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
 [<ffffffff81263146>] fb_set_suspend+0x46/0x60
 [<ffffffff810407a7>] ? console_lock+0x77/0x80
 [<ffffffffa0204d35>] ? nouveau_fbcon_set_suspend+0x25/0xc0 [nouveau]
 [<ffffffffa0204da2>] nouveau_fbcon_set_suspend+0x92/0xc0 [nouveau]
 [<ffffffffa01f5451>] nouveau_do_suspend+0x51/0x200 [nouveau]
 [<ffffffffa01f564f>] nouveau_pmops_suspend+0x2f/0x80 [nouveau]
 [<ffffffffa01f723c>] nouveau_switcheroo_set_state+0x5c/0xc0 [nouveau]
 [<ffffffff81300877>] vga_switchoff+0x17/0x40
 [<ffffffff81300f1a>] vga_switcheroo_debugfs_write+0xca/0x380
 [<ffffffff8110ec93>] vfs_write+0xa3/0x160
 [<ffffffff8110ef9d>] sys_write+0x4d/0xa0
 [<ffffffff8155ad56>] system_call_fastpath+0x1a/0x1f
nouveau [   DRM] suspending display...
nouveau [   DRM] unpinning framebuffer(s)...
nouveau [   DRM] evicting buffers...
nouveau [   DRM] suspending client object trees...
tg3 0000:0a:00.0 eth0: Link is up at 1000 Mbps, full duplex
tg3 0000:0a:00.0 eth0: Flow control is on for TX and on for RX
nouveau E[   I2C][0000:01:00.0] AUXCH(3): begin idle timeout 0xffffffff
nouveau E[   I2C][0000:01:00.0] AUXCH(2): begin idle timeout 0xffffffff
nouveau E[   I2C][0000:01:00.0] AUXCH(1): begin idle timeout 0xffffffff
-- 
Daniel J Blueman

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Alan Cox]

On Thu, 3 Jan 2013 20:56:30 +0800
Daniel J Blueman <daniel@quora.org> wrote:

> On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
> Retina), I see two releated lock inversion issues with the EFI
> framebuffer, leading to possible deadlock: when X takes over from the
> EFI framebuffer [1] and when nouveau releases the framebuffer when
> being vgaswitcherood [2].
> 
> Let me know if you'd like any testing or analysis when I can get the time.

The fb layer locking was broken. I posted patches early December which
should have fixed the ones we know about. ('fb: Rework locking to fix
lock ordering on takeover').

Alan

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Daniel J Blueman]

On 3 January 2013 21:11, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On Thu, 3 Jan 2013 20:56:30 +0800
> Daniel J Blueman <daniel@quora.org> wrote:
>
>> On 3.8-rc2 with lockdep enabled and dual-GPU setup (Macbook Pro
>> Retina), I see two releated lock inversion issues with the EFI
>> framebuffer, leading to possible deadlock: when X takes over from the
>> EFI framebuffer [1] and when nouveau releases the framebuffer when
>> being vgaswitcherood [2].
>>
>> Let me know if you'd like any testing or analysis when I can get the time.
>
> The fb layer locking was broken. I posted patches early December which
> should have fixed the ones we know about. ('fb: Rework locking to fix
> lock ordering on takeover').

Superb work, Alan!

The only patch I could find [1] (mid Nov) looks like it needs another
sites updating, since we now see an i915 vs efifb lock ordering issue
[2].

I can get some time next week to take a look if it helps.

Thanks,
  Daniel

--- [1] https://patchwork.kernel.org/patch/1757061/

--- [2]

[drm] Memory usable by graphics device = 2048M
checking generic (b0000000 1440000) vs hw (b0000000 10000000)
fb: conflicting fb hw usage inteldrmfb vs EFI VGA - removing generic driver

===========================
[ INFO: possible circular locking dependency detected ]
3.8.0-rc2-expert+ #2 Not tainted
-------------------------------------------------------
modprobe/603 is trying to acquire lock:
 (console_lock){+.+.+.}, at: [<ffffffff812c869f>] unbind_con_driver+0x3f/0x200

but task is already holding lock:
 ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 ((fb_notifier_list).rwsem){++++.+}:
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff81557c97>] down_read+0x47/0x5c
    [<ffffffff810697c1>] __blocking_notifier_call_chain+0x51/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff81264c20>] register_framebuffer+0x1c0/0x300
    [<ffffffff81ac2bd4>] efifb_probe+0x40f/0x496
    [<ffffffff81308fbe>] platform_drv_probe+0x3e/0x70
    [<ffffffff81306f86>] driver_probe_device+0x76/0x240
    [<ffffffff813071f3>] __driver_attach+0xa3/0xb0
    [<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
    [<ffffffff81306ae9>] driver_attach+0x19/0x20
    [<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
    [<ffffffff81307882>] driver_register+0x72/0x170
    [<ffffffff81308831>] platform_driver_register+0x41/0x50
    [<ffffffff81308856>] platform_driver_probe+0x16/0xa0
    [<ffffffff81ac2ece>] efifb_init+0x273/0x292
    [<ffffffff810002da>] do_one_initcall+0x11a/0x170
    [<ffffffff81541a3c>] kernel_init+0x11c/0x290
    [<ffffffff8155ae6c>] ret_from_fork+0x7c/0xb0

-> #0 (console_lock){+.+.+.}:
    [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
    [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
    [<ffffffff810916ea>] lock_acquire+0x5a/0x70
    [<ffffffff810407a7>] console_lock+0x77/0x80
    [<ffffffff812c869f>] unbind_con_driver+0x3f/0x200
    [<ffffffff81272bc7>] fbcon_event_notify+0x447/0x8b0
    [<ffffffff810693a5>] notifier_call_chain+0x55/0x110
    [<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
    [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
    [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
    [<ffffffff812647db>] do_unregister_framebuffer+0x5b/0x110
    [<ffffffff81264a28>] do_remove_conflicting_framebuffers+0x158/0x190
    [<ffffffff81264d9a>] remove_conflicting_framebuffers+0x3a/0x60
    [<ffffffffa007dbe4>] i915_driver_load+0x7d4/0xe70 [i915]
    [<ffffffff812ee1ee>] drm_get_pci_dev+0x17e/0x2b0
    [<ffffffffa0079616>] i915_pci_probe+0x36/0x90 [i915]
    [<ffffffff8124a146>] local_pci_probe+0x46/0x80
    [<ffffffff8124a9d1>] pci_device_probe+0x101/0x110
    [<ffffffff81306f86>] driver_probe_device+0x76/0x240
    [<ffffffff813071f3>] __driver_attach+0xa3/0xb0
    [<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
    [<ffffffff81306ae9>] driver_attach+0x19/0x20
    [<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
    [<ffffffff81307882>] driver_register+0x72/0x170
    [<ffffffff8124aacf>] __pci_register_driver+0x5f/0x70
    [<ffffffff812ee435>] drm_pci_init+0x115/0x130
    [<ffffffffa00ff066>] i915_init+0x66/0x68 [i915]
    [<ffffffff810002da>] do_one_initcall+0x11a/0x170
    [<ffffffff8109cf84>] load_module+0xfd4/0x13c0
    [<ffffffff8109d427>] sys_init_module+0xb7/0xe0
    [<ffffffff8155af16>] system_call_fastpath+0x1a/0x1f

other info that might help us debug this:

 Possible unsafe locking scenario:

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

 *** DEADLOCK ***

6 locks held by modprobe/603:
 #0: (&__lockdep_no_validate__){......}, at: [<ffffffff813071a3>]
__driver_attach+0x53/0xb0
 #1: (&__lockdep_no_validate__){......}, at: [<ffffffff813071b1>]
__driver_attach+0x61/0xb0
 #2: (drm_global_mutex){+.+.+.}, at: [<ffffffff812ee12c>]
drm_get_pci_dev+0xbc/0x2b0
 #3: (registration_lock){+.+.+.}, at: [<ffffffff81264d8b>]
remove_conflicting_framebuffers+0x2b/0x60
 #4: (&fb_info->lock){+.+.+.}, at: [<ffffffff81262ef1>] lock_fb_info+0x21/0x60
 #5: ((fb_notifier_list).rwsem){++++.+}, at: [<ffffffff810697c1>]
__blocking_notifier_call_chain+0x51/0xc0

stack backtrace:
Pid: 603, comm: modprobe Not tainted 3.8.0-rc2-expert+ #2
Call Trace:
 [<ffffffff8154f886>] print_circular_bug+0x28e/0x29f
 [<ffffffff8108ff10>] validate_chain.isra.33+0x1000/0x10d0
 [<ffffffff81090a61>] __lock_acquire+0x3a1/0xb60
 [<ffffffff8155a12a>] ? _raw_spin_unlock_irqrestore+0x3a/0x70
 [<ffffffff8109209d>] ? trace_hardirqs_on_caller+0x10d/0x1a0
 [<ffffffff810916ea>] lock_acquire+0x5a/0x70
 [<ffffffff812c869f>] ? unbind_con_driver+0x3f/0x200
 [<ffffffff810407a7>] console_lock+0x77/0x80
 [<ffffffff812c869f>] ? unbind_con_driver+0x3f/0x200
 [<ffffffff812c869f>] unbind_con_driver+0x3f/0x200
 [<ffffffff81090a61>] ? __lock_acquire+0x3a1/0xb60
 [<ffffffff81272bc7>] fbcon_event_notify+0x447/0x8b0
 [<ffffffff810693a5>] notifier_call_chain+0x55/0x110
 [<ffffffff810697d7>] __blocking_notifier_call_chain+0x67/0xc0
 [<ffffffff81069841>] blocking_notifier_call_chain+0x11/0x20
 [<ffffffff81262a16>] fb_notifier_call_chain+0x16/0x20
 [<ffffffff812647db>] do_unregister_framebuffer+0x5b/0x110
 [<ffffffff81264a28>] do_remove_conflicting_framebuffers+0x158/0x190
 [<ffffffff81264d9a>] remove_conflicting_framebuffers+0x3a/0x60
 [<ffffffffa007dbe4>] i915_driver_load+0x7d4/0xe70 [i915]
 [<ffffffff812ee1ee>] drm_get_pci_dev+0x17e/0x2b0
 [<ffffffff8109213d>] ? trace_hardirqs_on+0xd/0x10
 [<ffffffffa0079616>] i915_pci_probe+0x36/0x90 [i915]
 [<ffffffff8124a146>] local_pci_probe+0x46/0x80
 [<ffffffff8124a9d1>] pci_device_probe+0x101/0x110
 [<ffffffff81306f86>] driver_probe_device+0x76/0x240
 [<ffffffff813071f3>] __driver_attach+0xa3/0xb0
 [<ffffffff81307150>] ? driver_probe_device+0x240/0x240
 [<ffffffff813051fd>] bus_for_each_dev+0x4d/0x90
 [<ffffffff81306ae9>] driver_attach+0x19/0x20
 [<ffffffff813066a0>] bus_add_driver+0x1a0/0x270
 [<ffffffffa00ff000>] ? 0xffffffffa00fefff
 [<ffffffff81307882>] driver_register+0x72/0x170
 [<ffffffffa00ff000>] ? 0xffffffffa00fefff
 [<ffffffff8124aacf>] __pci_register_driver+0x5f/0x70
 [<ffffffff812ee435>] drm_pci_init+0x115/0x130
 [<ffffffffa00ff000>] ? 0xffffffffa00fefff
 [<ffffffffa00ff066>] i915_init+0x66/0x68 [i915]
 [<ffffffff810002da>] do_one_initcall+0x11a/0x170
 [<ffffffff8109cf84>] load_module+0xfd4/0x13c0
 [<ffffffff81098ab0>] ? in_lock_functions+0x20/0x20
 [<ffffffff8109d427>] sys_init_module+0xb7/0xe0
 [<ffffffff8155af16>] system_call_fastpath+0x1a/0x1f
Console: switching to colour dummy device 80x25
-- 
Daniel J Blueman

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

Hi Daniel,

just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
Me is also working with i915 KMS.

I looked at nouveau KMS driver and adapted the part for i915:

drivers/gpu/drm/nouveau/nouveau_drm.c-200-      /* remove conflicting
drivers (vesafb, efifb etc) */
drivers/gpu/drm/nouveau/nouveau_drm.c:201:      aper = alloc_apertures(3);
drivers/gpu/drm/nouveau/nouveau_drm.c-202-      if (!aper)
drivers/gpu/drm/nouveau/nouveau_drm.c-203-              return -ENOMEM;

Untested by me, feel free to test.

Maybe some of the i915 and/or fb driver experts can comment on the problem.

Regards,
- Sedat -

[1] http://marc.info/?t=135721787600001&r=1&w=2
[2] https://patchwork.kernel.org/patch/1757061/

[-- Attachment #2: i915-Remove-conflicting-fb-drivers.patch --]
[-- Type: application/octet-stream, Size: 508 bytes --]

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 99daa89..d9ae93d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1396,6 +1396,11 @@ static void i915_kick_out_firmware_fb(struct drm_i915_private *dev_priv)
 	struct pci_dev *pdev = dev_priv->dev->pdev;
 	bool primary;
 
+	/* remove conflicting drivers (vesafb, efifb etc) */
+	ap = alloc_apertures(3);
+	if (!ap)
+		return -ENOMEM;
+
 	ap = alloc_apertures(1);
 	if (!ap)
 		return;

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Alan Cox]

> The only patch I could find [1] (mid Nov) looks like it needs another
> sites updating, since we now see an i915 vs efifb lock ordering issue
> [2].
> 
> I can get some time next week to take a look if it helps.

That would be great. I've not got any EFI afflicted hardware and I'm
doing my best to avoid it.

Alan

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Daniel J Blueman]

On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> Hi Daniel,
>
> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
> Me is also working with i915 KMS.
>
> I looked at nouveau KMS driver and adapted the part for i915:
>
> drivers/gpu/drm/nouveau/nouveau_drm.c-200-      /* remove conflicting
> drivers (vesafb, efifb etc) */
> drivers/gpu/drm/nouveau/nouveau_drm.c:201:      aper = alloc_apertures(3);
> drivers/gpu/drm/nouveau/nouveau_drm.c-202-      if (!aper)
> drivers/gpu/drm/nouveau/nouveau_drm.c-203-              return -ENOMEM;
>
> Untested by me, feel free to test.
>
> Maybe some of the i915 and/or fb driver experts can comment on the problem.

The structure array from alloc_apertures is just used for the PCI base
address registers, so it's important here.

I'll take a look at the efifb locking later.

Thanks,
  Daniel
-- 
Daniel J Blueman

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Sedat Dilek]

On Thu, Jan 3, 2013 at 3:39 PM, Daniel J Blueman <daniel@quora.org> wrote:
> On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> Hi Daniel,
>>
>> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
>> Me is also working with i915 KMS.
>>
>> I looked at nouveau KMS driver and adapted the part for i915:
>>
>> drivers/gpu/drm/nouveau/nouveau_drm.c-200-      /* remove conflicting
>> drivers (vesafb, efifb etc) */
>> drivers/gpu/drm/nouveau/nouveau_drm.c:201:      aper = alloc_apertures(3);
>> drivers/gpu/drm/nouveau/nouveau_drm.c-202-      if (!aper)
>> drivers/gpu/drm/nouveau/nouveau_drm.c-203-              return -ENOMEM;
>>
>> Untested by me, feel free to test.
>>
>> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>
> The structure array from alloc_apertures is just used for the PCI base
> address registers, so it's important here.
>
> I'll take a look at the efifb locking later.
>

That is the code part [1] I looked at.
Maybe the next lines with ap(er)->ranges || pci_resource_start() and
pci_resource_len() are missing?

I also looked at "include/linux/fb.h" but could not get wiser.
Also I can't say what the value "1" or "3" means in alloc_apertures().

Wouldn't it make sense to remove the conflicting fb-drivers globally
not in each affected DRM/KMS driver?
Just an idea.

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/nouveau/nouveau_drm.c;hb=refs/tags/v3.8-rc2#l192

> Thanks,
>   Daniel
> --
> Daniel J Blueman

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Sedat Dilek]

Hi,

I have tested the fb-fix [1] from Alan on top of Linux v3.8-rc2.

Unfortunately, several people still complain about it.
As Boris shouted out... Apply directly to mainline?

Feel free to add a "Tested-by".

Just FYI: There seems to exist a locking problem with efifb reported
by Daniel J (also see [1]).

Regards,
- Sedat -

[1] http://marc.info/?t\x135721787600001&r=1&w=2
[2] https://patchwork.kernel.org/patch/1757061/

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Sedat Dilek]

On Thu, Jan 3, 2013 at 3:11 PM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> Hi Daniel,
>
> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
> Me is also working with i915 KMS.
>
> I looked at nouveau KMS driver and adapted the part for i915:
>
> drivers/gpu/drm/nouveau/nouveau_drm.c-200-      /* remove conflicting
> drivers (vesafb, efifb etc) */
> drivers/gpu/drm/nouveau/nouveau_drm.c:201:      aper = alloc_apertures(3);
> drivers/gpu/drm/nouveau/nouveau_drm.c-202-      if (!aper)
> drivers/gpu/drm/nouveau/nouveau_drm.c-203-              return -ENOMEM;
>
> Untested by me, feel free to test.
>
> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>

That is the i915 part [---> i915_kick_out_firmware_fb()] where I looked at [1].

- Sedat -

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/i915/i915_dma.c;hb=refs/tags/v3.8-rc2#l1393

> Regards,
> - Sedat -
>
> [1] http://marc.info/?t\x135721787600001&r=1&w=2
> [2] https://patchwork.kernel.org/patch/1757061/

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Linus Torvalds]

On Thu, Jan 3, 2013 at 5:11 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>
> The fb layer locking was broken. I posted patches early December which
> should have fixed the ones we know about. ('fb: Rework locking to fix
> lock ordering on takeover').

That patch causes compile errors with "allmodconfig":

  ERROR: "do_take_over_console" [drivers/video/console/fbcon.ko] undefined!
  make[1]: *** [__modpost] Error 1
  make: *** [modules] Error 2
  make: *** Waiting for unfinished jobs....

Hmm?

               Linus

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Andrew Morton]

On Thu, 3 Jan 2013 11:40:47 -0800
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Jan 3, 2013 at 5:11 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> >
> > The fb layer locking was broken. I posted patches early December which
> > should have fixed the ones we know about. ('fb: Rework locking to fix
> > lock ordering on takeover').
> 
> That patch causes compile errors with "allmodconfig":
> 
>   ERROR: "do_take_over_console" [drivers/video/console/fbcon.ko] undefined!
>   make[1]: *** [__modpost] Error 1
>   make: *** [modules] Error 2
>   make: *** Waiting for unfinished jobs....
> 
> Hmm?

I have a couple of fixes against
fb-rework-locking-to-fix-lock-ordering-on-takeover.patch:

http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix.patch
http://ozlabs.org/~akpm/mmots/broken-out/fb-rework-locking-to-fix-lock-ordering-on-takeover-fix-2.patch

Florian has been busy for a month or two - I've been waiting for him to
reappear to consider this patch.

Re: 3.8-rc2: EFI framebuffer lock inversion...

[Sedat Dilek]

On Thu, Jan 3, 2013 at 3:39 PM, Daniel J Blueman <daniel@quora.org> wrote:
> On 3 January 2013 22:11, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>> Hi Daniel,
>>
>> just wanted to test the fb-fix [2] from Alan and followed the thread in [1].
>> Me is also working with i915 KMS.
>>
>> I looked at nouveau KMS driver and adapted the part for i915:
>>
>> drivers/gpu/drm/nouveau/nouveau_drm.c-200-      /* remove conflicting
>> drivers (vesafb, efifb etc) */
>> drivers/gpu/drm/nouveau/nouveau_drm.c:201:      aper = alloc_apertures(3);
>> drivers/gpu/drm/nouveau/nouveau_drm.c-202-      if (!aper)
>> drivers/gpu/drm/nouveau/nouveau_drm.c-203-              return -ENOMEM;
>>
>> Untested by me, feel free to test.
>>
>> Maybe some of the i915 and/or fb driver experts can comment on the problem.
>
> The structure array from alloc_apertures is just used for the PCI base
> address registers, so it's important here.
>
> I'll take a look at the efifb locking later.
>

You had a chance to look at this?

- Sedat -

> Thanks,
>   Daniel
> --
> Daniel J Blueman