• Douglas Anderson's avatar
    serial: core: Allow processing sysrq at port unlock time · 2c299a22
    Douglas Anderson authored
    [ Upstream commit d6e19358 ]
    
    Right now serial drivers process sysrq keys deep in their character
    receiving code.  This means that they've already grabbed their
    port->lock spinlock.  This can end up getting in the way if we've go
    to do serial stuff (especially kgdb) in response to the sysrq.
    
    Serial drivers have various hacks in them to handle this.  Looking at
    '8250_port.c' you can see that the console_write() skips locking if
    we're in the sysrq handler.  Looking at 'msm_serial.c' you can see
    that the port lock is dropped around uart_handle_sysrq_char().
    
    It turns out that these hacks aren't exactly perfect.  If you have
    lockdep turned on and use something like the 8250_port hack you'll get
    a splat that looks like:
    
      WARNING: possible circular locking dependency detected
      [...] is trying to acquire lock:
      ... (console_owner){-.-.}, at: console_unlock+0x2e0/0x5e4
    
      but task is already holding lock:
      ... (&port_lock_key){-.-.}, at: serial8250_handle_irq+0x30/0xe4
    
      which lock already depends on the new lock.
    
      the existing dependency chain (in reverse order) is:
    
      -> #1 (&port_lock_key){-.-.}:
             _raw_spin_lock_irqsave+0x58/0x70
             serial8250_console_write+0xa8/0x250
             univ8250_console_write+0x40/0x4c
             console_unlock+0x528/0x5e4
             register_console+0x2c4/0x3b0
             uart_add_one_port+0x350/0x478
             serial8250_register_8250_port+0x350/0x3a8
             dw8250_probe+0x67c/0x754
             platform_drv_probe+0x58/0xa4
             really_probe+0x150/0x294
             driver_probe_device+0xac/0xe8
             __driver_attach+0x98/0xd0
             bus_for_each_dev+0x84/0xc8
             driver_attach+0x2c/0x34
             bus_add_driver+0xf0/0x1ec
             driver_register+0xb4/0x100
             __platform_driver_register+0x60/0x6c
             dw8250_platform_driver_init+0x20/0x28
    	 ...
    
      -> #0 (console_owner){-.-.}:
             lock_acquire+0x1e8/0x214
             console_unlock+0x35c/0x5e4
             vprintk_emit+0x230/0x274
             vprintk_default+0x7c/0x84
             vprintk_func+0x190/0x1bc
             printk+0x80/0xa0
             __handle_sysrq+0x104/0x21c
             handle_sysrq+0x30/0x3c
             serial8250_read_char+0x15c/0x18c
             serial8250_rx_chars+0x34/0x74
             serial8250_handle_irq+0x9c/0xe4
             dw8250_handle_irq+0x98/0xcc
             serial8250_interrupt+0x50/0xe8
             ...
    
      other info that might help us debug this:
    
       Possible unsafe locking scenario:
    
             CPU0                    CPU1
             ----                    ----
        lock(&port_lock_key);
                                     lock(console_owner);
                                     lock(&port_lock_key);
        lock(console_owner);
    
       *** DEADLOCK ***
    
    The hack used in 'msm_serial.c' doesn't cause the above splats but it
    seems a bit ugly to unlock / lock our spinlock deep in our irq
    handler.
    
    It seems like we could defer processing the sysrq until the end of the
    interrupt handler right after we've unlocked the port.  With this
    scheme if a whole batch of sysrq characters comes in one irq then we
    won't handle them all, but that seems like it should be a fine
    compromise.
    Signed-off-by: default avatarDouglas Anderson <dianders@chromium.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    Signed-off-by: default avatarSasha Levin <sashal@kernel.org>
    2c299a22
serial_core.h 17.5 KB