From patchwork Wed Mar 7 20:19:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 145337 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id BE29DB6EF3 for ; Thu, 8 Mar 2012 07:19:32 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932626Ab2CGUTa (ORCPT ); Wed, 7 Mar 2012 15:19:30 -0500 Received: from www.linutronix.de ([62.245.132.108]:38637 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757021Ab2CGUT3 convert rfc822-to-8bit (ORCPT ); Wed, 7 Mar 2012 15:19:29 -0500 Received: from bigeasy by Galois.linutronix.de with local (Exim 4.72) (envelope-from ) id 1S5NKa-0007tE-Bp; Wed, 07 Mar 2012 21:19:28 +0100 Date: Wed, 7 Mar 2012 21:19:28 +0100 From: Sebastian Andrzej Siewior To: Oliver Neukum Cc: netdev@vger.kernel.org, linux-usb@vger.kernel.org Subject: [REPOST PATCH] net/usbnet: avoid recursive locking in usbnet_stop() Message-ID: <20120307201928.GA30188@linutronix.de> MIME-Version: 1.0 Content-Disposition: inline X-Key-Id: 97C4700B X-Key-Fingerprint: 09E2 D1F3 9A3A FF13 C3D3 961C 0688 1C1E 97C4 700B User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org |kernel BUG at kernel/rtmutex.c:724! |[] (rt_spin_lock_slowlock+0x108/0x2bc) from [] (defer_bh+0x1c/0xb4) |[] (defer_bh+0x1c/0xb4) from [] (rx_complete+0x14c/0x194) |[] (rx_complete+0x14c/0x194) from [] (usb_hcd_giveback_urb+0xa0/0xf0) |[] (usb_hcd_giveback_urb+0xa0/0xf0) from [] (musb_giveback+0x34/0x40) |[] (musb_giveback+0x34/0x40) from [] (musb_advance_schedule+0xb4/0x1c0) |[] (musb_advance_schedule+0xb4/0x1c0) from [] (musb_cleanup_urb.isra.9+0x80/0x8c) |[] (musb_cleanup_urb.isra.9+0x80/0x8c) from [] (musb_urb_dequeue+0xec/0x108) |[] (musb_urb_dequeue+0xec/0x108) from [] (unlink1+0xbc/0xcc) |[] (unlink1+0xbc/0xcc) from [] (usb_hcd_unlink_urb+0x54/0xa8) |[] (usb_hcd_unlink_urb+0x54/0xa8) from [] (unlink_urbs.isra.17+0x2c/0x58) |[] (unlink_urbs.isra.17+0x2c/0x58) from [] (usbnet_terminate_urbs+0x94/0x10c) |[] (usbnet_terminate_urbs+0x94/0x10c) from [] (usbnet_stop+0x100/0x15c) |[] (usbnet_stop+0x100/0x15c) from [] (__dev_close_many+0x94/0xc8) defer_bh() takes the lock which is hold during unlink_urbs(). The safe walk suggest that the skb will be removed from the list and this is done by defer_bh() so it seems to be okay to drop the lock here. Cc: stable@kernel.org Reported-by: Aníbal Almeida Pinto Signed-off-by: Sebastian Andrzej Siewior Acked-by: Oliver Neukum --- According to [0] the usb driver has to assume that the HCD will call the ->complete() callback and therefore not hold any lock which are acquired in the ->complete() callback. [0] http://git.kernel.org/?p=linux/kernel/git/gregkh/usb.git;a=commitdiff;h=371f3b49f2cb1a8b6ac09b6b108841ca92349eb1;hp=2a5be8783e0016d15e7907ddd212b2c312e196eb drivers/net/usb/usbnet.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index fae0fbd..81b96e3 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -589,6 +589,7 @@ static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q) entry = (struct skb_data *) skb->cb; urb = entry->urb; + spin_unlock_irqrestore(&q->lock, flags); // during some PM-driven resume scenarios, // these (async) unlinks complete immediately retval = usb_unlink_urb (urb); @@ -596,6 +597,7 @@ static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q) netdev_dbg(dev->net, "unlink urb err, %d\n", retval); else count++; + spin_lock_irqsave(&q->lock, flags); } spin_unlock_irqrestore (&q->lock, flags); return count;