* [PATCH 0/2] wireless: fix 11d lockdep warning
@ 2009-07-30 0:46 Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 1/2] cfg80211: use goto out on 11d reg hint failure Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex Luis R. Rodriguez
0 siblings, 2 replies; 7+ messages in thread
From: Luis R. Rodriguez @ 2009-07-30 0:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
This fixes the lockdep warning for me. We take a much simpler
approach than that mess I was doing before. cfg80211 stuff will
go later.
Luis R. Rodriguez (2):
cfg80211: use goto out on 11d reg hint failure
cfg80211: decouple regulatory variables from cfg80211_mutex
net/wireless/core.c | 4 +---
net/wireless/reg.c | 32 +++++++++++++++++++++++---------
2 files changed, 24 insertions(+), 12 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] cfg80211: use goto out on 11d reg hint failure
2009-07-30 0:46 [PATCH 0/2] wireless: fix 11d lockdep warning Luis R. Rodriguez
@ 2009-07-30 0:46 ` Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex Luis R. Rodriguez
1 sibling, 0 replies; 7+ messages in thread
From: Luis R. Rodriguez @ 2009-07-30 0:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
This does not change functionality.
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
net/wireless/reg.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index fb40428..2d1d183 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1774,10 +1774,8 @@ void regulatory_hint_11d(struct wiphy *wiphy,
mutex_lock(&cfg80211_mutex);
- if (unlikely(!last_request)) {
- mutex_unlock(&cfg80211_mutex);
- return;
- }
+ if (unlikely(!last_request))
+ goto out;
/* IE len must be evenly divisible by 2 */
if (country_ie_len & 0x01)
--
1.6.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex
2009-07-30 0:46 [PATCH 0/2] wireless: fix 11d lockdep warning Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 1/2] cfg80211: use goto out on 11d reg hint failure Luis R. Rodriguez
@ 2009-07-30 0:46 ` Luis R. Rodriguez
2009-07-30 7:43 ` Johannes Berg
1 sibling, 1 reply; 7+ messages in thread
From: Luis R. Rodriguez @ 2009-07-30 0:46 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Luis R. Rodriguez
We change regulatory code to be protected by its own regulatory
spinlock and alleviate cfg80211_mutex to only be used to protect
cfg80211_rdev_list, the registered device list.
By doing this we will be able to work on regulatory core components
without having to have hog up the cfg80211_mutex. An example here is
we no longer need to use the cfg80211_mutex during the country IE
regulatory hint; by doing so we end up curing this new lockdep warning:
=======================================================
[ INFO: possible circular locking dependency detected ]
2.6.31-rc4-wl #12
-------------------------------------------------------
phy1/1709 is trying to acquire lock:
(cfg80211_mutex){+.+.+.}, at: [<ffffffffa00af852>] regulatory_hint_11d+0x32/0x3f0 [cfg80211]
but task is already holding lock:
(&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (&ifmgd->mtx){+.+.+.}:
[<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
[<ffffffff81085dd3>] lock_acquire+0xe3/0x120
[<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
[<ffffffffa0141bb8>] ieee80211_mgd_auth+0x108/0x1f0 [mac80211]
[<ffffffffa0148563>] ieee80211_auth+0x13/0x20 [mac80211]
[<ffffffffa00bc3a1>] __cfg80211_mlme_auth+0x1b1/0x2a0 [cfg80211]
[<ffffffffa00bc516>] cfg80211_mlme_auth+0x86/0xc0 [cfg80211]
[<ffffffffa00b368d>] nl80211_authenticate+0x21d/0x230 [cfg80211]
[<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
[<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
[<ffffffff814169d9>] genl_rcv+0x29/0x40
[<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
[<ffffffff81416514>] netlink_sendmsg+0x214/0x300
[<ffffffff813e4407>] sock_sendmsg+0x107/0x130
[<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
[<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
-> #2 (&wdev->mtx){+.+.+.}:
[<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
[<ffffffff81085dd3>] lock_acquire+0xe3/0x120
[<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
[<ffffffffa00ab304>] cfg80211_netdev_notifier_call+0x1a4/0x390 [cfg80211]
[<ffffffff814f3dff>] notifier_call_chain+0x3f/0x80
[<ffffffff81075a91>] raw_notifier_call_chain+0x11/0x20
[<ffffffff813f665a>] dev_open+0x10a/0x120
[<ffffffff813f59bd>] dev_change_flags+0x9d/0x1e0
[<ffffffff8144eb6e>] devinet_ioctl+0x6fe/0x760
[<ffffffff81450204>] inet_ioctl+0x94/0xc0
[<ffffffff813e25fa>] sock_ioctl+0x6a/0x290
[<ffffffff8111e911>] vfs_ioctl+0x31/0xa0
[<ffffffff8111ea9a>] do_vfs_ioctl+0x8a/0x5c0
[<ffffffff8111f069>] sys_ioctl+0x99/0xa0
[<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
-> #1 (&rdev->mtx){+.+.+.}:
[<ffffffff810857b6>] __lock_acquire+0xd76/0x12b0
[<ffffffff81085dd3>] lock_acquire+0xe3/0x120
[<ffffffff814eeae4>] mutex_lock_nested+0x44/0x350
[<ffffffffa00ac4d0>] cfg80211_get_dev_from_ifindex+0x60/0x90 [cfg80211]
[<ffffffffa00b21ff>] get_rdev_dev_by_info_ifindex+0x6f/0xa0 [cfg80211]
[<ffffffffa00b51eb>] nl80211_set_interface+0x3b/0x260 [cfg80211]
[<ffffffff81416ba6>] genl_rcv_msg+0x1b6/0x1f0
[<ffffffff81415c39>] netlink_rcv_skb+0x89/0xb0
[<ffffffff814169d9>] genl_rcv+0x29/0x40
[<ffffffff8141591d>] netlink_unicast+0x29d/0x2b0
[<ffffffff81416514>] netlink_sendmsg+0x214/0x300
[<ffffffff813e4407>] sock_sendmsg+0x107/0x130
[<ffffffff813e45b9>] sys_sendmsg+0x189/0x320
[<ffffffff81011f82>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
other info that might help us debug this:
3 locks held by phy1/1709:
#0: ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
#1: (&ifmgd->work){+.+.+.}, at: [<ffffffff8106b45d>] worker_thread+0x19d/0x340
#2: (&ifmgd->mtx){+.+.+.}, at: [<ffffffffa0144228>] ieee80211_sta_work+0x108/0x10f0 [mac80211]
Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
net/wireless/core.c | 4 +---
net/wireless/reg.c | 26 +++++++++++++++++++++-----
2 files changed, 22 insertions(+), 8 deletions(-)
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 755cdf1..d8dbd7f 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -34,9 +34,7 @@ MODULE_DESCRIPTION("wireless configuration support");
LIST_HEAD(cfg80211_rdev_list);
/*
- * This is used to protect the cfg80211_rdev_list, cfg80211_regdomain,
- * country_ie_regdomain, the reg_beacon_list and the the last regulatory
- * request receipt (last_request).
+ * This is used to protect the cfg80211_rdev_list
*/
DEFINE_MUTEX(cfg80211_mutex);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 2d1d183..41c16fc 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -62,6 +62,15 @@ const struct ieee80211_regdomain *cfg80211_regdomain;
*/
static const struct ieee80211_regdomain *country_ie_regdomain;
+/*
+ * Protects static reg.c components:
+ * - cfg80211_world_regdom
+ * - cfg80211_regdom
+ * - country_ie_regdomain
+ * - last_request
+ */
+static spinlock_t reg_lock;
+
/* Used to queue up regulatory hints */
static LIST_HEAD(reg_requests_list);
static spinlock_t reg_requests_lock;
@@ -1570,6 +1579,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
BUG_ON(!reg_request->alpha2);
mutex_lock(&cfg80211_mutex);
+ spin_lock(®_lock);
if (wiphy_idx_valid(reg_request->wiphy_idx))
wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
@@ -1585,6 +1595,7 @@ static void reg_process_hint(struct regulatory_request *reg_request)
if (r == -EALREADY && wiphy && wiphy->strict_regulatory)
wiphy_update_regulatory(wiphy, reg_request->initiator);
out:
+ spin_unlock(®_lock);
mutex_unlock(&cfg80211_mutex);
}
@@ -1734,13 +1745,12 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
}
EXPORT_SYMBOL(regulatory_hint);
+/* Caller must hold reg_lock */
static bool reg_same_country_ie_hint(struct wiphy *wiphy,
u32 country_ie_checksum)
{
struct wiphy *request_wiphy;
- assert_cfg80211_lock();
-
if (unlikely(last_request->initiator !=
NL80211_REGDOM_SET_BY_COUNTRY_IE))
return false;
@@ -1772,7 +1782,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
enum environment_cap env = ENVIRON_ANY;
struct regulatory_request *request;
- mutex_lock(&cfg80211_mutex);
+ spin_lock(®_lock);
if (unlikely(!last_request))
goto out;
@@ -1883,7 +1893,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
request->country_ie_checksum = checksum;
request->country_ie_env = env;
- mutex_unlock(&cfg80211_mutex);
+ spin_unlock(®_lock);
queue_regulatory_request(request);
@@ -1892,7 +1902,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
free_rd_out:
kfree(rd);
out:
- mutex_unlock(&cfg80211_mutex);
+ spin_unlock(®_lock);
}
EXPORT_SYMBOL(regulatory_hint_11d);
@@ -2225,10 +2235,13 @@ int set_regdom(const struct ieee80211_regdomain *rd)
assert_cfg80211_lock();
+ spin_lock(®_lock);
+
/* Note that this doesn't update the wiphys, this is done below */
r = __set_regdom(rd);
if (r) {
kfree(rd);
+ spin_unlock(®_lock);
return r;
}
@@ -2243,6 +2256,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
nl80211_send_reg_change_event(last_request);
+ spin_unlock(®_lock);
+
return r;
}
@@ -2275,6 +2290,7 @@ int regulatory_init(void)
spin_lock_init(®_requests_lock);
spin_lock_init(®_pending_beacons_lock);
+ spin_lock_init(®_lock);
#ifdef CONFIG_WIRELESS_OLD_REGULATORY
cfg80211_regdomain = static_regdom(ieee80211_regdom);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex
2009-07-30 0:46 ` [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex Luis R. Rodriguez
@ 2009-07-30 7:43 ` Johannes Berg
2009-07-30 14:54 ` Luis R. Rodriguez
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2009-07-30 7:43 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
On Wed, 2009-07-29 at 17:46 -0700, Luis R. Rodriguez wrote:
> We change regulatory code to be protected by its own regulatory
> spinlock and alleviate cfg80211_mutex to only be used to protect
> cfg80211_rdev_list, the registered device list.
>
> By doing this we will be able to work on regulatory core components
> without having to have hog up the cfg80211_mutex. An example here is
> we no longer need to use the cfg80211_mutex during the country IE
> regulatory hint; by doing so we end up curing this new lockdep warning:
This is ok with me, but be sure that it changes the API between drivers
and cfg80211 -- the reg notifier can no longer sleep now.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex
2009-07-30 7:43 ` Johannes Berg
@ 2009-07-30 14:54 ` Luis R. Rodriguez
2009-07-30 15:32 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Luis R. Rodriguez @ 2009-07-30 14:54 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
On Thu, Jul 30, 2009 at 12:43 AM, Johannes
Berg<johannes@sipsolutions.net> wrote:
> On Wed, 2009-07-29 at 17:46 -0700, Luis R. Rodriguez wrote:
>> We change regulatory code to be protected by its own regulatory
>> spinlock and alleviate cfg80211_mutex to only be used to protect
>> cfg80211_rdev_list, the registered device list.
>>
>> By doing this we will be able to work on regulatory core components
>> without having to have hog up the cfg80211_mutex. An example here is
>> we no longer need to use the cfg80211_mutex during the country IE
>> regulatory hint; by doing so we end up curing this new lockdep warning:
>
> This is ok with me, but be sure that it changes the API between drivers
> and cfg80211 -- the reg notifier can no longer sleep now.
We could use a mutex as well.
Luis
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex
2009-07-30 14:54 ` Luis R. Rodriguez
@ 2009-07-30 15:32 ` Johannes Berg
2009-07-30 15:34 ` Luis R. Rodriguez
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2009-07-30 15:32 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: linville, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 1110 bytes --]
On Thu, 2009-07-30 at 07:54 -0700, Luis R. Rodriguez wrote:
> On Thu, Jul 30, 2009 at 12:43 AM, Johannes
> Berg<johannes@sipsolutions.net> wrote:
> > On Wed, 2009-07-29 at 17:46 -0700, Luis R. Rodriguez wrote:
> >> We change regulatory code to be protected by its own regulatory
> >> spinlock and alleviate cfg80211_mutex to only be used to protect
> >> cfg80211_rdev_list, the registered device list.
> >>
> >> By doing this we will be able to work on regulatory core components
> >> without having to have hog up the cfg80211_mutex. An example here is
> >> we no longer need to use the cfg80211_mutex during the country IE
> >> regulatory hint; by doing so we end up curing this new lockdep warning:
> >
> > This is ok with me, but be sure that it changes the API between drivers
> > and cfg80211 -- the reg notifier can no longer sleep now.
>
> We could use a mutex as well.
Right. I don't care, just wanted you to be aware of the change. I don't
expect many drivers to really use the reg notifier stuff so if it
doesn't matter for ath.ko it probably just doesn't matter.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex
2009-07-30 15:32 ` Johannes Berg
@ 2009-07-30 15:34 ` Luis R. Rodriguez
0 siblings, 0 replies; 7+ messages in thread
From: Luis R. Rodriguez @ 2009-07-30 15:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
On Thu, Jul 30, 2009 at 8:32 AM, Johannes Berg<johannes@sipsolutions.net> wrote:
> On Thu, 2009-07-30 at 07:54 -0700, Luis R. Rodriguez wrote:
>> On Thu, Jul 30, 2009 at 12:43 AM, Johannes
>> Berg<johannes@sipsolutions.net> wrote:
>> > On Wed, 2009-07-29 at 17:46 -0700, Luis R. Rodriguez wrote:
>> >> We change regulatory code to be protected by its own regulatory
>> >> spinlock and alleviate cfg80211_mutex to only be used to protect
>> >> cfg80211_rdev_list, the registered device list.
>> >>
>> >> By doing this we will be able to work on regulatory core components
>> >> without having to have hog up the cfg80211_mutex. An example here is
>> >> we no longer need to use the cfg80211_mutex during the country IE
>> >> regulatory hint; by doing so we end up curing this new lockdep warning:
>> >
>> > This is ok with me, but be sure that it changes the API between drivers
>> > and cfg80211 -- the reg notifier can no longer sleep now.
>>
>> We could use a mutex as well.
>
> Right. I don't care, just wanted you to be aware of the change. I don't
> expect many drivers to really use the reg notifier stuff so if it
> doesn't matter for ath.ko it probably just doesn't matter.
Yeah understood, I'll still respin with a mutex.
Luis
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-07-30 15:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-30 0:46 [PATCH 0/2] wireless: fix 11d lockdep warning Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 1/2] cfg80211: use goto out on 11d reg hint failure Luis R. Rodriguez
2009-07-30 0:46 ` [PATCH 2/2] cfg80211: decouple regulatory variables from cfg80211_mutex Luis R. Rodriguez
2009-07-30 7:43 ` Johannes Berg
2009-07-30 14:54 ` Luis R. Rodriguez
2009-07-30 15:32 ` Johannes Berg
2009-07-30 15:34 ` Luis R. Rodriguez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).