Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757617AbaDWPQE (ORCPT ); Wed, 23 Apr 2014 11:16:04 -0400 Received: from mail-la0-f46.google.com ([209.85.215.46]:42606 "EHLO mail-la0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752655AbaDWPP6 (ORCPT ); Wed, 23 Apr 2014 11:15:58 -0400 Date: Wed, 23 Apr 2014 18:15:48 +0300 From: Johan Hedberg To: Andreas =?iso-8859-1?Q?Bie=DFmann?= Cc: linux-kernel@vger.kernel.org, Marcel Holtmann , Gustavo Padovan , linux-bluetooth@vger.kernel.org Subject: Re: [PATCH] bluetooth:hci_ldisc: add tasklet for deferred TX handling Message-ID: <20140423151548.GA10025@t440s.P-661HNU-F1> Mail-Followup-To: Andreas =?iso-8859-1?Q?Bie=DFmann?= , linux-kernel@vger.kernel.org, Marcel Holtmann , Gustavo Padovan , linux-bluetooth@vger.kernel.org References: <1397661950-30408-1-git-send-email-andreas@biessmann.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1397661950-30408-1-git-send-email-andreas@biessmann.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andreas, On Wed, Apr 16, 2014, Andreas Bie?mann wrote: > This patch fixes a recursive locking scenario when using BCSP connection via > 8250 driver. The 8250 driver may tty_wakeup() in interrupt context which > results in hci_uart_tx_wakeup(). This in turn will call uart_write() in the > very same context and therefore will spin_lock() the same lock within the > same context. > > Here is the call stack: > > ---8<--- > ============================================= > [ INFO: possible recursive locking detected ] > 3.4.87-gf1a3cc3 #3 Tainted: G O > --------------------------------------------- > swapper/0 is trying to acquire lock: > (&port_lock_key){-.-...}, at: [] uart_write+0x60/0xfc > > but task is already holding lock: > (&port_lock_key){-.-...}, at: [] serial8250_handle_irq+0x24/0x88 > > other info that might help us debug this: > Possible unsafe locking scenario: > > CPU0 > ---- > lock(&port_lock_key); > lock(&port_lock_key); > > *** DEADLOCK *** > > May be due to missing lock nesting notation > > 2 locks held by swapper/0: > #0: (&(&i->lock)->rlock){-.-...}, at: [] serial8250_interrupt+0x2c/0xc0 > #1: (&port_lock_key){-.-...}, at: [] serial8250_handle_irq+0x24/0x88 > > stack backtrace: > [] (unwind_backtrace+0x0/0xec) from [] (dump_stack+0x20/0x24) > [] (dump_stack+0x20/0x24) from [] (print_deadlock_bug+0xb4/0xe4) > [] (print_deadlock_bug+0xb4/0xe4) from [] (check_deadlock.isra.20+0x160/0x18c) > [] (check_deadlock.isra.20+0x160/0x18c) from [] (validate_chain.isra.24+0x4a4/0x4f0) > [] (validate_chain.isra.24+0x4a4/0x4f0) from [] (__lock_acquire+0x670/0x740) > [] (__lock_acquire+0x670/0x740) from [] (lock_acquire+0x138/0x15c) > [] (lock_acquire+0x138/0x15c) from [] (_raw_spin_lock_irqsave+0x54/0x68) > [] (_raw_spin_lock_irqsave+0x54/0x68) from [] (uart_write+0x60/0xfc) > [] (uart_write+0x60/0xfc) from [] (hci_uart_tx_wakeup+0x9c/0x160 [hci_uart]) > [] (hci_uart_tx_wakeup+0x9c/0x160 [hci_uart]) from [] (hci_uart_tty_wakeup+0x58/0x5c [hci_uart]) > [] (hci_uart_tty_wakeup+0x58/0x5c [hci_uart]) from [] (tty_wakeup+0x48/0x68) > [] (tty_wakeup+0x48/0x68) from [] (uart_write_wakeup+0x2c/0x30) > [] (uart_write_wakeup+0x2c/0x30) from [] (serial8250_tx_chars+0xf0/0x140) > [] (serial8250_tx_chars+0xf0/0x140) from [] (serial8250_handle_irq+0x6c/0x88) > [] (serial8250_handle_irq+0x6c/0x88) from [] (serial8250_default_handle_irq+0x30/0x34) > [] (serial8250_default_handle_irq+0x30/0x34) from [] (serial8250_interrupt+0x44/0xc0) > [] (serial8250_interrupt+0x44/0xc0) from [] (handle_irq_event_percpu+0xc4/0x2cc) > [] (handle_irq_event_percpu+0xc4/0x2cc) from [] (handle_irq_event+0x4c/0x6c) > [] (handle_irq_event+0x4c/0x6c) from [] (handle_edge_irq+0x114/0x14c) > [] (handle_edge_irq+0x114/0x14c) from [] (generic_handle_irq+0x40/0x54) > [] (generic_handle_irq+0x40/0x54) from [] (gpio_irq_handler+0x168/0x1ac) > [] (gpio_irq_handler+0x168/0x1ac) from [] (generic_handle_irq+0x40/0x54) > [] (generic_handle_irq+0x40/0x54) from [] (handle_IRQ+0x70/0x94) > [] (handle_IRQ+0x70/0x94) from [] (omap3_intc_handle_irq+0x64/0x78) > [] (omap3_intc_handle_irq+0x64/0x78) from [] (__irq_svc+0x44/0x78) > --->8--- > > Signed-off-by: Andreas Bie?mann > Cc: Marcel Holtmann > Cc: Gustavo Padovan > Cc: Johan Hedberg > Cc: linux-bluetooth@vger.kernel.org > --- > > It seems at least one other guy had the very same problem with another uart > (mpc52xx): http://www.spinics.net/lists/linux-rt-users/msg09246.html > > I wonder, if my approach is right. It is runtime tested with 3.4.87 on our > board and work around the mentioned recursive locking. But I do not know, if > it should be fixed in another way. > If it is right, I'd work some more on it to get the fix mainline. > > drivers/bluetooth/hci_ldisc.c | 49 ++++++++++++++++++++++++++++++++++++----- > drivers/bluetooth/hci_uart.h | 2 ++ > 2 files changed, 45 insertions(+), 6 deletions(-) This seems to be tackling the same problem as the following patch from Felipe Balbi (of which a new revision was sent earlier today): Subject: [PATCH 02/13] bluetooth: hci_ldisc: fix deadlock condition Johan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/