* [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).