mac80211: use synchronize_rcu() with rcu_barrier()

PATCH Superseded Bob Copeland 3 participants 4 messages Apr 18, 2013 – Apr 22, 2013 mac80211

Share & tools

Bob Copeland

author

2013-04-18 22:05 UTC #0 lore raw

PATCH 1/1 mac80211: use synchronize_rcu() with rcu_barrier()

The RCU docs used to state that rcu_barrier() included a wait for an RCU grace period; however the comments for rcu_barrier() as of commit f0a0e6f... "rcu: Clarify memory-ordering properties of grace-period primitives" contradict this.

So add back synchronize_{rcu,net}() to where they once were, but keep the rcu_barrier()s for the call_rcu() callbacks.

Cc stable <[email protected]>

Signed-off-by Bob Copeland <[email protected]>

---
 net/mac80211/iface.c |    5 +++--
 net/mac80211/pm.c    |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

net/mac80211/iface.c +3 -2

index b6abaaa..c634aff 100644
@@ -836,11 +836,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		 *
 		 * sta_info_flush_cleanup() requires rcu_barrier()
 		 * first to wait for the station call_rcu() calls
-		 * to complete, here we need at least sychronize_rcu()
-		 * it to wait for the RX path in case it is using the
+		 * to complete, and we also need synchronize_rcu()
+		 * to wait for the RX path in case it is using the
 		 * interface and enqueuing frames at this very time on
 		 * another CPU.
 		 */
+		synchronize_rcu();
 		rcu_barrier();
 		sta_info_flush_cleanup(sdata);

net/mac80211/pm.c +1 -0

index 4431f0f..7fc5d0d 100644
@@ -38,6 +38,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 					IEEE80211_QUEUE_STOP_REASON_SUSPEND);
 
 	/* flush out all packets and station cleanup call_rcu()s */
+	synchronize_net();
 	rcu_barrier();
 
 	ieee80211_flush_queues(local, NULL);

Signature · 4 lines

1.7.10.4

-- 
Bob Copeland %% www.bobcopeland.com

Paul E. McKenney

Reviewed-by on v1 1/1

2013-04-18 22:20 UTC #1 lore raw

Quoted 10 lines · Bob Copeland

The RCU docs used to state that rcu_barrier() included a wait for an RCU grace period; however the comments for rcu_barrier() as of commit f0a0e6f... "rcu: Clarify memory-ordering properties of grace-period primitives" contradict this.
So add back synchronize_{rcu,net}() to where they once were, but keep the rcu_barrier()s for the call_rcu() callbacks.
Cc stable <[email protected]>
Signed-off-by Bob Copeland <[email protected]>

Reviewed-by Paul E. McKenney <[email protected]>

Quoted 42 lines

---
 net/mac80211/iface.c |    5 +++--
 net/mac80211/pm.c    |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

net/mac80211/iface.c +3 -2

index b6abaaa..c634aff 100644
@@ -836,11 +836,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		 *
 		 * sta_info_flush_cleanup() requires rcu_barrier()
 		 * first to wait for the station call_rcu() calls
-		 * to complete, here we need at least sychronize_rcu()
-		 * it to wait for the RX path in case it is using the
+		 * to complete, and we also need synchronize_rcu()
+		 * to wait for the RX path in case it is using the
 		 * interface and enqueuing frames at this very time on
 		 * another CPU.
 		 */
+		synchronize_rcu();
 		rcu_barrier();
 		sta_info_flush_cleanup(sdata);

net/mac80211/pm.c +1 -0

index 4431f0f..7fc5d0d 100644
@@ -38,6 +38,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 					IEEE80211_QUEUE_STOP_REASON_SUSPEND);
 
 	/* flush out all packets and station cleanup call_rcu()s */
+	synchronize_net();
 	rcu_barrier();
 
 	ieee80211_flush_queues(local, NULL);

Signature · 4 lines

1.7.10.4

-- 
Bob Copeland %% www.bobcopeland.com

Bob Copeland

author

2013-04-18 22:27 UTC #2 lore raw

PATCH v2 1/1 mac80211: use synchronize_rcu() with rcu_barrier()

In reply to Bob Copeland

So add back synchronize_{rcu,net}() to where they once were, but keep the rcu_barrier()s for the call_rcu() callbacks.

Cc stable <[email protected]>

Signed-off-by Bob Copeland <[email protected]>

---
v2: also in cfg.c

 net/mac80211/cfg.c   |    1 +
 net/mac80211/iface.c |    5 +++--
 net/mac80211/pm.c    |    1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

net/mac80211/cfg.c +1 -0

index 72ab1c0..dcffca4 100644
@@ -1043,6 +1043,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
 	list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list)
 		sta_info_flush_defer(vlan);
 	sta_info_flush_defer(sdata);
+	synchronize_net();
 	rcu_barrier();
 	list_for_each_entry(vlan, &sdata->u.ap.vlans, u.vlan.list) {
 		sta_info_flush_cleanup(vlan);

net/mac80211/iface.c +3 -2

index b6abaaa..c634aff 100644
@@ -836,11 +836,12 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		 *
 		 * sta_info_flush_cleanup() requires rcu_barrier()
 		 * first to wait for the station call_rcu() calls
-		 * to complete, here we need at least sychronize_rcu()
-		 * it to wait for the RX path in case it is using the
+		 * to complete, and we also need synchronize_rcu()
+		 * to wait for the RX path in case it is using the
 		 * interface and enqueuing frames at this very time on
 		 * another CPU.
 		 */
+		synchronize_rcu();
 		rcu_barrier();
 		sta_info_flush_cleanup(sdata);

net/mac80211/pm.c +1 -0

index 4431f0f..7fc5d0d 100644
@@ -38,6 +38,7 @@ int __ieee80211_suspend(struct ieee80211_hw *hw, struct cfg80211_wowlan *wowlan)
 					IEEE80211_QUEUE_STOP_REASON_SUSPEND);
 
 	/* flush out all packets and station cleanup call_rcu()s */
+	synchronize_net();
 	rcu_barrier();
 
 	ieee80211_flush_queues(local, NULL);

Signature · 1 line

1.7.10.4

Johannes Berg

maintainer Applied

2013-04-22 13:41 UTC #3 lore raw

Quoted 7 lines · Bob Copeland

The RCU docs used to state that rcu_barrier() included a wait for an RCU grace period; however the comments for rcu_barrier() as of commit f0a0e6f... "rcu: Clarify memory-ordering properties of grace-period primitives" contradict this.
So add back synchronize_{rcu,net}() to where they once were, but keep the rcu_barrier()s for the call_rcu() callbacks.

Applied, thanks for spotting this!

johannes