From abc7381bcca6ce9dc101f112a13e14957bfbda7e Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 30 Jul 2009 17:38:08 -0700 Subject: [PATCH] cfg80211: decouple regulatory variables from cfg80211_mutex We change regulatory code to be protected by its own regulatory mutex 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 driver specific wiphy_apply_custom_regulatory(). We also no longer need it for the 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: [] regulatory_hint_11d+0x32/0x3f0 [cfg80211] but task is already holding lock: (&ifmgd->mtx){+.+.+.}, at: [] 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){+.+.+.}: [] __lock_acquire+0xd76/0x12b0 [] lock_acquire+0xe3/0x120 [] mutex_lock_nested+0x44/0x350 [] ieee80211_mgd_auth+0x108/0x1f0 [mac80211] [] ieee80211_auth+0x13/0x20 [mac80211] [] __cfg80211_mlme_auth+0x1b1/0x2a0 [cfg80211] [] cfg80211_mlme_auth+0x86/0xc0 [cfg80211] [] nl80211_authenticate+0x21d/0x230 [cfg80211] [] genl_rcv_msg+0x1b6/0x1f0 [] netlink_rcv_skb+0x89/0xb0 [] genl_rcv+0x29/0x40 [] netlink_unicast+0x29d/0x2b0 [] netlink_sendmsg+0x214/0x300 [] sock_sendmsg+0x107/0x130 [] sys_sendmsg+0x189/0x320 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff -> #2 (&wdev->mtx){+.+.+.}: [] __lock_acquire+0xd76/0x12b0 [] lock_acquire+0xe3/0x120 [] mutex_lock_nested+0x44/0x350 [] cfg80211_netdev_notifier_call+0x1a4/0x390 [cfg80211] [] notifier_call_chain+0x3f/0x80 [] raw_notifier_call_chain+0x11/0x20 [] dev_open+0x10a/0x120 [] dev_change_flags+0x9d/0x1e0 [] devinet_ioctl+0x6fe/0x760 [] inet_ioctl+0x94/0xc0 [] sock_ioctl+0x6a/0x290 [] vfs_ioctl+0x31/0xa0 [] do_vfs_ioctl+0x8a/0x5c0 [] sys_ioctl+0x99/0xa0 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff -> #1 (&rdev->mtx){+.+.+.}: [] __lock_acquire+0xd76/0x12b0 [] lock_acquire+0xe3/0x120 [] mutex_lock_nested+0x44/0x350 [] cfg80211_get_dev_from_ifindex+0x60/0x90 [cfg80211] [] get_rdev_dev_by_info_ifindex+0x6f/0xa0 [cfg80211] [] nl80211_set_interface+0x3b/0x260 [cfg80211] [] genl_rcv_msg+0x1b6/0x1f0 [] netlink_rcv_skb+0x89/0xb0 [] genl_rcv+0x29/0x40 [] netlink_unicast+0x29d/0x2b0 [] netlink_sendmsg+0x214/0x300 [] sock_sendmsg+0x107/0x130 [] sys_sendmsg+0x189/0x320 [] system_call_fastpath+0x16/0x1b [] 0xffffffffffffffff other info that might help us debug this: 3 locks held by phy1/1709: #0: ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [] worker_thread+0x19d/0x340 #1: (&ifmgd->work){+.+.+.}, at: [] worker_thread+0x19d/0x340 #2: (&ifmgd->mtx){+.+.+.}, at: [] ieee80211_sta_work+0x108/0x10f0 [mac80211] Reported-by: Reinette Chatre Acked-by: Johannes Berg Signed-off-by: Luis R. Rodriguez Signed-off-by: John W. Linville --- net/wireless/core.c | 4 +--- net/wireless/reg.c | 46 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/net/wireless/core.c b/net/wireless/core.c index 1493285b6a4d..cd7dff9c75f4 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 f386981734a0..6ab56f098de1 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -62,6 +62,16 @@ 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 + */ +DEFINE_MUTEX(reg_mutex); +#define assert_reg_lock() WARN_ON(!mutex_is_locked(®_mutex)) + /* Used to queue up regulatory hints */ static LIST_HEAD(reg_requests_list); static spinlock_t reg_requests_lock; @@ -1293,7 +1303,7 @@ static void handle_channel_custom(struct wiphy *wiphy, struct ieee80211_supported_band *sband; struct ieee80211_channel *chan; - assert_cfg80211_lock(); + assert_reg_lock(); sband = wiphy->bands[band]; BUG_ON(chan_idx >= sband->n_channels); @@ -1342,14 +1352,14 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy, enum ieee80211_band band; unsigned int bands_set = 0; - mutex_lock(&cfg80211_mutex); + mutex_lock(®_mutex); for (band = 0; band < IEEE80211_NUM_BANDS; band++) { if (!wiphy->bands[band]) continue; handle_band_custom(wiphy, band, regd); bands_set++; } - mutex_unlock(&cfg80211_mutex); + mutex_unlock(®_mutex); /* * no point in calling this if it won't have any effect @@ -1495,7 +1505,7 @@ static int ignore_request(struct wiphy *wiphy, * Returns zero if all went fine, %-EALREADY if a regulatory domain had * already been set or other standard error codes. * - * Caller must hold &cfg80211_mutex + * Caller must hold &cfg80211_mutex and ®_mutex */ static int __regulatory_hint(struct wiphy *wiphy, struct regulatory_request *pending_request) @@ -1570,6 +1580,7 @@ static void reg_process_hint(struct regulatory_request *reg_request) BUG_ON(!reg_request->alpha2); mutex_lock(&cfg80211_mutex); + mutex_lock(®_mutex); if (wiphy_idx_valid(reg_request->wiphy_idx)) wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx); @@ -1585,6 +1596,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: + mutex_unlock(®_mutex); mutex_unlock(&cfg80211_mutex); } @@ -1613,6 +1625,10 @@ static void reg_process_pending_beacon_hints(void) struct cfg80211_registered_device *rdev; struct reg_beacon *pending_beacon, *tmp; + /* + * No need to hold the reg_mutex here as we just touch wiphys + * and do not read or access regulatory variables. + */ mutex_lock(&cfg80211_mutex); /* This goes through the _pending_ beacon list */ @@ -1734,12 +1750,13 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2) } EXPORT_SYMBOL(regulatory_hint); +/* Caller must hold reg_mutex */ static bool reg_same_country_ie_hint(struct wiphy *wiphy, u32 country_ie_checksum) { struct wiphy *request_wiphy; - assert_cfg80211_lock(); + assert_reg_lock(); if (unlikely(last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE)) @@ -1776,7 +1793,7 @@ void regulatory_hint_11d(struct wiphy *wiphy, enum environment_cap env = ENVIRON_ANY; struct regulatory_request *request; - mutex_lock(&cfg80211_mutex); + mutex_lock(®_mutex); if (unlikely(!last_request)) goto out; @@ -1850,7 +1867,7 @@ void regulatory_hint_11d(struct wiphy *wiphy, request->country_ie_checksum = checksum; request->country_ie_env = env; - mutex_unlock(&cfg80211_mutex); + mutex_unlock(®_mutex); queue_regulatory_request(request); @@ -1859,7 +1876,7 @@ void regulatory_hint_11d(struct wiphy *wiphy, free_rd_out: kfree(rd); out: - mutex_unlock(&cfg80211_mutex); + mutex_unlock(®_mutex); } EXPORT_SYMBOL(regulatory_hint_11d); @@ -2192,10 +2209,13 @@ int set_regdom(const struct ieee80211_regdomain *rd) assert_cfg80211_lock(); + mutex_lock(®_mutex); + /* Note that this doesn't update the wiphys, this is done below */ r = __set_regdom(rd); if (r) { kfree(rd); + mutex_unlock(®_mutex); return r; } @@ -2210,6 +2230,8 @@ int set_regdom(const struct ieee80211_regdomain *rd) nl80211_send_reg_change_event(last_request); + mutex_unlock(®_mutex); + return r; } @@ -2220,16 +2242,20 @@ void reg_device_remove(struct wiphy *wiphy) assert_cfg80211_lock(); + mutex_lock(®_mutex); + kfree(wiphy->regd); if (last_request) request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx); if (!request_wiphy || request_wiphy != wiphy) - return; + goto out; last_request->wiphy_idx = WIPHY_IDX_STALE; last_request->country_ie_env = ENVIRON_ANY; +out: + mutex_unlock(®_mutex); } int regulatory_init(void) @@ -2290,6 +2316,7 @@ void regulatory_exit(void) cancel_work_sync(®_work); mutex_lock(&cfg80211_mutex); + mutex_lock(®_mutex); reset_regdomains(); @@ -2328,5 +2355,6 @@ void regulatory_exit(void) } spin_unlock(®_requests_lock); + mutex_unlock(®_mutex); mutex_unlock(&cfg80211_mutex); } -- 2.39.2