Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752901Ab1BUQZS (ORCPT ); Mon, 21 Feb 2011 11:25:18 -0500 Received: from he.sipsolutions.net ([78.46.109.217]:34247 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751699Ab1BUQZR (ORCPT ); Mon, 21 Feb 2011 11:25:17 -0500 Subject: Re: [2.6.38-rc5, patch] fix cfg80211_wext_siwfreq lock ordering... From: Johannes Berg To: Daniel J Blueman Cc: linux-wireless@vger.kernel.org, Linux Kernel In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Date: Mon, 21 Feb 2011 17:25:10 +0100 Message-ID: <1298305510.3836.7.camel@jlt3.sipsolutions.net> Mime-Version: 1.0 X-Mailer: Evolution 2.32.2 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5513 Lines: 129 On Tue, 2011-02-22 at 00:11 +0800, Daniel J Blueman wrote: > I previously managed to reproduce a hang while scanning wireless > channels (reproducible with airodump-ng hopping channels); subsequent > lockdep instrumentation revealed a lock ordering issue [1]. > > Without knowing the design intent, it looks like the locks should be > taken in reverse order; please comment. > > Signed-off-by: Daniel J Blueman > > --- [1] Yeah, looks this way, thanks. Acked-by: Johannes Berg > ======================================================= > [ INFO: possible circular locking dependency detected ] > 2.6.38-rc5-341cd #4 > ------------------------------------------------------- > airodump-ng/15445 is trying to acquire lock: > (&rdev->devlist_mtx){+.+.+.}, at: [] > cfg80211_wext_siwfreq+0xc6/0x100 > > but task is already holding lock: > (&wdev->mtx){+.+.+.}, at: [] cfg80211_wext_siwfreq+0xbc/0x100 > > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > > -> #1 (&wdev->mtx){+.+.+.}: > [] lock_acquire+0xc6/0x280 > [] mutex_lock_nested+0x6e/0x4b0 > [] cfg80211_netdev_notifier_call+0x430/0x5f0 > [] notifier_call_chain+0x8b/0x100 > [] raw_notifier_call_chain+0x11/0x20 > [] call_netdevice_notifiers+0x32/0x60 > [] __dev_notify_flags+0x34/0x80 > [] dev_change_flags+0x40/0x70 > [] do_setlink+0x1fc/0x8d0 > [] rtnl_setlink+0xf2/0x140 > [] rtnetlink_rcv_msg+0x163/0x270 > [] netlink_rcv_skb+0xa1/0xd0 > [] rtnetlink_rcv+0x20/0x30 > [] netlink_unicast+0x2ba/0x300 > [] netlink_sendmsg+0x267/0x3e0 > [] sock_sendmsg+0xe4/0x110 > [] sys_sendmsg+0x253/0x3b0 > [] system_call_fastpath+0x16/0x1b > > -> #0 (&rdev->devlist_mtx){+.+.+.}: > [] __lock_acquire+0x1622/0x1d10 > [] lock_acquire+0xc6/0x280 > [] mutex_lock_nested+0x6e/0x4b0 > [] cfg80211_wext_siwfreq+0xc6/0x100 > [] ioctl_standard_call+0x5d/0xd0 > [] T.808+0x163/0x170 > [] wext_handle_ioctl+0x3a/0x90 > [] dev_ioctl+0x6f2/0x830 > [] sock_ioctl+0xfd/0x290 > [] do_vfs_ioctl+0x9d/0x590 > [] sys_ioctl+0x4a/0x80 > [] system_call_fastpath+0x16/0x1b > > other info that might help us debug this: > > 2 locks held by airodump-ng/15445: > #0: (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x12/0x20 > #1: (&wdev->mtx){+.+.+.}, at: [] > cfg80211_wext_siwfreq+0xbc/0x100 > > stack backtrace: > Pid: 15445, comm: airodump-ng Not tainted 2.6.38-rc5-341cd #4 > Call Trace: > [] ? print_circular_bug+0xfa/0x100 > [] ? __lock_acquire+0x1622/0x1d10 > [] ? trace_hardirqs_off_caller+0x29/0xc0 > [] ? lock_acquire+0xc6/0x280 > [] ? cfg80211_wext_siwfreq+0xc6/0x100 > [] ? mark_held_locks+0x67/0x90 > [] ? mutex_lock_nested+0x6e/0x4b0 > [] ? cfg80211_wext_siwfreq+0xc6/0x100 > [] ? mark_held_locks+0x67/0x90 > [] ? cfg80211_wext_siwfreq+0xc6/0x100 > [] ? cfg80211_wext_siwfreq+0xc6/0x100 > [] ? ioctl_standard_call+0x5d/0xd0 > [] ? __dev_get_by_name+0x9b/0xc0 > [] ? ioctl_standard_call+0x0/0xd0 > [] ? T.808+0x163/0x170 > [] ? might_fault+0x72/0xd0 > [] ? wext_handle_ioctl+0x3a/0x90 > [] ? might_fault+0xbb/0xd0 > [] ? dev_ioctl+0x6f2/0x830 > [] ? put_lock_stats+0xe/0x40 > [] ? lock_release_holdtime+0xac/0x150 > [] ? sock_ioctl+0xfd/0x290 > [] ? do_vfs_ioctl+0x9d/0x590 > [] ? fget_light+0x1df/0x3c0 > [] ? sys_ioctl+0x4a/0x80 > [] ? system_call_fastpath+0x16/0x1b > > --- [2] > > diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c > index 3e5dbd4..d112f03 100644 > --- a/net/wireless/wext-compat.c > +++ b/net/wireless/wext-compat.c > @@ -802,11 +802,11 @@ int cfg80211_wext_siwfreq(struct net_device *dev, > return freq; > if (freq == 0) > return -EINVAL; > - wdev_lock(wdev); > mutex_lock(&rdev->devlist_mtx); > + wdev_lock(wdev); > err = cfg80211_set_freq(rdev, wdev, freq, NL80211_CHAN_NO_HT); > - mutex_unlock(&rdev->devlist_mtx); > wdev_unlock(wdev); > + mutex_unlock(&rdev->devlist_mtx); > return err; > default: > return -EOPNOTSUPP; -- 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/