linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/24] regulatory fixes, cleanups & improvements
@ 2012-12-06 16:47 Johannes Berg
  2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
                   ` (24 more replies)
  0 siblings, 25 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless

The biggest improvement is using RCU, this will allow checking
against the regdomain from any context and makes the wiphy's
regd pointer safe to use for drivers. It also simplifies the
locking since cfg80211_mutex isn't needed any more for those
things, only for wdev iteration/access. 

Over my previous [RFC] series, the changes are:
 old 18: dropped, will be part of VHT changes
 new 01: fixing an array access bug
 new 02: clean up the intersect more, in particular
         don't memset all the time
 new 20: fix a memory leak
 new 21: don't let cfg80211_regdomain pointer be NULL
 new 22/23: RCU
 new 24: use IS_ERR macro family

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 01/24] regulatory: don't write past array when intersecting rules
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-06 23:43   ` Luis R. Rodriguez
  2012-12-10 21:55   ` Johannes Berg
  2012-12-06 16:47 ` [PATCH 02/24] regulatory: don't allocate too much memory Johannes Berg
                   ` (23 subsequent siblings)
  24 siblings, 2 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

When intersecting rules, we count first to know how many
rules need to be allocated, and then do the intersection
into the allocated array. However, the code doing this
writes past the end of the array because it attempts to
do all intersections. Make it stop when the right number
of rules has been reached.

Cc: stable@vger.kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index b6c7ea6..4197359 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect(
 	if (!rd)
 		return NULL;
 
-	for (x = 0; x < rd1->n_reg_rules; x++) {
+	for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) {
 		rule1 = &rd1->reg_rules[x];
-		for (y = 0; y < rd2->n_reg_rules; y++) {
+		for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) {
 			rule2 = &rd2->reg_rules[y];
 			/*
 			 * This time around instead of using the stack lets
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 02/24] regulatory: don't allocate too much memory
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
  2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-06 23:47   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 03/24] regulatory: clean up regdom_intersect Johannes Berg
                   ` (22 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need to allocate one reg rule more
than will be used, reduce the allocations. The
allocation in nl80211 already doesn't allocate
too much space.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4197359..70255c9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -319,8 +319,9 @@ static int reg_copy_regd(const struct ieee80211_regdomain **dst_regd,
 	int size_of_regd = 0;
 	unsigned int i;
 
-	size_of_regd = sizeof(struct ieee80211_regdomain) +
-	  ((src_regd->n_reg_rules + 1) * sizeof(struct ieee80211_reg_rule));
+	size_of_regd =
+		sizeof(struct ieee80211_regdomain) +
+		src_regd->n_reg_rules * sizeof(struct ieee80211_reg_rule);
 
 	regd = kzalloc(size_of_regd, GFP_KERNEL);
 	if (!regd)
@@ -642,7 +643,7 @@ static struct ieee80211_regdomain *regdom_intersect(
 		return NULL;
 
 	size_of_regd = sizeof(struct ieee80211_regdomain) +
-		((num_rules + 1) * sizeof(struct ieee80211_reg_rule));
+		       num_rules * sizeof(struct ieee80211_reg_rule);
 
 	rd = kzalloc(size_of_regd, GFP_KERNEL);
 	if (!rd)
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 03/24] regulatory: clean up regdom_intersect
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
  2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
  2012-12-06 16:47 ` [PATCH 02/24] regulatory: don't allocate too much memory Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-06 23:55   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 04/24] regulatory: clean up reg_copy_regd() Johannes Berg
                   ` (21 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

As the dummy_rule (also renamed from irule) is only
used for output by the reg_rules_intersect() function
there's no need to clear it at all, remove that.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 70255c9..0f25a1a 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -609,12 +609,7 @@ static struct ieee80211_regdomain *regdom_intersect(
 	struct ieee80211_reg_rule *intersected_rule;
 	struct ieee80211_regdomain *rd;
 	/* This is just a dummy holder to help us count */
-	struct ieee80211_reg_rule irule;
-
-	/* Uses the stack temporarily for counter arithmetic */
-	intersected_rule = &irule;
-
-	memset(intersected_rule, 0, sizeof(struct ieee80211_reg_rule));
+	struct ieee80211_reg_rule dummy_rule;
 
 	if (!rd1 || !rd2)
 		return NULL;
@@ -631,11 +626,8 @@ static struct ieee80211_regdomain *regdom_intersect(
 		rule1 = &rd1->reg_rules[x];
 		for (y = 0; y < rd2->n_reg_rules; y++) {
 			rule2 = &rd2->reg_rules[y];
-			if (!reg_rules_intersect(rule1, rule2,
-					intersected_rule))
+			if (!reg_rules_intersect(rule1, rule2, &dummy_rule))
 				num_rules++;
-			memset(intersected_rule, 0,
-					sizeof(struct ieee80211_reg_rule));
 		}
 	}
 
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 04/24] regulatory: clean up reg_copy_regd()
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (2 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 03/24] regulatory: clean up regdom_intersect Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-06 23:59   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 05/24] regulatory: don't test list before iterating Johannes Berg
                   ` (20 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Use ERR_PTR/IS_ERR to return the result or errors,
also do some code cleanups.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 48 +++++++++++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 0f25a1a..45adc8f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -312,11 +312,11 @@ static bool is_user_regdom_saved(void)
 	return true;
 }
 
-static int reg_copy_regd(const struct ieee80211_regdomain **dst_regd,
-			 const struct ieee80211_regdomain *src_regd)
+static const struct ieee80211_regdomain *
+reg_copy_regd(const struct ieee80211_regdomain *src_regd)
 {
 	struct ieee80211_regdomain *regd;
-	int size_of_regd = 0;
+	int size_of_regd;
 	unsigned int i;
 
 	size_of_regd =
@@ -325,16 +325,15 @@ static int reg_copy_regd(const struct ieee80211_regdomain **dst_regd,
 
 	regd = kzalloc(size_of_regd, GFP_KERNEL);
 	if (!regd)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	memcpy(regd, src_regd, sizeof(struct ieee80211_regdomain));
 
 	for (i = 0; i < src_regd->n_reg_rules; i++)
 		memcpy(&regd->reg_rules[i], &src_regd->reg_rules[i],
-			sizeof(struct ieee80211_reg_rule));
+		       sizeof(struct ieee80211_reg_rule));
 
-	*dst_regd = regd;
-	return 0;
+	return regd;
 }
 
 #ifdef CONFIG_CFG80211_INTERNAL_REGDB
@@ -349,9 +348,8 @@ static DEFINE_MUTEX(reg_regdb_search_mutex);
 static void reg_regdb_search(struct work_struct *work)
 {
 	struct reg_regdb_search_request *request;
-	const struct ieee80211_regdomain *curdom, *regdom;
+	const struct ieee80211_regdomain *curdom, *regdom = NULL;
 	int i, r;
-	bool set_reg = false;
 
 	mutex_lock(&cfg80211_mutex);
 
@@ -366,10 +364,7 @@ static void reg_regdb_search(struct work_struct *work)
 			curdom = reg_regdb[i];
 
 			if (!memcmp(request->alpha2, curdom->alpha2, 2)) {
-				r = reg_copy_regd(&regdom, curdom);
-				if (r)
-					break;
-				set_reg = true;
+				regdom = reg_copy_regd(curdom);
 				break;
 			}
 		}
@@ -378,7 +373,7 @@ static void reg_regdb_search(struct work_struct *work)
 	}
 	mutex_unlock(&reg_regdb_search_mutex);
 
-	if (set_reg)
+	if (!IS_ERR_OR_NULL(regdom))
 		set_regdom(regdom);
 
 	mutex_unlock(&cfg80211_mutex);
@@ -1510,6 +1505,7 @@ static void reg_set_request_processed(void)
 static int __regulatory_hint(struct wiphy *wiphy,
 			     struct regulatory_request *pending_request)
 {
+	const struct ieee80211_regdomain *regd;
 	bool intersect = false;
 	int r = 0;
 
@@ -1520,11 +1516,12 @@ static int __regulatory_hint(struct wiphy *wiphy,
 	if (r == REG_INTERSECT) {
 		if (pending_request->initiator ==
 		    NL80211_REGDOM_SET_BY_DRIVER) {
-			r = reg_copy_regd(&wiphy->regd, cfg80211_regdomain);
-			if (r) {
+			regd = reg_copy_regd(cfg80211_regdomain);
+			if (IS_ERR(regd)) {
 				kfree(pending_request);
-				return r;
+				return PTR_ERR(regd);
 			}
+			wiphy->regd = regd;
 		}
 		intersect = true;
 	} else if (r) {
@@ -1536,12 +1533,13 @@ static int __regulatory_hint(struct wiphy *wiphy,
 		if (r == -EALREADY &&
 		    pending_request->initiator ==
 		    NL80211_REGDOM_SET_BY_DRIVER) {
-			r = reg_copy_regd(&wiphy->regd, cfg80211_regdomain);
-			if (r) {
+			regd = reg_copy_regd(cfg80211_regdomain);
+			if (IS_ERR(regd)) {
 				kfree(pending_request);
-				return r;
+				return PTR_ERR(regd);
 			}
 			r = -EALREADY;
+			wiphy->regd = regd;
 			goto new_request;
 		}
 		kfree(pending_request);
@@ -2201,6 +2199,7 @@ static void print_regdomain_info(const struct ieee80211_regdomain *rd)
 /* Takes ownership of rd only if it doesn't fail */
 static int __set_regdom(const struct ieee80211_regdomain *rd)
 {
+	const struct ieee80211_regdomain *regd;
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 	struct wiphy *request_wiphy;
 	/* Some basic sanity checks first */
@@ -2258,8 +2257,6 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	if (!last_request->intersect) {
-		int r;
-
 		if (last_request->initiator != NL80211_REGDOM_SET_BY_DRIVER) {
 			reset_regdomains(false);
 			cfg80211_regdomain = rd;
@@ -2278,10 +2275,11 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		if (request_wiphy->regd)
 			return -EALREADY;
 
-		r = reg_copy_regd(&request_wiphy->regd, rd);
-		if (r)
-			return r;
+		regd = reg_copy_regd(rd);
+		if (IS_ERR(regd))
+			return PTR_ERR(regd);
 
+		request_wiphy->regd = regd;
 		reset_regdomains(false);
 		cfg80211_regdomain = rd;
 		return 0;
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 05/24] regulatory: don't test list before iterating
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (3 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 04/24] regulatory: clean up reg_copy_regd() Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:02   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 06/24] regulatory: simplify regulatory_hint_11d Johannes Berg
                   ` (19 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need to test whether a list is
empty or not before iterating.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 66 ++++++++++++++++--------------------------------------
 1 file changed, 19 insertions(+), 47 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 45adc8f..333a921 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1105,9 +1105,6 @@ static void wiphy_update_beacon_reg(struct wiphy *wiphy)
 
 	assert_cfg80211_lock();
 
-	if (list_empty(&reg_beacon_list))
-		return;
-
 	list_for_each_entry(reg_beacon, &reg_beacon_list, list) {
 		if (!wiphy->bands[reg_beacon->chan.band])
 			continue;
@@ -1667,11 +1664,6 @@ static void reg_process_pending_beacon_hints(void)
 	/* This goes through the _pending_ beacon list */
 	spin_lock_bh(&reg_pending_beacons_lock);
 
-	if (list_empty(&reg_pending_beacons)) {
-		spin_unlock_bh(&reg_pending_beacons_lock);
-		goto out;
-	}
-
 	list_for_each_entry_safe(pending_beacon, tmp,
 				 &reg_pending_beacons, list) {
 
@@ -1686,7 +1678,6 @@ static void reg_process_pending_beacon_hints(void)
 	}
 
 	spin_unlock_bh(&reg_pending_beacons_lock);
-out:
 	mutex_unlock(&cfg80211_mutex);
 }
 
@@ -1950,34 +1941,24 @@ static void restore_regulatory_settings(bool reset_user)
 	 * settings.
 	 */
 	spin_lock(&reg_requests_lock);
-	if (!list_empty(&reg_requests_list)) {
-		list_for_each_entry_safe(reg_request, tmp,
-					 &reg_requests_list, list) {
-			if (reg_request->initiator !=
-			    NL80211_REGDOM_SET_BY_USER)
-				continue;
-			list_move_tail(&reg_request->list, &tmp_reg_req_list);
-		}
+	list_for_each_entry_safe(reg_request, tmp, &reg_requests_list, list) {
+		if (reg_request->initiator != NL80211_REGDOM_SET_BY_USER)
+			continue;
+		list_move_tail(&reg_request->list, &tmp_reg_req_list);
 	}
 	spin_unlock(&reg_requests_lock);
 
 	/* Clear beacon hints */
 	spin_lock_bh(&reg_pending_beacons_lock);
-	if (!list_empty(&reg_pending_beacons)) {
-		list_for_each_entry_safe(reg_beacon, btmp,
-					 &reg_pending_beacons, list) {
-			list_del(&reg_beacon->list);
-			kfree(reg_beacon);
-		}
+	list_for_each_entry_safe(reg_beacon, btmp, &reg_pending_beacons, list) {
+		list_del(&reg_beacon->list);
+		kfree(reg_beacon);
 	}
 	spin_unlock_bh(&reg_pending_beacons_lock);
 
-	if (!list_empty(&reg_beacon_list)) {
-		list_for_each_entry_safe(reg_beacon, btmp,
-					 &reg_beacon_list, list) {
-			list_del(&reg_beacon->list);
-			kfree(reg_beacon);
-		}
+	list_for_each_entry_safe(reg_beacon, btmp, &reg_beacon_list, list) {
+		list_del(&reg_beacon->list);
+		kfree(reg_beacon);
 	}
 
 	/* First restore to the basic regulatory settings */
@@ -2491,30 +2472,21 @@ void /* __init_or_exit */ regulatory_exit(void)
 	platform_device_unregister(reg_pdev);
 
 	spin_lock_bh(&reg_pending_beacons_lock);
-	if (!list_empty(&reg_pending_beacons)) {
-		list_for_each_entry_safe(reg_beacon, btmp,
-					 &reg_pending_beacons, list) {
-			list_del(&reg_beacon->list);
-			kfree(reg_beacon);
-		}
+	list_for_each_entry_safe(reg_beacon, btmp, &reg_pending_beacons, list) {
+		list_del(&reg_beacon->list);
+		kfree(reg_beacon);
 	}
 	spin_unlock_bh(&reg_pending_beacons_lock);
 
-	if (!list_empty(&reg_beacon_list)) {
-		list_for_each_entry_safe(reg_beacon, btmp,
-					 &reg_beacon_list, list) {
-			list_del(&reg_beacon->list);
-			kfree(reg_beacon);
-		}
+	list_for_each_entry_safe(reg_beacon, btmp, &reg_beacon_list, list) {
+		list_del(&reg_beacon->list);
+		kfree(reg_beacon);
 	}
 
 	spin_lock(&reg_requests_lock);
-	if (!list_empty(&reg_requests_list)) {
-		list_for_each_entry_safe(reg_request, tmp,
-					 &reg_requests_list, list) {
-			list_del(&reg_request->list);
-			kfree(reg_request);
-		}
+	list_for_each_entry_safe(reg_request, tmp, &reg_requests_list, list) {
+		list_del(&reg_request->list);
+		kfree(reg_request);
 	}
 	spin_unlock(&reg_requests_lock);
 
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 06/24] regulatory: simplify regulatory_hint_11d
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (4 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 05/24] regulatory: don't test list before iterating Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:10   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 07/24] regulatory: code cleanup Johannes Berg
                   ` (18 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's no need to unlock before calling
queue_regulatory_request(), so simplify
the function.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 333a921..902e138 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1826,12 +1826,7 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 	request->initiator = NL80211_REGDOM_SET_BY_COUNTRY_IE;
 	request->country_ie_env = env;
 
-	mutex_unlock(&reg_mutex);
-
 	queue_regulatory_request(request);
-
-	return;
-
 out:
 	mutex_unlock(&reg_mutex);
 }
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 07/24] regulatory: code cleanup
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (5 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 06/24] regulatory: simplify regulatory_hint_11d Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:11   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 08/24] regulatory: remove useless locking on exit Johannes Berg
                   ` (17 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Clean up various things like indentation, extra
parentheses, too many/few line breaks, etc.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c |  15 ++-
 net/wireless/reg.c     | 286 ++++++++++++++++++-------------------------------
 net/wireless/reg.h     |   4 +-
 net/wireless/sme.c     |   6 +-
 4 files changed, 116 insertions(+), 195 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 0a8f2c6..e5c2988 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4283,7 +4283,7 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 		dfs_region = nla_get_u8(info->attrs[NL80211_ATTR_DFS_REGION]);
 
 	nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
-			rem_reg_rules) {
+			    rem_reg_rules) {
 		num_rules++;
 		if (num_rules > NL80211_MAX_SUPP_REG_RULES)
 			return -EINVAL;
@@ -4297,7 +4297,7 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	size_of_regd = sizeof(struct ieee80211_regdomain) +
-		(num_rules * sizeof(struct ieee80211_reg_rule));
+		       num_rules * sizeof(struct ieee80211_reg_rule);
 
 	rd = kzalloc(size_of_regd, GFP_KERNEL);
 	if (!rd) {
@@ -4317,10 +4317,10 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 		rd->dfs_region = dfs_region;
 
 	nla_for_each_nested(nl_reg_rule, info->attrs[NL80211_ATTR_REG_RULES],
-			rem_reg_rules) {
+			    rem_reg_rules) {
 		nla_parse(tb, NL80211_REG_RULE_ATTR_MAX,
-			nla_data(nl_reg_rule), nla_len(nl_reg_rule),
-			reg_rule_policy);
+			  nla_data(nl_reg_rule), nla_len(nl_reg_rule),
+			  reg_rule_policy);
 		r = parse_reg_rule(tb, &rd->reg_rules[rule_idx]);
 		if (r)
 			goto bad_reg;
@@ -4336,10 +4336,7 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 	BUG_ON(rule_idx != num_rules);
 
 	r = set_regdom(rd);
-
-	mutex_unlock(&cfg80211_mutex);
-
-	return r;
+	rd = NULL;
 
  bad_reg:
 	mutex_unlock(&cfg80211_mutex);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 902e138..7391624 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -48,7 +48,6 @@
 #include <linux/export.h>
 #include <linux/slab.h>
 #include <linux/list.h>
-#include <linux/random.h>
 #include <linux/ctype.h>
 #include <linux/nl80211.h>
 #include <linux/platform_device.h>
@@ -220,18 +219,14 @@ bool is_world_regdom(const char *alpha2)
 {
 	if (!alpha2)
 		return false;
-	if (alpha2[0] == '0' && alpha2[1] == '0')
-		return true;
-	return false;
+	return alpha2[0] == '0' && alpha2[1] == '0';
 }
 
 static bool is_alpha2_set(const char *alpha2)
 {
 	if (!alpha2)
 		return false;
-	if (alpha2[0] != 0 && alpha2[1] != 0)
-		return true;
-	return false;
+	return alpha2[0] && alpha2[1];
 }
 
 static bool is_unknown_alpha2(const char *alpha2)
@@ -242,9 +237,7 @@ static bool is_unknown_alpha2(const char *alpha2)
 	 * Special case where regulatory domain was built by driver
 	 * but a specific alpha2 cannot be determined
 	 */
-	if (alpha2[0] == '9' && alpha2[1] == '9')
-		return true;
-	return false;
+	return alpha2[0] == '9' && alpha2[1] == '9';
 }
 
 static bool is_intersected_alpha2(const char *alpha2)
@@ -256,28 +249,21 @@ static bool is_intersected_alpha2(const char *alpha2)
 	 * result of an intersection between two regulatory domain
 	 * structures
 	 */
-	if (alpha2[0] == '9' && alpha2[1] == '8')
-		return true;
-	return false;
+	return alpha2[0] == '9' && alpha2[1] == '8';
 }
 
 static bool is_an_alpha2(const char *alpha2)
 {
 	if (!alpha2)
 		return false;
-	if (isalpha(alpha2[0]) && isalpha(alpha2[1]))
-		return true;
-	return false;
+	return isalpha(alpha2[0]) && isalpha(alpha2[1]);
 }
 
 static bool alpha2_equal(const char *alpha2_x, const char *alpha2_y)
 {
 	if (!alpha2_x || !alpha2_y)
 		return false;
-	if (alpha2_x[0] == alpha2_y[0] &&
-		alpha2_x[1] == alpha2_y[1])
-		return true;
-	return false;
+	return alpha2_x[0] == alpha2_y[0] && alpha2_x[1] == alpha2_y[1];
 }
 
 static bool regdom_changes(const char *alpha2)
@@ -286,9 +272,7 @@ static bool regdom_changes(const char *alpha2)
 
 	if (!cfg80211_regdomain)
 		return true;
-	if (alpha2_equal(cfg80211_regdomain->alpha2, alpha2))
-		return false;
-	return true;
+	return !alpha2_equal(cfg80211_regdomain->alpha2, alpha2);
 }
 
 /*
@@ -302,11 +286,9 @@ static bool is_user_regdom_saved(void)
 		return false;
 
 	/* This would indicate a mistake on the design */
-	if (WARN((!is_world_regdom(user_alpha2) &&
-		  !is_an_alpha2(user_alpha2)),
+	if (WARN(!is_world_regdom(user_alpha2) && !is_an_alpha2(user_alpha2),
 		 "Unexpected user alpha2: %c%c\n",
-		 user_alpha2[0],
-	         user_alpha2[1]))
+		 user_alpha2[0], user_alpha2[1]))
 		return false;
 
 	return true;
@@ -360,10 +342,10 @@ static void reg_regdb_search(struct work_struct *work)
 					   list);
 		list_del(&request->list);
 
-		for (i=0; i<reg_regdb_size; i++) {
+		for (i = 0; i < reg_regdb_size; i++) {
 			curdom = reg_regdb[i];
 
-			if (!memcmp(request->alpha2, curdom->alpha2, 2)) {
+			if (alpha2_equal(request->alpha2, curdom->alpha2)) {
 				regdom = reg_copy_regd(curdom);
 				break;
 			}
@@ -457,7 +439,7 @@ static bool is_valid_reg_rule(const struct ieee80211_reg_rule *rule)
 	freq_diff = freq_range->end_freq_khz - freq_range->start_freq_khz;
 
 	if (freq_range->end_freq_khz <= freq_range->start_freq_khz ||
-			freq_range->max_bandwidth_khz > freq_diff)
+	    freq_range->max_bandwidth_khz > freq_diff)
 		return false;
 
 	return true;
@@ -515,7 +497,7 @@ static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
  * regulatory rule support for other "bands".
  **/
 static bool freq_in_rule_band(const struct ieee80211_freq_range *freq_range,
-	u32 freq_khz)
+			      u32 freq_khz)
 {
 #define ONE_GHZ_IN_KHZ	1000000
 	/*
@@ -537,10 +519,9 @@ static bool freq_in_rule_band(const struct ieee80211_freq_range *freq_range,
  * Helper for regdom_intersect(), this does the real
  * mathematical intersection fun
  */
-static int reg_rules_intersect(
-	const struct ieee80211_reg_rule *rule1,
-	const struct ieee80211_reg_rule *rule2,
-	struct ieee80211_reg_rule *intersected_rule)
+static int reg_rules_intersect(const struct ieee80211_reg_rule *rule1,
+			       const struct ieee80211_reg_rule *rule2,
+			       struct ieee80211_reg_rule *intersected_rule)
 {
 	const struct ieee80211_freq_range *freq_range1, *freq_range2;
 	struct ieee80211_freq_range *freq_range;
@@ -557,11 +538,11 @@ static int reg_rules_intersect(
 	power_rule = &intersected_rule->power_rule;
 
 	freq_range->start_freq_khz = max(freq_range1->start_freq_khz,
-		freq_range2->start_freq_khz);
+					 freq_range2->start_freq_khz);
 	freq_range->end_freq_khz = min(freq_range1->end_freq_khz,
-		freq_range2->end_freq_khz);
+				       freq_range2->end_freq_khz);
 	freq_range->max_bandwidth_khz = min(freq_range1->max_bandwidth_khz,
-		freq_range2->max_bandwidth_khz);
+					    freq_range2->max_bandwidth_khz);
 
 	freq_diff = freq_range->end_freq_khz - freq_range->start_freq_khz;
 	if (freq_range->max_bandwidth_khz > freq_diff)
@@ -572,7 +553,7 @@ static int reg_rules_intersect(
 	power_rule->max_antenna_gain = min(power_rule1->max_antenna_gain,
 		power_rule2->max_antenna_gain);
 
-	intersected_rule->flags = (rule1->flags | rule2->flags);
+	intersected_rule->flags = rule1->flags | rule2->flags;
 
 	if (!is_valid_reg_rule(intersected_rule))
 		return -EINVAL;
@@ -593,9 +574,9 @@ static int reg_rules_intersect(
  * resulting intersection of rules between rd1 and rd2. We will
  * kzalloc() this structure for you.
  */
-static struct ieee80211_regdomain *regdom_intersect(
-	const struct ieee80211_regdomain *rd1,
-	const struct ieee80211_regdomain *rd2)
+static struct ieee80211_regdomain *
+regdom_intersect(const struct ieee80211_regdomain *rd1,
+		 const struct ieee80211_regdomain *rd2)
 {
 	int r, size_of_regd;
 	unsigned int x, y;
@@ -646,8 +627,7 @@ static struct ieee80211_regdomain *regdom_intersect(
 			 * a memcpy()
 			 */
 			intersected_rule = &rd->reg_rules[rule_idx];
-			r = reg_rules_intersect(rule1, rule2,
-				intersected_rule);
+			r = reg_rules_intersect(rule1, rule2, intersected_rule);
 			/*
 			 * No need to memset here the intersected rule here as
 			 * we're not using the stack anymore
@@ -732,9 +712,7 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 		if (!band_rule_found)
 			band_rule_found = freq_in_rule_band(fr, center_freq);
 
-		bw_fits = reg_does_bw_fit(fr,
-					  center_freq,
-					  desired_bw_khz);
+		bw_fits = reg_does_bw_fit(fr, center_freq, desired_bw_khz);
 
 		if (band_rule_found && bw_fits) {
 			*reg_rule = rr;
@@ -748,17 +726,13 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 	return -EINVAL;
 }
 
-int freq_reg_info(struct wiphy *wiphy,
-		  u32 center_freq,
-		  u32 desired_bw_khz,
+int freq_reg_info(struct wiphy *wiphy, u32 center_freq, u32 desired_bw_khz,
 		  const struct ieee80211_reg_rule **reg_rule)
 {
 	assert_cfg80211_lock();
-	return freq_reg_info_regd(wiphy,
-				  center_freq,
-				  desired_bw_khz,
-				  reg_rule,
-				  NULL);
+
+	return freq_reg_info_regd(wiphy, center_freq, desired_bw_khz,
+				  reg_rule, NULL);
 }
 EXPORT_SYMBOL(freq_reg_info);
 
@@ -796,16 +770,12 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
 	else
 		snprintf(max_antenna_gain, 32, "%d", power_rule->max_antenna_gain);
 
-	REG_DBG_PRINT("Updating information on frequency %d MHz "
-		      "for a %d MHz width channel with regulatory rule:\n",
-		      chan->center_freq,
-		      KHZ_TO_MHZ(desired_bw_khz));
+	REG_DBG_PRINT("Updating information on frequency %d MHz for a %d MHz width channel with regulatory rule:\n",
+		      chan->center_freq, KHZ_TO_MHZ(desired_bw_khz));
 
 	REG_DBG_PRINT("%d KHz - %d KHz @ %d KHz), (%s mBi, %d mBm)\n",
-		      freq_range->start_freq_khz,
-		      freq_range->end_freq_khz,
-		      freq_range->max_bandwidth_khz,
-		      max_antenna_gain,
+		      freq_range->start_freq_khz, freq_range->end_freq_khz,
+		      freq_range->max_bandwidth_khz, max_antenna_gain,
 		      power_rule->max_eirp);
 }
 #else
@@ -851,11 +821,8 @@ static void handle_channel(struct wiphy *wiphy,
 
 	flags = chan->orig_flags;
 
-	r = freq_reg_info(wiphy,
-			  MHZ_TO_KHZ(chan->center_freq),
-			  desired_bw_khz,
-			  &reg_rule);
-
+	r = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq),
+			  desired_bw_khz, &reg_rule);
 	if (r) {
 		/*
 		 * We will disable all channels that do not match our
@@ -903,8 +870,9 @@ static void handle_channel(struct wiphy *wiphy,
 
 	chan->beacon_found = false;
 	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
-	chan->max_antenna_gain = min(chan->orig_mag,
-		(int) MBI_TO_DBI(power_rule->max_antenna_gain));
+	chan->max_antenna_gain =
+		min_t(int, chan->orig_mag,
+		      MBI_TO_DBI(power_rule->max_antenna_gain));
 	chan->max_reg_power = (int) MBM_TO_DBM(power_rule->max_eirp);
 	if (chan->orig_mpwr) {
 		/*
@@ -923,8 +891,7 @@ static void handle_channel(struct wiphy *wiphy,
 		chan->max_power = chan->max_reg_power;
 }
 
-static void handle_band(struct wiphy *wiphy,
-			enum ieee80211_band band,
+static void handle_band(struct wiphy *wiphy, enum ieee80211_band band,
 			enum nl80211_reg_initiator initiator)
 {
 	unsigned int i;
@@ -941,51 +908,48 @@ static bool reg_request_cell_base(struct regulatory_request *request)
 {
 	if (request->initiator != NL80211_REGDOM_SET_BY_USER)
 		return false;
-	if (request->user_reg_hint_type != NL80211_USER_REG_HINT_CELL_BASE)
-		return false;
-	return true;
+	return request->user_reg_hint_type == NL80211_USER_REG_HINT_CELL_BASE;
 }
 
 bool reg_last_request_cell_base(void)
 {
 	bool val;
+
 	assert_cfg80211_lock();
 
 	mutex_lock(&reg_mutex);
 	val = reg_request_cell_base(last_request);
 	mutex_unlock(&reg_mutex);
+
 	return val;
 }
 
 #ifdef CONFIG_CFG80211_CERTIFICATION_ONUS
-
 /* Core specific check */
 static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
 {
 	if (!reg_num_devs_support_basehint)
 		return -EOPNOTSUPP;
 
-	if (reg_request_cell_base(last_request)) {
-		if (!regdom_changes(pending_request->alpha2))
-			return -EALREADY;
-		return 0;
-	}
+	if (reg_request_cell_base(last_request) &&
+	    !regdom_changes(pending_request->alpha2))
+		return -EALREADY;
+
 	return 0;
 }
 
 /* Device specific check */
 static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
 {
-	if (!(wiphy->features & NL80211_FEATURE_CELL_BASE_REG_HINTS))
-		return true;
-	return false;
+	return !(wiphy->features & NL80211_FEATURE_CELL_BASE_REG_HINTS);
 }
 #else
 static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
 {
 	return -EOPNOTSUPP;
 }
-static int reg_dev_ignore_cell_hint(struct wiphy *wiphy)
+
+static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
 {
 	return true;
 }
@@ -996,17 +960,14 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 			      enum nl80211_reg_initiator initiator)
 {
 	if (!last_request) {
-		REG_DBG_PRINT("Ignoring regulatory request %s since "
-			      "last_request is not set\n",
+		REG_DBG_PRINT("Ignoring regulatory request %s since last_request is not set\n",
 			      reg_initiator_name(initiator));
 		return true;
 	}
 
 	if (initiator == NL80211_REGDOM_SET_BY_CORE &&
 	    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY) {
-		REG_DBG_PRINT("Ignoring regulatory request %s "
-			      "since the driver uses its own custom "
-			      "regulatory domain\n",
+		REG_DBG_PRINT("Ignoring regulatory request %s since the driver uses its own custom regulatory domain\n",
 			      reg_initiator_name(initiator));
 		return true;
 	}
@@ -1018,9 +979,7 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	if (wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY && !wiphy->regd &&
 	    initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
 	    !is_world_regdom(last_request->alpha2)) {
-		REG_DBG_PRINT("Ignoring regulatory request %s "
-			      "since the driver requires its own regulatory "
-			      "domain to be set first\n",
+		REG_DBG_PRINT("Ignoring regulatory request %s since the driver requires its own regulatory domain to be set first\n",
 			      reg_initiator_name(initiator));
 		return true;
 	}
@@ -1031,8 +990,7 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	return false;
 }
 
-static void handle_reg_beacon(struct wiphy *wiphy,
-			      unsigned int chan_idx,
+static void handle_reg_beacon(struct wiphy *wiphy, unsigned int chan_idx,
 			      struct reg_beacon *reg_beacon)
 {
 	struct ieee80211_supported_band *sband;
@@ -1140,16 +1098,14 @@ static void reg_process_beacons(struct wiphy *wiphy)
 	wiphy_update_beacon_reg(wiphy);
 }
 
-static bool is_ht40_not_allowed(struct ieee80211_channel *chan)
+static bool is_ht40_allowed(struct ieee80211_channel *chan)
 {
 	if (!chan)
-		return true;
+		return false;
 	if (chan->flags & IEEE80211_CHAN_DISABLED)
-		return true;
+		return false;
 	/* This would happen when regulatory rules disallow HT40 completely */
-	if (IEEE80211_CHAN_NO_HT40 == (chan->flags & (IEEE80211_CHAN_NO_HT40)))
-		return true;
-	return false;
+	return !(chan->flags & IEEE80211_CHAN_NO_HT40);
 }
 
 static void reg_process_ht_flags_channel(struct wiphy *wiphy,
@@ -1167,7 +1123,7 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
 	BUG_ON(chan_idx >= sband->n_channels);
 	channel = &sband->channels[chan_idx];
 
-	if (is_ht40_not_allowed(channel)) {
+	if (!is_ht40_allowed(channel)) {
 		channel->flags |= IEEE80211_CHAN_NO_HT40;
 		return;
 	}
@@ -1178,6 +1134,7 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
 	 */
 	for (i = 0; i < sband->n_channels; i++) {
 		struct ieee80211_channel *c = &sband->channels[i];
+
 		if (c->center_freq == (channel->center_freq - 20))
 			channel_before = c;
 		if (c->center_freq == (channel->center_freq + 20))
@@ -1189,12 +1146,12 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
 	 * if that ever changes we also need to change the below logic
 	 * to include that as well.
 	 */
-	if (is_ht40_not_allowed(channel_before))
+	if (!is_ht40_allowed(channel_before))
 		channel->flags |= IEEE80211_CHAN_NO_HT40MINUS;
 	else
 		channel->flags &= ~IEEE80211_CHAN_NO_HT40MINUS;
 
-	if (is_ht40_not_allowed(channel_after))
+	if (!is_ht40_allowed(channel_after))
 		channel->flags |= IEEE80211_CHAN_NO_HT40PLUS;
 	else
 		channel->flags &= ~IEEE80211_CHAN_NO_HT40PLUS;
@@ -1246,6 +1203,7 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 
 	reg_process_beacons(wiphy);
 	reg_process_ht_flags(wiphy);
+
 	if (wiphy->reg_notifier)
 		wiphy->reg_notifier(wiphy, last_request);
 }
@@ -1290,18 +1248,12 @@ static void handle_channel_custom(struct wiphy *wiphy,
 	BUG_ON(chan_idx >= sband->n_channels);
 	chan = &sband->channels[chan_idx];
 
-	r = freq_reg_info_regd(wiphy,
-			       MHZ_TO_KHZ(chan->center_freq),
-			       desired_bw_khz,
-			       &reg_rule,
-			       regd);
+	r = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
+			       desired_bw_khz, &reg_rule, regd);
 
 	if (r) {
-		REG_DBG_PRINT("Disabling freq %d MHz as custom "
-			      "regd has no rule that fits a %d MHz "
-			      "wide channel\n",
-			      chan->center_freq,
-			      KHZ_TO_MHZ(desired_bw_khz));
+		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits a %d MHz wide channel\n",
+			      chan->center_freq, KHZ_TO_MHZ(desired_bw_khz));
 		chan->flags = IEEE80211_CHAN_DISABLED;
 		return;
 	}
@@ -1351,7 +1303,7 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 
 	/*
 	 * no point in calling this if it won't have any effect
-	 * on your device's supportd bands.
+	 * on your device's supported bands.
 	 */
 	WARN_ON(!bands_set);
 }
@@ -1380,7 +1332,6 @@ static int ignore_request(struct wiphy *wiphy,
 	case NL80211_REGDOM_SET_BY_CORE:
 		return 0;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
-
 		if (reg_request_cell_base(last_request)) {
 			/* Trust a Cell base station over the AP's country IE */
 			if (regdom_changes(pending_request->alpha2))
@@ -1445,18 +1396,17 @@ static int ignore_request(struct wiphy *wiphy,
 		 * to their country before the IE is picked up
 		 */
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_USER &&
-			  last_request->intersect)
+		    last_request->intersect)
 			return -EOPNOTSUPP;
 		/*
 		 * Process user requests only after previous user/driver/core
 		 * requests have been processed
 		 */
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_CORE ||
-		    last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
-		    last_request->initiator == NL80211_REGDOM_SET_BY_USER) {
-			if (regdom_changes(last_request->alpha2))
-				return -EAGAIN;
-		}
+		if ((last_request->initiator == NL80211_REGDOM_SET_BY_CORE ||
+		     last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
+		     last_request->initiator == NL80211_REGDOM_SET_BY_USER) &&
+		    regdom_changes(last_request->alpha2))
+			return -EAGAIN;
 
 		if (!regdom_changes(pending_request->alpha2))
 			return -EALREADY;
@@ -1586,8 +1536,7 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 	if (wiphy_idx_valid(reg_request->wiphy_idx))
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
 
-	if (reg_initiator == NL80211_REGDOM_SET_BY_DRIVER &&
-	    !wiphy) {
+	if (reg_initiator == NL80211_REGDOM_SET_BY_DRIVER && !wiphy) {
 		kfree(reg_request);
 		return;
 	}
@@ -1604,8 +1553,7 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 	 * We only time out user hints, given that they should be the only
 	 * source of bogus requests.
 	 */
-	if (r != -EALREADY &&
-	    reg_initiator == NL80211_REGDOM_SET_BY_USER)
+	if (r != -EALREADY && reg_initiator == NL80211_REGDOM_SET_BY_USER)
 		schedule_delayed_work(&reg_timeout, msecs_to_jiffies(3142));
 }
 
@@ -1623,8 +1571,7 @@ static void reg_process_pending_hints(void)
 
 	/* When last_request->processed becomes true this will be rescheduled */
 	if (last_request && !last_request->processed) {
-		REG_DBG_PRINT("Pending regulatory request, waiting "
-			      "for it to be processed...\n");
+		REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
 		goto out;
 	}
 
@@ -1666,7 +1613,6 @@ static void reg_process_pending_beacon_hints(void)
 
 	list_for_each_entry_safe(pending_beacon, tmp,
 				 &reg_pending_beacons, list) {
-
 		list_del_init(&pending_beacon->list);
 
 		/* Applies the beacon hint to current wiphys */
@@ -1709,8 +1655,7 @@ static int regulatory_hint_core(const char *alpha2)
 {
 	struct regulatory_request *request;
 
-	request = kzalloc(sizeof(struct regulatory_request),
-			  GFP_KERNEL);
+	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
 	if (!request)
 		return -ENOMEM;
 
@@ -1777,10 +1722,8 @@ EXPORT_SYMBOL(regulatory_hint);
  * We hold wdev_lock() here so we cannot hold cfg80211_mutex() and
  * therefore cannot iterate over the rdev list here.
  */
-void regulatory_hint_11d(struct wiphy *wiphy,
-			 enum ieee80211_band band,
-			 const u8 *country_ie,
-			 u8 country_ie_len)
+void regulatory_hint_11d(struct wiphy *wiphy, enum ieee80211_band band,
+			 const u8 *country_ie, u8 country_ie_len)
 {
 	char alpha2[2];
 	enum environment_cap env = ENVIRON_ANY;
@@ -1812,8 +1755,8 @@ void regulatory_hint_11d(struct wiphy *wiphy,
 	 * cfg80211_mutex.
 	 */
 	if (likely(last_request->initiator ==
-	    NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    wiphy_idx_valid(last_request->wiphy_idx)))
+		   NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+		   wiphy_idx_valid(last_request->wiphy_idx)))
 		goto out;
 
 	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
@@ -1841,8 +1784,7 @@ static void restore_alpha2(char *alpha2, bool reset_user)
 	if (is_user_regdom_saved()) {
 		/* Unless we're asked to ignore it and reset it */
 		if (reset_user) {
-			REG_DBG_PRINT("Restoring regulatory settings "
-			       "including user preference\n");
+			REG_DBG_PRINT("Restoring regulatory settings including user preference\n");
 			user_alpha2[0] = '9';
 			user_alpha2[1] = '7';
 
@@ -1852,26 +1794,20 @@ static void restore_alpha2(char *alpha2, bool reset_user)
 			 * back as they were for a full restore.
 			 */
 			if (!is_world_regdom(ieee80211_regdom)) {
-				REG_DBG_PRINT("Keeping preference on "
-				       "module parameter ieee80211_regdom: %c%c\n",
-				       ieee80211_regdom[0],
-				       ieee80211_regdom[1]);
+				REG_DBG_PRINT("Keeping preference on module parameter ieee80211_regdom: %c%c\n",
+					      ieee80211_regdom[0], ieee80211_regdom[1]);
 				alpha2[0] = ieee80211_regdom[0];
 				alpha2[1] = ieee80211_regdom[1];
 			}
 		} else {
-			REG_DBG_PRINT("Restoring regulatory settings "
-			       "while preserving user preference for: %c%c\n",
-			       user_alpha2[0],
-			       user_alpha2[1]);
+			REG_DBG_PRINT("Restoring regulatory settings while preserving user preference for: %c%c\n",
+				      user_alpha2[0], user_alpha2[1]);
 			alpha2[0] = user_alpha2[0];
 			alpha2[1] = user_alpha2[1];
 		}
 	} else if (!is_world_regdom(ieee80211_regdom)) {
-		REG_DBG_PRINT("Keeping preference on "
-		       "module parameter ieee80211_regdom: %c%c\n",
-		       ieee80211_regdom[0],
-		       ieee80211_regdom[1]);
+		REG_DBG_PRINT("Keeping preference on module parameter ieee80211_regdom: %c%c\n",
+			      ieee80211_regdom[0], ieee80211_regdom[1]);
 		alpha2[0] = ieee80211_regdom[0];
 		alpha2[1] = ieee80211_regdom[1];
 	} else
@@ -1987,10 +1923,8 @@ static void restore_regulatory_settings(bool reset_user)
 
 	spin_lock(&reg_requests_lock);
 	list_for_each_entry_safe(reg_request, tmp, &tmp_reg_req_list, list) {
-		REG_DBG_PRINT("Adding request for country %c%c back "
-			      "into the queue\n",
-			      reg_request->alpha2[0],
-			      reg_request->alpha2[1]);
+		REG_DBG_PRINT("Adding request for country %c%c back into the queue\n",
+			      reg_request->alpha2[0], reg_request->alpha2[1]);
 		list_move_tail(&reg_request->list, &reg_requests_list);
 	}
 	spin_unlock(&reg_requests_lock);
@@ -2005,8 +1939,7 @@ static void restore_regulatory_settings(bool reset_user)
 
 void regulatory_hint_disconnect(void)
 {
-	REG_DBG_PRINT("All devices are disconnected, going to "
-		      "restore regulatory settings\n");
+	REG_DBG_PRINT("All devices are disconnected, going to restore regulatory settings\n");
 	restore_regulatory_settings(false);
 }
 
@@ -2025,25 +1958,23 @@ int regulatory_hint_found_beacon(struct wiphy *wiphy,
 {
 	struct reg_beacon *reg_beacon;
 
-	if (likely((beacon_chan->beacon_found ||
-	    (beacon_chan->flags & IEEE80211_CHAN_RADAR) ||
+	if (beacon_chan->beacon_found ||
+	    beacon_chan->flags & IEEE80211_CHAN_RADAR ||
 	    (beacon_chan->band == IEEE80211_BAND_2GHZ &&
-	     !freq_is_chan_12_13_14(beacon_chan->center_freq)))))
+	     !freq_is_chan_12_13_14(beacon_chan->center_freq)))
 		return 0;
 
 	reg_beacon = kzalloc(sizeof(struct reg_beacon), gfp);
 	if (!reg_beacon)
 		return -ENOMEM;
 
-	REG_DBG_PRINT("Found new beacon on "
-		      "frequency: %d MHz (Ch %d) on %s\n",
+	REG_DBG_PRINT("Found new beacon on frequency: %d MHz (Ch %d) on %s\n",
 		      beacon_chan->center_freq,
 		      ieee80211_frequency_to_channel(beacon_chan->center_freq),
 		      wiphy_name(wiphy));
 
 	memcpy(&reg_beacon->chan, beacon_chan,
-		sizeof(struct ieee80211_channel));
-
+	       sizeof(struct ieee80211_channel));
 
 	/*
 	 * Since we can be called from BH or and non-BH context
@@ -2123,7 +2054,7 @@ static void print_dfs_region(u8 dfs_region)
 		pr_info(" DFS Master region JP");
 		break;
 	default:
-		pr_info(" DFS Master region Uknown");
+		pr_info(" DFS Master region Unknown");
 		break;
 	}
 }
@@ -2132,7 +2063,6 @@ static void print_regdomain(const struct ieee80211_regdomain *rd)
 {
 
 	if (is_intersected_alpha2(rd->alpha2)) {
-
 		if (last_request->initiator ==
 		    NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 			struct cfg80211_registered_device *rdev;
@@ -2146,22 +2076,21 @@ static void print_regdomain(const struct ieee80211_regdomain *rd)
 				pr_info("Current regulatory domain intersected:\n");
 		} else
 			pr_info("Current regulatory domain intersected:\n");
-	} else if (is_world_regdom(rd->alpha2))
+	} else if (is_world_regdom(rd->alpha2)) {
 		pr_info("World regulatory domain updated:\n");
-	else {
+	} else {
 		if (is_unknown_alpha2(rd->alpha2))
 			pr_info("Regulatory domain changed to driver built-in settings (unknown country)\n");
 		else {
 			if (reg_request_cell_base(last_request))
-				pr_info("Regulatory domain changed "
-					"to country: %c%c by Cell Station\n",
+				pr_info("Regulatory domain changed to country: %c%c by Cell Station\n",
 					rd->alpha2[0], rd->alpha2[1]);
 			else
-				pr_info("Regulatory domain changed "
-					"to country: %c%c\n",
+				pr_info("Regulatory domain changed to country: %c%c\n",
 					rd->alpha2[0], rd->alpha2[1]);
 		}
 	}
+
 	print_dfs_region(rd->dfs_region);
 	print_rd_rules(rd);
 }
@@ -2188,7 +2117,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	if (!is_alpha2_set(rd->alpha2) && !is_an_alpha2(rd->alpha2) &&
-			!is_unknown_alpha2(rd->alpha2))
+	    !is_unknown_alpha2(rd->alpha2))
 		return -EINVAL;
 
 	if (!last_request)
@@ -2264,7 +2193,6 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	/* Intersection requires a bit more work */
 
 	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-
 		intersected_rd = regdom_intersect(rd, cfg80211_regdomain);
 		if (!intersected_rd)
 			return -EINVAL;
@@ -2316,8 +2244,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	/* This would make this whole thing pointless */
-	if (!last_request->intersect)
-		BUG_ON(rd != cfg80211_regdomain);
+	BUG_ON(!last_request->intersect && rd != cfg80211_regdomain);
 
 	/* update all wiphys now with the new established regulatory domain */
 	update_all_wiphy_regulatory(last_request->initiator);
@@ -2394,8 +2321,7 @@ out:
 
 static void reg_timeout_work(struct work_struct *work)
 {
-	REG_DBG_PRINT("Timeout while waiting for CRDA to reply, "
-		      "restoring regulatory settings\n");
+	REG_DBG_PRINT("Timeout while waiting for CRDA to reply, restoring regulatory settings\n");
 	restore_regulatory_settings(true);
 }
 
@@ -2449,7 +2375,7 @@ int __init regulatory_init(void)
 	return 0;
 }
 
-void /* __init_or_exit */ regulatory_exit(void)
+void regulatory_exit(void)
 {
 	struct regulatory_request *reg_request, *tmp;
 	struct reg_beacon *reg_beacon, *btmp;
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 4c0a32f..37891e8 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -55,8 +55,8 @@ bool reg_last_request_cell_base(void);
  * set the wiphy->disable_beacon_hints to true.
  */
 int regulatory_hint_found_beacon(struct wiphy *wiphy,
-					struct ieee80211_channel *beacon_chan,
-					gfp_t gfp);
+				 struct ieee80211_channel *beacon_chan,
+				 gfp_t gfp);
 
 /**
  * regulatory_hint_11d - hints a country IE as a regulatory domain
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index f2431e4..d2d2651 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -519,10 +519,8 @@ void __cfg80211_connect_result(struct net_device *dev, const u8 *bssid,
 	 * - country_ie + 2, the start of the country ie data, and
 	 * - and country_ie[1] which is the IE length
 	 */
-	regulatory_hint_11d(wdev->wiphy,
-			    bss->channel->band,
-			    country_ie + 2,
-			    country_ie[1]);
+	regulatory_hint_11d(wdev->wiphy, bss->channel->band,
+			    country_ie + 2, country_ie[1]);
 	kfree(country_ie);
 }
 
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 08/24] regulatory: remove useless locking on exit
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (6 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 07/24] regulatory: code cleanup Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:16   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 09/24] regulatory: use proper enum for return values Johannes Berg
                   ` (16 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

It would be a major problem if anything were to run
concurrently while the module is being unloaded so
remove the locking that doesn't help anything.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 7391624..4bb953f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2383,34 +2383,29 @@ void regulatory_exit(void)
 	cancel_work_sync(&reg_work);
 	cancel_delayed_work_sync(&reg_timeout);
 
+	/* Lock to suppress warnings */
 	mutex_lock(&cfg80211_mutex);
 	mutex_lock(&reg_mutex);
-
 	reset_regdomains(true);
+	mutex_unlock(&cfg80211_mutex);
+	mutex_unlock(&reg_mutex);
 
 	dev_set_uevent_suppress(&reg_pdev->dev, true);
 
 	platform_device_unregister(reg_pdev);
 
-	spin_lock_bh(&reg_pending_beacons_lock);
 	list_for_each_entry_safe(reg_beacon, btmp, &reg_pending_beacons, list) {
 		list_del(&reg_beacon->list);
 		kfree(reg_beacon);
 	}
-	spin_unlock_bh(&reg_pending_beacons_lock);
 
 	list_for_each_entry_safe(reg_beacon, btmp, &reg_beacon_list, list) {
 		list_del(&reg_beacon->list);
 		kfree(reg_beacon);
 	}
 
-	spin_lock(&reg_requests_lock);
 	list_for_each_entry_safe(reg_request, tmp, &reg_requests_list, list) {
 		list_del(&reg_request->list);
 		kfree(reg_request);
 	}
-	spin_unlock(&reg_requests_lock);
-
-	mutex_unlock(&reg_mutex);
-	mutex_unlock(&cfg80211_mutex);
 }
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 09/24] regulatory: use proper enum for return values
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (7 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 08/24] regulatory: remove useless locking on exit Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:20   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 10/24] cfg80211: remove wiphy_idx_valid Johannes Berg
                   ` (15 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Instead of treating special error codes specially,
like -EALREADY, introduce a real enum for all the
needed possibilities and use it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 133 +++++++++++++++++++++++++++--------------------------
 1 file changed, 69 insertions(+), 64 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 4bb953f..12576a4 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -65,6 +65,13 @@
 #define REG_DBG_PRINT(args...)
 #endif
 
+enum reg_request_treatment {
+	REG_REQ_OK,
+	REG_REQ_IGNORE,
+	REG_REQ_INTERSECT,
+	REG_REQ_ALREADY_SET,
+};
+
 static struct regulatory_request core_request_world = {
 	.initiator = NL80211_REGDOM_SET_BY_CORE,
 	.alpha2[0] = '0',
@@ -926,16 +933,17 @@ bool reg_last_request_cell_base(void)
 
 #ifdef CONFIG_CFG80211_CERTIFICATION_ONUS
 /* Core specific check */
-static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
+static enum reg_request_treatment
+reg_ignore_cell_hint(struct regulatory_request *pending_request)
 {
 	if (!reg_num_devs_support_basehint)
-		return -EOPNOTSUPP;
+		return REG_REQ_IGNORE;
 
 	if (reg_request_cell_base(last_request) &&
 	    !regdom_changes(pending_request->alpha2))
-		return -EALREADY;
+		return REG_REQ_ALREADY_SET;
 
-	return 0;
+	return REG_REQ_OK;
 }
 
 /* Device specific check */
@@ -946,7 +954,7 @@ static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
 #else
 static int reg_ignore_cell_hint(struct regulatory_request *pending_request)
 {
-	return -EOPNOTSUPP;
+	return REG_REQ_IGNORE;
 }
 
 static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
@@ -1309,15 +1317,10 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 }
 EXPORT_SYMBOL(wiphy_apply_custom_regulatory);
 
-/*
- * Return value which can be used by ignore_request() to indicate
- * it has been determined we should intersect two regulatory domains
- */
-#define REG_INTERSECT	1
-
 /* This has the logic which determines when a new request
  * should be ignored. */
-static int ignore_request(struct wiphy *wiphy,
+static enum reg_request_treatment
+get_reg_request_treatment(struct wiphy *wiphy,
 			  struct regulatory_request *pending_request)
 {
 	struct wiphy *last_wiphy = NULL;
@@ -1326,17 +1329,17 @@ static int ignore_request(struct wiphy *wiphy,
 
 	/* All initial requests are respected */
 	if (!last_request)
-		return 0;
+		return REG_REQ_OK;
 
 	switch (pending_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
-		return 0;
+		return REG_REQ_OK;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
 		if (reg_request_cell_base(last_request)) {
 			/* Trust a Cell base station over the AP's country IE */
 			if (regdom_changes(pending_request->alpha2))
-				return -EOPNOTSUPP;
-			return -EALREADY;
+				return REG_REQ_IGNORE;
+			return REG_REQ_ALREADY_SET;
 		}
 
 		last_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
@@ -1353,23 +1356,23 @@ static int ignore_request(struct wiphy *wiphy,
 				 * to be correct. Reject second one for now.
 				 */
 				if (regdom_changes(pending_request->alpha2))
-					return -EOPNOTSUPP;
-				return -EALREADY;
+					return REG_REQ_IGNORE;
+				return REG_REQ_ALREADY_SET;
 			}
 			/*
 			 * Two consecutive Country IE hints on the same wiphy.
 			 * This should be picked up early by the driver/stack
 			 */
 			if (WARN_ON(regdom_changes(pending_request->alpha2)))
-				return 0;
-			return -EALREADY;
+				return REG_REQ_OK;
+			return REG_REQ_ALREADY_SET;
 		}
 		return 0;
 	case NL80211_REGDOM_SET_BY_DRIVER:
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_CORE) {
 			if (regdom_changes(pending_request->alpha2))
-				return 0;
-			return -EALREADY;
+				return REG_REQ_OK;
+			return REG_REQ_ALREADY_SET;
 		}
 
 		/*
@@ -1379,25 +1382,25 @@ static int ignore_request(struct wiphy *wiphy,
 		 */
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
 		    !regdom_changes(pending_request->alpha2))
-			return -EALREADY;
+			return REG_REQ_ALREADY_SET;
 
-		return REG_INTERSECT;
+		return REG_REQ_INTERSECT;
 	case NL80211_REGDOM_SET_BY_USER:
 		if (reg_request_cell_base(pending_request))
 			return reg_ignore_cell_hint(pending_request);
 
 		if (reg_request_cell_base(last_request))
-			return -EOPNOTSUPP;
+			return REG_REQ_IGNORE;
 
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
-			return REG_INTERSECT;
+			return REG_REQ_INTERSECT;
 		/*
 		 * If the user knows better the user should set the regdom
 		 * to their country before the IE is picked up
 		 */
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_USER &&
 		    last_request->intersect)
-			return -EOPNOTSUPP;
+			return REG_REQ_IGNORE;
 		/*
 		 * Process user requests only after previous user/driver/core
 		 * requests have been processed
@@ -1406,15 +1409,15 @@ static int ignore_request(struct wiphy *wiphy,
 		     last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
 		     last_request->initiator == NL80211_REGDOM_SET_BY_USER) &&
 		    regdom_changes(last_request->alpha2))
-			return -EAGAIN;
+			return REG_REQ_IGNORE;
 
 		if (!regdom_changes(pending_request->alpha2))
-			return -EALREADY;
+			return REG_REQ_ALREADY_SET;
 
-		return 0;
+		return REG_REQ_OK;
 	}
 
-	return -EINVAL;
+	return REG_REQ_IGNORE;
 }
 
 static void reg_set_request_processed(void)
@@ -1444,23 +1447,24 @@ static void reg_set_request_processed(void)
  * The Wireless subsystem can use this function to hint to the wireless core
  * what it believes should be the current regulatory domain.
  *
- * Returns zero if all went fine, %-EALREADY if a regulatory domain had
- * already been set or other standard error codes.
+ * Returns one of the different reg request treatment values.
  *
  * Caller must hold &cfg80211_mutex and &reg_mutex
  */
-static int __regulatory_hint(struct wiphy *wiphy,
-			     struct regulatory_request *pending_request)
+static enum reg_request_treatment
+__regulatory_hint(struct wiphy *wiphy,
+		  struct regulatory_request *pending_request)
 {
 	const struct ieee80211_regdomain *regd;
 	bool intersect = false;
-	int r = 0;
+	enum reg_request_treatment treatment;
 
 	assert_cfg80211_lock();
 
-	r = ignore_request(wiphy, pending_request);
+	treatment = get_reg_request_treatment(wiphy, pending_request);
 
-	if (r == REG_INTERSECT) {
+	switch (treatment) {
+	case REG_REQ_INTERSECT:
 		if (pending_request->initiator ==
 		    NL80211_REGDOM_SET_BY_DRIVER) {
 			regd = reg_copy_regd(cfg80211_regdomain);
@@ -1471,26 +1475,28 @@ static int __regulatory_hint(struct wiphy *wiphy,
 			wiphy->regd = regd;
 		}
 		intersect = true;
-	} else if (r) {
+		break;
+	case REG_REQ_OK:
+		break;
+	default:
 		/*
 		 * If the regulatory domain being requested by the
 		 * driver has already been set just copy it to the
 		 * wiphy
 		 */
-		if (r == -EALREADY &&
-		    pending_request->initiator ==
-		    NL80211_REGDOM_SET_BY_DRIVER) {
+		if (treatment == REG_REQ_ALREADY_SET &&
+		    pending_request->initiator == NL80211_REGDOM_SET_BY_DRIVER) {
 			regd = reg_copy_regd(cfg80211_regdomain);
 			if (IS_ERR(regd)) {
 				kfree(pending_request);
-				return PTR_ERR(regd);
+				return REG_REQ_IGNORE;
 			}
-			r = -EALREADY;
+			treatment = REG_REQ_ALREADY_SET;
 			wiphy->regd = regd;
 			goto new_request;
 		}
 		kfree(pending_request);
-		return r;
+		return treatment;
 	}
 
 new_request:
@@ -1507,28 +1513,29 @@ new_request:
 		user_alpha2[1] = last_request->alpha2[1];
 	}
 
-	/* When r == REG_INTERSECT we do need to call CRDA */
-	if (r < 0) {
+	/* When r == REG_REQ_INTERSECT we do need to call CRDA */
+	if (treatment != REG_REQ_OK && treatment != REG_REQ_INTERSECT) {
 		/*
 		 * Since CRDA will not be called in this case as we already
 		 * have applied the requested regulatory domain before we just
 		 * inform userspace we have processed the request
 		 */
-		if (r == -EALREADY) {
+		if (treatment == REG_REQ_ALREADY_SET) {
 			nl80211_send_reg_change_event(last_request);
 			reg_set_request_processed();
 		}
-		return r;
+		return treatment;
 	}
 
-	return call_crda(last_request->alpha2);
+	if (call_crda(last_request->alpha2))
+		return REG_REQ_IGNORE;
+	return REG_REQ_OK;
 }
 
 /* This processes *all* regulatory hints */
 static void reg_process_hint(struct regulatory_request *reg_request,
 			     enum nl80211_reg_initiator reg_initiator)
 {
-	int r = 0;
 	struct wiphy *wiphy = NULL;
 
 	BUG_ON(!reg_request->alpha2);
@@ -1541,20 +1548,18 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 		return;
 	}
 
-	r = __regulatory_hint(wiphy, reg_request);
-	/* This is required so that the orig_* parameters are saved */
-	if (r == -EALREADY && wiphy &&
-	    wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) {
-		wiphy_update_regulatory(wiphy, reg_initiator);
-		return;
+	switch (__regulatory_hint(wiphy, reg_request)) {
+	case REG_REQ_ALREADY_SET:
+		/* This is required so that the orig_* parameters are saved */
+		if (wiphy && wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY)
+			wiphy_update_regulatory(wiphy, reg_initiator);
+		break;
+	default:
+		if (reg_initiator == NL80211_REGDOM_SET_BY_USER)
+			schedule_delayed_work(&reg_timeout,
+					      msecs_to_jiffies(3142));
+		break;
 	}
-
-	/*
-	 * We only time out user hints, given that they should be the only
-	 * source of bogus requests.
-	 */
-	if (r != -EALREADY && reg_initiator == NL80211_REGDOM_SET_BY_USER)
-		schedule_delayed_work(&reg_timeout, msecs_to_jiffies(3142));
 }
 
 /*
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 10/24] cfg80211: remove wiphy_idx_valid
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (8 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 09/24] regulatory: use proper enum for return values Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:34   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 11/24] regulatory: remove BUG_ON Johannes Berg
                   ` (14 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

This is pretty much useless since get_wiphy_idx()
always returns true since it's always called with
a valid wiphy pointer.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.c    | 14 +++-----------
 net/wireless/core.h    | 16 +++-------------
 net/wireless/nl80211.c |  2 +-
 net/wireless/reg.c     | 14 +++++---------
 4 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 14d9904..747dd93 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -57,9 +57,6 @@ struct cfg80211_registered_device *cfg80211_rdev_by_wiphy_idx(int wiphy_idx)
 {
 	struct cfg80211_registered_device *result = NULL, *rdev;
 
-	if (!wiphy_idx_valid(wiphy_idx))
-		return NULL;
-
 	assert_cfg80211_lock();
 
 	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
@@ -74,10 +71,8 @@ struct cfg80211_registered_device *cfg80211_rdev_by_wiphy_idx(int wiphy_idx)
 
 int get_wiphy_idx(struct wiphy *wiphy)
 {
-	struct cfg80211_registered_device *rdev;
-	if (!wiphy)
-		return WIPHY_IDX_STALE;
-	rdev = wiphy_to_dev(wiphy);
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
 	return rdev->wiphy_idx;
 }
 
@@ -86,9 +81,6 @@ struct wiphy *wiphy_idx_to_wiphy(int wiphy_idx)
 {
 	struct cfg80211_registered_device *rdev;
 
-	if (!wiphy_idx_valid(wiphy_idx))
-		return NULL;
-
 	assert_cfg80211_lock();
 
 	rdev = cfg80211_rdev_by_wiphy_idx(wiphy_idx);
@@ -309,7 +301,7 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 
 	rdev->wiphy_idx = wiphy_counter++;
 
-	if (unlikely(!wiphy_idx_valid(rdev->wiphy_idx))) {
+	if (unlikely(rdev->wiphy_idx < 0)) {
 		wiphy_counter--;
 		mutex_unlock(&cfg80211_mutex);
 		/* ugh, wrapped! */
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 3563097..b8f4630 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -18,6 +18,9 @@
 #include <net/cfg80211.h>
 #include "reg.h"
 
+
+#define WIPHY_IDX_INVALID	-1
+
 struct cfg80211_registered_device {
 	const struct cfg80211_ops *ops;
 	struct list_head list;
@@ -96,13 +99,6 @@ struct cfg80211_registered_device *wiphy_to_dev(struct wiphy *wiphy)
 	return container_of(wiphy, struct cfg80211_registered_device, wiphy);
 }
 
-/* Note 0 is valid, hence phy0 */
-static inline
-bool wiphy_idx_valid(int wiphy_idx)
-{
-	return wiphy_idx >= 0;
-}
-
 static inline void
 cfg80211_rdev_free_wowlan(struct cfg80211_registered_device *rdev)
 {
@@ -126,12 +122,6 @@ static inline void assert_cfg80211_lock(void)
 	lockdep_assert_held(&cfg80211_mutex);
 }
 
-/*
- * You can use this to mark a wiphy_idx as not having an associated wiphy.
- * It guarantees cfg80211_rdev_by_wiphy_idx(wiphy_idx) will return NULL
- */
-#define WIPHY_IDX_STALE -1
-
 struct cfg80211_internal_bss {
 	struct list_head list;
 	struct rb_node rbn;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e5c2988..fb81208 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8072,7 +8072,7 @@ void nl80211_send_reg_change_event(struct regulatory_request *request)
 			goto nla_put_failure;
 	}
 
-	if (wiphy_idx_valid(request->wiphy_idx) &&
+	if (request->wiphy_idx != WIPHY_IDX_INVALID &&
 	    nla_put_u32(msg, NL80211_ATTR_WIPHY, request->wiphy_idx))
 		goto nla_put_failure;
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 12576a4..5f83d8e 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1540,7 +1540,7 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 
 	BUG_ON(!reg_request->alpha2);
 
-	if (wiphy_idx_valid(reg_request->wiphy_idx))
+	if (reg_request->wiphy_idx != WIPHY_IDX_INVALID)
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
 
 	if (reg_initiator == NL80211_REGDOM_SET_BY_DRIVER && !wiphy) {
@@ -1685,7 +1685,7 @@ int regulatory_hint_user(const char *alpha2,
 	if (!request)
 		return -ENOMEM;
 
-	request->wiphy_idx = WIPHY_IDX_STALE;
+	request->wiphy_idx = WIPHY_IDX_INVALID;
 	request->alpha2[0] = alpha2[0];
 	request->alpha2[1] = alpha2[1];
 	request->initiator = NL80211_REGDOM_SET_BY_USER;
@@ -1710,9 +1710,6 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
 
 	request->wiphy_idx = get_wiphy_idx(wiphy);
 
-	/* Must have registered wiphy first */
-	BUG_ON(!wiphy_idx_valid(request->wiphy_idx));
-
 	request->alpha2[0] = alpha2[0];
 	request->alpha2[1] = alpha2[1];
 	request->initiator = NL80211_REGDOM_SET_BY_DRIVER;
@@ -1759,9 +1756,8 @@ void regulatory_hint_11d(struct wiphy *wiphy, enum ieee80211_band band,
 	 * We leave conflict resolution to the workqueue, where can hold
 	 * cfg80211_mutex.
 	 */
-	if (likely(last_request->initiator ==
-		   NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-		   wiphy_idx_valid(last_request->wiphy_idx)))
+	if (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    last_request->wiphy_idx != WIPHY_IDX_INVALID)
 		goto out;
 
 	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
@@ -2318,7 +2314,7 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 	if (!request_wiphy || request_wiphy != wiphy)
 		goto out;
 
-	last_request->wiphy_idx = WIPHY_IDX_STALE;
+	last_request->wiphy_idx = WIPHY_IDX_INVALID;
 	last_request->country_ie_env = ENVIRON_ANY;
 out:
 	mutex_unlock(&reg_mutex);
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 11/24] regulatory: remove BUG_ON
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (9 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 10/24] cfg80211: remove wiphy_idx_valid Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:39   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 12/24] regulatory: simplify restore_regulatory_settings Johannes Berg
                   ` (13 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

This code is a bit too BUG_ON happy, remove all
instances and while doing so make some code a bit
smarter by passing the right pointer instead of
indices into arrays.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c |  2 --
 net/wireless/reg.c     | 95 ++++++++++++++++++++------------------------------
 2 files changed, 37 insertions(+), 60 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index fb81208..260b5d8 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4333,8 +4333,6 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
-	BUG_ON(rule_idx != num_rules);
-
 	r = set_regdom(rd);
 	rd = NULL;
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5f83d8e..f40e90b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -214,7 +214,7 @@ static void reset_regdomains(bool full_reset)
  */
 static void update_world_regdomain(const struct ieee80211_regdomain *rd)
 {
-	BUG_ON(!last_request);
+	WARN_ON(!last_request);
 
 	reset_regdomains(false);
 
@@ -805,8 +805,7 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
  */
 static void handle_channel(struct wiphy *wiphy,
 			   enum nl80211_reg_initiator initiator,
-			   enum ieee80211_band band,
-			   unsigned int chan_idx)
+			   struct ieee80211_channel *chan)
 {
 	int r;
 	u32 flags, bw_flags = 0;
@@ -814,18 +813,12 @@ static void handle_channel(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
-	struct ieee80211_supported_band *sband;
-	struct ieee80211_channel *chan;
 	struct wiphy *request_wiphy = NULL;
 
 	assert_cfg80211_lock();
 
 	request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
 
-	sband = wiphy->bands[band];
-	BUG_ON(chan_idx >= sband->n_channels);
-	chan = &sband->channels[chan_idx];
-
 	flags = chan->orig_flags;
 
 	r = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq),
@@ -898,17 +891,17 @@ static void handle_channel(struct wiphy *wiphy,
 		chan->max_power = chan->max_reg_power;
 }
 
-static void handle_band(struct wiphy *wiphy, enum ieee80211_band band,
-			enum nl80211_reg_initiator initiator)
+static void handle_band(struct wiphy *wiphy,
+			enum nl80211_reg_initiator initiator,
+			struct ieee80211_supported_band *sband)
 {
 	unsigned int i;
-	struct ieee80211_supported_band *sband;
 
-	BUG_ON(!wiphy->bands[band]);
-	sband = wiphy->bands[band];
+	if (!sband)
+		return;
 
 	for (i = 0; i < sband->n_channels; i++)
-		handle_channel(wiphy, initiator, band, i);
+		handle_channel(wiphy, initiator, &sband->channels[i]);
 }
 
 static bool reg_request_cell_base(struct regulatory_request *request)
@@ -1117,20 +1110,14 @@ static bool is_ht40_allowed(struct ieee80211_channel *chan)
 }
 
 static void reg_process_ht_flags_channel(struct wiphy *wiphy,
-					 enum ieee80211_band band,
-					 unsigned int chan_idx)
+					 struct ieee80211_channel *channel)
 {
-	struct ieee80211_supported_band *sband;
-	struct ieee80211_channel *channel;
+	struct ieee80211_supported_band *sband = wiphy->bands[channel->band];
 	struct ieee80211_channel *channel_before = NULL, *channel_after = NULL;
 	unsigned int i;
 
 	assert_cfg80211_lock();
 
-	sband = wiphy->bands[band];
-	BUG_ON(chan_idx >= sband->n_channels);
-	channel = &sband->channels[chan_idx];
-
 	if (!is_ht40_allowed(channel)) {
 		channel->flags |= IEEE80211_CHAN_NO_HT40;
 		return;
@@ -1166,16 +1153,15 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
 }
 
 static void reg_process_ht_flags_band(struct wiphy *wiphy,
-				      enum ieee80211_band band)
+				      struct ieee80211_supported_band *sband)
 {
 	unsigned int i;
-	struct ieee80211_supported_band *sband;
 
-	BUG_ON(!wiphy->bands[band]);
-	sband = wiphy->bands[band];
+	if (!sband)
+		return;
 
 	for (i = 0; i < sband->n_channels; i++)
-		reg_process_ht_flags_channel(wiphy, band, i);
+		reg_process_ht_flags_channel(wiphy, &sband->channels[i]);
 }
 
 static void reg_process_ht_flags(struct wiphy *wiphy)
@@ -1185,11 +1171,8 @@ static void reg_process_ht_flags(struct wiphy *wiphy)
 	if (!wiphy)
 		return;
 
-	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
-		if (wiphy->bands[band])
-			reg_process_ht_flags_band(wiphy, band);
-	}
-
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++)
+		reg_process_ht_flags_band(wiphy, wiphy->bands[band]);
 }
 
 static void wiphy_update_regulatory(struct wiphy *wiphy,
@@ -1204,10 +1187,8 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 
 	last_request->dfs_region = cfg80211_regdomain->dfs_region;
 
-	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
-		if (wiphy->bands[band])
-			handle_band(wiphy, band, initiator);
-	}
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++)
+		handle_band(wiphy, initiator, wiphy->bands[band]);
 
 	reg_process_beacons(wiphy);
 	reg_process_ht_flags(wiphy);
@@ -1237,8 +1218,7 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
 }
 
 static void handle_channel_custom(struct wiphy *wiphy,
-				  enum ieee80211_band band,
-				  unsigned int chan_idx,
+				  struct ieee80211_channel *chan,
 				  const struct ieee80211_regdomain *regd)
 {
 	int r;
@@ -1247,15 +1227,9 @@ static void handle_channel_custom(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
-	struct ieee80211_supported_band *sband;
-	struct ieee80211_channel *chan;
 
 	assert_reg_lock();
 
-	sband = wiphy->bands[band];
-	BUG_ON(chan_idx >= sband->n_channels);
-	chan = &sband->channels[chan_idx];
-
 	r = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
 			       desired_bw_khz, &reg_rule, regd);
 
@@ -1280,17 +1254,17 @@ static void handle_channel_custom(struct wiphy *wiphy,
 		(int) MBM_TO_DBM(power_rule->max_eirp);
 }
 
-static void handle_band_custom(struct wiphy *wiphy, enum ieee80211_band band,
+static void handle_band_custom(struct wiphy *wiphy,
+			       struct ieee80211_supported_band *sband,
 			       const struct ieee80211_regdomain *regd)
 {
 	unsigned int i;
-	struct ieee80211_supported_band *sband;
 
-	BUG_ON(!wiphy->bands[band]);
-	sband = wiphy->bands[band];
+	if (!sband)
+		return;
 
 	for (i = 0; i < sband->n_channels; i++)
-		handle_channel_custom(wiphy, band, i, regd);
+		handle_channel_custom(wiphy, &sband->channels[i], regd);
 }
 
 /* Used by drivers prior to wiphy registration */
@@ -1304,7 +1278,7 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 		if (!wiphy->bands[band])
 			continue;
-		handle_band_custom(wiphy, band, regd);
+		handle_band_custom(wiphy, wiphy->bands[band], regd);
 		bands_set++;
 	}
 	mutex_unlock(&reg_mutex);
@@ -1538,7 +1512,8 @@ static void reg_process_hint(struct regulatory_request *reg_request,
 {
 	struct wiphy *wiphy = NULL;
 
-	BUG_ON(!reg_request->alpha2);
+	if (WARN_ON(!reg_request->alpha2))
+		return;
 
 	if (reg_request->wiphy_idx != WIPHY_IDX_INVALID)
 		wiphy = wiphy_idx_to_wiphy(reg_request->wiphy_idx);
@@ -1679,7 +1654,8 @@ int regulatory_hint_user(const char *alpha2,
 {
 	struct regulatory_request *request;
 
-	BUG_ON(!alpha2);
+	if (WARN_ON(!alpha2))
+		return -EINVAL;
 
 	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
 	if (!request)
@@ -1701,8 +1677,8 @@ int regulatory_hint(struct wiphy *wiphy, const char *alpha2)
 {
 	struct regulatory_request *request;
 
-	BUG_ON(!alpha2);
-	BUG_ON(!wiphy);
+	if (WARN_ON(!alpha2 || !wiphy))
+		return -EINVAL;
 
 	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
 	if (!request)
@@ -2240,12 +2216,14 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 			reg_set_request_processed();
 
 		kfree(rd);
-		mutex_unlock(&reg_mutex);
-		return r;
+		goto out;
 	}
 
 	/* This would make this whole thing pointless */
-	BUG_ON(!last_request->intersect && rd != cfg80211_regdomain);
+	if (WARN_ON(!last_request->intersect && rd != cfg80211_regdomain)) {
+		r = -EINVAL;
+		goto out;
+	}
 
 	/* update all wiphys now with the new established regulatory domain */
 	update_all_wiphy_regulatory(last_request->initiator);
@@ -2256,6 +2234,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 
 	reg_set_request_processed();
 
+ out:
 	mutex_unlock(&reg_mutex);
 
 	return r;
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 12/24] regulatory: simplify restore_regulatory_settings
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (10 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 11/24] regulatory: remove BUG_ON Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:53   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 13/24] regulatory: remove redundant isalpha() check Johannes Berg
                   ` (12 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Use list_splice_tail_init() and also simplify the locking.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index f40e90b..1c19d70 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1879,9 +1879,6 @@ static void restore_regulatory_settings(bool reset_user)
 			restore_custom_reg_settings(&rdev->wiphy);
 	}
 
-	mutex_unlock(&reg_mutex);
-	mutex_unlock(&cfg80211_mutex);
-
 	regulatory_hint_core(world_alpha2);
 
 	/*
@@ -1892,18 +1889,8 @@ static void restore_regulatory_settings(bool reset_user)
 	if (is_an_alpha2(alpha2))
 		regulatory_hint_user(user_alpha2, NL80211_USER_REG_HINT_USER);
 
-	if (list_empty(&tmp_reg_req_list))
-		return;
-
-	mutex_lock(&cfg80211_mutex);
-	mutex_lock(&reg_mutex);
-
 	spin_lock(&reg_requests_lock);
-	list_for_each_entry_safe(reg_request, tmp, &tmp_reg_req_list, list) {
-		REG_DBG_PRINT("Adding request for country %c%c back into the queue\n",
-			      reg_request->alpha2[0], reg_request->alpha2[1]);
-		list_move_tail(&reg_request->list, &reg_requests_list);
-	}
+	list_splice_tail_init(&tmp_reg_req_list, &reg_requests_list);
 	spin_unlock(&reg_requests_lock);
 
 	mutex_unlock(&reg_mutex);
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 13/24] regulatory: remove redundant isalpha() check
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (11 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 12/24] regulatory: simplify restore_regulatory_settings Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:54   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 14/24] regulatory: remove useless warning Johannes Berg
                   ` (11 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

toupper() only modifies lower-case letters, so
the isalpha() check is redundant; remove it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 1c19d70..de3cf71 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1615,10 +1615,8 @@ static void reg_todo(struct work_struct *work)
 
 static void queue_regulatory_request(struct regulatory_request *request)
 {
-	if (isalpha(request->alpha2[0]))
-		request->alpha2[0] = toupper(request->alpha2[0]);
-	if (isalpha(request->alpha2[1]))
-		request->alpha2[1] = toupper(request->alpha2[1]);
+	request->alpha2[0] = toupper(request->alpha2[0]);
+	request->alpha2[1] = toupper(request->alpha2[1]);
 
 	spin_lock(&reg_requests_lock);
 	list_add_tail(&request->list, &reg_requests_list);
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 14/24] regulatory: remove useless warning
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (12 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 13/24] regulatory: remove redundant isalpha() check Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  0:55   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 15/24] regulatory: simplify freq_reg_info_regd Johannes Berg
                   ` (10 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Even if it never happens and is hidden behind the
debug config option, it's completely useless: the
calltrace will only show module loading.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index de3cf71..fe8a13f 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2323,10 +2323,6 @@ int __init regulatory_init(void)
 		 * errors as non-fatal.
 		 */
 		pr_err("kobject_uevent_env() was unable to call CRDA during init\n");
-#ifdef CONFIG_CFG80211_REG_DEBUG
-		/* We want to find out exactly why when debugging */
-		WARN_ON(err);
-#endif
 	}
 
 	/*
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 15/24] regulatory: simplify freq_reg_info_regd
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (13 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 14/24] regulatory: remove useless warning Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07  1:02   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 16/24] regulatory: clarify locking rules and assertions Johannes Berg
                   ` (9 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

The function itself has dual-purpose: it can
retrieve from a given regdomain or from the
globally installed one. Change it to have a
single purpose only: to look up from a given
regdomain. Pass the correct regdomain in the
freq_reg_info() function instead.

This also changes the locking rules for it,
no locking is required any more.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index fe8a13f..ab8740b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -679,28 +679,15 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 			      u32 center_freq,
 			      u32 desired_bw_khz,
 			      const struct ieee80211_reg_rule **reg_rule,
-			      const struct ieee80211_regdomain *custom_regd)
+			      const struct ieee80211_regdomain *regd)
 {
 	int i;
 	bool band_rule_found = false;
-	const struct ieee80211_regdomain *regd;
 	bool bw_fits = false;
 
 	if (!desired_bw_khz)
 		desired_bw_khz = MHZ_TO_KHZ(20);
 
-	regd = custom_regd ? custom_regd : cfg80211_regdomain;
-
-	/*
-	 * Follow the driver's regulatory domain, if present, unless a country
-	 * IE has been processed or a user wants to help complaince further
-	 */
-	if (!custom_regd &&
-	    last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    last_request->initiator != NL80211_REGDOM_SET_BY_USER &&
-	    wiphy->regd)
-		regd = wiphy->regd;
-
 	if (!regd)
 		return -EINVAL;
 
@@ -736,10 +723,24 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 int freq_reg_info(struct wiphy *wiphy, u32 center_freq, u32 desired_bw_khz,
 		  const struct ieee80211_reg_rule **reg_rule)
 {
+	const struct ieee80211_regdomain *regd;
+
+	assert_reg_lock();
 	assert_cfg80211_lock();
 
+	/*
+	 * Follow the driver's regulatory domain, if present, unless a country
+	 * IE has been processed or a user wants to help complaince further
+	 */
+	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    last_request->initiator != NL80211_REGDOM_SET_BY_USER &&
+	    wiphy->regd)
+		regd = wiphy->regd;
+	else
+		regd = cfg80211_regdomain;
+
 	return freq_reg_info_regd(wiphy, center_freq, desired_bw_khz,
-				  reg_rule, NULL);
+				  reg_rule, regd);
 }
 EXPORT_SYMBOL(freq_reg_info);
 
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 16/24] regulatory: clarify locking rules and assertions
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (14 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 15/24] regulatory: simplify freq_reg_info_regd Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-07 23:11   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory Johannes Berg
                   ` (8 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Many places that currently check that cfg80211_mutex
is held don't actually use any data protected by it.
The functions that need to hold the cfg80211_mutex
are the ones using the cfg80211_regdomain variable,
so add the lock assertion to those and clarify this
in the comments.

The reason for this is that nl80211 uses the regdom
without being able to hold reg_mutex.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ab8740b..ea6f057 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -94,14 +94,14 @@ static struct device_type reg_device_type = {
 /*
  * Central wireless core regulatory domains, we only need two,
  * the current one and a world regulatory domain in case we have no
- * information to give us an alpha2
+ * information to give us an alpha2.
+ * Protected by the cfg80211_mutex.
  */
 const struct ieee80211_regdomain *cfg80211_regdomain;
 
 /*
  * Protects static reg.c components:
  *     - cfg80211_world_regdom
- *     - cfg80211_regdom
  *     - last_request
  *     - reg_num_devs_support_basehint
  */
@@ -186,6 +186,9 @@ MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
 static void reset_regdomains(bool full_reset)
 {
+	assert_cfg80211_lock();
+	assert_reg_lock();
+
 	/* avoid freeing static information or freeing something twice */
 	if (cfg80211_regdomain == cfg80211_world_regdom)
 		cfg80211_regdomain = NULL;
@@ -216,6 +219,9 @@ static void update_world_regdomain(const struct ieee80211_regdomain *rd)
 {
 	WARN_ON(!last_request);
 
+	assert_cfg80211_lock();
+	assert_reg_lock();
+
 	reset_regdomains(false);
 
 	cfg80211_world_regdom = rd;
@@ -423,8 +429,6 @@ static int call_crda(const char *alpha2)
 /* Used by nl80211 before kmalloc'ing our regulatory domain */
 bool reg_is_valid_request(const char *alpha2)
 {
-	assert_cfg80211_lock();
-
 	if (!last_request)
 		return false;
 
@@ -916,8 +920,6 @@ bool reg_last_request_cell_base(void)
 {
 	bool val;
 
-	assert_cfg80211_lock();
-
 	mutex_lock(&reg_mutex);
 	val = reg_request_cell_base(last_request);
 	mutex_unlock(&reg_mutex);
@@ -1000,8 +1002,6 @@ static void handle_reg_beacon(struct wiphy *wiphy, unsigned int chan_idx,
 	bool channel_changed = false;
 	struct ieee80211_channel chan_before;
 
-	assert_cfg80211_lock();
-
 	sband = wiphy->bands[reg_beacon->chan.band];
 	chan = &sband->channels[chan_idx];
 
@@ -1043,8 +1043,6 @@ static void wiphy_update_new_beacon(struct wiphy *wiphy,
 	unsigned int i;
 	struct ieee80211_supported_band *sband;
 
-	assert_cfg80211_lock();
-
 	if (!wiphy->bands[reg_beacon->chan.band])
 		return;
 
@@ -1063,8 +1061,6 @@ static void wiphy_update_beacon_reg(struct wiphy *wiphy)
 	struct ieee80211_supported_band *sband;
 	struct reg_beacon *reg_beacon;
 
-	assert_cfg80211_lock();
-
 	list_for_each_entry(reg_beacon, &reg_beacon_list, list) {
 		if (!wiphy->bands[reg_beacon->chan.band])
 			continue;
@@ -1076,6 +1072,8 @@ static void wiphy_update_beacon_reg(struct wiphy *wiphy)
 
 static bool reg_is_world_roaming(struct wiphy *wiphy)
 {
+	assert_cfg80211_lock();
+
 	if (is_world_regdom(cfg80211_regdomain->alpha2) ||
 	    (wiphy->regd && is_world_regdom(wiphy->regd->alpha2)))
 		return true;
@@ -1117,8 +1115,6 @@ static void reg_process_ht_flags_channel(struct wiphy *wiphy,
 	struct ieee80211_channel *channel_before = NULL, *channel_after = NULL;
 	unsigned int i;
 
-	assert_cfg80211_lock();
-
 	if (!is_ht40_allowed(channel)) {
 		channel->flags |= IEEE80211_CHAN_NO_HT40;
 		return;
@@ -1181,6 +1177,7 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 {
 	enum ieee80211_band band;
 
+	assert_cfg80211_lock();
 	assert_reg_lock();
 
 	if (ignore_reg_update(wiphy, initiator))
@@ -1300,8 +1297,6 @@ get_reg_request_treatment(struct wiphy *wiphy,
 {
 	struct wiphy *last_wiphy = NULL;
 
-	assert_cfg80211_lock();
-
 	/* All initial requests are respected */
 	if (!last_request)
 		return REG_REQ_OK;
@@ -2247,8 +2242,6 @@ int reg_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 void wiphy_regulatory_register(struct wiphy *wiphy)
 {
-	assert_cfg80211_lock();
-
 	mutex_lock(&reg_mutex);
 
 	if (!reg_dev_ignore_cell_hint(wiphy))
@@ -2264,8 +2257,6 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 {
 	struct wiphy *request_wiphy = NULL;
 
-	assert_cfg80211_lock();
-
 	mutex_lock(&reg_mutex);
 
 	if (!reg_dev_ignore_cell_hint(wiphy))
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (15 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 16/24] regulatory: clarify locking rules and assertions Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 21:56   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 18/24] regulatory: fix reg_is_valid_request handling Johannes Berg
                   ` (7 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

wiphy_apply_custom_regulatory() doesn't have to hold
the regulatory mutex as it only modifies the given
wiphy with the given regulatory domain, it doesn't
access any global regulatory data.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ea6f057..dfab71b 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1226,8 +1226,6 @@ static void handle_channel_custom(struct wiphy *wiphy,
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
 
-	assert_reg_lock();
-
 	r = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
 			       desired_bw_khz, &reg_rule, regd);
 
@@ -1272,14 +1270,12 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy,
 	enum ieee80211_band band;
 	unsigned int bands_set = 0;
 
-	mutex_lock(&reg_mutex);
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 		if (!wiphy->bands[band])
 			continue;
 		handle_band_custom(wiphy, wiphy->bands[band], regd);
 		bands_set++;
 	}
-	mutex_unlock(&reg_mutex);
 
 	/*
 	 * no point in calling this if it won't have any effect
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 18/24] regulatory: fix reg_is_valid_request handling
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (16 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 21:27   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 19/24] regulatory: remove handling of channel bandwidth Johannes Berg
                   ` (6 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

There's a bug with the world regulatory domain, it
can be updated any time which is different from all
other regdomains that can only be updated once after
a request for them. Fix this by adding a check for
"processed" to the reg_is_valid_request() function
and clear that when doing a request.

While looking at this I also found another locking
bug, last_request is protected by the reg_mutex not
the cfg80211_mutex so the code in nl80211 is racy.
Remove that code as it only tries to prevent an
allocation in an error case, which isn't necessary.
Then the function can also become static and locking
in nl80211 can have a smaller scope.

Also change __set_regdom() to do the checks earlier
and not different for world/other regdomains.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c |  9 ++-------
 net/wireless/reg.c     | 21 +++++++++++----------
 net/wireless/reg.h     |  1 -
 3 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 260b5d8..370cd31 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4289,13 +4289,6 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 			return -EINVAL;
 	}
 
-	mutex_lock(&cfg80211_mutex);
-
-	if (!reg_is_valid_request(alpha2)) {
-		r = -EINVAL;
-		goto bad_reg;
-	}
-
 	size_of_regd = sizeof(struct ieee80211_regdomain) +
 		       num_rules * sizeof(struct ieee80211_reg_rule);
 
@@ -4333,6 +4326,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
 		}
 	}
 
+	mutex_lock(&cfg80211_mutex);
+
 	r = set_regdom(rd);
 	rd = NULL;
 
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index dfab71b..10d8edb 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -426,12 +426,16 @@ static int call_crda(const char *alpha2)
 	return kobject_uevent(&reg_pdev->dev.kobj, KOBJ_CHANGE);
 }
 
-/* Used by nl80211 before kmalloc'ing our regulatory domain */
-bool reg_is_valid_request(const char *alpha2)
+static bool reg_is_valid_request(const char *alpha2)
 {
+	assert_reg_lock();
+
 	if (!last_request)
 		return false;
 
+	if (last_request->processed)
+		return false;
+
 	return alpha2_equal(last_request->alpha2, alpha2);
 }
 
@@ -1471,6 +1475,7 @@ new_request:
 
 	last_request = pending_request;
 	last_request->intersect = intersect;
+	last_request->processed = false;
 
 	pending_request = NULL;
 
@@ -2061,11 +2066,13 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	const struct ieee80211_regdomain *regd;
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 	struct wiphy *request_wiphy;
+
 	/* Some basic sanity checks first */
 
+	if (!reg_is_valid_request(rd->alpha2))
+		return -EINVAL;
+
 	if (is_world_regdom(rd->alpha2)) {
-		if (WARN_ON(!reg_is_valid_request(rd->alpha2)))
-			return -EINVAL;
 		update_world_regdomain(rd);
 		return 0;
 	}
@@ -2074,9 +2081,6 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	    !is_unknown_alpha2(rd->alpha2))
 		return -EINVAL;
 
-	if (!last_request)
-		return -EINVAL;
-
 	/*
 	 * Lets only bother proceeding on the same alpha2 if the current
 	 * rd is non static (it means CRDA was present and was used last)
@@ -2098,9 +2102,6 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	 * internal EEPROM data
 	 */
 
-	if (WARN_ON(!reg_is_valid_request(rd->alpha2)))
-		return -EINVAL;
-
 	if (!is_valid_rd(rd)) {
 		pr_err("Invalid regulatory domain detected:\n");
 		print_regdomain_info(rd);
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index 37891e8..d391b50 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -19,7 +19,6 @@
 extern const struct ieee80211_regdomain *cfg80211_regdomain;
 
 bool is_world_regdom(const char *alpha2);
-bool reg_is_valid_request(const char *alpha2);
 bool reg_supported_dfs_region(u8 dfs_region);
 
 int regulatory_hint_user(const char *alpha2,
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 19/24] regulatory: remove handling of channel bandwidth
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (17 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 18/24] regulatory: fix reg_is_valid_request handling Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 22:11   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 20/24] regulatory: fix memory leak Johannes Berg
                   ` (5 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

The channel bandwidth handling isn't really quite right,
it assumes that a 40 MHz channel is really two 20 MHz
channels, which isn't strictly true. This is the way the
regulatory database handling is defined right now though
so remove the logic to handle other channel widths.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/ath/regd.c                   |  7 ++--
 drivers/net/wireless/brcm80211/brcmsmac/channel.c |  2 +-
 drivers/net/wireless/rtlwifi/regd.c               |  9 ++---
 include/net/cfg80211.h                            |  8 +----
 net/wireless/reg.c                                | 44 ++++++++---------------
 5 files changed, 21 insertions(+), 49 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index d816980..8ae58c40 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -195,7 +195,6 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule;
 	struct ieee80211_channel *ch;
 	unsigned int i;
-	u32 bandwidth = 0;
 	int r;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
@@ -216,7 +215,6 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 				r = freq_reg_info(wiphy,
 						  ch->center_freq,
-						  bandwidth,
 						  &reg_rule);
 				if (r)
 					continue;
@@ -254,7 +252,6 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *ch;
 	const struct ieee80211_reg_rule *reg_rule;
-	u32 bandwidth = 0;
 	int r;
 
 	sband = wiphy->bands[IEEE80211_BAND_2GHZ];
@@ -283,7 +280,7 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	 */
 
 	ch = &sband->channels[11]; /* CH 12 */
-	r = freq_reg_info(wiphy, ch->center_freq, bandwidth, &reg_rule);
+	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
 	if (!r) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
@@ -291,7 +288,7 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	}
 
 	ch = &sband->channels[12]; /* CH 13 */
-	r = freq_reg_info(wiphy, ch->center_freq, bandwidth, &reg_rule);
+	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
 	if (!r) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/channel.c b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
index 64a48f0..f754efa 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/channel.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
@@ -687,7 +687,7 @@ brcms_reg_apply_beaconing_flags(struct wiphy *wiphy,
 
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 				ret = freq_reg_info(wiphy, ch->center_freq,
-						    0, &rule);
+						    &rule);
 				if (ret)
 					continue;
 
diff --git a/drivers/net/wireless/rtlwifi/regd.c b/drivers/net/wireless/rtlwifi/regd.c
index c1608cd..be55dc9 100644
--- a/drivers/net/wireless/rtlwifi/regd.c
+++ b/drivers/net/wireless/rtlwifi/regd.c
@@ -158,7 +158,6 @@ static void _rtl_reg_apply_beaconing_flags(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule;
 	struct ieee80211_channel *ch;
 	unsigned int i;
-	u32 bandwidth = 0;
 	int r;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
@@ -174,8 +173,7 @@ static void _rtl_reg_apply_beaconing_flags(struct wiphy *wiphy,
 			    (ch->flags & IEEE80211_CHAN_RADAR))
 				continue;
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-				r = freq_reg_info(wiphy, ch->center_freq,
-						  bandwidth, &reg_rule);
+				r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
 				if (r)
 					continue;
 
@@ -211,7 +209,6 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *ch;
 	const struct ieee80211_reg_rule *reg_rule;
-	u32 bandwidth = 0;
 	int r;
 
 	if (!wiphy->bands[IEEE80211_BAND_2GHZ])
@@ -240,7 +237,7 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	 */
 
 	ch = &sband->channels[11];	/* CH 12 */
-	r = freq_reg_info(wiphy, ch->center_freq, bandwidth, &reg_rule);
+	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
 	if (!r) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
@@ -248,7 +245,7 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	}
 
 	ch = &sband->channels[12];	/* CH 13 */
-	r = freq_reg_info(wiphy, ch->center_freq, bandwidth, &reg_rule);
+	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
 	if (!r) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 3ba1784..84fcb10 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2938,10 +2938,6 @@ extern void wiphy_apply_custom_regulatory(
  * freq_reg_info - get regulatory information for the given frequency
  * @wiphy: the wiphy for which we want to process this rule for
  * @center_freq: Frequency in KHz for which we want regulatory information for
- * @desired_bw_khz: the desired max bandwidth you want to use per
- *	channel. Note that this is still 20 MHz if you want to use HT40
- *	as HT40 makes use of two channels for its 40 MHz width bandwidth.
- *	If set to 0 we'll assume you want the standard 20 MHz.
  * @reg_rule: the regulatory rule which we have for this frequency
  *
  * Use this function to get the regulatory rule for a specific frequency on
@@ -2956,9 +2952,7 @@ extern void wiphy_apply_custom_regulatory(
  * freq_in_rule_band() for our current definition of a band -- this is purely
  * subjective and right now its 802.11 specific.
  */
-extern int freq_reg_info(struct wiphy *wiphy,
-			 u32 center_freq,
-			 u32 desired_bw_khz,
+extern int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 			 const struct ieee80211_reg_rule **reg_rule);
 
 /*
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 10d8edb..011fab9 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -481,8 +481,7 @@ static bool is_valid_rd(const struct ieee80211_regdomain *rd)
 }
 
 static bool reg_does_bw_fit(const struct ieee80211_freq_range *freq_range,
-			    u32 center_freq_khz,
-			    u32 bw_khz)
+			    u32 center_freq_khz, u32 bw_khz)
 {
 	u32 start_freq_khz, end_freq_khz;
 
@@ -683,9 +682,7 @@ static u32 map_regdom_flags(u32 rd_flags)
 	return channel_flags;
 }
 
-static int freq_reg_info_regd(struct wiphy *wiphy,
-			      u32 center_freq,
-			      u32 desired_bw_khz,
+static int freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
 			      const struct ieee80211_reg_rule **reg_rule,
 			      const struct ieee80211_regdomain *regd)
 {
@@ -693,9 +690,6 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 	bool band_rule_found = false;
 	bool bw_fits = false;
 
-	if (!desired_bw_khz)
-		desired_bw_khz = MHZ_TO_KHZ(20);
-
 	if (!regd)
 		return -EINVAL;
 
@@ -714,7 +708,7 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 		if (!band_rule_found)
 			band_rule_found = freq_in_rule_band(fr, center_freq);
 
-		bw_fits = reg_does_bw_fit(fr, center_freq, desired_bw_khz);
+		bw_fits = reg_does_bw_fit(fr, center_freq, MHZ_TO_KHZ(20));
 
 		if (band_rule_found && bw_fits) {
 			*reg_rule = rr;
@@ -728,7 +722,7 @@ static int freq_reg_info_regd(struct wiphy *wiphy,
 	return -EINVAL;
 }
 
-int freq_reg_info(struct wiphy *wiphy, u32 center_freq, u32 desired_bw_khz,
+int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 		  const struct ieee80211_reg_rule **reg_rule)
 {
 	const struct ieee80211_regdomain *regd;
@@ -747,8 +741,7 @@ int freq_reg_info(struct wiphy *wiphy, u32 center_freq, u32 desired_bw_khz,
 	else
 		regd = cfg80211_regdomain;
 
-	return freq_reg_info_regd(wiphy, center_freq, desired_bw_khz,
-				  reg_rule, regd);
+	return freq_reg_info_regd(wiphy, center_freq, reg_rule, regd);
 }
 EXPORT_SYMBOL(freq_reg_info);
 
@@ -771,7 +764,6 @@ static const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
 }
 
 static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
-				    u32 desired_bw_khz,
 				    const struct ieee80211_reg_rule *reg_rule)
 {
 	const struct ieee80211_power_rule *power_rule;
@@ -786,8 +778,8 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
 	else
 		snprintf(max_antenna_gain, 32, "%d", power_rule->max_antenna_gain);
 
-	REG_DBG_PRINT("Updating information on frequency %d MHz for a %d MHz width channel with regulatory rule:\n",
-		      chan->center_freq, KHZ_TO_MHZ(desired_bw_khz));
+	REG_DBG_PRINT("Updating information on frequency %d MHz with regulatory rule:\n",
+		      chan->center_freq);
 
 	REG_DBG_PRINT("%d KHz - %d KHz @ %d KHz), (%s mBi, %d mBm)\n",
 		      freq_range->start_freq_khz, freq_range->end_freq_khz,
@@ -796,7 +788,6 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
 }
 #else
 static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
-				    u32 desired_bw_khz,
 				    const struct ieee80211_reg_rule *reg_rule)
 {
 	return;
@@ -806,11 +797,7 @@ static void chan_reg_rule_print_dbg(struct ieee80211_channel *chan,
 /*
  * Note that right now we assume the desired channel bandwidth
  * is always 20 MHz for each individual channel (HT40 uses 20 MHz
- * per channel, the primary and the extension channel). To support
- * smaller custom bandwidths such as 5 MHz or 10 MHz we'll need a
- * new ieee80211_channel.target_bw and re run the regulatory check
- * on the wiphy with the target_bw specified. Then we can simply use
- * that below for the desired_bw_khz below.
+ * per channel, the primary and the extension channel).
  */
 static void handle_channel(struct wiphy *wiphy,
 			   enum nl80211_reg_initiator initiator,
@@ -818,7 +805,6 @@ static void handle_channel(struct wiphy *wiphy,
 {
 	int r;
 	u32 flags, bw_flags = 0;
-	u32 desired_bw_khz = MHZ_TO_KHZ(20);
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
@@ -830,8 +816,7 @@ static void handle_channel(struct wiphy *wiphy,
 
 	flags = chan->orig_flags;
 
-	r = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq),
-			  desired_bw_khz, &reg_rule);
+	r = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq), &reg_rule);
 	if (r) {
 		/*
 		 * We will disable all channels that do not match our
@@ -852,7 +837,7 @@ static void handle_channel(struct wiphy *wiphy,
 		return;
 	}
 
-	chan_reg_rule_print_dbg(chan, desired_bw_khz, reg_rule);
+	chan_reg_rule_print_dbg(chan, reg_rule);
 
 	power_rule = &reg_rule->power_rule;
 	freq_range = &reg_rule->freq_range;
@@ -1224,23 +1209,22 @@ static void handle_channel_custom(struct wiphy *wiphy,
 				  const struct ieee80211_regdomain *regd)
 {
 	int r;
-	u32 desired_bw_khz = MHZ_TO_KHZ(20);
 	u32 bw_flags = 0;
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
 
 	r = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
-			       desired_bw_khz, &reg_rule, regd);
+			       &reg_rule, regd);
 
 	if (r) {
-		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits a %d MHz wide channel\n",
-			      chan->center_freq, KHZ_TO_MHZ(desired_bw_khz));
+		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
+			      chan->center_freq);
 		chan->flags = IEEE80211_CHAN_DISABLED;
 		return;
 	}
 
-	chan_reg_rule_print_dbg(chan, desired_bw_khz, reg_rule);
+	chan_reg_rule_print_dbg(chan, reg_rule);
 
 	power_rule = &reg_rule->power_rule;
 	freq_range = &reg_rule->freq_range;
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 20/24] regulatory: fix memory leak
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (18 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 19/24] regulatory: remove handling of channel bandwidth Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 21:35   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 21/24] regulatory: pass new regdomain to reset function Johannes Berg
                   ` (4 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

If duplicating here fails, we leak the original.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 011fab9..5a34830 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2120,8 +2120,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 			return -EALREADY;
 
 		regd = reg_copy_regd(rd);
-		if (IS_ERR(regd))
+		if (IS_ERR(regd)) {
+			kfree(rd);
 			return PTR_ERR(regd);
+		}
 
 		request_wiphy->regd = regd;
 		reset_regdomains(false);
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 21/24] regulatory: pass new regdomain to reset function
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (19 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 20/24] regulatory: fix memory leak Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 21:42   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains Johannes Berg
                   ` (3 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Instead of assigning after calling the function do
it inside the function. This will later avoid a
period of time where the pointer is NULL.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/reg.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5a34830..ff5ac4a 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -184,7 +184,8 @@ static char user_alpha2[2];
 module_param(ieee80211_regdom, charp, 0444);
 MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 
-static void reset_regdomains(bool full_reset)
+static void reset_regdomains(bool full_reset,
+			     const struct ieee80211_regdomain *new_regdom)
 {
 	assert_cfg80211_lock();
 	assert_reg_lock();
@@ -201,7 +202,7 @@ static void reset_regdomains(bool full_reset)
 	kfree(cfg80211_world_regdom);
 
 	cfg80211_world_regdom = &world_regdom;
-	cfg80211_regdomain = NULL;
+	cfg80211_regdomain = new_regdom;
 
 	if (!full_reset)
 		return;
@@ -222,10 +223,9 @@ static void update_world_regdomain(const struct ieee80211_regdomain *rd)
 	assert_cfg80211_lock();
 	assert_reg_lock();
 
-	reset_regdomains(false);
+	reset_regdomains(false, rd);
 
 	cfg80211_world_regdom = rd;
-	cfg80211_regdomain = rd;
 }
 
 bool is_world_regdom(const char *alpha2)
@@ -1818,7 +1818,7 @@ static void restore_regulatory_settings(bool reset_user)
 	mutex_lock(&cfg80211_mutex);
 	mutex_lock(&reg_mutex);
 
-	reset_regdomains(true);
+	reset_regdomains(true, cfg80211_world_regdom);
 	restore_alpha2(alpha2, reset_user);
 
 	/*
@@ -1849,9 +1849,8 @@ static void restore_regulatory_settings(bool reset_user)
 	}
 
 	/* First restore to the basic regulatory settings */
-	cfg80211_regdomain = cfg80211_world_regdom;
-	world_alpha2[0] = cfg80211_regdomain->alpha2[0];
-	world_alpha2[1] = cfg80211_regdomain->alpha2[1];
+	world_alpha2[0] = cfg80211_world_regdom->alpha2[0];
+	world_alpha2[1] = cfg80211_world_regdom->alpha2[1];
 
 	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
 		if (rdev->wiphy.flags & WIPHY_FLAG_CUSTOM_REGULATORY)
@@ -2102,8 +2101,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 
 	if (!last_request->intersect) {
 		if (last_request->initiator != NL80211_REGDOM_SET_BY_DRIVER) {
-			reset_regdomains(false);
-			cfg80211_regdomain = rd;
+			reset_regdomains(false, rd);
 			return 0;
 		}
 
@@ -2126,8 +2124,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		}
 
 		request_wiphy->regd = regd;
-		reset_regdomains(false);
-		cfg80211_regdomain = rd;
+		reset_regdomains(false, rd);
 		return 0;
 	}
 
@@ -2150,8 +2147,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 
 		rd = NULL;
 
-		reset_regdomains(false);
-		cfg80211_regdomain = intersected_rd;
+		reset_regdomains(false, intersected_rd);
 
 		return 0;
 	}
@@ -2322,7 +2318,7 @@ void regulatory_exit(void)
 	/* Lock to suppress warnings */
 	mutex_lock(&cfg80211_mutex);
 	mutex_lock(&reg_mutex);
-	reset_regdomains(true);
+	reset_regdomains(true, NULL);
 	mutex_unlock(&cfg80211_mutex);
 	mutex_unlock(&reg_mutex);
 
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (20 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 21/24] regulatory: pass new regdomain to reset function Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-10 17:05   ` Johannes Berg
  2012-12-06 16:47 ` [PATCH 23/24] regulatory: use RCU to protect last_request Johannes Berg
                   ` (2 subsequent siblings)
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

To simplify the locking and not require cfg80211_mutex
(which nl80211 uses to access the global regdomain) and
also to make it possible for drivers to access their
wiphy->regd safely, use RCU to protect these pointers.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h   |   2 +-
 include/net/regulatory.h |   2 +
 net/wireless/nl80211.c   |  36 +++++++--------
 net/wireless/reg.c       | 113 +++++++++++++++++++++++++++--------------------
 net/wireless/reg.h       |   2 +-
 5 files changed, 88 insertions(+), 67 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 84fcb10..461d979 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2369,7 +2369,7 @@ struct wiphy {
 
 	/* fields below are read-only, assigned by cfg80211 */
 
-	const struct ieee80211_regdomain *regd;
+	const struct ieee80211_regdomain __rcu *regd;
 
 	/* the item in /sys/class/ieee80211/ points to this,
 	 * you need use set_wiphy_dev() (see below) */
diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 7dcaa27..96b0f07 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -18,6 +18,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <linux/rcupdate.h>
 
 /**
  * enum environment_cap - Environment parsed from country IE
@@ -101,6 +102,7 @@ struct ieee80211_reg_rule {
 };
 
 struct ieee80211_regdomain {
+	struct rcu_head rcu_head;
 	u32 n_reg_rules;
 	char alpha2[2];
 	u8 dfs_region;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 370cd31..dd3df5c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3811,12 +3811,8 @@ static int nl80211_req_set_reg(struct sk_buff *skb, struct genl_info *info)
 	 * window between nl80211_init() and regulatory_init(), if that is
 	 * even possible.
 	 */
-	mutex_lock(&cfg80211_mutex);
-	if (unlikely(!cfg80211_regdomain)) {
-		mutex_unlock(&cfg80211_mutex);
+	if (unlikely(!rcu_access_pointer(cfg80211_regdomain)))
 		return -EINPROGRESS;
-	}
-	mutex_unlock(&cfg80211_mutex);
 
 	if (!info->attrs[NL80211_ATTR_REG_ALPHA2])
 		return -EINVAL;
@@ -4176,6 +4172,7 @@ static int nl80211_update_mesh_config(struct sk_buff *skb,
 
 static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 {
+	const struct ieee80211_regdomain *regdom;
 	struct sk_buff *msg;
 	void *hdr = NULL;
 	struct nlattr *nl_reg_rules;
@@ -4198,35 +4195,36 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 	if (!hdr)
 		goto put_failure;
 
-	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2,
-			   cfg80211_regdomain->alpha2) ||
-	    (cfg80211_regdomain->dfs_region &&
-	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION,
-			cfg80211_regdomain->dfs_region)))
-		goto nla_put_failure;
+	rcu_read_lock();
+	regdom = rcu_dereference(cfg80211_regdomain);
+
+	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) ||
+	    (regdom->dfs_region &&
+	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region)))
+		goto nla_put_failure_rcu;
 
 	if (reg_last_request_cell_base() &&
 	    nla_put_u32(msg, NL80211_ATTR_USER_REG_HINT_TYPE,
 			NL80211_USER_REG_HINT_CELL_BASE))
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
 	nl_reg_rules = nla_nest_start(msg, NL80211_ATTR_REG_RULES);
 	if (!nl_reg_rules)
-		goto nla_put_failure;
+		goto nla_put_failure_rcu;
 
-	for (i = 0; i < cfg80211_regdomain->n_reg_rules; i++) {
+	for (i = 0; i < regdom->n_reg_rules; i++) {
 		struct nlattr *nl_reg_rule;
 		const struct ieee80211_reg_rule *reg_rule;
 		const struct ieee80211_freq_range *freq_range;
 		const struct ieee80211_power_rule *power_rule;
 
-		reg_rule = &cfg80211_regdomain->reg_rules[i];
+		reg_rule = &regdom->reg_rules[i];
 		freq_range = &reg_rule->freq_range;
 		power_rule = &reg_rule->power_rule;
 
 		nl_reg_rule = nla_nest_start(msg, i);
 		if (!nl_reg_rule)
-			goto nla_put_failure;
+			goto nla_put_failure_rcu;
 
 		if (nla_put_u32(msg, NL80211_ATTR_REG_RULE_FLAGS,
 				reg_rule->flags) ||
@@ -4240,10 +4238,11 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 				power_rule->max_antenna_gain) ||
 		    nla_put_u32(msg, NL80211_ATTR_POWER_RULE_MAX_EIRP,
 				power_rule->max_eirp))
-			goto nla_put_failure;
+			goto nla_put_failure_rcu;
 
 		nla_nest_end(msg, nl_reg_rule);
 	}
+	rcu_read_unlock();
 
 	nla_nest_end(msg, nl_reg_rules);
 
@@ -4251,7 +4250,8 @@ static int nl80211_get_reg(struct sk_buff *skb, struct genl_info *info)
 	err = genlmsg_reply(msg, info);
 	goto out;
 
-nla_put_failure:
+nla_put_failure_rcu:
+	rcu_read_unlock();
 	genlmsg_cancel(msg, hdr);
 put_failure:
 	nlmsg_free(msg);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index ff5ac4a..6082c68 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -95,15 +95,15 @@ static struct device_type reg_device_type = {
  * Central wireless core regulatory domains, we only need two,
  * the current one and a world regulatory domain in case we have no
  * information to give us an alpha2.
- * Protected by the cfg80211_mutex.
  */
-const struct ieee80211_regdomain *cfg80211_regdomain;
+const struct ieee80211_regdomain __rcu *cfg80211_regdomain;
 
 /*
  * Protects static reg.c components:
- *     - cfg80211_world_regdom
- *     - last_request
- *     - reg_num_devs_support_basehint
+ *	- cfg80211_regdomain (if not used with RCU)
+ *	- cfg80211_world_regdom
+ *	- last_request
+ *	- reg_num_devs_support_basehint
  */
 static DEFINE_MUTEX(reg_mutex);
 
@@ -118,6 +118,25 @@ static inline void assert_reg_lock(void)
 	lockdep_assert_held(&reg_mutex);
 }
 
+static const struct ieee80211_regdomain *get_cfg80211_regdom(void)
+{
+	return rcu_dereference_protected(cfg80211_regdomain,
+					 lockdep_is_held(&reg_mutex));
+}
+
+static const struct ieee80211_regdomain *get_wiphy_regdom(struct wiphy *wiphy)
+{
+	return rcu_dereference_protected(wiphy->regd,
+					 lockdep_is_held(&reg_mutex));
+}
+
+static void rcu_free_regdom(const struct ieee80211_regdomain *r)
+{
+	if (!r)
+		return;
+	kfree_rcu((struct ieee80211_regdomain *)r, rcu_head);
+}
+
 /* Used to queue up regulatory hints */
 static LIST_HEAD(reg_requests_list);
 static spinlock_t reg_requests_lock;
@@ -187,22 +206,25 @@ MODULE_PARM_DESC(ieee80211_regdom, "IEEE 802.11 regulatory domain code");
 static void reset_regdomains(bool full_reset,
 			     const struct ieee80211_regdomain *new_regdom)
 {
-	assert_cfg80211_lock();
+	const struct ieee80211_regdomain *r;
+
 	assert_reg_lock();
 
+	r = get_cfg80211_regdom();
+
 	/* avoid freeing static information or freeing something twice */
-	if (cfg80211_regdomain == cfg80211_world_regdom)
-		cfg80211_regdomain = NULL;
+	if (r == cfg80211_world_regdom)
+		r = NULL;
 	if (cfg80211_world_regdom == &world_regdom)
 		cfg80211_world_regdom = NULL;
-	if (cfg80211_regdomain == &world_regdom)
-		cfg80211_regdomain = NULL;
+	if (r == &world_regdom)
+		r = NULL;
 
-	kfree(cfg80211_regdomain);
-	kfree(cfg80211_world_regdom);
+	rcu_free_regdom(r);
+	rcu_free_regdom(cfg80211_world_regdom);
 
 	cfg80211_world_regdom = &world_regdom;
-	cfg80211_regdomain = new_regdom;
+	rcu_assign_pointer(cfg80211_regdomain, new_regdom);
 
 	if (!full_reset)
 		return;
@@ -220,7 +242,6 @@ static void update_world_regdomain(const struct ieee80211_regdomain *rd)
 {
 	WARN_ON(!last_request);
 
-	assert_cfg80211_lock();
 	assert_reg_lock();
 
 	reset_regdomains(false, rd);
@@ -281,11 +302,11 @@ static bool alpha2_equal(const char *alpha2_x, const char *alpha2_y)
 
 static bool regdom_changes(const char *alpha2)
 {
-	assert_cfg80211_lock();
+	const struct ieee80211_regdomain *r = get_cfg80211_regdom();
 
-	if (!cfg80211_regdomain)
+	if (!r)
 		return true;
-	return !alpha2_equal(cfg80211_regdomain->alpha2, alpha2);
+	return !alpha2_equal(r->alpha2, alpha2);
 }
 
 /*
@@ -728,7 +749,6 @@ int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 	const struct ieee80211_regdomain *regd;
 
 	assert_reg_lock();
-	assert_cfg80211_lock();
 
 	/*
 	 * Follow the driver's regulatory domain, if present, unless a country
@@ -737,9 +757,9 @@ int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
 	    last_request->initiator != NL80211_REGDOM_SET_BY_USER &&
 	    wiphy->regd)
-		regd = wiphy->regd;
+		regd = get_wiphy_regdom(wiphy);
 	else
-		regd = cfg80211_regdomain;
+		regd = get_cfg80211_regdom();
 
 	return freq_reg_info_regd(wiphy, center_freq, reg_rule, regd);
 }
@@ -810,8 +830,6 @@ static void handle_channel(struct wiphy *wiphy,
 	const struct ieee80211_freq_range *freq_range = NULL;
 	struct wiphy *request_wiphy = NULL;
 
-	assert_cfg80211_lock();
-
 	request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
 
 	flags = chan->orig_flags;
@@ -1061,15 +1079,17 @@ static void wiphy_update_beacon_reg(struct wiphy *wiphy)
 
 static bool reg_is_world_roaming(struct wiphy *wiphy)
 {
-	assert_cfg80211_lock();
+	const struct ieee80211_regdomain *cr = get_cfg80211_regdom();
+	const struct ieee80211_regdomain *wr = get_wiphy_regdom(wiphy);
 
-	if (is_world_regdom(cfg80211_regdomain->alpha2) ||
-	    (wiphy->regd && is_world_regdom(wiphy->regd->alpha2)))
+	if (is_world_regdom(cr->alpha2) || (wr && is_world_regdom(wr->alpha2)))
 		return true;
+
 	if (last_request &&
 	    last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
 	    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY)
 		return true;
+
 	return false;
 }
 
@@ -1166,13 +1186,12 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 {
 	enum ieee80211_band band;
 
-	assert_cfg80211_lock();
 	assert_reg_lock();
 
 	if (ignore_reg_update(wiphy, initiator))
 		return;
 
-	last_request->dfs_region = cfg80211_regdomain->dfs_region;
+	last_request->dfs_region = get_cfg80211_regdom()->dfs_region;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++)
 		handle_band(wiphy, initiator, wiphy->bands[band]);
@@ -1189,6 +1208,8 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
 	struct cfg80211_registered_device *rdev;
 	struct wiphy *wiphy;
 
+	assert_cfg80211_lock();
+
 	list_for_each_entry(rdev, &cfg80211_rdev_list, list) {
 		wiphy = &rdev->wiphy;
 		wiphy_update_regulatory(wiphy, initiator);
@@ -1403,7 +1424,7 @@ static void reg_set_request_processed(void)
  *
  * Returns one of the different reg request treatment values.
  *
- * Caller must hold &cfg80211_mutex and &reg_mutex
+ * Caller must hold &reg_mutex
  */
 static enum reg_request_treatment
 __regulatory_hint(struct wiphy *wiphy,
@@ -1413,20 +1434,18 @@ __regulatory_hint(struct wiphy *wiphy,
 	bool intersect = false;
 	enum reg_request_treatment treatment;
 
-	assert_cfg80211_lock();
-
 	treatment = get_reg_request_treatment(wiphy, pending_request);
 
 	switch (treatment) {
 	case REG_REQ_INTERSECT:
 		if (pending_request->initiator ==
 		    NL80211_REGDOM_SET_BY_DRIVER) {
-			regd = reg_copy_regd(cfg80211_regdomain);
+			regd = reg_copy_regd(get_cfg80211_regdom());
 			if (IS_ERR(regd)) {
 				kfree(pending_request);
 				return PTR_ERR(regd);
 			}
-			wiphy->regd = regd;
+			rcu_assign_pointer(wiphy->regd, regd);
 		}
 		intersect = true;
 		break;
@@ -1440,13 +1459,13 @@ __regulatory_hint(struct wiphy *wiphy,
 		 */
 		if (treatment == REG_REQ_ALREADY_SET &&
 		    pending_request->initiator == NL80211_REGDOM_SET_BY_DRIVER) {
-			regd = reg_copy_regd(cfg80211_regdomain);
+			regd = reg_copy_regd(get_cfg80211_regdom());
 			if (IS_ERR(regd)) {
 				kfree(pending_request);
 				return REG_REQ_IGNORE;
 			}
 			treatment = REG_REQ_ALREADY_SET;
-			wiphy->regd = regd;
+			rcu_assign_pointer(wiphy->regd, regd);
 			goto new_request;
 		}
 		kfree(pending_request);
@@ -2052,6 +2071,8 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 
 	/* Some basic sanity checks first */
 
+	assert_reg_lock();
+
 	if (!reg_is_valid_request(rd->alpha2))
 		return -EINVAL;
 
@@ -2123,7 +2144,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 			return PTR_ERR(regd);
 		}
 
-		request_wiphy->regd = regd;
+		rcu_assign_pointer(request_wiphy->regd, regd);
 		reset_regdomains(false, rd);
 		return 0;
 	}
@@ -2131,7 +2152,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	/* Intersection requires a bit more work */
 
 	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-		intersected_rd = regdom_intersect(rd, cfg80211_regdomain);
+		intersected_rd = regdom_intersect(rd, get_cfg80211_regdom());
 		if (!intersected_rd)
 			return -EINVAL;
 
@@ -2141,7 +2162,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		 * domain we keep it for its private use
 		 */
 		if (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER)
-			request_wiphy->regd = rd;
+			rcu_assign_pointer(request_wiphy->regd, rd);
 		else
 			kfree(rd);
 
@@ -2159,14 +2180,12 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 /*
  * Use this call to set the current regulatory domain. Conflicts with
  * multiple drivers can be ironed out later. Caller must've already
- * kmalloc'd the rd structure. Caller must hold cfg80211_mutex
+ * kmalloc'd the rd structure.
  */
 int set_regdom(const struct ieee80211_regdomain *rd)
 {
 	int r;
 
-	assert_cfg80211_lock();
-
 	mutex_lock(&reg_mutex);
 
 	/* Note that this doesn't update the wiphys, this is done below */
@@ -2180,7 +2199,8 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	/* This would make this whole thing pointless */
-	if (WARN_ON(!last_request->intersect && rd != cfg80211_regdomain)) {
+	if (WARN_ON(!last_request->intersect &&
+		    rd != get_cfg80211_regdom())) {
 		r = -EINVAL;
 		goto out;
 	}
@@ -2188,7 +2208,7 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	/* update all wiphys now with the new established regulatory domain */
 	update_all_wiphy_regulatory(last_request->initiator);
 
-	print_regdomain(cfg80211_regdomain);
+	print_regdomain(get_cfg80211_regdom());
 
 	nl80211_send_reg_change_event(last_request);
 
@@ -2241,7 +2261,8 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 	if (!reg_dev_ignore_cell_hint(wiphy))
 		reg_num_devs_support_basehint--;
 
-	kfree(wiphy->regd);
+	rcu_free_regdom(get_wiphy_regdom(wiphy));
+	rcu_assign_pointer(wiphy->regd, NULL);
 
 	if (last_request)
 		request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
@@ -2276,13 +2297,13 @@ int __init regulatory_init(void)
 
 	reg_regdb_size_check();
 
-	cfg80211_regdomain = cfg80211_world_regdom;
+	rcu_assign_pointer(cfg80211_regdomain, cfg80211_world_regdom);
 
 	user_alpha2[0] = '9';
 	user_alpha2[1] = '7';
 
 	/* We always try to get an update for the static regdomain */
-	err = regulatory_hint_core(cfg80211_regdomain->alpha2);
+	err = regulatory_hint_core(cfg80211_world_regdom->alpha2);
 	if (err) {
 		if (err == -ENOMEM)
 			return err;
@@ -2316,10 +2337,8 @@ void regulatory_exit(void)
 	cancel_delayed_work_sync(&reg_timeout);
 
 	/* Lock to suppress warnings */
-	mutex_lock(&cfg80211_mutex);
 	mutex_lock(&reg_mutex);
 	reset_regdomains(true, NULL);
-	mutex_unlock(&cfg80211_mutex);
 	mutex_unlock(&reg_mutex);
 
 	dev_set_uevent_suppress(&reg_pdev->dev, true);
diff --git a/net/wireless/reg.h b/net/wireless/reg.h
index d391b50..af2d5f8 100644
--- a/net/wireless/reg.h
+++ b/net/wireless/reg.h
@@ -16,7 +16,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-extern const struct ieee80211_regdomain *cfg80211_regdomain;
+extern const struct ieee80211_regdomain __rcu *cfg80211_regdomain;
 
 bool is_world_regdom(const char *alpha2);
 bool reg_supported_dfs_region(u8 dfs_region);
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 23/24] regulatory: use RCU to protect last_request
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (21 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 22:07   ` Luis R. Rodriguez
  2012-12-06 16:47 ` [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info Johannes Berg
  2012-12-20 11:15 ` [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

This will allow making freq_reg_info() lock-free.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/regulatory.h |   2 +
 net/wireless/reg.c       | 195 ++++++++++++++++++++++++++---------------------
 2 files changed, 108 insertions(+), 89 deletions(-)

diff --git a/include/net/regulatory.h b/include/net/regulatory.h
index 96b0f07..f17ed59 100644
--- a/include/net/regulatory.h
+++ b/include/net/regulatory.h
@@ -36,6 +36,7 @@ enum environment_cap {
 /**
  * struct regulatory_request - used to keep track of regulatory requests
  *
+ * @rcu_head: RCU head struct used to free the request
  * @wiphy_idx: this is set if this request's initiator is
  * 	%REGDOM_SET_BY_COUNTRY_IE or %REGDOM_SET_BY_DRIVER. This
  * 	can be used by the wireless core to deal with conflicts
@@ -73,6 +74,7 @@ enum environment_cap {
  * @list: used to insert into the reg_requests_list linked list
  */
 struct regulatory_request {
+	struct rcu_head rcu_head;
 	int wiphy_idx;
 	enum nl80211_reg_initiator initiator;
 	enum nl80211_user_reg_hint_type user_reg_hint_type;
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 6082c68..b289131 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -82,7 +82,8 @@ static struct regulatory_request core_request_world = {
 };
 
 /* Receipt of information from last regulatory request */
-static struct regulatory_request *last_request = &core_request_world;
+static struct regulatory_request __rcu *last_request =
+	(void __rcu *)&core_request_world;
 
 /* To trigger userspace events */
 static struct platform_device *reg_pdev;
@@ -102,7 +103,7 @@ const struct ieee80211_regdomain __rcu *cfg80211_regdomain;
  * Protects static reg.c components:
  *	- cfg80211_regdomain (if not used with RCU)
  *	- cfg80211_world_regdom
- *	- last_request
+ *	- last_request (if not used with RCU)
  *	- reg_num_devs_support_basehint
  */
 static DEFINE_MUTEX(reg_mutex);
@@ -137,6 +138,12 @@ static void rcu_free_regdom(const struct ieee80211_regdomain *r)
 	kfree_rcu((struct ieee80211_regdomain *)r, rcu_head);
 }
 
+static struct regulatory_request *get_last_request(void)
+{
+	return rcu_dereference_protected(last_request,
+					 lockdep_is_held(&reg_mutex));
+}
+
 /* Used to queue up regulatory hints */
 static LIST_HEAD(reg_requests_list);
 static spinlock_t reg_requests_lock;
@@ -207,6 +214,7 @@ static void reset_regdomains(bool full_reset,
 			     const struct ieee80211_regdomain *new_regdom)
 {
 	const struct ieee80211_regdomain *r;
+	struct regulatory_request *lr;
 
 	assert_reg_lock();
 
@@ -229,9 +237,10 @@ static void reset_regdomains(bool full_reset,
 	if (!full_reset)
 		return;
 
-	if (last_request != &core_request_world)
-		kfree(last_request);
-	last_request = &core_request_world;
+	lr = get_last_request();
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
+	rcu_assign_pointer(last_request, &core_request_world);
 }
 
 /*
@@ -240,9 +249,11 @@ static void reset_regdomains(bool full_reset,
  */
 static void update_world_regdomain(const struct ieee80211_regdomain *rd)
 {
-	WARN_ON(!last_request);
+	struct regulatory_request *lr;
 
-	assert_reg_lock();
+	lr = get_last_request();
+
+	WARN_ON(!lr);
 
 	reset_regdomains(false, rd);
 
@@ -449,15 +460,12 @@ static int call_crda(const char *alpha2)
 
 static bool reg_is_valid_request(const char *alpha2)
 {
-	assert_reg_lock();
-
-	if (!last_request)
-		return false;
+	struct regulatory_request *lr = get_last_request();
 
-	if (last_request->processed)
+	if (!lr || lr->processed)
 		return false;
 
-	return alpha2_equal(last_request->alpha2, alpha2);
+	return alpha2_equal(lr->alpha2, alpha2);
 }
 
 /* Sanity check on a regulatory rule */
@@ -747,15 +755,14 @@ int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 		  const struct ieee80211_reg_rule **reg_rule)
 {
 	const struct ieee80211_regdomain *regd;
-
-	assert_reg_lock();
+	struct regulatory_request *lr = get_last_request();
 
 	/*
 	 * Follow the driver's regulatory domain, if present, unless a country
 	 * IE has been processed or a user wants to help complaince further
 	 */
-	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    last_request->initiator != NL80211_REGDOM_SET_BY_USER &&
+	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    lr->initiator != NL80211_REGDOM_SET_BY_USER &&
 	    wiphy->regd)
 		regd = get_wiphy_regdom(wiphy);
 	else
@@ -829,8 +836,9 @@ static void handle_channel(struct wiphy *wiphy,
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
 	struct wiphy *request_wiphy = NULL;
+	struct regulatory_request *lr = get_last_request();
 
-	request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 
 	flags = chan->orig_flags;
 
@@ -863,7 +871,7 @@ static void handle_channel(struct wiphy *wiphy,
 	if (freq_range->max_bandwidth_khz < MHZ_TO_KHZ(40))
 		bw_flags = IEEE80211_CHAN_NO_HT40;
 
-	if (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
+	if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
 	    request_wiphy && request_wiphy == wiphy &&
 	    request_wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) {
 		/*
@@ -928,7 +936,7 @@ bool reg_last_request_cell_base(void)
 	bool val;
 
 	mutex_lock(&reg_mutex);
-	val = reg_request_cell_base(last_request);
+	val = reg_request_cell_base(get_last_request());
 	mutex_unlock(&reg_mutex);
 
 	return val;
@@ -970,7 +978,9 @@ static bool reg_dev_ignore_cell_hint(struct wiphy *wiphy)
 static bool ignore_reg_update(struct wiphy *wiphy,
 			      enum nl80211_reg_initiator initiator)
 {
-	if (!last_request) {
+	struct regulatory_request *lr = get_last_request();
+
+	if (!lr) {
 		REG_DBG_PRINT("Ignoring regulatory request %s since last_request is not set\n",
 			      reg_initiator_name(initiator));
 		return true;
@@ -989,13 +999,13 @@ static bool ignore_reg_update(struct wiphy *wiphy,
 	 */
 	if (wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY && !wiphy->regd &&
 	    initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    !is_world_regdom(last_request->alpha2)) {
+	    !is_world_regdom(lr->alpha2)) {
 		REG_DBG_PRINT("Ignoring regulatory request %s since the driver requires its own regulatory domain to be set first\n",
 			      reg_initiator_name(initiator));
 		return true;
 	}
 
-	if (reg_request_cell_base(last_request))
+	if (reg_request_cell_base(lr))
 		return reg_dev_ignore_cell_hint(wiphy);
 
 	return false;
@@ -1081,12 +1091,12 @@ static bool reg_is_world_roaming(struct wiphy *wiphy)
 {
 	const struct ieee80211_regdomain *cr = get_cfg80211_regdom();
 	const struct ieee80211_regdomain *wr = get_wiphy_regdom(wiphy);
+	struct regulatory_request *lr = get_last_request();
 
 	if (is_world_regdom(cr->alpha2) || (wr && is_world_regdom(wr->alpha2)))
 		return true;
 
-	if (last_request &&
-	    last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	if (lr && lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE &&
 	    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY)
 		return true;
 
@@ -1185,13 +1195,12 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 				    enum nl80211_reg_initiator initiator)
 {
 	enum ieee80211_band band;
-
-	assert_reg_lock();
+	struct regulatory_request *lr = get_last_request();
 
 	if (ignore_reg_update(wiphy, initiator))
 		return;
 
-	last_request->dfs_region = get_cfg80211_regdom()->dfs_region;
+	lr->dfs_region = get_cfg80211_regdom()->dfs_region;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++)
 		handle_band(wiphy, initiator, wiphy->bands[band]);
@@ -1200,7 +1209,7 @@ static void wiphy_update_regulatory(struct wiphy *wiphy,
 	reg_process_ht_flags(wiphy);
 
 	if (wiphy->reg_notifier)
-		wiphy->reg_notifier(wiphy, last_request);
+		wiphy->reg_notifier(wiphy, lr);
 }
 
 static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
@@ -1221,7 +1230,7 @@ static void update_all_wiphy_regulatory(enum nl80211_reg_initiator initiator)
 		if (initiator == NL80211_REGDOM_SET_BY_CORE &&
 		    wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY &&
 		    wiphy->reg_notifier)
-			wiphy->reg_notifier(wiphy, last_request);
+			wiphy->reg_notifier(wiphy, get_last_request());
 	}
 }
 
@@ -1301,28 +1310,28 @@ get_reg_request_treatment(struct wiphy *wiphy,
 			  struct regulatory_request *pending_request)
 {
 	struct wiphy *last_wiphy = NULL;
+	struct regulatory_request *lr = get_last_request();
 
 	/* All initial requests are respected */
-	if (!last_request)
+	if (!lr)
 		return REG_REQ_OK;
 
 	switch (pending_request->initiator) {
 	case NL80211_REGDOM_SET_BY_CORE:
 		return REG_REQ_OK;
 	case NL80211_REGDOM_SET_BY_COUNTRY_IE:
-		if (reg_request_cell_base(last_request)) {
+		if (reg_request_cell_base(lr)) {
 			/* Trust a Cell base station over the AP's country IE */
 			if (regdom_changes(pending_request->alpha2))
 				return REG_REQ_IGNORE;
 			return REG_REQ_ALREADY_SET;
 		}
 
-		last_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+		last_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 
 		if (unlikely(!is_an_alpha2(pending_request->alpha2)))
 			return -EINVAL;
-		if (last_request->initiator ==
-		    NL80211_REGDOM_SET_BY_COUNTRY_IE) {
+		if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 			if (last_wiphy != wiphy) {
 				/*
 				 * Two cards with two APs claiming different
@@ -1344,7 +1353,7 @@ get_reg_request_treatment(struct wiphy *wiphy,
 		}
 		return 0;
 	case NL80211_REGDOM_SET_BY_DRIVER:
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_CORE) {
+		if (lr->initiator == NL80211_REGDOM_SET_BY_CORE) {
 			if (regdom_changes(pending_request->alpha2))
 				return REG_REQ_OK;
 			return REG_REQ_ALREADY_SET;
@@ -1355,7 +1364,7 @@ get_reg_request_treatment(struct wiphy *wiphy,
 		 * back in or if you add a new device for which the previously
 		 * loaded card also agrees on the regulatory domain.
 		 */
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
+		if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER &&
 		    !regdom_changes(pending_request->alpha2))
 			return REG_REQ_ALREADY_SET;
 
@@ -1364,26 +1373,26 @@ get_reg_request_treatment(struct wiphy *wiphy,
 		if (reg_request_cell_base(pending_request))
 			return reg_ignore_cell_hint(pending_request);
 
-		if (reg_request_cell_base(last_request))
+		if (reg_request_cell_base(lr))
 			return REG_REQ_IGNORE;
 
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
+		if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)
 			return REG_REQ_INTERSECT;
 		/*
 		 * If the user knows better the user should set the regdom
 		 * to their country before the IE is picked up
 		 */
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_USER &&
-		    last_request->intersect)
+		if (lr->initiator == NL80211_REGDOM_SET_BY_USER &&
+		    lr->intersect)
 			return REG_REQ_IGNORE;
 		/*
 		 * Process user requests only after previous user/driver/core
 		 * requests have been processed
 		 */
-		if ((last_request->initiator == NL80211_REGDOM_SET_BY_CORE ||
-		     last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
-		     last_request->initiator == NL80211_REGDOM_SET_BY_USER) &&
-		    regdom_changes(last_request->alpha2))
+		if ((lr->initiator == NL80211_REGDOM_SET_BY_CORE ||
+		     lr->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
+		     lr->initiator == NL80211_REGDOM_SET_BY_USER) &&
+		    regdom_changes(lr->alpha2))
 			return REG_REQ_IGNORE;
 
 		if (!regdom_changes(pending_request->alpha2))
@@ -1398,15 +1407,16 @@ get_reg_request_treatment(struct wiphy *wiphy,
 static void reg_set_request_processed(void)
 {
 	bool need_more_processing = false;
+	struct regulatory_request *lr = get_last_request();
 
-	last_request->processed = true;
+	lr->processed = true;
 
 	spin_lock(&reg_requests_lock);
 	if (!list_empty(&reg_requests_list))
 		need_more_processing = true;
 	spin_unlock(&reg_requests_lock);
 
-	if (last_request->initiator == NL80211_REGDOM_SET_BY_USER)
+	if (lr->initiator == NL80211_REGDOM_SET_BY_USER)
 		cancel_delayed_work(&reg_timeout);
 
 	if (need_more_processing)
@@ -1433,6 +1443,7 @@ __regulatory_hint(struct wiphy *wiphy,
 	const struct ieee80211_regdomain *regd;
 	bool intersect = false;
 	enum reg_request_treatment treatment;
+	struct regulatory_request *lr;
 
 	treatment = get_reg_request_treatment(wiphy, pending_request);
 
@@ -1473,18 +1484,20 @@ __regulatory_hint(struct wiphy *wiphy,
 	}
 
 new_request:
-	if (last_request != &core_request_world)
-		kfree(last_request);
+	lr = get_last_request();
+	if (lr != &core_request_world && lr)
+		kfree_rcu(lr, rcu_head);
 
-	last_request = pending_request;
-	last_request->intersect = intersect;
-	last_request->processed = false;
+	pending_request->intersect = intersect;
+	pending_request->processed = false;
+	rcu_assign_pointer(last_request, pending_request);
+	lr = pending_request;
 
 	pending_request = NULL;
 
-	if (last_request->initiator == NL80211_REGDOM_SET_BY_USER) {
-		user_alpha2[0] = last_request->alpha2[0];
-		user_alpha2[1] = last_request->alpha2[1];
+	if (lr->initiator == NL80211_REGDOM_SET_BY_USER) {
+		user_alpha2[0] = lr->alpha2[0];
+		user_alpha2[1] = lr->alpha2[1];
 	}
 
 	/* When r == REG_REQ_INTERSECT we do need to call CRDA */
@@ -1495,13 +1508,13 @@ new_request:
 		 * inform userspace we have processed the request
 		 */
 		if (treatment == REG_REQ_ALREADY_SET) {
-			nl80211_send_reg_change_event(last_request);
+			nl80211_send_reg_change_event(lr);
 			reg_set_request_processed();
 		}
 		return treatment;
 	}
 
-	if (call_crda(last_request->alpha2))
+	if (call_crda(lr->alpha2))
 		return REG_REQ_IGNORE;
 	return REG_REQ_OK;
 }
@@ -1544,13 +1557,14 @@ static void reg_process_hint(struct regulatory_request *reg_request,
  */
 static void reg_process_pending_hints(void)
 {
-	struct regulatory_request *reg_request;
+	struct regulatory_request *reg_request, *lr;
 
 	mutex_lock(&cfg80211_mutex);
 	mutex_lock(&reg_mutex);
+	lr = get_last_request();
 
 	/* When last_request->processed becomes true this will be rescheduled */
-	if (last_request && !last_request->processed) {
+	if (lr && !lr->processed) {
 		REG_DBG_PRINT("Pending regulatory request, waiting for it to be processed...\n");
 		goto out;
 	}
@@ -1703,11 +1717,12 @@ void regulatory_hint_11d(struct wiphy *wiphy, enum ieee80211_band band,
 {
 	char alpha2[2];
 	enum environment_cap env = ENVIRON_ANY;
-	struct regulatory_request *request;
+	struct regulatory_request *request, *lr;
 
 	mutex_lock(&reg_mutex);
+	lr = get_last_request();
 
-	if (unlikely(!last_request))
+	if (unlikely(!lr))
 		goto out;
 
 	/* IE len must be evenly divisible by 2 */
@@ -1730,8 +1745,8 @@ void regulatory_hint_11d(struct wiphy *wiphy, enum ieee80211_band band,
 	 * We leave conflict resolution to the workqueue, where can hold
 	 * cfg80211_mutex.
 	 */
-	if (last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-	    last_request->wiphy_idx != WIPHY_IDX_INVALID)
+	if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
+	    lr->wiphy_idx != WIPHY_IDX_INVALID)
 		goto out;
 
 	request = kzalloc(sizeof(struct regulatory_request), GFP_KERNEL);
@@ -2022,13 +2037,12 @@ static void print_dfs_region(u8 dfs_region)
 
 static void print_regdomain(const struct ieee80211_regdomain *rd)
 {
+	struct regulatory_request *lr = get_last_request();
 
 	if (is_intersected_alpha2(rd->alpha2)) {
-		if (last_request->initiator ==
-		    NL80211_REGDOM_SET_BY_COUNTRY_IE) {
+		if (lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 			struct cfg80211_registered_device *rdev;
-			rdev = cfg80211_rdev_by_wiphy_idx(
-				last_request->wiphy_idx);
+			rdev = cfg80211_rdev_by_wiphy_idx(lr->wiphy_idx);
 			if (rdev) {
 				pr_info("Current regulatory domain updated by AP to: %c%c\n",
 					rdev->country_ie_alpha2[0],
@@ -2043,7 +2057,7 @@ static void print_regdomain(const struct ieee80211_regdomain *rd)
 		if (is_unknown_alpha2(rd->alpha2))
 			pr_info("Regulatory domain changed to driver built-in settings (unknown country)\n");
 		else {
-			if (reg_request_cell_base(last_request))
+			if (reg_request_cell_base(lr))
 				pr_info("Regulatory domain changed to country: %c%c by Cell Station\n",
 					rd->alpha2[0], rd->alpha2[1]);
 			else
@@ -2068,11 +2082,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	const struct ieee80211_regdomain *regd;
 	const struct ieee80211_regdomain *intersected_rd = NULL;
 	struct wiphy *request_wiphy;
+	struct regulatory_request *lr = get_last_request();
 
 	/* Some basic sanity checks first */
 
-	assert_reg_lock();
-
 	if (!reg_is_valid_request(rd->alpha2))
 		return -EINVAL;
 
@@ -2090,7 +2103,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 	 * rd is non static (it means CRDA was present and was used last)
 	 * and the pending request came in from a country IE
 	 */
-	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
+	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 		/*
 		 * If someone else asked us to change the rd lets only bother
 		 * checking if the alpha2 changes if CRDA was already called
@@ -2112,16 +2125,16 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		return -EINVAL;
 	}
 
-	request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+	request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 	if (!request_wiphy &&
-	    (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
-	     last_request->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)) {
+	    (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER ||
+	     lr->initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE)) {
 		schedule_delayed_work(&reg_timeout, 0);
 		return -ENODEV;
 	}
 
-	if (!last_request->intersect) {
-		if (last_request->initiator != NL80211_REGDOM_SET_BY_DRIVER) {
+	if (!lr->intersect) {
+		if (lr->initiator != NL80211_REGDOM_SET_BY_DRIVER) {
 			reset_regdomains(false, rd);
 			return 0;
 		}
@@ -2151,7 +2164,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 
 	/* Intersection requires a bit more work */
 
-	if (last_request->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
+	if (lr->initiator != NL80211_REGDOM_SET_BY_COUNTRY_IE) {
 		intersected_rd = regdom_intersect(rd, get_cfg80211_regdom());
 		if (!intersected_rd)
 			return -EINVAL;
@@ -2161,7 +2174,7 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
 		 * However if a driver requested this specific regulatory
 		 * domain we keep it for its private use
 		 */
-		if (last_request->initiator == NL80211_REGDOM_SET_BY_DRIVER)
+		if (lr->initiator == NL80211_REGDOM_SET_BY_DRIVER)
 			rcu_assign_pointer(request_wiphy->regd, rd);
 		else
 			kfree(rd);
@@ -2184,9 +2197,11 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
  */
 int set_regdom(const struct ieee80211_regdomain *rd)
 {
+	struct regulatory_request *lr;
 	int r;
 
 	mutex_lock(&reg_mutex);
+	lr = get_last_request();
 
 	/* Note that this doesn't update the wiphys, this is done below */
 	r = __set_regdom(rd);
@@ -2199,18 +2214,17 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 	}
 
 	/* This would make this whole thing pointless */
-	if (WARN_ON(!last_request->intersect &&
-		    rd != get_cfg80211_regdom())) {
+	if (WARN_ON(!lr->intersect && rd != get_cfg80211_regdom())) {
 		r = -EINVAL;
 		goto out;
 	}
 
 	/* update all wiphys now with the new established regulatory domain */
-	update_all_wiphy_regulatory(last_request->initiator);
+	update_all_wiphy_regulatory(lr->initiator);
 
 	print_regdomain(get_cfg80211_regdom());
 
-	nl80211_send_reg_change_event(last_request);
+	nl80211_send_reg_change_event(lr);
 
 	reg_set_request_processed();
 
@@ -2223,10 +2237,11 @@ int set_regdom(const struct ieee80211_regdomain *rd)
 #ifdef CONFIG_HOTPLUG
 int reg_device_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
-	if (last_request && !last_request->processed) {
+	struct regulatory_request *lr = get_last_request();
+
+	if (lr && !lr->processed) {
 		if (add_uevent_var(env, "COUNTRY=%c%c",
-				   last_request->alpha2[0],
-				   last_request->alpha2[1]))
+				   lr->alpha2[0], lr->alpha2[1]))
 			return -ENOMEM;
 	}
 
@@ -2255,8 +2270,10 @@ void wiphy_regulatory_register(struct wiphy *wiphy)
 void wiphy_regulatory_deregister(struct wiphy *wiphy)
 {
 	struct wiphy *request_wiphy = NULL;
+	struct regulatory_request *lr;
 
 	mutex_lock(&reg_mutex);
+	lr = get_last_request();
 
 	if (!reg_dev_ignore_cell_hint(wiphy))
 		reg_num_devs_support_basehint--;
@@ -2264,14 +2281,14 @@ void wiphy_regulatory_deregister(struct wiphy *wiphy)
 	rcu_free_regdom(get_wiphy_regdom(wiphy));
 	rcu_assign_pointer(wiphy->regd, NULL);
 
-	if (last_request)
-		request_wiphy = wiphy_idx_to_wiphy(last_request->wiphy_idx);
+	if (lr)
+		request_wiphy = wiphy_idx_to_wiphy(lr->wiphy_idx);
 
 	if (!request_wiphy || request_wiphy != wiphy)
 		goto out;
 
-	last_request->wiphy_idx = WIPHY_IDX_INVALID;
-	last_request->country_ie_env = ENVIRON_ANY;
+	lr->wiphy_idx = WIPHY_IDX_INVALID;
+	lr->country_ie_env = ENVIRON_ANY;
 out:
 	mutex_unlock(&reg_mutex);
 }
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (22 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 23/24] regulatory: use RCU to protect last_request Johannes Berg
@ 2012-12-06 16:47 ` Johannes Berg
  2012-12-13 22:10   ` Luis R. Rodriguez
  2012-12-20 11:15 ` [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
  24 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-06 16:47 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg

From: Johannes Berg <johannes.berg@intel.com>

Instead of returning an error and filling a pointer
return the pointer and an ERR_PTR value in error cases.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/ath/regd.c                   | 16 ++++------
 drivers/net/wireless/brcm80211/brcmsmac/channel.c |  5 ++-
 drivers/net/wireless/rtlwifi/regd.c               | 14 ++++-----
 include/net/cfg80211.h                            | 18 +++++------
 net/wireless/reg.c                                | 38 ++++++++++-------------
 5 files changed, 40 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/ath/regd.c b/drivers/net/wireless/ath/regd.c
index 8ae58c40..7a6c79e 100644
--- a/drivers/net/wireless/ath/regd.c
+++ b/drivers/net/wireless/ath/regd.c
@@ -195,7 +195,6 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule;
 	struct ieee80211_channel *ch;
 	unsigned int i;
-	int r;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 
@@ -213,10 +212,8 @@ ath_reg_apply_beaconing_flags(struct wiphy *wiphy,
 				continue;
 
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-				r = freq_reg_info(wiphy,
-						  ch->center_freq,
-						  &reg_rule);
-				if (r)
+				reg_rule = freq_reg_info(wiphy, ch->center_freq);
+				if (IS_ERR(reg_rule))
 					continue;
 				/*
 				 * If 11d had a rule for this channel ensure
@@ -252,7 +249,6 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *ch;
 	const struct ieee80211_reg_rule *reg_rule;
-	int r;
 
 	sband = wiphy->bands[IEEE80211_BAND_2GHZ];
 	if (!sband)
@@ -280,16 +276,16 @@ ath_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	 */
 
 	ch = &sband->channels[11]; /* CH 12 */
-	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
-	if (!r) {
+	reg_rule = freq_reg_info(wiphy, ch->center_freq);
+	if (!IS_ERR(reg_rule)) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
 				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
 	}
 
 	ch = &sband->channels[12]; /* CH 13 */
-	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
-	if (!r) {
+	reg_rule = freq_reg_info(wiphy, ch->center_freq);
+	if (!IS_ERR(reg_rule)) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
 				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/channel.c b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
index f754efa..e6f6d92 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/channel.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/channel.c
@@ -686,9 +686,8 @@ brcms_reg_apply_beaconing_flags(struct wiphy *wiphy,
 				continue;
 
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-				ret = freq_reg_info(wiphy, ch->center_freq,
-						    &rule);
-				if (ret)
+				rule = freq_reg_info(wiphy, ch->center_freq);
+				if (IS_ERR(rule))
 					continue;
 
 				if (!(rule->flags & NL80211_RRF_NO_IBSS))
diff --git a/drivers/net/wireless/rtlwifi/regd.c b/drivers/net/wireless/rtlwifi/regd.c
index be55dc9..7e3ead7 100644
--- a/drivers/net/wireless/rtlwifi/regd.c
+++ b/drivers/net/wireless/rtlwifi/regd.c
@@ -158,7 +158,6 @@ static void _rtl_reg_apply_beaconing_flags(struct wiphy *wiphy,
 	const struct ieee80211_reg_rule *reg_rule;
 	struct ieee80211_channel *ch;
 	unsigned int i;
-	int r;
 
 	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
 
@@ -173,8 +172,8 @@ static void _rtl_reg_apply_beaconing_flags(struct wiphy *wiphy,
 			    (ch->flags & IEEE80211_CHAN_RADAR))
 				continue;
 			if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE) {
-				r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
-				if (r)
+				reg_rule = freq_reg_info(wiphy, ch->center_freq);
+				if (IS_ERR(reg_rule))
 					continue;
 
 				/*
@@ -209,7 +208,6 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	struct ieee80211_supported_band *sband;
 	struct ieee80211_channel *ch;
 	const struct ieee80211_reg_rule *reg_rule;
-	int r;
 
 	if (!wiphy->bands[IEEE80211_BAND_2GHZ])
 		return;
@@ -237,16 +235,16 @@ static void _rtl_reg_apply_active_scan_flags(struct wiphy *wiphy,
 	 */
 
 	ch = &sband->channels[11];	/* CH 12 */
-	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
-	if (!r) {
+	reg_rule = freq_reg_info(wiphy, ch->center_freq);
+	if (!IS_ERR(reg_rule)) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
 				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
 	}
 
 	ch = &sband->channels[12];	/* CH 13 */
-	r = freq_reg_info(wiphy, ch->center_freq, &reg_rule);
-	if (!r) {
+	reg_rule = freq_reg_info(wiphy, ch->center_freq);
+	if (!IS_ERR(reg_rule)) {
 		if (!(reg_rule->flags & NL80211_RRF_PASSIVE_SCAN))
 			if (ch->flags & IEEE80211_CHAN_PASSIVE_SCAN)
 				ch->flags &= ~IEEE80211_CHAN_PASSIVE_SCAN;
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 461d979..acbf5e0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2938,22 +2938,22 @@ extern void wiphy_apply_custom_regulatory(
  * freq_reg_info - get regulatory information for the given frequency
  * @wiphy: the wiphy for which we want to process this rule for
  * @center_freq: Frequency in KHz for which we want regulatory information for
- * @reg_rule: the regulatory rule which we have for this frequency
  *
  * Use this function to get the regulatory rule for a specific frequency on
  * a given wireless device. If the device has a specific regulatory domain
  * it wants to follow we respect that unless a country IE has been received
  * and processed already.
  *
- * Returns 0 if it was able to find a valid regulatory rule which does
- * apply to the given center_freq otherwise it returns non-zero. It will
- * also return -ERANGE if we determine the given center_freq does not even have
- * a regulatory rule for a frequency range in the center_freq's band. See
- * freq_in_rule_band() for our current definition of a band -- this is purely
- * subjective and right now its 802.11 specific.
+ * When an error occurs, for example if no rule can be found, the return value
+ * is encoded using ERR_PTR(). Use IS_ERR() to check and PTR_ERR() to obtain
+ * the numeric return value. The numeric return value will be -ERANGE if we
+ * determine the given center_freq does not even have a regulatory rule for a
+ * frequency range in the center_freq's band. See freq_in_rule_band() for our
+ * current definition of a band -- this is purely subjective and right now it's
+ * 802.11 specific.
  */
-extern int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
-			 const struct ieee80211_reg_rule **reg_rule);
+const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
+					       u32 center_freq);
 
 /*
  * callbacks for asynchronous cfg80211 methods, notification
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index b289131..041f22c 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -711,16 +711,16 @@ static u32 map_regdom_flags(u32 rd_flags)
 	return channel_flags;
 }
 
-static int freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
-			      const struct ieee80211_reg_rule **reg_rule,
-			      const struct ieee80211_regdomain *regd)
+static const struct ieee80211_reg_rule *
+freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
+		   const struct ieee80211_regdomain *regd)
 {
 	int i;
 	bool band_rule_found = false;
 	bool bw_fits = false;
 
 	if (!regd)
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 
 	for (i = 0; i < regd->n_reg_rules; i++) {
 		const struct ieee80211_reg_rule *rr;
@@ -739,20 +739,18 @@ static int freq_reg_info_regd(struct wiphy *wiphy, u32 center_freq,
 
 		bw_fits = reg_does_bw_fit(fr, center_freq, MHZ_TO_KHZ(20));
 
-		if (band_rule_found && bw_fits) {
-			*reg_rule = rr;
-			return 0;
-		}
+		if (band_rule_found && bw_fits)
+			return rr;
 	}
 
 	if (!band_rule_found)
-		return -ERANGE;
+		return ERR_PTR(-ERANGE);
 
-	return -EINVAL;
+	return ERR_PTR(-EINVAL);
 }
 
-int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
-		  const struct ieee80211_reg_rule **reg_rule)
+const struct ieee80211_reg_rule *freq_reg_info(struct wiphy *wiphy,
+					       u32 center_freq)
 {
 	const struct ieee80211_regdomain *regd;
 	struct regulatory_request *lr = get_last_request();
@@ -768,7 +766,7 @@ int freq_reg_info(struct wiphy *wiphy, u32 center_freq,
 	else
 		regd = get_cfg80211_regdom();
 
-	return freq_reg_info_regd(wiphy, center_freq, reg_rule, regd);
+	return freq_reg_info_regd(wiphy, center_freq, regd);
 }
 EXPORT_SYMBOL(freq_reg_info);
 
@@ -830,7 +828,6 @@ static void handle_channel(struct wiphy *wiphy,
 			   enum nl80211_reg_initiator initiator,
 			   struct ieee80211_channel *chan)
 {
-	int r;
 	u32 flags, bw_flags = 0;
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
@@ -842,8 +839,8 @@ static void handle_channel(struct wiphy *wiphy,
 
 	flags = chan->orig_flags;
 
-	r = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq), &reg_rule);
-	if (r) {
+	reg_rule = freq_reg_info(wiphy, MHZ_TO_KHZ(chan->center_freq));
+	if (IS_ERR(reg_rule)) {
 		/*
 		 * We will disable all channels that do not match our
 		 * received regulatory rule unless the hint is coming
@@ -855,7 +852,7 @@ static void handle_channel(struct wiphy *wiphy,
 		 * while 5 GHz is still supported.
 		 */
 		if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE &&
-		    r == -ERANGE)
+		    PTR_ERR(reg_rule) == -ERANGE)
 			return;
 
 		REG_DBG_PRINT("Disabling freq %d MHz\n", chan->center_freq);
@@ -1238,16 +1235,15 @@ static void handle_channel_custom(struct wiphy *wiphy,
 				  struct ieee80211_channel *chan,
 				  const struct ieee80211_regdomain *regd)
 {
-	int r;
 	u32 bw_flags = 0;
 	const struct ieee80211_reg_rule *reg_rule = NULL;
 	const struct ieee80211_power_rule *power_rule = NULL;
 	const struct ieee80211_freq_range *freq_range = NULL;
 
-	r = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
-			       &reg_rule, regd);
+	reg_rule = freq_reg_info_regd(wiphy, MHZ_TO_KHZ(chan->center_freq),
+				      regd);
 
-	if (r) {
+	if (IS_ERR(reg_rule)) {
 		REG_DBG_PRINT("Disabling freq %d MHz as custom regd has no rule that fits it\n",
 			      chan->center_freq);
 		chan->flags = IEEE80211_CHAN_DISABLED;
-- 
1.8.0


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/24] regulatory: don't write past array when intersecting rules
  2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
@ 2012-12-06 23:43   ` Luis R. Rodriguez
  2012-12-07  7:53     ` Johannes Berg
  2012-12-10 21:55   ` Johannes Berg
  1 sibling, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-06 23:43 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> When intersecting rules, we count first to know how many
> rules need to be allocated, and then do the intersection
> into the allocated array. However, the code doing this
> writes past the end of the array because it attempts to
> do all intersections. Make it stop when the right number
> of rules has been reached.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

A comment below though.

> ---
>  net/wireless/reg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index b6c7ea6..4197359 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect(
>         if (!rd)
>                 return NULL;
>
> -       for (x = 0; x < rd1->n_reg_rules; x++) {
> +       for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) {
>                 rule1 = &rd1->reg_rules[x];
> -               for (y = 0; y < rd2->n_reg_rules; y++) {
> +               for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) {
>                         rule2 = &rd2->reg_rules[y];
>                         /*

Does rule_idx ever become > num_rules though? The check that builds
num_rules are the same as we traverse and increment rule_idx.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 02/24] regulatory: don't allocate too much memory
  2012-12-06 16:47 ` [PATCH 02/24] regulatory: don't allocate too much memory Johannes Berg
@ 2012-12-06 23:47   ` Luis R. Rodriguez
  2012-12-07  7:54     ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-06 23:47 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> There's no need to allocate one reg rule more
> than will be used, reduce the allocations. The
> allocation in nl80211 already doesn't allocate
> too much space.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

Just for the record without this I ran into odd issues that I could
not root cause but glad you have verified this should be a non-issue.
I'll ACK-it provided we let it rip and find any issues during in the
development cycle.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 03/24] regulatory: clean up regdom_intersect
  2012-12-06 16:47 ` [PATCH 03/24] regulatory: clean up regdom_intersect Johannes Berg
@ 2012-12-06 23:55   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-06 23:55 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> As the dummy_rule (also renamed from irule) is only
> used for output by the reg_rules_intersect() function
> there's no need to clear it at all, remove that.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

The removal of the pointer is fine but the removal of the memset() is
not trivial but given that reg_rules_intersect() does indeed re-set
all elements of the struct this works OK. If reg_rules_intersect()
ever branches though we may need to re-add a memset() back, but its
fine like this. The issue I was trying to avoid was if you do not
memset you potentially piggy backed data set from the previous
intersect and this could potentially reduce the number of possible
valid rules.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 04/24] regulatory: clean up reg_copy_regd()
  2012-12-06 16:47 ` [PATCH 04/24] regulatory: clean up reg_copy_regd() Johannes Berg
@ 2012-12-06 23:59   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-06 23:59 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Use ERR_PTR/IS_ERR to return the result or errors,
> also do some code cleanups.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 05/24] regulatory: don't test list before iterating
  2012-12-06 16:47 ` [PATCH 05/24] regulatory: don't test list before iterating Johannes Berg
@ 2012-12-07  0:02   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:02 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> There's no need to test whether a list is
> empty or not before iterating.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

I had also ran into issues without the checks, but glad you have
verified things are kosher.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 06/24] regulatory: simplify regulatory_hint_11d
  2012-12-06 16:47 ` [PATCH 06/24] regulatory: simplify regulatory_hint_11d Johannes Berg
@ 2012-12-07  0:10   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:10 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> There's no need to unlock before calling
> queue_regulatory_request(), so simplify
> the function.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

Sure, I was just trying to be consistent with the rest of the context
of the other queue_regulatory_request() calls. Either way.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 07/24] regulatory: code cleanup
  2012-12-06 16:47 ` [PATCH 07/24] regulatory: code cleanup Johannes Berg
@ 2012-12-07  0:11   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:11 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Clean up various things like indentation, extra
> parentheses, too many/few line breaks, etc.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 08/24] regulatory: remove useless locking on exit
  2012-12-06 16:47 ` [PATCH 08/24] regulatory: remove useless locking on exit Johannes Berg
@ 2012-12-07  0:16   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:16 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> It would be a major problem if anything were to run
> concurrently while the module is being unloaded so
> remove the locking that doesn't help anything.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

This seems to work well because we do the cancel_work_sync() and
cancel_delayed_work_sync() before, so sure, and given you know the
core better I'm for it.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 09/24] regulatory: use proper enum for return values
  2012-12-06 16:47 ` [PATCH 09/24] regulatory: use proper enum for return values Johannes Berg
@ 2012-12-07  0:20   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:20 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Instead of treating special error codes specially,
> like -EALREADY, introduce a real enum for all the
> needed possibilities and use it.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 10/24] cfg80211: remove wiphy_idx_valid
  2012-12-06 16:47 ` [PATCH 10/24] cfg80211: remove wiphy_idx_valid Johannes Berg
@ 2012-12-07  0:34   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> This is pretty much useless since get_wiphy_idx()
> always returns true since it's always called with
> a valid wiphy pointer.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

This seems more like a rename of the stale -> invalid and unwrapping
the check into code in place, and then removal of some checks you are
sure are unnecessary.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 11/24] regulatory: remove BUG_ON
  2012-12-06 16:47 ` [PATCH 11/24] regulatory: remove BUG_ON Johannes Berg
@ 2012-12-07  0:39   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:39 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> This code is a bit too BUG_ON happy, remove all
> instances and while doing so make some code a bit
> smarter by passing the right pointer instead of
> indices into arrays.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 12/24] regulatory: simplify restore_regulatory_settings
  2012-12-06 16:47 ` [PATCH 12/24] regulatory: simplify restore_regulatory_settings Johannes Berg
@ 2012-12-07  0:53   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:53 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Use list_splice_tail_init() and also simplify the locking.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

Super sexy.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 13/24] regulatory: remove redundant isalpha() check
  2012-12-06 16:47 ` [PATCH 13/24] regulatory: remove redundant isalpha() check Johannes Berg
@ 2012-12-07  0:54   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> toupper() only modifies lower-case letters, so
> the isalpha() check is redundant; remove it.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 14/24] regulatory: remove useless warning
  2012-12-06 16:47 ` [PATCH 14/24] regulatory: remove useless warning Johannes Berg
@ 2012-12-07  0:55   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  0:55 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Even if it never happens and is hidden behind the
> debug config option, it's completely useless: the
> calltrace will only show module loading.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 15/24] regulatory: simplify freq_reg_info_regd
  2012-12-06 16:47 ` [PATCH 15/24] regulatory: simplify freq_reg_info_regd Johannes Berg
@ 2012-12-07  1:02   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07  1:02 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The function itself has dual-purpose: it can
> retrieve from a given regdomain or from the
> globally installed one. Change it to have a
> single purpose only: to look up from a given
> regdomain. Pass the correct regdomain in the
> freq_reg_info() function instead.
>
> This also changes the locking rules for it,
> no locking is required any more.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/24] regulatory: don't write past array when intersecting rules
  2012-12-06 23:43   ` Luis R. Rodriguez
@ 2012-12-07  7:53     ` Johannes Berg
  0 siblings, 0 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-07  7:53 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Thu, 2012-12-06 at 15:43 -0800, Luis R. Rodriguez wrote:

> > diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > index b6c7ea6..4197359 100644
> > --- a/net/wireless/reg.c
> > +++ b/net/wireless/reg.c
> > @@ -648,9 +648,9 @@ static struct ieee80211_regdomain *regdom_intersect(
> >         if (!rd)
> >                 return NULL;
> >
> > -       for (x = 0; x < rd1->n_reg_rules; x++) {
> > +       for (x = 0; x < rd1->n_reg_rules && rule_idx < num_rules; x++) {
> >                 rule1 = &rd1->reg_rules[x];
> > -               for (y = 0; y < rd2->n_reg_rules; y++) {
> > +               for (y = 0; y < rd2->n_reg_rules && rule_idx < num_rules; y++) {
> >                         rule2 = &rd2->reg_rules[y];
> >                         /*
> 
> Does rule_idx ever become > num_rules though? The check that builds
> num_rules are the same as we traverse and increment rule_idx.

It doesn't become great, but it becomes equal. Say you have the
following rules:

rd1: 1000-2000, 3000-4000
rd2: 1000-1500, 5000-6000

The result will be 1000-1500, so 1 rule. But while iterating, that's the
very first thing, so rule_idx becomes 1 after the first iteration of the
inner/outer loops, and then without the fix we still check 1000-2000 vs.
5000-6000, 3000-4000 vs. 1000-1500 and finally 3000-4000 vs. 5000-6000
and rule_idx is 1 all the time while checking that so we write past the
array ...

This makes it stop when it knows it has found the right number of rules.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 02/24] regulatory: don't allocate too much memory
  2012-12-06 23:47   ` Luis R. Rodriguez
@ 2012-12-07  7:54     ` Johannes Berg
  0 siblings, 0 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-07  7:54 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Thu, 2012-12-06 at 15:47 -0800, Luis R. Rodriguez wrote:

> Just for the record without this I ran into odd issues that I could
> not root cause but glad you have verified this should be a non-issue.
> I'll ACK-it provided we let it rip and find any issues during in the
> development cycle.

I'm pretty that was the same issue I just fixed in patch 1 :-)

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 16/24] regulatory: clarify locking rules and assertions
  2012-12-06 16:47 ` [PATCH 16/24] regulatory: clarify locking rules and assertions Johannes Berg
@ 2012-12-07 23:11   ` Luis R. Rodriguez
  2012-12-07 23:16     ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-07 23:11 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Many places that currently check that cfg80211_mutex
> is held don't actually use any data protected by it.
> The functions that need to hold the cfg80211_mutex
> are the ones using the cfg80211_regdomain variable,
> so add the lock assertion to those and clarify this
> in the comments.
>
> The reason for this is that nl80211 uses the regdom
> without being able to hold reg_mutex.

Hm, do we want to do this or add a caller to reg.c that gets nl80211
to copy over the current regdomain over for it to parse and then send?
In that case we'd only need the cfg80211_mutex for
update_all_wiphy_regulatory(). Thoughts?

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 16/24] regulatory: clarify locking rules and assertions
  2012-12-07 23:11   ` Luis R. Rodriguez
@ 2012-12-07 23:16     ` Johannes Berg
  2012-12-13 20:57       ` Luis R. Rodriguez
  0 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-07 23:16 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Fri, 2012-12-07 at 15:11 -0800, Luis R. Rodriguez wrote:

> > The reason for this is that nl80211 uses the regdom
> > without being able to hold reg_mutex.
> 
> Hm, do we want to do this or add a caller to reg.c that gets nl80211
> to copy over the current regdomain over for it to parse and then send?
> In that case we'd only need the cfg80211_mutex for
> update_all_wiphy_regulatory(). Thoughts?

I think you should look at the later patches :)

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains
  2012-12-06 16:47 ` [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains Johannes Berg
@ 2012-12-10 17:05   ` Johannes Berg
  2012-12-13 21:54     ` Luis R. Rodriguez
  0 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-10 17:05 UTC (permalink / raw)
  To: linux-wireless

On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote:

> -	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2,
> -			   cfg80211_regdomain->alpha2) ||
> -	    (cfg80211_regdomain->dfs_region &&
> -	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION,
> -			cfg80211_regdomain->dfs_region)))
> -		goto nla_put_failure;
> +	rcu_read_lock();
> +	regdom = rcu_dereference(cfg80211_regdomain);
> +
> +	if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) ||
> +	    (regdom->dfs_region &&
> +	     nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region)))
> +		goto nla_put_failure_rcu;
>  
>  	if (reg_last_request_cell_base() &&

This causes a locking issue -- mutex is taken in
reg_last_request_cell_base(), so it needs to be before the
rcu_read_lock(). I fixed the version in my mac80211-next.git tree's wip
branch.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/24] regulatory: don't write past array when intersecting rules
  2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
  2012-12-06 23:43   ` Luis R. Rodriguez
@ 2012-12-10 21:55   ` Johannes Berg
  2012-12-12  1:08     ` Luis R. Rodriguez
  1 sibling, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-10 21:55 UTC (permalink / raw)
  To: linux-wireless

On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> When intersecting rules, we count first to know how many
> rules need to be allocated, and then do the intersection
> into the allocated array. However, the code doing this
> writes past the end of the array because it attempts to
> do all intersections. Make it stop when the right number
> of rules has been reached.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

FWIW, since we currently allocate enough memory here to actually write
past the end of the intended array, I've decided to remove the stable
tag. It doesn't really fix anything -- with the next patch it fixes the
allocation to not be too large, but that doesn't really need to go to
stable.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 01/24] regulatory: don't write past array when intersecting rules
  2012-12-10 21:55   ` Johannes Berg
@ 2012-12-12  1:08     ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-12  1:08 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Mon, Dec 10, 2012 at 1:55 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote:
>> From: Johannes Berg <johannes.berg@intel.com>
>>
>> When intersecting rules, we count first to know how many
>> rules need to be allocated, and then do the intersection
>> into the allocated array. However, the code doing this
>> writes past the end of the array because it attempts to
>> do all intersections. Make it stop when the right number
>> of rules has been reached.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> FWIW, since we currently allocate enough memory here to actually write
> past the end of the intended array, I've decided to remove the stable
> tag. It doesn't really fix anything -- with the next patch it fixes the
> allocation to not be too large, but that doesn't really need to go to
> stable.

That was likely the issue I ran into that caused things to burp
without the +1 ;)

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 16/24] regulatory: clarify locking rules and assertions
  2012-12-07 23:16     ` Johannes Berg
@ 2012-12-13 20:57       ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 20:57 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Fri, Dec 7, 2012 at 3:16 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2012-12-07 at 15:11 -0800, Luis R. Rodriguez wrote:
>
>> > The reason for this is that nl80211 uses the regdom
>> > without being able to hold reg_mutex.
>>
>> Hm, do we want to do this or add a caller to reg.c that gets nl80211
>> to copy over the current regdomain over for it to parse and then send?
>> In that case we'd only need the cfg80211_mutex for
>> update_all_wiphy_regulatory(). Thoughts?
>
> I think you should look at the later patches :)

I'm on it now.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 18/24] regulatory: fix reg_is_valid_request handling
  2012-12-06 16:47 ` [PATCH 18/24] regulatory: fix reg_is_valid_request handling Johannes Berg
@ 2012-12-13 21:27   ` Luis R. Rodriguez
  2012-12-13 21:35     ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 21:27 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> There's a bug with the world regulatory domain, it
> can be updated any time which is different from all
> other regdomains that can only be updated once after
> a request for them. Fix this by adding a check for
> "processed" to the reg_is_valid_request() function
> and clear that when doing a request.
>
> While looking at this I also found another locking
> bug, last_request is protected by the reg_mutex not
> the cfg80211_mutex so the code in nl80211 is racy.
> Remove that code as it only tries to prevent an
> allocation in an error case, which isn't necessary.
> Then the function can also become static and locking
> in nl80211 can have a smaller scope.
>
> Also change __set_regdom() to do the checks earlier
> and not different for world/other regdomains.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/wireless/nl80211.c |  9 ++-------
>  net/wireless/reg.c     | 21 +++++++++++----------
>  net/wireless/reg.h     |  1 -
>  3 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 260b5d8..370cd31 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -4333,6 +4326,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
>                 }
>         }
>
> +       mutex_lock(&cfg80211_mutex);
> +
>         r = set_regdom(rd);
>         rd = NULL;

Unless you removed it from previous patches in the series the
"bad_reg" goto label has an unlock but above you removed the top
locking call so I think you just forgot to remove the unlock on the
goto label.

> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index dfab71b..10d8edb 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -426,12 +426,16 @@ static int call_crda(const char *alpha2)
>         return kobject_uevent(&reg_pdev->dev.kobj, KOBJ_CHANGE);
>  }
>
> -/* Used by nl80211 before kmalloc'ing our regulatory domain */
> -bool reg_is_valid_request(const char *alpha2)
> +static bool reg_is_valid_request(const char *alpha2)
>  {
> +       assert_reg_lock();
> +
>         if (!last_request)
>                 return false;
>
> +       if (last_request->processed)
> +               return false;
> +
>         return alpha2_equal(last_request->alpha2, alpha2);
>  }
>
> @@ -1471,6 +1475,7 @@ new_request:
>
>         last_request = pending_request;
>         last_request->intersect = intersect;
> +       last_request->processed = false;

The clearing you are doing is on the last_request but it comes from
the pending regulatory request which is always false given that the
request is kzalloc'd, not sure if that was needed or how that helped.

Apart from that looks good.

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 18/24] regulatory: fix reg_is_valid_request handling
  2012-12-13 21:27   ` Luis R. Rodriguez
@ 2012-12-13 21:35     ` Johannes Berg
  2012-12-13 22:00       ` Luis R. Rodriguez
  0 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-13 21:35 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Thu, 2012-12-13 at 13:27 -0800, Luis R. Rodriguez wrote:

> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 260b5d8..370cd31 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -4333,6 +4326,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
> >                 }
> >         }
> >
> > +       mutex_lock(&cfg80211_mutex);
> > +
> >         r = set_regdom(rd);
> >         rd = NULL;
> 
> Unless you removed it from previous patches in the series the
> "bad_reg" goto label has an unlock but above you removed the top
> locking call so I think you just forgot to remove the unlock on the
> goto label.

Good catch. Wonder why smatch didn't complain? Fixed.


> > @@ -1471,6 +1475,7 @@ new_request:

        if (last_request != &core_request_world)
                kfree(last_request);

> >
> >         last_request = pending_request;
> >         last_request->intersect = intersect;
> > +       last_request->processed = false;
> 
> The clearing you are doing is on the last_request but it comes from
> the pending regulatory request which is always false given that the
> request is kzalloc'd, not sure if that was needed or how that helped.

Not necessarily, I added a bit more context above -- if it's
core_request_world the clearing is needed I believe.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 20/24] regulatory: fix memory leak
  2012-12-06 16:47 ` [PATCH 20/24] regulatory: fix memory leak Johannes Berg
@ 2012-12-13 21:35   ` Luis R. Rodriguez
  2012-12-13 21:37     ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 21:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> If duplicating here fails, we leak the original.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/wireless/reg.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 011fab9..5a34830 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -2120,8 +2120,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
>                         return -EALREADY;
>
>                 regd = reg_copy_regd(rd);
> -               if (IS_ERR(regd))
> +               if (IS_ERR(regd)) {
> +                       kfree(rd);
>                         return PTR_ERR(regd);
> +               }

The caller, set_regdom() free's t for us if we fail though so I don't
see how this was leaked.

int set_regdom(const struct ieee80211_regdomain *rd)
{
        int r;

        assert_cfg80211_lock();

        mutex_lock(&reg_mutex);

        /* Note that this doesn't update the wiphys, this is done below */
        r = __set_regdom(rd);
        if (r) {
                if (r == -EALREADY)
                        reg_set_request_processed();

                kfree(rd);
                mutex_unlock(&reg_mutex);
                return r;
        }
...
}

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 20/24] regulatory: fix memory leak
  2012-12-13 21:35   ` Luis R. Rodriguez
@ 2012-12-13 21:37     ` Johannes Berg
  0 siblings, 0 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-13 21:37 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Thu, 2012-12-13 at 13:35 -0800, Luis R. Rodriguez wrote:
> On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > If duplicating here fails, we leak the original.
> >
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > ---
> >  net/wireless/reg.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> > index 011fab9..5a34830 100644
> > --- a/net/wireless/reg.c
> > +++ b/net/wireless/reg.c
> > @@ -2120,8 +2120,10 @@ static int __set_regdom(const struct ieee80211_regdomain *rd)
> >                         return -EALREADY;
> >
> >                 regd = reg_copy_regd(rd);
> > -               if (IS_ERR(regd))
> > +               if (IS_ERR(regd)) {
> > +                       kfree(rd);
> >                         return PTR_ERR(regd);
> > +               }
> 
> The caller, set_regdom() free's t for us if we fail though so I don't
> see how this was leaked.

Oops, so this patch is wrong, I'll drop it.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 21/24] regulatory: pass new regdomain to reset function
  2012-12-06 16:47 ` [PATCH 21/24] regulatory: pass new regdomain to reset function Johannes Berg
@ 2012-12-13 21:42   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 21:42 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Instead of assigning after calling the function do
> it inside the function. This will later avoid a
> period of time where the pointer is NULL.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains
  2012-12-10 17:05   ` Johannes Berg
@ 2012-12-13 21:54     ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 21:54 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Mon, Dec 10, 2012 at 9:05 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote:
>
>> -     if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2,
>> -                        cfg80211_regdomain->alpha2) ||
>> -         (cfg80211_regdomain->dfs_region &&
>> -          nla_put_u8(msg, NL80211_ATTR_DFS_REGION,
>> -                     cfg80211_regdomain->dfs_region)))
>> -             goto nla_put_failure;
>> +     rcu_read_lock();
>> +     regdom = rcu_dereference(cfg80211_regdomain);
>> +
>> +     if (nla_put_string(msg, NL80211_ATTR_REG_ALPHA2, regdom->alpha2) ||
>> +         (regdom->dfs_region &&
>> +          nla_put_u8(msg, NL80211_ATTR_DFS_REGION, regdom->dfs_region)))
>> +             goto nla_put_failure_rcu;
>>
>>       if (reg_last_request_cell_base() &&
>
> This causes a locking issue -- mutex is taken in
> reg_last_request_cell_base(), so it needs to be before the
> rcu_read_lock(). I fixed the version in my mac80211-next.git tree's wip
> branch.

Nice!

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory
  2012-12-06 16:47 ` [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory Johannes Berg
@ 2012-12-13 21:56   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 21:56 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> wiphy_apply_custom_regulatory() doesn't have to hold
> the regulatory mutex as it only modifies the given
> wiphy with the given regulatory domain, it doesn't
> access any global regulatory data.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 18/24] regulatory: fix reg_is_valid_request handling
  2012-12-13 21:35     ` Johannes Berg
@ 2012-12-13 22:00       ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 22:00 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless

On Thu, Dec 13, 2012 at 1:35 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Thu, 2012-12-13 at 13:27 -0800, Luis R. Rodriguez wrote:
>
>> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> > index 260b5d8..370cd31 100644
>> > --- a/net/wireless/nl80211.c
>> > +++ b/net/wireless/nl80211.c
>> > @@ -4333,6 +4326,8 @@ static int nl80211_set_reg(struct sk_buff *skb, struct genl_info *info)
>> >                 }
>> >         }
>> >
>> > +       mutex_lock(&cfg80211_mutex);
>> > +
>> >         r = set_regdom(rd);
>> >         rd = NULL;
>>
>> Unless you removed it from previous patches in the series the
>> "bad_reg" goto label has an unlock but above you removed the top
>> locking call so I think you just forgot to remove the unlock on the
>> goto label.
>
> Good catch. Wonder why smatch didn't complain? Fixed.


>> > @@ -1471,6 +1475,7 @@ new_request:
>
>         if (last_request != &core_request_world)
>                 kfree(last_request);
>
>> >
>> >         last_request = pending_request;
>> >         last_request->intersect = intersect;
>> > +       last_request->processed = false;
>>
>> The clearing you are doing is on the last_request but it comes from
>> the pending regulatory request which is always false given that the
>> request is kzalloc'd, not sure if that was needed or how that helped.
>
> Not necessarily, I added a bit more context above -- if it's
> core_request_world the clearing is needed I believe.

Good catch yes, upon reset only if we actually run the core request
upon reset and I do think we do. Yeap:

static void reset_regdomains(bool full_reset)
...
     last_request = &core_request_world;
}

With your fix in place:

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 23/24] regulatory: use RCU to protect last_request
  2012-12-06 16:47 ` [PATCH 23/24] regulatory: use RCU to protect last_request Johannes Berg
@ 2012-12-13 22:07   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 22:07 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> This will allow making freq_reg_info() lock-free.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

Neat!

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info
  2012-12-06 16:47 ` [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info Johannes Berg
@ 2012-12-13 22:10   ` Luis R. Rodriguez
  0 siblings, 0 replies; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 22:10 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Instead of returning an error and filling a pointer
> return the pointer and an ERR_PTR value in error cases.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 19/24] regulatory: remove handling of channel bandwidth
  2012-12-06 16:47 ` [PATCH 19/24] regulatory: remove handling of channel bandwidth Johannes Berg
@ 2012-12-13 22:11   ` Luis R. Rodriguez
  2012-12-14 10:35     ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Luis R. Rodriguez @ 2012-12-13 22:11 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Johannes Berg

On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The channel bandwidth handling isn't really quite right,
> it assumes that a 40 MHz channel is really two 20 MHz
> channels, which isn't strictly true. This is the way the
> regulatory database handling is defined right now though
> so remove the logic to handle other channel widths.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

I didn't see the replacement work in place but I assume its coming
through RFCs or already posted so:

Acked-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>

I'm done reviewing this series!

  Luis

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 19/24] regulatory: remove handling of channel bandwidth
  2012-12-13 22:11   ` Luis R. Rodriguez
@ 2012-12-14 10:35     ` Johannes Berg
  2012-12-14 10:36       ` Johannes Berg
  0 siblings, 1 reply; 62+ messages in thread
From: Johannes Berg @ 2012-12-14 10:35 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Thu, 2012-12-13 at 14:11 -0800, Luis R. Rodriguez wrote:
> On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > The channel bandwidth handling isn't really quite right,
> > it assumes that a 40 MHz channel is really two 20 MHz
> > channels, which isn't strictly true. This is the way the
> > regulatory database handling is defined right now though
> > so remove the logic to handle other channel widths.

> I didn't see the replacement work in place but I assume its coming
> through RFCs or already posted so:

I'm not planning to replace this (dead) code, at least not right now.

All current users of the function pass 0, which assumes 20 MHz, so
basically all I'm doing is remove this unused argument and code
associated with it.

Now, due to the way the regulatory database works, the argument was
never actually useful unless somebody wanted to support 5/10 MHz
channels and had a regulatory domain that only allowed those (and didn't
allow 20 MHz!)

For wider bandwidths, the way the regulatory db is currently defined (by
the way it's always been handled) is that all the secondary channels
must exist and allow the bandwidth, but that's a little different so I
also don't see any way this argument would currently be useful.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 19/24] regulatory: remove handling of channel bandwidth
  2012-12-14 10:35     ` Johannes Berg
@ 2012-12-14 10:36       ` Johannes Berg
  0 siblings, 0 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-14 10:36 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: linux-wireless

On Fri, 2012-12-14 at 11:35 +0100, Johannes Berg wrote:
> On Thu, 2012-12-13 at 14:11 -0800, Luis R. Rodriguez wrote:
> > On Thu, Dec 6, 2012 at 8:47 AM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > > From: Johannes Berg <johannes.berg@intel.com>
> > >
> > > The channel bandwidth handling isn't really quite right,
> > > it assumes that a 40 MHz channel is really two 20 MHz
> > > channels, which isn't strictly true. This is the way the
> > > regulatory database handling is defined right now though
> > > so remove the logic to handle other channel widths.
> 
> > I didn't see the replacement work in place but I assume its coming
> > through RFCs or already posted so:
> 
> I'm not planning to replace this (dead) code, at least not right now.
> 
> All current users of the function pass 0, which assumes 20 MHz, so
> basically all I'm doing is remove this unused argument and code
> associated with it.
> 
> Now, due to the way the regulatory database works, the argument was
> never actually useful unless somebody wanted to support 5/10 MHz
> channels and had a regulatory domain that only allowed those (and didn't
> allow 20 MHz!)
> 
> For wider bandwidths, the way the regulatory db is currently defined (by
> the way it's always been handled) is that all the secondary channels
> must exist and allow the bandwidth, but that's a little different so I
> also don't see any way this argument would currently be useful.

And "currently" really means "until an entirely new regulatory framework
with different rule interpretation is defined".

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 00/24] regulatory fixes, cleanups & improvements
  2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
                   ` (23 preceding siblings ...)
  2012-12-06 16:47 ` [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info Johannes Berg
@ 2012-12-20 11:15 ` Johannes Berg
  24 siblings, 0 replies; 62+ messages in thread
From: Johannes Berg @ 2012-12-20 11:15 UTC (permalink / raw)
  To: linux-wireless

On Thu, 2012-12-06 at 17:47 +0100, Johannes Berg wrote:
> The biggest improvement is using RCU, this will allow checking
> against the regdomain from any context and makes the wiphy's
> regd pointer safe to use for drivers. It also simplifies the
> locking since cfg80211_mutex isn't needed any more for those
> things, only for wdev iteration/access. 

Applied, except for patch 20.

johannes


^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2012-12-20 11:15 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-06 16:47 [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg
2012-12-06 16:47 ` [PATCH 01/24] regulatory: don't write past array when intersecting rules Johannes Berg
2012-12-06 23:43   ` Luis R. Rodriguez
2012-12-07  7:53     ` Johannes Berg
2012-12-10 21:55   ` Johannes Berg
2012-12-12  1:08     ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 02/24] regulatory: don't allocate too much memory Johannes Berg
2012-12-06 23:47   ` Luis R. Rodriguez
2012-12-07  7:54     ` Johannes Berg
2012-12-06 16:47 ` [PATCH 03/24] regulatory: clean up regdom_intersect Johannes Berg
2012-12-06 23:55   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 04/24] regulatory: clean up reg_copy_regd() Johannes Berg
2012-12-06 23:59   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 05/24] regulatory: don't test list before iterating Johannes Berg
2012-12-07  0:02   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 06/24] regulatory: simplify regulatory_hint_11d Johannes Berg
2012-12-07  0:10   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 07/24] regulatory: code cleanup Johannes Berg
2012-12-07  0:11   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 08/24] regulatory: remove useless locking on exit Johannes Berg
2012-12-07  0:16   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 09/24] regulatory: use proper enum for return values Johannes Berg
2012-12-07  0:20   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 10/24] cfg80211: remove wiphy_idx_valid Johannes Berg
2012-12-07  0:34   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 11/24] regulatory: remove BUG_ON Johannes Berg
2012-12-07  0:39   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 12/24] regulatory: simplify restore_regulatory_settings Johannes Berg
2012-12-07  0:53   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 13/24] regulatory: remove redundant isalpha() check Johannes Berg
2012-12-07  0:54   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 14/24] regulatory: remove useless warning Johannes Berg
2012-12-07  0:55   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 15/24] regulatory: simplify freq_reg_info_regd Johannes Berg
2012-12-07  1:02   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 16/24] regulatory: clarify locking rules and assertions Johannes Berg
2012-12-07 23:11   ` Luis R. Rodriguez
2012-12-07 23:16     ` Johannes Berg
2012-12-13 20:57       ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 17/24] regulatory: remove locking from wiphy_apply_custom_regulatory Johannes Berg
2012-12-13 21:56   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 18/24] regulatory: fix reg_is_valid_request handling Johannes Berg
2012-12-13 21:27   ` Luis R. Rodriguez
2012-12-13 21:35     ` Johannes Berg
2012-12-13 22:00       ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 19/24] regulatory: remove handling of channel bandwidth Johannes Berg
2012-12-13 22:11   ` Luis R. Rodriguez
2012-12-14 10:35     ` Johannes Berg
2012-12-14 10:36       ` Johannes Berg
2012-12-06 16:47 ` [PATCH 20/24] regulatory: fix memory leak Johannes Berg
2012-12-13 21:35   ` Luis R. Rodriguez
2012-12-13 21:37     ` Johannes Berg
2012-12-06 16:47 ` [PATCH 21/24] regulatory: pass new regdomain to reset function Johannes Berg
2012-12-13 21:42   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 22/24] regulatory: use RCU to protect global and wiphy regdomains Johannes Berg
2012-12-10 17:05   ` Johannes Berg
2012-12-13 21:54     ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 23/24] regulatory: use RCU to protect last_request Johannes Berg
2012-12-13 22:07   ` Luis R. Rodriguez
2012-12-06 16:47 ` [PATCH 24/24] regulatory: use IS_ERR macro family for freq_reg_info Johannes Berg
2012-12-13 22:10   ` Luis R. Rodriguez
2012-12-20 11:15 ` [PATCH 00/24] regulatory fixes, cleanups & improvements Johannes Berg

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