All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/23] aggregation locking rework
@ 2010-06-10  8:21 Johannes Berg
  2010-06-10  8:21 ` [PATCH 01/23] mac80211: simplify station/aggregation code Johannes Berg
                   ` (22 more replies)
  0 siblings, 23 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

Hi,

I'm finally happy with it now after having tested
it in more detail.

Also a big thanks to Sujith for testing and review.

I'll let the patch changelogs speak for themselves,
the overarching theme should be clear: ampdu_action
can sleep after this patch series.

For the curious:

 drivers/net/wireless/ath/ath9k/htc_drv_main.c |    6 
 drivers/net/wireless/ath/ath9k/main.c         |    4 
 include/net/mac80211.h                        |   34 -
 net/mac80211/agg-rx.c                         |  133 ++--
 net/mac80211/agg-tx.c                         |  695 ++++++++++++++------------
 net/mac80211/debugfs_sta.c                    |   19 
 net/mac80211/driver-ops.h                     |    8 
 net/mac80211/ht.c                             |   62 +-
 net/mac80211/ibss.c                           |   77 --
 net/mac80211/ieee80211_i.h                    |   57 +-
 net/mac80211/iface.c                          |  191 +++++--
 net/mac80211/main.c                           |   19 
 net/mac80211/mesh.c                           |   83 ---
 net/mac80211/mesh.h                           |    2 
 net/mac80211/mesh_hwmp.c                      |    4 
 net/mac80211/mesh_pathtbl.c                   |    4 
 net/mac80211/mlme.c                           |  128 ----
 net/mac80211/pm.c                             |   18 
 net/mac80211/rc80211_minstrel_ht.c            |    2 
 net/mac80211/rx.c                             |  159 +++--
 net/mac80211/sta_info.c                       |   16 
 net/mac80211/sta_info.h                       |   63 +-
 net/mac80211/tx.c                             |   93 ++-
 net/mac80211/util.c                           |   31 -
 24 files changed, 998 insertions(+), 910 deletions(-)


johannes


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

* [PATCH 01/23] mac80211: simplify station/aggregation code
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 02/23] mac80211: use common skb queue Johannes Berg
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

A number of places use RCU locking for accessing
the station list, even though they do not need
to. Use mutex locking instead to prepare for the
locking changes I want to make. The mlme code is
also using a WLAN_STA_DISASSOC flag that has the
same meaning as WLAN_STA_BLOCK_BA, so use that.

While doing so, combine places where we loop
over stations twice, and optimise away some of
the loops by checking if the hardware supports
aggregation at all first.

Also fix a more theoretical race condition: right
now we could resume, set up an aggregation session,
and right after tear it down again due to the code
that is needed for hardware reconfiguration here.
Also mark add a comment to that code marking it as
a workaround.

Finally, remove a pointless aggregation disabling
loop when an interface is stopped, directly after
that we remove all stations from it which will also
disable all aggregation sessions that may still be
active, and does so in a race-free way unlike the
current loop that doesn't block new sessions.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c   |   10 +---------
 net/mac80211/iface.c    |   13 -------------
 net/mac80211/mlme.c     |    6 +++---
 net/mac80211/pm.c       |   16 ++++------------
 net/mac80211/sta_info.h |    4 ----
 net/mac80211/util.c     |   31 ++++++++++++++++---------------
 6 files changed, 24 insertions(+), 56 deletions(-)

--- wireless-testing.orig/net/mac80211/pm.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/pm.c	2010-06-09 17:21:05.000000000 +0200
@@ -40,22 +40,14 @@ int __ieee80211_suspend(struct ieee80211
 	list_for_each_entry(sdata, &local->interfaces, list)
 		ieee80211_disable_keys(sdata);
 
-	/* Tear down aggregation sessions */
-
-	rcu_read_lock();
-
-	if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
-		list_for_each_entry_rcu(sta, &local->sta_list, list) {
+	/* tear down aggregation sessions and remove STAs */
+	mutex_lock(&local->sta_mtx);
+	list_for_each_entry(sta, &local->sta_list, list) {
+		if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
 			set_sta_flags(sta, WLAN_STA_BLOCK_BA);
 			ieee80211_sta_tear_down_BA_sessions(sta);
 		}
-	}
 
-	rcu_read_unlock();
-
-	/* remove STAs */
-	mutex_lock(&local->sta_mtx);
-	list_for_each_entry(sta, &local->sta_list, list) {
 		if (sta->uploaded) {
 			sdata = sta->sdata;
 			if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
--- wireless-testing.orig/net/mac80211/util.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/util.c	2010-06-09 17:21:05.000000000 +0200
@@ -1138,18 +1138,6 @@ int ieee80211_reconfig(struct ieee80211_
 	}
 	mutex_unlock(&local->sta_mtx);
 
-	/* Clear Suspend state so that ADDBA requests can be processed */
-
-	rcu_read_lock();
-
-	if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
-		list_for_each_entry_rcu(sta, &local->sta_list, list) {
-			clear_sta_flags(sta, WLAN_STA_BLOCK_BA);
-		}
-	}
-
-	rcu_read_unlock();
-
 	/* setup RTS threshold */
 	drv_set_rts_threshold(local, hw->wiphy->rts_threshold);
 
@@ -1202,13 +1190,26 @@ int ieee80211_reconfig(struct ieee80211_
 		}
 	}
 
-	rcu_read_lock();
+	/*
+	 * Clear the WLAN_STA_BLOCK_BA flag so new aggregation
+	 * sessions can be established after a resume.
+	 *
+	 * Also tear down aggregation sessions since reconfiguring
+	 * them in a hardware restart scenario is not easily done
+	 * right now, and the hardware will have lost information
+	 * about the sessions, but we and the AP still think they
+	 * are active. This is really a workaround though.
+	 */
 	if (hw->flags & IEEE80211_HW_AMPDU_AGGREGATION) {
-		list_for_each_entry_rcu(sta, &local->sta_list, list) {
+		mutex_lock(&local->sta_mtx);
+
+		list_for_each_entry(sta, &local->sta_list, list) {
 			ieee80211_sta_tear_down_BA_sessions(sta);
+			clear_sta_flags(sta, WLAN_STA_BLOCK_BA);
 		}
+
+		mutex_unlock(&local->sta_mtx);
 	}
-	rcu_read_unlock();
 
 	/* add back keys */
 	list_for_each_entry(sdata, &local->interfaces, list)
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:05.000000000 +0200
@@ -339,7 +339,6 @@ static int ieee80211_stop(struct net_dev
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
-	struct sta_info *sta;
 	unsigned long flags;
 	struct sk_buff *skb, *tmp;
 	u32 hw_reconf_flags = 0;
@@ -356,18 +355,6 @@ static int ieee80211_stop(struct net_dev
 	ieee80211_work_purge(sdata);
 
 	/*
-	 * Now delete all active aggregation sessions.
-	 */
-	rcu_read_lock();
-
-	list_for_each_entry_rcu(sta, &local->sta_list, list) {
-		if (sta->sdata == sdata)
-			ieee80211_sta_tear_down_BA_sessions(sta);
-	}
-
-	rcu_read_unlock();
-
-	/*
 	 * Remove all stations associated with this interface.
 	 *
 	 * This must be done before calling ops->remove_interface()
--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:05.000000000 +0200
@@ -239,17 +239,9 @@ int ieee80211_start_tx_ba_session(struct
 	    sdata->vif.type != NL80211_IFTYPE_AP)
 		return -EINVAL;
 
-	if (test_sta_flags(sta, WLAN_STA_DISASSOC)) {
-#ifdef CONFIG_MAC80211_HT_DEBUG
-		printk(KERN_DEBUG "Disassociation is in progress. "
-		       "Denying BA session request\n");
-#endif
-		return -EINVAL;
-	}
-
 	if (test_sta_flags(sta, WLAN_STA_BLOCK_BA)) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
-		printk(KERN_DEBUG "Suspend in progress. "
+		printk(KERN_DEBUG "BA sessions blocked. "
 		       "Denying BA session request\n");
 #endif
 		return -EINVAL;
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:05.000000000 +0200
@@ -898,13 +898,13 @@ static void ieee80211_set_disassoc(struc
 	netif_tx_stop_all_queues(sdata->dev);
 	netif_carrier_off(sdata->dev);
 
-	rcu_read_lock();
+	mutex_lock(&local->sta_mtx);
 	sta = sta_info_get(sdata, bssid);
 	if (sta) {
-		set_sta_flags(sta, WLAN_STA_DISASSOC);
+		set_sta_flags(sta, WLAN_STA_BLOCK_BA);
 		ieee80211_sta_tear_down_BA_sessions(sta);
 	}
-	rcu_read_unlock();
+	mutex_unlock(&local->sta_mtx);
 
 	changed |= ieee80211_reset_erp_info(sdata);
 
--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:05.000000000 +0200
@@ -42,9 +42,6 @@
  *	be in the queues
  * @WLAN_STA_PSPOLL: Station sent PS-poll while driver was keeping
  *	station in power-save mode, reply when the driver unblocks.
- * @WLAN_STA_DISASSOC: Disassociation in progress.
- *	This is used to reject TX BA session requests when disassociation
- *	is in progress.
  */
 enum ieee80211_sta_info_flags {
 	WLAN_STA_AUTH		= 1<<0,
@@ -60,7 +57,6 @@ enum ieee80211_sta_info_flags {
 	WLAN_STA_BLOCK_BA	= 1<<11,
 	WLAN_STA_PS_DRIVER	= 1<<12,
 	WLAN_STA_PSPOLL		= 1<<13,
-	WLAN_STA_DISASSOC       = 1<<14,
 };
 
 #define STA_TID_NUM 16



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

* [PATCH 02/23] mac80211: use common skb queue
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
  2010-06-10  8:21 ` [PATCH 01/23] mac80211: simplify station/aggregation code Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 03/23] mac80211: use common work struct Johannes Berg
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

IBSS, managed and mesh modes all have an
skb queue, and in the future we want to
also use it in other modes, so make them
all use a common skb queue already.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ibss.c        |    7 +++----
 net/mac80211/ieee80211_i.h |    7 ++-----
 net/mac80211/iface.c       |   21 +++++++++++----------
 net/mac80211/mesh.c        |   14 ++------------
 net/mac80211/mlme.c        |    5 ++---
 5 files changed, 20 insertions(+), 34 deletions(-)

--- wireless-testing.orig/net/mac80211/ibss.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-06-09 17:21:06.000000000 +0200
@@ -779,7 +779,7 @@ static void ieee80211_ibss_work(struct w
 		return;
 	ifibss = &sdata->u.ibss;
 
-	while ((skb = skb_dequeue(&ifibss->skb_queue)))
+	while ((skb = skb_dequeue(&sdata->skb_queue)))
 		ieee80211_ibss_rx_queued_mgmt(sdata, skb);
 
 	if (!test_and_clear_bit(IEEE80211_IBSS_REQ_RUN, &ifibss->request))
@@ -850,7 +850,6 @@ void ieee80211_ibss_setup_sdata(struct i
 	INIT_WORK(&ifibss->work, ieee80211_ibss_work);
 	setup_timer(&ifibss->timer, ieee80211_ibss_timer,
 		    (unsigned long) sdata);
-	skb_queue_head_init(&ifibss->skb_queue);
 }
 
 /* scan finished notification */
@@ -890,7 +889,7 @@ ieee80211_ibss_rx_mgmt(struct ieee80211_
 	case IEEE80211_STYPE_BEACON:
 	case IEEE80211_STYPE_PROBE_REQ:
 	case IEEE80211_STYPE_AUTH:
-		skb_queue_tail(&sdata->u.ibss.skb_queue, skb);
+		skb_queue_tail(&sdata->skb_queue, skb);
 		ieee80211_queue_work(&local->hw, &sdata->u.ibss.work);
 		return RX_QUEUED;
 	}
@@ -983,7 +982,7 @@ int ieee80211_ibss_leave(struct ieee8021
 	synchronize_rcu();
 	kfree_skb(skb);
 
-	skb_queue_purge(&sdata->u.ibss.skb_queue);
+	skb_queue_purge(&sdata->skb_queue);
 	memset(sdata->u.ibss.bssid, 0, ETH_ALEN);
 	sdata->u.ibss.ssid_len = 0;
 
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:06.000000000 +0200
@@ -340,8 +340,6 @@ struct ieee80211_if_managed {
 
 	u16 aid;
 
-	struct sk_buff_head skb_queue;
-
 	unsigned long timers_running; /* used for quiesce/restart */
 	bool powersave; /* powersave requested for this iface */
 	enum ieee80211_smps_mode req_smps, /* requested smps mode */
@@ -388,8 +386,6 @@ struct ieee80211_if_ibss {
 	struct timer_list timer;
 	struct work_struct work;
 
-	struct sk_buff_head skb_queue;
-
 	unsigned long request;
 	unsigned long last_scan_completed;
 
@@ -420,7 +416,6 @@ struct ieee80211_if_mesh {
 	struct timer_list housekeeping_timer;
 	struct timer_list mesh_path_timer;
 	struct timer_list mesh_path_root_timer;
-	struct sk_buff_head skb_queue;
 
 	unsigned long timers_running;
 
@@ -517,6 +512,8 @@ struct ieee80211_sub_if_data {
 
 	u16 sequence_number;
 
+	struct sk_buff_head skb_queue;
+
 	/*
 	 * AP this belongs to: self in AP mode and
 	 * corresponding AP in VLAN mode, NULL for
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:06.000000000 +0200
@@ -465,21 +465,11 @@ static int ieee80211_stop(struct net_dev
 		cancel_work_sync(&sdata->u.mgd.monitor_work);
 		cancel_work_sync(&sdata->u.mgd.beacon_connection_loss_work);
 
-		/*
-		 * When we get here, the interface is marked down.
-		 * Call synchronize_rcu() to wait for the RX path
-		 * should it be using the interface and enqueuing
-		 * frames at this very time on another CPU.
-		 */
-		synchronize_rcu();
-		skb_queue_purge(&sdata->u.mgd.skb_queue);
 		/* fall through */
 	case NL80211_IFTYPE_ADHOC:
 		if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
 			del_timer_sync(&sdata->u.ibss.timer);
 			cancel_work_sync(&sdata->u.ibss.work);
-			synchronize_rcu();
-			skb_queue_purge(&sdata->u.ibss.skb_queue);
 		}
 		/* fall through */
 	case NL80211_IFTYPE_MESH_POINT:
@@ -495,6 +485,15 @@ static int ieee80211_stop(struct net_dev
 		}
 		/* fall through */
 	default:
+		/*
+		 * When we get here, the interface is marked down.
+		 * Call synchronize_rcu() to wait for the RX path
+		 * should it be using the interface and enqueuing
+		 * frames at this very time on another CPU.
+		 */
+		synchronize_rcu();
+		skb_queue_purge(&sdata->skb_queue);
+
 		if (local->scan_sdata == sdata)
 			ieee80211_scan_cancel(local);
 
@@ -721,6 +720,8 @@ static void ieee80211_setup_sdata(struct
 	/* only monitor differs */
 	sdata->dev->type = ARPHRD_ETHER;
 
+	skb_queue_head_init(&sdata->skb_queue);
+
 	switch (type) {
 	case NL80211_IFTYPE_AP:
 		skb_queue_head_init(&sdata->u.ap.ps_bc_buf);
--- wireless-testing.orig/net/mac80211/mesh.c	2010-06-09 17:20:52.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.c	2010-06-09 17:21:06.000000000 +0200
@@ -537,15 +537,6 @@ void ieee80211_stop_mesh(struct ieee8021
 	 * it no longer is.
 	 */
 	cancel_work_sync(&sdata->u.mesh.work);
-
-	/*
-	 * When we get here, the interface is marked down.
-	 * Call synchronize_rcu() to wait for the RX path
-	 * should it be using the interface and enqueuing
-	 * frames at this very time on another CPU.
-	 */
-	rcu_barrier(); /* Wait for RX path and call_rcu()'s */
-	skb_queue_purge(&sdata->u.mesh.skb_queue);
 }
 
 static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
@@ -650,7 +641,7 @@ static void ieee80211_mesh_work(struct w
 	if (local->scanning)
 		return;
 
-	while ((skb = skb_dequeue(&ifmsh->skb_queue)))
+	while ((skb = skb_dequeue(&sdata->skb_queue)))
 		ieee80211_mesh_rx_queued_mgmt(sdata, skb);
 
 	if (ifmsh->preq_queue_len &&
@@ -690,7 +681,6 @@ void ieee80211_mesh_init_sdata(struct ie
 	setup_timer(&ifmsh->housekeeping_timer,
 		    ieee80211_mesh_housekeeping_timer,
 		    (unsigned long) sdata);
-	skb_queue_head_init(&sdata->u.mesh.skb_queue);
 
 	ifmsh->mshcfg.dot11MeshRetryTimeout = MESH_RET_T;
 	ifmsh->mshcfg.dot11MeshConfirmTimeout = MESH_CONF_T;
@@ -750,7 +740,7 @@ ieee80211_mesh_rx_mgmt(struct ieee80211_
 	case IEEE80211_STYPE_ACTION:
 	case IEEE80211_STYPE_PROBE_RESP:
 	case IEEE80211_STYPE_BEACON:
-		skb_queue_tail(&ifmsh->skb_queue, skb);
+		skb_queue_tail(&sdata->skb_queue, skb);
 		ieee80211_queue_work(&local->hw, &ifmsh->work);
 		return RX_QUEUED;
 	}
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:06.000000000 +0200
@@ -1652,7 +1652,7 @@ ieee80211_rx_result ieee80211_sta_rx_mgm
 	case IEEE80211_STYPE_DEAUTH:
 	case IEEE80211_STYPE_DISASSOC:
 	case IEEE80211_STYPE_ACTION:
-		skb_queue_tail(&sdata->u.mgd.skb_queue, skb);
+		skb_queue_tail(&sdata->skb_queue, skb);
 		ieee80211_queue_work(&local->hw, &sdata->u.mgd.work);
 		return RX_QUEUED;
 	}
@@ -1847,7 +1847,7 @@ static void ieee80211_sta_work(struct wo
 	ifmgd = &sdata->u.mgd;
 
 	/* first process frames to avoid timing out while a frame is pending */
-	while ((skb = skb_dequeue(&ifmgd->skb_queue)))
+	while ((skb = skb_dequeue(&sdata->skb_queue)))
 		ieee80211_sta_rx_queued_mgmt(sdata, skb);
 
 	/* then process the rest of the work */
@@ -2004,7 +2004,6 @@ void ieee80211_sta_setup_sdata(struct ie
 		    (unsigned long) sdata);
 	setup_timer(&ifmgd->chswitch_timer, ieee80211_chswitch_timer,
 		    (unsigned long) sdata);
-	skb_queue_head_init(&ifmgd->skb_queue);
 
 	ifmgd->flags = 0;
 



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

* [PATCH 03/23] mac80211: use common work struct
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
  2010-06-10  8:21 ` [PATCH 01/23] mac80211: simplify station/aggregation code Johannes Berg
  2010-06-10  8:21 ` [PATCH 02/23] mac80211: use common skb queue Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 04/23] mac80211: use common work function Johannes Berg
                   ` (19 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

IBSS, managed and mesh modes all have their
own work struct, and in the future we want
to also use it in other modes to process
frames from the now common skb queue.

This also makes the skb queue and work safe
to use from other interface types.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ibss.c         |   13 ++++++-------
 net/mac80211/ieee80211_i.h  |    4 +---
 net/mac80211/iface.c        |    6 ++----
 net/mac80211/mesh.c         |   22 +++++++++-------------
 net/mac80211/mesh_hwmp.c    |    4 ++--
 net/mac80211/mesh_pathtbl.c |    4 ++--
 net/mac80211/mlme.c         |   12 +++++-------
 net/mac80211/pm.c           |    2 ++
 8 files changed, 29 insertions(+), 38 deletions(-)

--- wireless-testing.orig/net/mac80211/ibss.c	2010-06-09 17:21:06.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-06-09 17:21:07.000000000 +0200
@@ -761,7 +761,7 @@ static void ieee80211_ibss_rx_queued_mgm
 static void ieee80211_ibss_work(struct work_struct *work)
 {
 	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, u.ibss.work);
+		container_of(work, struct ieee80211_sub_if_data, work);
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_ibss *ifibss;
 	struct sk_buff *skb;
@@ -804,7 +804,7 @@ static void ieee80211_queue_ibss_work(st
 	struct ieee80211_local *local = sdata->local;
 
 	set_bit(IEEE80211_IBSS_REQ_RUN, &ifibss->request);
-	ieee80211_queue_work(&local->hw, &ifibss->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 static void ieee80211_ibss_timer(unsigned long data)
@@ -827,7 +827,6 @@ void ieee80211_ibss_quiesce(struct ieee8
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
-	cancel_work_sync(&ifibss->work);
 	if (del_timer_sync(&ifibss->timer))
 		ifibss->timer_running = true;
 }
@@ -847,7 +846,7 @@ void ieee80211_ibss_setup_sdata(struct i
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
-	INIT_WORK(&ifibss->work, ieee80211_ibss_work);
+	INIT_WORK(&sdata->work, ieee80211_ibss_work);
 	setup_timer(&ifibss->timer, ieee80211_ibss_timer,
 		    (unsigned long) sdata);
 }
@@ -890,7 +889,7 @@ ieee80211_ibss_rx_mgmt(struct ieee80211_
 	case IEEE80211_STYPE_PROBE_REQ:
 	case IEEE80211_STYPE_AUTH:
 		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &sdata->u.ibss.work);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 		return RX_QUEUED;
 	}
 
@@ -956,7 +955,7 @@ int ieee80211_ibss_join(struct ieee80211
 	ieee80211_recalc_idle(sdata->local);
 
 	set_bit(IEEE80211_IBSS_REQ_RUN, &sdata->u.ibss.request);
-	ieee80211_queue_work(&sdata->local->hw, &sdata->u.ibss.work);
+	ieee80211_queue_work(&sdata->local->hw, &sdata->work);
 
 	return 0;
 }
@@ -967,7 +966,7 @@ int ieee80211_ibss_leave(struct ieee8021
 
 	del_timer_sync(&sdata->u.ibss.timer);
 	clear_bit(IEEE80211_IBSS_REQ_RUN, &sdata->u.ibss.request);
-	cancel_work_sync(&sdata->u.ibss.work);
+	cancel_work_sync(&sdata->work);
 	clear_bit(IEEE80211_IBSS_REQ_RUN, &sdata->u.ibss.request);
 
 	sta_info_flush(sdata->local, sdata);
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:06.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:07.000000000 +0200
@@ -325,7 +325,6 @@ struct ieee80211_if_managed {
 	struct timer_list conn_mon_timer;
 	struct timer_list bcn_mon_timer;
 	struct timer_list chswitch_timer;
-	struct work_struct work;
 	struct work_struct monitor_work;
 	struct work_struct chswitch_work;
 	struct work_struct beacon_connection_loss_work;
@@ -384,7 +383,6 @@ enum ieee80211_ibss_request {
 
 struct ieee80211_if_ibss {
 	struct timer_list timer;
-	struct work_struct work;
 
 	unsigned long request;
 	unsigned long last_scan_completed;
@@ -412,7 +410,6 @@ struct ieee80211_if_ibss {
 };
 
 struct ieee80211_if_mesh {
-	struct work_struct work;
 	struct timer_list housekeeping_timer;
 	struct timer_list mesh_path_timer;
 	struct timer_list mesh_path_root_timer;
@@ -512,6 +509,7 @@ struct ieee80211_sub_if_data {
 
 	u16 sequence_number;
 
+	struct work_struct work;
 	struct sk_buff_head skb_queue;
 
 	/*
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:06.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:07.000000000 +0200
@@ -1653,7 +1653,7 @@ ieee80211_rx_result ieee80211_sta_rx_mgm
 	case IEEE80211_STYPE_DISASSOC:
 	case IEEE80211_STYPE_ACTION:
 		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &sdata->u.mgd.work);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 		return RX_QUEUED;
 	}
 
@@ -1816,13 +1816,13 @@ static void ieee80211_sta_timer(unsigned
 		return;
 	}
 
-	ieee80211_queue_work(&local->hw, &ifmgd->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 static void ieee80211_sta_work(struct work_struct *work)
 {
 	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, u.mgd.work);
+		container_of(work, struct ieee80211_sub_if_data, work);
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_managed *ifmgd;
 	struct sk_buff *skb;
@@ -1943,8 +1943,7 @@ static void ieee80211_restart_sta_timer(
 		ieee80211_queue_work(&sdata->local->hw,
 			   &sdata->u.mgd.monitor_work);
 		/* and do all the other regular work too */
-		ieee80211_queue_work(&sdata->local->hw,
-			   &sdata->u.mgd.work);
+		ieee80211_queue_work(&sdata->local->hw, &sdata->work);
 	}
 }
 
@@ -1959,7 +1958,6 @@ void ieee80211_sta_quiesce(struct ieee80
 	 * time -- the code here is properly synchronised.
 	 */
 
-	cancel_work_sync(&ifmgd->work);
 	cancel_work_sync(&ifmgd->beacon_connection_loss_work);
 	if (del_timer_sync(&ifmgd->timer))
 		set_bit(TMR_RUNNING_TIMER, &ifmgd->timers_running);
@@ -1991,7 +1989,7 @@ void ieee80211_sta_setup_sdata(struct ie
 	struct ieee80211_if_managed *ifmgd;
 
 	ifmgd = &sdata->u.mgd;
-	INIT_WORK(&ifmgd->work, ieee80211_sta_work);
+	INIT_WORK(&sdata->work, ieee80211_sta_work);
 	INIT_WORK(&ifmgd->monitor_work, ieee80211_sta_monitor_work);
 	INIT_WORK(&ifmgd->chswitch_work, ieee80211_chswitch_work);
 	INIT_WORK(&ifmgd->beacon_connection_loss_work,
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:06.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:07.000000000 +0200
@@ -460,17 +460,14 @@ static int ieee80211_stop(struct net_dev
 		 * whether the interface is running, which, at this point,
 		 * it no longer is.
 		 */
-		cancel_work_sync(&sdata->u.mgd.work);
 		cancel_work_sync(&sdata->u.mgd.chswitch_work);
 		cancel_work_sync(&sdata->u.mgd.monitor_work);
 		cancel_work_sync(&sdata->u.mgd.beacon_connection_loss_work);
 
 		/* fall through */
 	case NL80211_IFTYPE_ADHOC:
-		if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
+		if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
 			del_timer_sync(&sdata->u.ibss.timer);
-			cancel_work_sync(&sdata->u.ibss.work);
-		}
 		/* fall through */
 	case NL80211_IFTYPE_MESH_POINT:
 		if (ieee80211_vif_is_mesh(&sdata->vif)) {
@@ -485,6 +482,7 @@ static int ieee80211_stop(struct net_dev
 		}
 		/* fall through */
 	default:
+		cancel_work_sync(&sdata->work);
 		/*
 		 * When we get here, the interface is marked down.
 		 * Call synchronize_rcu() to wait for the RX path
--- wireless-testing.orig/net/mac80211/mesh.c	2010-06-09 17:21:06.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.c	2010-06-09 17:21:07.000000000 +0200
@@ -54,7 +54,7 @@ static void ieee80211_mesh_housekeeping_
 		return;
 	}
 
-	ieee80211_queue_work(&local->hw, &ifmsh->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 /**
@@ -345,7 +345,7 @@ static void ieee80211_mesh_path_timer(un
 		return;
 	}
 
-	ieee80211_queue_work(&local->hw, &ifmsh->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 static void ieee80211_mesh_path_root_timer(unsigned long data)
@@ -362,7 +362,7 @@ static void ieee80211_mesh_path_root_tim
 		return;
 	}
 
-	ieee80211_queue_work(&local->hw, &ifmsh->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh)
@@ -484,9 +484,6 @@ void ieee80211_mesh_quiesce(struct ieee8
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 
-	/* might restart the timer but that doesn't matter */
-	cancel_work_sync(&ifmsh->work);
-
 	/* use atomic bitops in case both timers fire at the same time */
 
 	if (del_timer_sync(&ifmsh->housekeeping_timer))
@@ -518,7 +515,7 @@ void ieee80211_start_mesh(struct ieee802
 
 	set_bit(MESH_WORK_HOUSEKEEPING, &ifmsh->wrkq_flags);
 	ieee80211_mesh_root_setup(ifmsh);
-	ieee80211_queue_work(&local->hw, &ifmsh->work);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 	sdata->vif.bss_conf.beacon_int = MESH_DEFAULT_BEACON_INTERVAL;
 	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON |
 						BSS_CHANGED_BEACON_ENABLED |
@@ -536,7 +533,7 @@ void ieee80211_stop_mesh(struct ieee8021
 	 * whether the interface is running, which, at this point,
 	 * it no longer is.
 	 */
-	cancel_work_sync(&sdata->u.mesh.work);
+	cancel_work_sync(&sdata->work);
 }
 
 static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
@@ -630,7 +627,7 @@ static void ieee80211_mesh_rx_queued_mgm
 static void ieee80211_mesh_work(struct work_struct *work)
 {
 	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, u.mesh.work);
+		container_of(work, struct ieee80211_sub_if_data, work);
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	struct sk_buff *skb;
@@ -669,7 +666,7 @@ void ieee80211_mesh_notify_scan_complete
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdata, &local->interfaces, list)
 		if (ieee80211_vif_is_mesh(&sdata->vif))
-			ieee80211_queue_work(&local->hw, &sdata->u.mesh.work);
+			ieee80211_queue_work(&local->hw, &sdata->work);
 	rcu_read_unlock();
 }
 
@@ -677,7 +674,7 @@ void ieee80211_mesh_init_sdata(struct ie
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 
-	INIT_WORK(&ifmsh->work, ieee80211_mesh_work);
+	INIT_WORK(&sdata->work, ieee80211_mesh_work);
 	setup_timer(&ifmsh->housekeeping_timer,
 		    ieee80211_mesh_housekeeping_timer,
 		    (unsigned long) sdata);
@@ -726,7 +723,6 @@ ieee80211_rx_result
 ieee80211_mesh_rx_mgmt(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb)
 {
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 	struct ieee80211_mgmt *mgmt;
 	u16 fc;
 
@@ -741,7 +737,7 @@ ieee80211_mesh_rx_mgmt(struct ieee80211_
 	case IEEE80211_STYPE_PROBE_RESP:
 	case IEEE80211_STYPE_BEACON:
 		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &ifmsh->work);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 		return RX_QUEUED;
 	}
 
--- wireless-testing.orig/net/mac80211/mesh_hwmp.c	2010-06-09 17:20:51.000000000 +0200
+++ wireless-testing/net/mac80211/mesh_hwmp.c	2010-06-09 17:21:07.000000000 +0200
@@ -805,14 +805,14 @@ static void mesh_queue_preq(struct mesh_
 	spin_unlock(&ifmsh->mesh_preq_queue_lock);
 
 	if (time_after(jiffies, ifmsh->last_preq + min_preq_int_jiff(sdata)))
-		ieee80211_queue_work(&sdata->local->hw, &ifmsh->work);
+		ieee80211_queue_work(&sdata->local->hw, &sdata->work);
 
 	else if (time_before(jiffies, ifmsh->last_preq)) {
 		/* avoid long wait if did not send preqs for a long time
 		 * and jiffies wrapped around
 		 */
 		ifmsh->last_preq = jiffies - min_preq_int_jiff(sdata) - 1;
-		ieee80211_queue_work(&sdata->local->hw, &ifmsh->work);
+		ieee80211_queue_work(&sdata->local->hw, &sdata->work);
 	} else
 		mod_timer(&ifmsh->mesh_path_timer, ifmsh->last_preq +
 						min_preq_int_jiff(sdata));
--- wireless-testing.orig/net/mac80211/mesh_pathtbl.c	2010-06-09 17:20:51.000000000 +0200
+++ wireless-testing/net/mac80211/mesh_pathtbl.c	2010-06-09 17:21:07.000000000 +0200
@@ -315,7 +315,7 @@ int mesh_path_add(u8 *dst, struct ieee80
 	read_unlock(&pathtbl_resize_lock);
 	if (grow) {
 		set_bit(MESH_WORK_GROW_MPATH_TABLE,  &ifmsh->wrkq_flags);
-		ieee80211_queue_work(&local->hw, &ifmsh->work);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 	}
 	return 0;
 
@@ -425,7 +425,7 @@ int mpp_path_add(u8 *dst, u8 *mpp, struc
 	read_unlock(&pathtbl_resize_lock);
 	if (grow) {
 		set_bit(MESH_WORK_GROW_MPP_TABLE,  &ifmsh->wrkq_flags);
-		ieee80211_queue_work(&local->hw, &ifmsh->work);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 	}
 	return 0;
 
--- wireless-testing.orig/net/mac80211/pm.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/pm.c	2010-06-09 17:21:07.000000000 +0200
@@ -64,6 +64,8 @@ int __ieee80211_suspend(struct ieee80211
 
 	/* remove all interfaces */
 	list_for_each_entry(sdata, &local->interfaces, list) {
+		cancel_work_sync(&sdata->work);
+
 		switch(sdata->vif.type) {
 		case NL80211_IFTYPE_STATION:
 			ieee80211_sta_quiesce(sdata);



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

* [PATCH 04/23] mac80211: use common work function
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (2 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 03/23] mac80211: use common work struct Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 05/23] mac80211: common work skb freeing Johannes Berg
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Even with the previous patch, IBSS, managed
and mesh modes all attach their own work
function to the shared work struct, which
means some duplicated code. Change that to
only have a frame processing function and a
further work function for each of them and
share some common code.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ibss.c        |   29 ++-------------------
 net/mac80211/ieee80211_i.h |   11 +++++++
 net/mac80211/iface.c       |   62 +++++++++++++++++++++++++++++++++++++++++++++
 net/mac80211/mesh.c        |   20 ++------------
 net/mac80211/mlme.c        |   35 ++-----------------------
 5 files changed, 84 insertions(+), 73 deletions(-)

--- wireless-testing.orig/net/mac80211/ibss.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-06-09 17:21:07.000000000 +0200
@@ -727,8 +727,8 @@ static void ieee80211_rx_mgmt_beacon(str
 	ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems, true);
 }
 
-static void ieee80211_ibss_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
-					  struct sk_buff *skb)
+void ieee80211_ibss_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				   struct sk_buff *skb)
 {
 	struct ieee80211_rx_status *rx_status;
 	struct ieee80211_mgmt *mgmt;
@@ -758,29 +758,9 @@ static void ieee80211_ibss_rx_queued_mgm
 	kfree_skb(skb);
 }
 
-static void ieee80211_ibss_work(struct work_struct *work)
+void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, work);
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_if_ibss *ifibss;
-	struct sk_buff *skb;
-
-	if (WARN_ON(local->suspended))
-		return;
-
-	if (!ieee80211_sdata_running(sdata))
-		return;
-
-	if (local->scanning)
-		return;
-
-	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_ADHOC))
-		return;
-	ifibss = &sdata->u.ibss;
-
-	while ((skb = skb_dequeue(&sdata->skb_queue)))
-		ieee80211_ibss_rx_queued_mgmt(sdata, skb);
+	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
 	if (!test_and_clear_bit(IEEE80211_IBSS_REQ_RUN, &ifibss->request))
 		return;
@@ -846,7 +826,6 @@ void ieee80211_ibss_setup_sdata(struct i
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
-	INIT_WORK(&sdata->work, ieee80211_ibss_work);
 	setup_timer(&ifibss->timer, ieee80211_ibss_timer,
 		    (unsigned long) sdata);
 }
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:07.000000000 +0200
@@ -995,6 +995,9 @@ void ieee80211_sta_process_chanswitch(st
 				      u64 timestamp);
 void ieee80211_sta_quiesce(struct ieee80211_sub_if_data *sdata);
 void ieee80211_sta_restart(struct ieee80211_sub_if_data *sdata);
+void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata);
+void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				  struct sk_buff *skb);
 
 /* IBSS code */
 void ieee80211_ibss_notify_scan_completed(struct ieee80211_local *local);
@@ -1009,6 +1012,14 @@ int ieee80211_ibss_join(struct ieee80211
 int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata);
 void ieee80211_ibss_quiesce(struct ieee80211_sub_if_data *sdata);
 void ieee80211_ibss_restart(struct ieee80211_sub_if_data *sdata);
+void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata);
+void ieee80211_ibss_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				   struct sk_buff *skb);
+
+/* mesh code */
+void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata);
+void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				   struct sk_buff *skb);
 
 /* scan/BSS handling */
 void ieee80211_scan_work(struct work_struct *work);
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:07.000000000 +0200
@@ -701,6 +701,67 @@ static void ieee80211_if_setup(struct ne
 	dev->destructor = free_netdev;
 }
 
+static void ieee80211_iface_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data, work);
+	struct ieee80211_local *local = sdata->local;
+	struct sk_buff *skb;
+
+	if (!ieee80211_sdata_running(sdata))
+		return;
+
+	if (local->scanning)
+		return;
+
+	/*
+	 * ieee80211_queue_work() should have picked up most cases,
+	 * here we'll pick the rest.
+	 */
+	if (WARN(local->suspended,
+		 "interface work scheduled while going to suspend\n"))
+		return;
+
+	/* first process frames */
+	while ((skb = skb_dequeue(&sdata->skb_queue))) {
+		switch (sdata->vif.type) {
+		case NL80211_IFTYPE_STATION:
+			ieee80211_sta_rx_queued_mgmt(sdata, skb);
+			break;
+		case NL80211_IFTYPE_ADHOC:
+			ieee80211_ibss_rx_queued_mgmt(sdata, skb);
+			break;
+		case NL80211_IFTYPE_MESH_POINT:
+			if (!ieee80211_vif_is_mesh(&sdata->vif))
+				break;
+			ieee80211_mesh_rx_queued_mgmt(sdata, skb);
+			break;
+		default:
+			WARN(1, "frame for unexpected interface type");
+			kfree_skb(skb);
+			break;
+		}
+	}
+
+	/* then other type-dependent work */
+	switch (sdata->vif.type) {
+	case NL80211_IFTYPE_STATION:
+		ieee80211_sta_work(sdata);
+		break;
+	case NL80211_IFTYPE_ADHOC:
+		ieee80211_ibss_work(sdata);
+		break;
+	case NL80211_IFTYPE_MESH_POINT:
+		if (!ieee80211_vif_is_mesh(&sdata->vif))
+			break;
+		ieee80211_mesh_work(sdata);
+		break;
+	default:
+		break;
+	}
+}
+
+
 /*
  * Helper function to initialise an interface to a specific type.
  */
@@ -719,6 +780,7 @@ static void ieee80211_setup_sdata(struct
 	sdata->dev->type = ARPHRD_ETHER;
 
 	skb_queue_head_init(&sdata->skb_queue);
+	INIT_WORK(&sdata->work, ieee80211_iface_work);
 
 	switch (type) {
 	case NL80211_IFTYPE_AP:
--- wireless-testing.orig/net/mac80211/mesh.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.c	2010-06-09 17:21:07.000000000 +0200
@@ -596,8 +596,8 @@ static void ieee80211_mesh_rx_mgmt_actio
 	}
 }
 
-static void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
-					  struct sk_buff *skb)
+void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				   struct sk_buff *skb)
 {
 	struct ieee80211_rx_status *rx_status;
 	struct ieee80211_if_mesh *ifmsh;
@@ -624,22 +624,9 @@ static void ieee80211_mesh_rx_queued_mgm
 	kfree_skb(skb);
 }
 
-static void ieee80211_mesh_work(struct work_struct *work)
+void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, work);
-	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
-	struct sk_buff *skb;
-
-	if (!ieee80211_sdata_running(sdata))
-		return;
-
-	if (local->scanning)
-		return;
-
-	while ((skb = skb_dequeue(&sdata->skb_queue)))
-		ieee80211_mesh_rx_queued_mgmt(sdata, skb);
 
 	if (ifmsh->preq_queue_len &&
 	    time_after(jiffies,
@@ -674,7 +661,6 @@ void ieee80211_mesh_init_sdata(struct ie
 {
 	struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
 
-	INIT_WORK(&sdata->work, ieee80211_mesh_work);
 	setup_timer(&ifmsh->housekeeping_timer,
 		    ieee80211_mesh_housekeeping_timer,
 		    (unsigned long) sdata);
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:07.000000000 +0200
@@ -1660,8 +1660,8 @@ ieee80211_rx_result ieee80211_sta_rx_mgm
 	return RX_DROP_MONITOR;
 }
 
-static void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
-					 struct sk_buff *skb)
+void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
+				  struct sk_buff *skb)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	struct ieee80211_rx_status *rx_status;
@@ -1819,36 +1819,10 @@ static void ieee80211_sta_timer(unsigned
 	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
-static void ieee80211_sta_work(struct work_struct *work)
+void ieee80211_sta_work(struct ieee80211_sub_if_data *sdata)
 {
-	struct ieee80211_sub_if_data *sdata =
-		container_of(work, struct ieee80211_sub_if_data, work);
 	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_if_managed *ifmgd;
-	struct sk_buff *skb;
-
-	if (!ieee80211_sdata_running(sdata))
-		return;
-
-	if (local->scanning)
-		return;
-
-	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_STATION))
-		return;
-
-	/*
-	 * ieee80211_queue_work() should have picked up most cases,
-	 * here we'll pick the rest.
-	 */
-	if (WARN(local->suspended, "STA MLME work scheduled while "
-		 "going to suspend\n"))
-		return;
-
-	ifmgd = &sdata->u.mgd;
-
-	/* first process frames to avoid timing out while a frame is pending */
-	while ((skb = skb_dequeue(&sdata->skb_queue)))
-		ieee80211_sta_rx_queued_mgmt(sdata, skb);
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 
 	/* then process the rest of the work */
 	mutex_lock(&ifmgd->mtx);
@@ -1989,7 +1963,6 @@ void ieee80211_sta_setup_sdata(struct ie
 	struct ieee80211_if_managed *ifmgd;
 
 	ifmgd = &sdata->u.mgd;
-	INIT_WORK(&sdata->work, ieee80211_sta_work);
 	INIT_WORK(&ifmgd->monitor_work, ieee80211_sta_monitor_work);
 	INIT_WORK(&ifmgd->chswitch_work, ieee80211_chswitch_work);
 	INIT_WORK(&ifmgd->beacon_connection_loss_work,



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

* [PATCH 05/23] mac80211: common work skb freeing
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (3 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 04/23] mac80211: use common work function Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 06/23] mac80211: pull mgmt frame rx into rx handler Johannes Berg
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

All the management processing functions free the
skb after they are done, so this can be done in
the new common code instead.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ibss.c  |    2 --
 net/mac80211/iface.c |    3 ++-
 net/mac80211/mesh.c  |    2 --
 net/mac80211/mlme.c  |    5 +----
 4 files changed, 3 insertions(+), 9 deletions(-)

--- wireless-testing.orig/net/mac80211/ibss.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-06-09 17:21:08.000000000 +0200
@@ -754,8 +754,6 @@ void ieee80211_ibss_rx_queued_mgmt(struc
 		ieee80211_rx_mgmt_auth_ibss(sdata, mgmt, skb->len);
 		break;
 	}
-
-	kfree_skb(skb);
 }
 
 void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
--- wireless-testing.orig/net/mac80211/mesh.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.c	2010-06-09 17:21:08.000000000 +0200
@@ -620,8 +620,6 @@ void ieee80211_mesh_rx_queued_mgmt(struc
 		ieee80211_mesh_rx_mgmt_action(sdata, mgmt, skb->len, rx_status);
 		break;
 	}
-
-	kfree_skb(skb);
 }
 
 void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata)
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:08.000000000 +0200
@@ -1754,7 +1754,7 @@ void ieee80211_sta_rx_queued_mgmt(struct
 		default:
 			WARN(1, "unexpected: %d", rma);
 		}
-		goto out;
+		return;
 	}
 
 	mutex_unlock(&ifmgd->mtx);
@@ -1799,9 +1799,6 @@ void ieee80211_sta_rx_queued_mgmt(struct
 
 		cfg80211_send_deauth(sdata->dev, (u8 *)mgmt, skb->len);
 	}
-
- out:
-	kfree_skb(skb);
 }
 
 static void ieee80211_sta_timer(unsigned long data)
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:08.000000000 +0200
@@ -738,9 +738,10 @@ static void ieee80211_iface_work(struct
 			break;
 		default:
 			WARN(1, "frame for unexpected interface type");
-			kfree_skb(skb);
 			break;
 		}
+
+		kfree_skb(skb);
 	}
 
 	/* then other type-dependent work */



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

* [PATCH 06/23] mac80211: pull mgmt frame rx into rx handler
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (4 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 05/23] mac80211: common work skb freeing Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 07/23] mac80211: always process blockack action from workqueue Johannes Berg
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Some code is duplicated between ibss, mesh and
managed mode regarding the queueing of management
frames. Since all modes now use a common skb
queue and a common work function, we can pull
the queueing code into the rx handler directly
and remove the duplicated length checks etc.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ibss.c        |   26 -------------------
 net/mac80211/ieee80211_i.h |    4 --
 net/mac80211/mesh.c        |   25 ------------------
 net/mac80211/mesh.h        |    2 -
 net/mac80211/mlme.c        |   27 -------------------
 net/mac80211/rx.c          |   61 +++++++++++++++++++++++++++++++++++----------
 6 files changed, 48 insertions(+), 97 deletions(-)

--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:07.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:08.000000000 +0200
@@ -981,8 +981,6 @@ int ieee80211_mgd_deauth(struct ieee8021
 int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 			   struct cfg80211_disassoc_request *req,
 			   void *cookie);
-ieee80211_rx_result ieee80211_sta_rx_mgmt(struct ieee80211_sub_if_data *sdata,
-					  struct sk_buff *skb);
 void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata);
 void ieee80211_recalc_ps(struct ieee80211_local *local, s32 latency);
@@ -1002,8 +1000,6 @@ void ieee80211_sta_rx_queued_mgmt(struct
 /* IBSS code */
 void ieee80211_ibss_notify_scan_completed(struct ieee80211_local *local);
 void ieee80211_ibss_setup_sdata(struct ieee80211_sub_if_data *sdata);
-ieee80211_rx_result
-ieee80211_ibss_rx_mgmt(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb);
 struct sta_info *ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
 					u8 *bssid, u8 *addr, u32 supp_rates,
 					gfp_t gfp);
--- wireless-testing.orig/net/mac80211/mesh.h	2010-06-09 17:20:50.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.h	2010-06-09 17:21:08.000000000 +0200
@@ -237,8 +237,6 @@ void ieee80211s_update_metric(struct iee
 		struct sta_info *stainfo, struct sk_buff *skb);
 void ieee80211s_stop(void);
 void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata);
-ieee80211_rx_result
-ieee80211_mesh_rx_mgmt(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb);
 void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata);
 void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata);
 void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh);
--- wireless-testing.orig/net/mac80211/ibss.c	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-06-09 17:21:08.000000000 +0200
@@ -847,32 +847,6 @@ void ieee80211_ibss_notify_scan_complete
 	mutex_unlock(&local->iflist_mtx);
 }
 
-ieee80211_rx_result
-ieee80211_ibss_rx_mgmt(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb)
-{
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_mgmt *mgmt;
-	u16 fc;
-
-	if (skb->len < 24)
-		return RX_DROP_MONITOR;
-
-	mgmt = (struct ieee80211_mgmt *) skb->data;
-	fc = le16_to_cpu(mgmt->frame_control);
-
-	switch (fc & IEEE80211_FCTL_STYPE) {
-	case IEEE80211_STYPE_PROBE_RESP:
-	case IEEE80211_STYPE_BEACON:
-	case IEEE80211_STYPE_PROBE_REQ:
-	case IEEE80211_STYPE_AUTH:
-		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &sdata->work);
-		return RX_QUEUED;
-	}
-
-	return RX_DROP_MONITOR;
-}
-
 int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 			struct cfg80211_ibss_params *params)
 {
--- wireless-testing.orig/net/mac80211/mesh.c	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/mesh.c	2010-06-09 17:21:08.000000000 +0200
@@ -702,28 +702,3 @@ void ieee80211_mesh_init_sdata(struct ie
 	INIT_LIST_HEAD(&ifmsh->preq_queue.list);
 	spin_lock_init(&ifmsh->mesh_preq_queue_lock);
 }
-
-ieee80211_rx_result
-ieee80211_mesh_rx_mgmt(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb)
-{
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_mgmt *mgmt;
-	u16 fc;
-
-	if (skb->len < 24)
-		return RX_DROP_MONITOR;
-
-	mgmt = (struct ieee80211_mgmt *) skb->data;
-	fc = le16_to_cpu(mgmt->frame_control);
-
-	switch (fc & IEEE80211_FCTL_STYPE) {
-	case IEEE80211_STYPE_ACTION:
-	case IEEE80211_STYPE_PROBE_RESP:
-	case IEEE80211_STYPE_BEACON:
-		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &sdata->work);
-		return RX_QUEUED;
-	}
-
-	return RX_CONTINUE;
-}
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:08.000000000 +0200
@@ -1633,33 +1633,6 @@ static void ieee80211_rx_mgmt_beacon(str
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
 
-ieee80211_rx_result ieee80211_sta_rx_mgmt(struct ieee80211_sub_if_data *sdata,
-					  struct sk_buff *skb)
-{
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_mgmt *mgmt;
-	u16 fc;
-
-	if (skb->len < 24)
-		return RX_DROP_MONITOR;
-
-	mgmt = (struct ieee80211_mgmt *) skb->data;
-	fc = le16_to_cpu(mgmt->frame_control);
-
-	switch (fc & IEEE80211_FCTL_STYPE) {
-	case IEEE80211_STYPE_PROBE_RESP:
-	case IEEE80211_STYPE_BEACON:
-	case IEEE80211_STYPE_DEAUTH:
-	case IEEE80211_STYPE_DISASSOC:
-	case IEEE80211_STYPE_ACTION:
-		skb_queue_tail(&sdata->skb_queue, skb);
-		ieee80211_queue_work(&local->hw, &sdata->work);
-		return RX_QUEUED;
-	}
-
-	return RX_DROP_MONITOR;
-}
-
 void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 				  struct sk_buff *skb)
 {
--- wireless-testing.orig/net/mac80211/rx.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-09 17:21:09.000000000 +0200
@@ -1950,8 +1950,11 @@ ieee80211_rx_h_action(struct ieee80211_r
 		if (len < IEEE80211_MIN_ACTION_SIZE + 1)
 			break;
 
-		if (sdata->vif.type == NL80211_IFTYPE_STATION)
-			return ieee80211_sta_rx_mgmt(sdata, rx->skb);
+		if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+			skb_queue_tail(&sdata->skb_queue, rx->skb);
+			ieee80211_queue_work(&local->hw, &sdata->work);
+			return RX_QUEUED;
+		}
 
 		switch (mgmt->u.action.u.addba_req.action_code) {
 		case WLAN_ACTION_ADDBA_REQ:
@@ -2003,7 +2006,9 @@ ieee80211_rx_h_action(struct ieee80211_r
 			if (memcmp(mgmt->bssid, sdata->u.mgd.bssid, ETH_ALEN))
 				break;
 
-			return ieee80211_sta_rx_mgmt(sdata, rx->skb);
+			skb_queue_tail(&sdata->skb_queue, rx->skb);
+			ieee80211_queue_work(&local->hw, &sdata->work);
+			return RX_QUEUED;
 		}
 		break;
 	case WLAN_CATEGORY_SA_QUERY:
@@ -2021,9 +2026,11 @@ ieee80211_rx_h_action(struct ieee80211_r
 		break;
 	case WLAN_CATEGORY_MESH_PLINK:
 	case WLAN_CATEGORY_MESH_PATH_SEL:
-		if (ieee80211_vif_is_mesh(&sdata->vif))
-			return ieee80211_mesh_rx_mgmt(sdata, rx->skb);
-		break;
+		if (!ieee80211_vif_is_mesh(&sdata->vif))
+			break;
+		skb_queue_tail(&sdata->skb_queue, rx->skb);
+		ieee80211_queue_work(&local->hw, &sdata->work);
+		return RX_QUEUED;
 	}
 
 	/*
@@ -2080,10 +2087,15 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_
 {
 	struct ieee80211_sub_if_data *sdata = rx->sdata;
 	ieee80211_rx_result rxs;
+	struct ieee80211_mgmt *mgmt = (void *)rx->skb->data;
+	__le16 stype;
 
 	if (!(rx->flags & IEEE80211_RX_RA_MATCH))
 		return RX_DROP_MONITOR;
 
+	if (rx->skb->len < 24)
+		return RX_DROP_MONITOR;
+
 	if (ieee80211_drop_unencrypted_mgmt(rx))
 		return RX_DROP_UNUSABLE;
 
@@ -2091,16 +2103,39 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_
 	if (rxs != RX_CONTINUE)
 		return rxs;
 
-	if (ieee80211_vif_is_mesh(&sdata->vif))
-		return ieee80211_mesh_rx_mgmt(sdata, rx->skb);
+	stype = mgmt->frame_control & cpu_to_le16(IEEE80211_FCTL_STYPE);
+
+	if (!ieee80211_vif_is_mesh(&sdata->vif) &&
+	    sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+	    sdata->vif.type != NL80211_IFTYPE_STATION)
+		return RX_DROP_MONITOR;
 
-	if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
-		return ieee80211_ibss_rx_mgmt(sdata, rx->skb);
+	switch (stype) {
+	case cpu_to_le16(IEEE80211_STYPE_BEACON):
+	case cpu_to_le16(IEEE80211_STYPE_PROBE_RESP):
+		/* process for all: mesh, mlme, ibss */
+		break;
+	case cpu_to_le16(IEEE80211_STYPE_DEAUTH):
+	case cpu_to_le16(IEEE80211_STYPE_DISASSOC):
+		/* process only for station */
+		if (sdata->vif.type != NL80211_IFTYPE_STATION)
+			return RX_DROP_MONITOR;
+		break;
+	case cpu_to_le16(IEEE80211_STYPE_PROBE_REQ):
+	case cpu_to_le16(IEEE80211_STYPE_AUTH):
+		/* process only for ibss */
+		if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
+			return RX_DROP_MONITOR;
+		break;
+	default:
+		return RX_DROP_MONITOR;
+	}
 
-	if (sdata->vif.type == NL80211_IFTYPE_STATION)
-		return ieee80211_sta_rx_mgmt(sdata, rx->skb);
+	/* queue up frame and kick off work to process it */
+	skb_queue_tail(&sdata->skb_queue, rx->skb);
+	ieee80211_queue_work(&rx->local->hw, &sdata->work);
 
-	return RX_DROP_MONITOR;
+	return RX_QUEUED;
 }
 
 static void ieee80211_rx_michael_mic_report(struct ieee80211_hdr *hdr,



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

* [PATCH 07/23] mac80211: always process blockack action from workqueue
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (5 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 06/23] mac80211: pull mgmt frame rx into rx handler Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 08/23] mac80211: move blockack stop due to fragmentation Johannes Berg
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

To prepare for making the ampdu_action callback
sleep, make mac80211 always process blockack
action frames from the skb queue. This gets rid
of the current special case for managed mode
interfaces as well.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c |   31 ++++++++++++++++++++++++++++++-
 net/mac80211/mlme.c  |   38 --------------------------------------
 net/mac80211/rx.c    |   29 +++++++++++++----------------
 3 files changed, 43 insertions(+), 55 deletions(-)

--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:09.000000000 +0200
@@ -724,7 +724,36 @@ static void ieee80211_iface_work(struct
 
 	/* first process frames */
 	while ((skb = skb_dequeue(&sdata->skb_queue))) {
-		switch (sdata->vif.type) {
+		struct ieee80211_mgmt *mgmt = (void *)skb->data;
+
+		if (ieee80211_is_action(mgmt->frame_control) &&
+		    mgmt->u.action.category == WLAN_CATEGORY_BACK) {
+			int len = skb->len;
+			struct sta_info *sta;
+
+			rcu_read_lock();
+			sta = sta_info_get(sdata, mgmt->sa);
+			if (sta) {
+				switch (mgmt->u.action.u.addba_req.action_code) {
+				case WLAN_ACTION_ADDBA_REQ:
+					ieee80211_process_addba_request(
+							local, sta, mgmt, len);
+					break;
+				case WLAN_ACTION_ADDBA_RESP:
+					ieee80211_process_addba_resp(local, sta,
+								     mgmt, len);
+					break;
+				case WLAN_ACTION_DELBA:
+					ieee80211_process_delba(sdata, sta,
+								mgmt, len);
+					break;
+				default:
+					WARN_ON(1);
+					break;
+				}
+			}
+			rcu_read_unlock();
+		} else switch (sdata->vif.type) {
 		case NL80211_IFTYPE_STATION:
 			ieee80211_sta_rx_queued_mgmt(sdata, skb);
 			break;
--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-09 17:21:09.000000000 +0200
@@ -1666,44 +1666,6 @@ void ieee80211_sta_rx_queued_mgmt(struct
 			break;
 		case IEEE80211_STYPE_ACTION:
 			switch (mgmt->u.action.category) {
-			case WLAN_CATEGORY_BACK: {
-				struct ieee80211_local *local = sdata->local;
-				int len = skb->len;
-				struct sta_info *sta;
-
-				rcu_read_lock();
-				sta = sta_info_get(sdata, mgmt->sa);
-				if (!sta) {
-					rcu_read_unlock();
-					break;
-				}
-
-				local_bh_disable();
-
-				switch (mgmt->u.action.u.addba_req.action_code) {
-				case WLAN_ACTION_ADDBA_REQ:
-					if (len < (IEEE80211_MIN_ACTION_SIZE +
-						   sizeof(mgmt->u.action.u.addba_req)))
-						break;
-					ieee80211_process_addba_request(local, sta, mgmt, len);
-					break;
-				case WLAN_ACTION_ADDBA_RESP:
-					if (len < (IEEE80211_MIN_ACTION_SIZE +
-						   sizeof(mgmt->u.action.u.addba_resp)))
-						break;
-					ieee80211_process_addba_resp(local, sta, mgmt, len);
-					break;
-				case WLAN_ACTION_DELBA:
-					if (len < (IEEE80211_MIN_ACTION_SIZE +
-						   sizeof(mgmt->u.action.u.delba)))
-						break;
-					ieee80211_process_delba(sdata, sta, mgmt, len);
-					break;
-				}
-				local_bh_enable();
-				rcu_read_unlock();
-				break;
-				}
 			case WLAN_CATEGORY_SPECTRUM_MGMT:
 				ieee80211_sta_process_chanswitch(sdata,
 						&mgmt->u.action.u.chan_switch.sw_elem,
--- wireless-testing.orig/net/mac80211/rx.c	2010-06-09 17:21:09.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-09 17:21:09.000000000 +0200
@@ -1950,33 +1950,29 @@ ieee80211_rx_h_action(struct ieee80211_r
 		if (len < IEEE80211_MIN_ACTION_SIZE + 1)
 			break;
 
-		if (sdata->vif.type == NL80211_IFTYPE_STATION) {
-			skb_queue_tail(&sdata->skb_queue, rx->skb);
-			ieee80211_queue_work(&local->hw, &sdata->work);
-			return RX_QUEUED;
-		}
-
 		switch (mgmt->u.action.u.addba_req.action_code) {
 		case WLAN_ACTION_ADDBA_REQ:
 			if (len < (IEEE80211_MIN_ACTION_SIZE +
 				   sizeof(mgmt->u.action.u.addba_req)))
-				return RX_DROP_MONITOR;
-			ieee80211_process_addba_request(local, rx->sta, mgmt, len);
-			goto handled;
+				goto invalid;
+			break;
 		case WLAN_ACTION_ADDBA_RESP:
 			if (len < (IEEE80211_MIN_ACTION_SIZE +
 				   sizeof(mgmt->u.action.u.addba_resp)))
-				break;
-			ieee80211_process_addba_resp(local, rx->sta, mgmt, len);
-			goto handled;
+				goto invalid;
+			break;
 		case WLAN_ACTION_DELBA:
 			if (len < (IEEE80211_MIN_ACTION_SIZE +
 				   sizeof(mgmt->u.action.u.delba)))
-				break;
-			ieee80211_process_delba(sdata, rx->sta, mgmt, len);
-			goto handled;
+				goto invalid;
+			break;
+		default:
+			goto invalid;
 		}
-		break;
+
+		skb_queue_tail(&sdata->skb_queue, rx->skb);
+		ieee80211_queue_work(&local->hw, &sdata->work);
+		return RX_QUEUED;
 	case WLAN_CATEGORY_SPECTRUM_MGMT:
 		if (local->hw.conf.channel->band != IEEE80211_BAND_5GHZ)
 			break;
@@ -2033,6 +2029,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 		return RX_QUEUED;
 	}
 
+ invalid:
 	/*
 	 * For AP mode, hostapd is responsible for handling any action
 	 * frames that we didn't handle, including returning unknown



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

* [PATCH 08/23] mac80211: move blockack stop due to fragmentation
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (6 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 07/23] mac80211: always process blockack action from workqueue Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 09/23] mac80211: move aggregation callback processing Johannes Berg
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

There's a corner case where we receive a fragmented
frame during a blockack session, in which case we
will terminate that session. To simplify future work
in this area that will culminate in allowing the
driver callbacks for aggregation to sleep, move the
processing of this case out of the RX path into the
interface work.

This will simplify future work because the new place
for this code doesn't require that the function will
always be atomic, which the RX path needs.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/iface.c |   32 +++++++++++++++++++++++++++++++-
 net/mac80211/rx.c    |    5 ++---
 2 files changed, 33 insertions(+), 4 deletions(-)

--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:09.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:10.000000000 +0200
@@ -707,6 +707,7 @@ static void ieee80211_iface_work(struct
 		container_of(work, struct ieee80211_sub_if_data, work);
 	struct ieee80211_local *local = sdata->local;
 	struct sk_buff *skb;
+	struct sta_info *sta;
 
 	if (!ieee80211_sdata_running(sdata))
 		return;
@@ -729,7 +730,6 @@ static void ieee80211_iface_work(struct
 		if (ieee80211_is_action(mgmt->frame_control) &&
 		    mgmt->u.action.category == WLAN_CATEGORY_BACK) {
 			int len = skb->len;
-			struct sta_info *sta;
 
 			rcu_read_lock();
 			sta = sta_info_get(sdata, mgmt->sa);
@@ -753,6 +753,36 @@ static void ieee80211_iface_work(struct
 				}
 			}
 			rcu_read_unlock();
+		} else if (ieee80211_is_data_qos(mgmt->frame_control)) {
+			struct ieee80211_hdr *hdr = (void *)mgmt;
+			/*
+			 * So the frame isn't mgmt, but frame_control
+			 * is at the right place anyway, of course, so
+			 * the if statement is correct.
+			 *
+			 * Warn if we have other data frame types here,
+			 * they must not get here.
+			 */
+			WARN_ON(hdr->frame_control &
+					cpu_to_le16(IEEE80211_STYPE_NULLFUNC));
+			WARN_ON(!(hdr->seq_ctrl &
+					cpu_to_le16(IEEE80211_SCTL_FRAG)));
+			/*
+			 * This was a fragment of a frame, received while
+			 * a block-ack session was active. That cannot be
+			 * right, so terminate the session.
+			 */
+			rcu_read_lock();
+			sta = sta_info_get(sdata, mgmt->sa);
+			if (sta) {
+				u16 tid = *ieee80211_get_qos_ctl(hdr) &
+						IEEE80211_QOS_CTL_TID_MASK;
+
+				__ieee80211_stop_rx_ba_session(
+					sta, tid, WLAN_BACK_RECIPIENT,
+					WLAN_REASON_QSTA_REQUIRE_SETUP);
+			}
+			rcu_read_unlock();
 		} else switch (sdata->vif.type) {
 		case NL80211_IFTYPE_STATION:
 			ieee80211_sta_rx_queued_mgmt(sdata, skb);
--- wireless-testing.orig/net/mac80211/rx.c	2010-06-09 17:21:09.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-09 17:21:10.000000000 +0200
@@ -741,9 +741,8 @@ static void ieee80211_rx_reorder_ampdu(s
 	sc = le16_to_cpu(hdr->seq_ctrl);
 	if (sc & IEEE80211_SCTL_FRAG) {
 		spin_unlock(&sta->lock);
-		__ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_RECIPIENT,
-					       WLAN_REASON_QSTA_REQUIRE_SETUP);
-		dev_kfree_skb(skb);
+		skb_queue_tail(&rx->sdata->skb_queue, skb);
+		ieee80211_queue_work(&local->hw, &rx->sdata->work);
 		return;
 	}
 



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

* [PATCH 09/23] mac80211: move aggregation callback processing
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (7 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 08/23] mac80211: move blockack stop due to fragmentation Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 10/23] mac80211: use RCU for RX aggregation Johannes Berg
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

This moves the aggregation callback processing
to the per-sdata skb queue and a work function
rather than the tasklet.

Unfortunately, this means that it extends the
pkt_type hack to that skb queue. However, it
will enable making ampdu_action API changes
gradually, my current plan is to get rid of
this again by forcing drivers to only return
from ampdu_action() when everything is done,
thus removing the callbacks completely.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c      |   14 ++++++--------
 net/mac80211/ieee80211_i.h |   11 +++++++----
 net/mac80211/iface.c       |   15 ++++++++++++---
 net/mac80211/main.c        |   13 -------------
 net/mac80211/rx.c          |    5 +++++
 5 files changed, 30 insertions(+), 28 deletions(-)

--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:08.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:10.000000000 +0200
@@ -564,11 +564,15 @@ ieee80211_sdata_set_mesh_id(struct ieee8
 #endif
 }
 
+enum sdata_queue_type {
+	IEEE80211_SDATA_QUEUE_TYPE_FRAME	= 0,
+	IEEE80211_SDATA_QUEUE_AGG_START		= 1,
+	IEEE80211_SDATA_QUEUE_AGG_STOP		= 2,
+};
+
 enum {
 	IEEE80211_RX_MSG	= 1,
 	IEEE80211_TX_STATUS_MSG	= 2,
-	IEEE80211_DELBA_MSG	= 3,
-	IEEE80211_ADDBA_MSG	= 4,
 };
 
 enum queue_stop_reason {
@@ -870,9 +874,8 @@ IEEE80211_DEV_TO_SUB_IF(struct net_devic
 	return netdev_priv(dev);
 }
 
-/* this struct represents 802.11n's RA/TID combination along with our vif */
+/* this struct represents 802.11n's RA/TID combination */
 struct ieee80211_ra_tid {
-	struct ieee80211_vif *vif;
 	u8 ra[ETH_ALEN];
 	u16 tid;
 };
--- wireless-testing.orig/net/mac80211/main.c	2010-06-09 17:20:49.000000000 +0200
+++ wireless-testing/net/mac80211/main.c	2010-06-09 17:21:10.000000000 +0200
@@ -259,7 +259,6 @@ static void ieee80211_tasklet_handler(un
 {
 	struct ieee80211_local *local = (struct ieee80211_local *) data;
 	struct sk_buff *skb;
-	struct ieee80211_ra_tid *ra_tid;
 
 	while ((skb = skb_dequeue(&local->skb_queue)) ||
 	       (skb = skb_dequeue(&local->skb_queue_unreliable))) {
@@ -274,18 +273,6 @@ static void ieee80211_tasklet_handler(un
 			skb->pkt_type = 0;
 			ieee80211_tx_status(local_to_hw(local), skb);
 			break;
-		case IEEE80211_DELBA_MSG:
-			ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
-			ieee80211_stop_tx_ba_cb(ra_tid->vif, ra_tid->ra,
-						ra_tid->tid);
-			dev_kfree_skb(skb);
-			break;
-		case IEEE80211_ADDBA_MSG:
-			ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
-			ieee80211_start_tx_ba_cb(ra_tid->vif, ra_tid->ra,
-						 ra_tid->tid);
-			dev_kfree_skb(skb);
-			break ;
 		default:
 			WARN(1, "mac80211: Packet is of unknown type %d\n",
 			     skb->pkt_type);
--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:10.000000000 +0200
@@ -502,11 +502,10 @@ void ieee80211_start_tx_ba_cb_irqsafe(st
 	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
 	memcpy(&ra_tid->ra, ra, ETH_ALEN);
 	ra_tid->tid = tid;
-	ra_tid->vif = vif;
 
-	skb->pkt_type = IEEE80211_ADDBA_MSG;
-	skb_queue_tail(&local->skb_queue, skb);
-	tasklet_schedule(&local->tasklet);
+	skb->pkt_type = IEEE80211_SDATA_QUEUE_AGG_START;
+	skb_queue_tail(&sdata->skb_queue, skb);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_irqsafe);
 
@@ -637,11 +636,10 @@ void ieee80211_stop_tx_ba_cb_irqsafe(str
 	ra_tid = (struct ieee80211_ra_tid *) &skb->cb;
 	memcpy(&ra_tid->ra, ra, ETH_ALEN);
 	ra_tid->tid = tid;
-	ra_tid->vif = vif;
 
-	skb->pkt_type = IEEE80211_DELBA_MSG;
-	skb_queue_tail(&local->skb_queue, skb);
-	tasklet_schedule(&local->tasklet);
+	skb->pkt_type = IEEE80211_SDATA_QUEUE_AGG_STOP;
+	skb_queue_tail(&sdata->skb_queue, skb);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb_irqsafe);
 
--- wireless-testing.orig/net/mac80211/rx.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-09 17:21:10.000000000 +0200
@@ -741,6 +741,7 @@ static void ieee80211_rx_reorder_ampdu(s
 	sc = le16_to_cpu(hdr->seq_ctrl);
 	if (sc & IEEE80211_SCTL_FRAG) {
 		spin_unlock(&sta->lock);
+		skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 		skb_queue_tail(&rx->sdata->skb_queue, skb);
 		ieee80211_queue_work(&local->hw, &rx->sdata->work);
 		return;
@@ -1969,6 +1970,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 			goto invalid;
 		}
 
+		rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 		skb_queue_tail(&sdata->skb_queue, rx->skb);
 		ieee80211_queue_work(&local->hw, &sdata->work);
 		return RX_QUEUED;
@@ -2001,6 +2003,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 			if (memcmp(mgmt->bssid, sdata->u.mgd.bssid, ETH_ALEN))
 				break;
 
+			rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 			skb_queue_tail(&sdata->skb_queue, rx->skb);
 			ieee80211_queue_work(&local->hw, &sdata->work);
 			return RX_QUEUED;
@@ -2023,6 +2026,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 	case WLAN_CATEGORY_MESH_PATH_SEL:
 		if (!ieee80211_vif_is_mesh(&sdata->vif))
 			break;
+		rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 		skb_queue_tail(&sdata->skb_queue, rx->skb);
 		ieee80211_queue_work(&local->hw, &sdata->work);
 		return RX_QUEUED;
@@ -2128,6 +2132,7 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_
 	}
 
 	/* queue up frame and kick off work to process it */
+	rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 	skb_queue_tail(&sdata->skb_queue, rx->skb);
 	ieee80211_queue_work(&rx->local->hw, &sdata->work);
 
--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:10.000000000 +0200
@@ -482,7 +482,7 @@ static int ieee80211_stop(struct net_dev
 		}
 		/* fall through */
 	default:
-		cancel_work_sync(&sdata->work);
+		flush_work(&sdata->work);
 		/*
 		 * When we get here, the interface is marked down.
 		 * Call synchronize_rcu() to wait for the RX path
@@ -708,6 +708,7 @@ static void ieee80211_iface_work(struct
 	struct ieee80211_local *local = sdata->local;
 	struct sk_buff *skb;
 	struct sta_info *sta;
+	struct ieee80211_ra_tid *ra_tid;
 
 	if (!ieee80211_sdata_running(sdata))
 		return;
@@ -727,8 +728,16 @@ static void ieee80211_iface_work(struct
 	while ((skb = skb_dequeue(&sdata->skb_queue))) {
 		struct ieee80211_mgmt *mgmt = (void *)skb->data;
 
-		if (ieee80211_is_action(mgmt->frame_control) &&
-		    mgmt->u.action.category == WLAN_CATEGORY_BACK) {
+		if (skb->pkt_type == IEEE80211_SDATA_QUEUE_AGG_START) {
+			ra_tid = (void *)&skb->cb;
+			ieee80211_start_tx_ba_cb(&sdata->vif, ra_tid->ra,
+						 ra_tid->tid);
+		} else if (skb->pkt_type == IEEE80211_SDATA_QUEUE_AGG_STOP) {
+			ra_tid = (void *)&skb->cb;
+			ieee80211_stop_tx_ba_cb(&sdata->vif, ra_tid->ra,
+						ra_tid->tid);
+		} else if (ieee80211_is_action(mgmt->frame_control) &&
+			   mgmt->u.action.category == WLAN_CATEGORY_BACK) {
 			int len = skb->len;
 
 			rcu_read_lock();



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

* [PATCH 10/23] mac80211: use RCU for RX aggregation
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (8 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 09/23] mac80211: move aggregation callback processing Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 11/23] mac80211: use RCU for TX aggregation Johannes Berg
                   ` (12 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Currently we allocate some memory for each RX
aggregation session and additionally keep a
flag indicating whether or not it is valid.
By using RCU to protect the pointer and making
sure that the memory is fully set up before it
becomes visible to the RX path, we can remove
the need for the bool that indicates validity,
as well as for locking on the RX path since it
is always synchronised against itself, and we
can guarantee that all other modifications are
done when the structure is not visible to the
RX path.

The net result is that since we remove locking
requirements from the RX path, we can in the
future use any kind of lock for the setup and
teardown code paths.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-rx.c      |   70 ++++++++++++++++++++-------------------------
 net/mac80211/debugfs_sta.c |    6 +--
 net/mac80211/rx.c          |   34 +++++++++------------
 net/mac80211/sta_info.h    |   14 +++++++--
 4 files changed, 61 insertions(+), 63 deletions(-)

--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:21:05.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:11.000000000 +0200
@@ -102,8 +102,18 @@ struct tid_ampdu_tx {
  * @buf_size: buffer size for incoming A-MPDUs
  * @timeout: reset timer value (in TUs).
  * @dialog_token: dialog token for aggregation session
+ * @rcu_head: RCU head used for freeing this struct
+ *
+ * This structure is protected by RCU and the per-station
+ * spinlock. Assignments to the array holding it must hold
+ * the spinlock, only the RX path can access it under RCU
+ * lock-free. The RX path, since it is single-threaded,
+ * can even modify the structure without locking since the
+ * only other modifications to it are done when the struct
+ * can not yet or no longer be found by the RX path.
  */
 struct tid_ampdu_rx {
+	struct rcu_head rcu_head;
 	struct sk_buff **reorder_buf;
 	unsigned long *reorder_time;
 	struct timer_list session_timer;
@@ -118,8 +128,7 @@ struct tid_ampdu_rx {
 /**
  * struct sta_ampdu_mlme - STA aggregation information.
  *
- * @tid_active_rx: TID's state in Rx session state machine.
- * @tid_rx: aggregation info for Rx per TID
+ * @tid_rx: aggregation info for Rx per TID -- RCU protected
  * @tid_state_tx: TID's state in Tx session state machine.
  * @tid_tx: aggregation info for Tx per TID
  * @addba_req_num: number of times addBA request has been sent.
@@ -127,7 +136,6 @@ struct tid_ampdu_rx {
  */
 struct sta_ampdu_mlme {
 	/* rx */
-	bool tid_active_rx[STA_TID_NUM];
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
 	/* tx */
 	u8 tid_state_tx[STA_TID_NUM];
--- wireless-testing.orig/net/mac80211/agg-rx.c	2010-06-09 17:20:49.000000000 +0200
+++ wireless-testing/net/mac80211/agg-rx.c	2010-06-09 17:21:11.000000000 +0200
@@ -19,25 +19,36 @@
 #include "ieee80211_i.h"
 #include "driver-ops.h"
 
+static void ieee80211_free_tid_rx(struct rcu_head *h)
+{
+	struct tid_ampdu_rx *tid_rx =
+		container_of(h, struct tid_ampdu_rx, rcu_head);
+	int i;
+
+	for (i = 0; i < tid_rx->buf_size; i++)
+		dev_kfree_skb(tid_rx->reorder_buf[i]);
+	kfree(tid_rx->reorder_buf);
+	kfree(tid_rx->reorder_time);
+	kfree(tid_rx);
+}
+
 static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
 					    u16 initiator, u16 reason,
 					    bool from_timer)
 {
 	struct ieee80211_local *local = sta->local;
 	struct tid_ampdu_rx *tid_rx;
-	int i;
 
 	spin_lock_bh(&sta->lock);
 
-	/* check if TID is in operational state */
-	if (!sta->ampdu_mlme.tid_active_rx[tid]) {
+	tid_rx = sta->ampdu_mlme.tid_rx[tid];
+
+	if (!tid_rx) {
 		spin_unlock_bh(&sta->lock);
 		return;
 	}
 
-	sta->ampdu_mlme.tid_active_rx[tid] = false;
-
-	tid_rx = sta->ampdu_mlme.tid_rx[tid];
+	rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], NULL);
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Rx BA session stop requested for %pM tid %u\n",
@@ -54,26 +65,12 @@ static void ___ieee80211_stop_rx_ba_sess
 		ieee80211_send_delba(sta->sdata, sta->sta.addr,
 				     tid, 0, reason);
 
-	/* free the reordering buffer */
-	for (i = 0; i < tid_rx->buf_size; i++) {
-		if (tid_rx->reorder_buf[i]) {
-			/* release the reordered frames */
-			dev_kfree_skb(tid_rx->reorder_buf[i]);
-			tid_rx->stored_mpdu_num--;
-			tid_rx->reorder_buf[i] = NULL;
-		}
-	}
-
-	/* free resources */
-	kfree(tid_rx->reorder_buf);
-	kfree(tid_rx->reorder_time);
-	sta->ampdu_mlme.tid_rx[tid] = NULL;
-
 	spin_unlock_bh(&sta->lock);
 
 	if (!from_timer)
 		del_timer_sync(&tid_rx->session_timer);
-	kfree(tid_rx);
+
+	call_rcu(&tid_rx->rcu_head, ieee80211_free_tid_rx);
 }
 
 void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
@@ -214,7 +211,7 @@ void ieee80211_process_addba_request(str
 	/* examine state machine */
 	spin_lock_bh(&sta->lock);
 
-	if (sta->ampdu_mlme.tid_active_rx[tid]) {
+	if (sta->ampdu_mlme.tid_rx[tid]) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		if (net_ratelimit())
 			printk(KERN_DEBUG "unexpected AddBA Req from "
@@ -225,9 +222,8 @@ void ieee80211_process_addba_request(str
 	}
 
 	/* prepare A-MPDU MLME for Rx aggregation */
-	sta->ampdu_mlme.tid_rx[tid] =
-			kmalloc(sizeof(struct tid_ampdu_rx), GFP_ATOMIC);
-	if (!sta->ampdu_mlme.tid_rx[tid]) {
+	tid_agg_rx = kmalloc(sizeof(struct tid_ampdu_rx), GFP_ATOMIC);
+	if (!tid_agg_rx) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		if (net_ratelimit())
 			printk(KERN_ERR "allocate rx mlme to tid %d failed\n",
@@ -235,14 +231,11 @@ void ieee80211_process_addba_request(str
 #endif
 		goto end;
 	}
-	/* rx timer */
-	sta->ampdu_mlme.tid_rx[tid]->session_timer.function =
-				sta_rx_agg_session_timer_expired;
-	sta->ampdu_mlme.tid_rx[tid]->session_timer.data =
-				(unsigned long)&sta->timer_to_tid[tid];
-	init_timer(&sta->ampdu_mlme.tid_rx[tid]->session_timer);
 
-	tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
+	/* rx timer */
+	tid_agg_rx->session_timer.function = sta_rx_agg_session_timer_expired;
+	tid_agg_rx->session_timer.data = (unsigned long)&sta->timer_to_tid[tid];
+	init_timer(&tid_agg_rx->session_timer);
 
 	/* prepare reordering buffer */
 	tid_agg_rx->reorder_buf =
@@ -257,8 +250,7 @@ void ieee80211_process_addba_request(str
 #endif
 		kfree(tid_agg_rx->reorder_buf);
 		kfree(tid_agg_rx->reorder_time);
-		kfree(sta->ampdu_mlme.tid_rx[tid]);
-		sta->ampdu_mlme.tid_rx[tid] = NULL;
+		kfree(tid_agg_rx);
 		goto end;
 	}
 
@@ -270,13 +262,12 @@ void ieee80211_process_addba_request(str
 
 	if (ret) {
 		kfree(tid_agg_rx->reorder_buf);
+		kfree(tid_agg_rx->reorder_time);
 		kfree(tid_agg_rx);
-		sta->ampdu_mlme.tid_rx[tid] = NULL;
 		goto end;
 	}
 
-	/* change state and send addba resp */
-	sta->ampdu_mlme.tid_active_rx[tid] = true;
+	/* update data */
 	tid_agg_rx->dialog_token = dialog_token;
 	tid_agg_rx->ssn = start_seq_num;
 	tid_agg_rx->head_seq_num = start_seq_num;
@@ -284,6 +275,9 @@ void ieee80211_process_addba_request(str
 	tid_agg_rx->timeout = timeout;
 	tid_agg_rx->stored_mpdu_num = 0;
 	status = WLAN_STATUS_SUCCESS;
+
+	/* activate it for RX */
+	rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], tid_agg_rx);
 end:
 	spin_unlock_bh(&sta->lock);
 
--- wireless-testing.orig/net/mac80211/rx.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-09 17:21:11.000000000 +0200
@@ -719,16 +719,13 @@ static void ieee80211_rx_reorder_ampdu(s
 
 	tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
 
-	spin_lock(&sta->lock);
-
-	if (!sta->ampdu_mlme.tid_active_rx[tid])
-		goto dont_reorder_unlock;
-
-	tid_agg_rx = sta->ampdu_mlme.tid_rx[tid];
+	tid_agg_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[tid]);
+	if (!tid_agg_rx)
+		goto dont_reorder;
 
 	/* qos null data frames are excluded */
 	if (unlikely(hdr->frame_control & cpu_to_le16(IEEE80211_STYPE_NULLFUNC)))
-		goto dont_reorder_unlock;
+		goto dont_reorder;
 
 	/* new, potentially un-ordered, ampdu frame - process it */
 
@@ -740,20 +737,22 @@ static void ieee80211_rx_reorder_ampdu(s
 	/* if this mpdu is fragmented - terminate rx aggregation session */
 	sc = le16_to_cpu(hdr->seq_ctrl);
 	if (sc & IEEE80211_SCTL_FRAG) {
-		spin_unlock(&sta->lock);
 		skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 		skb_queue_tail(&rx->sdata->skb_queue, skb);
 		ieee80211_queue_work(&local->hw, &rx->sdata->work);
 		return;
 	}
 
-	if (ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, frames)) {
-		spin_unlock(&sta->lock);
+	/*
+	 * No locking needed -- we will only ever process one
+	 * RX packet at a time, and thus own tid_agg_rx. All
+	 * other code manipulating it needs to (and does) make
+	 * sure that we cannot get to it any more before doing
+	 * anything with it.
+	 */
+	if (ieee80211_sta_manage_reorder_buf(hw, tid_agg_rx, skb, frames))
 		return;
-	}
 
- dont_reorder_unlock:
-	spin_unlock(&sta->lock);
  dont_reorder:
 	__skb_queue_tail(frames, skb);
 }
@@ -1830,13 +1829,11 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_
 				  &bar_data, sizeof(bar_data)))
 			return RX_DROP_MONITOR;
 
-		spin_lock(&rx->sta->lock);
 		tid = le16_to_cpu(bar_data.control) >> 12;
-		if (!rx->sta->ampdu_mlme.tid_active_rx[tid]) {
-			spin_unlock(&rx->sta->lock);
+
+		tid_agg_rx = rcu_dereference(rx->sta->ampdu_mlme.tid_rx[tid]);
+		if (!tid_agg_rx)
 			return RX_DROP_MONITOR;
-		}
-		tid_agg_rx = rx->sta->ampdu_mlme.tid_rx[tid];
 
 		start_seq_num = le16_to_cpu(bar_data.start_seq_num) >> 4;
 
@@ -1849,7 +1846,6 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_
 		ieee80211_release_reorder_frames(hw, tid_agg_rx, start_seq_num,
 						 frames);
 		kfree_skb(skb);
-		spin_unlock(&rx->sta->lock);
 		return RX_QUEUED;
 	}
 
--- wireless-testing.orig/net/mac80211/debugfs_sta.c	2010-06-09 17:20:49.000000000 +0200
+++ wireless-testing/net/mac80211/debugfs_sta.c	2010-06-09 17:21:11.000000000 +0200
@@ -125,12 +125,12 @@ static ssize_t sta_agg_status_read(struc
 	for (i = 0; i < STA_TID_NUM; i++) {
 		p += scnprintf(p, sizeof(buf) + buf - p, "%02d", i);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",
-				sta->ampdu_mlme.tid_active_rx[i]);
+				!!sta->ampdu_mlme.tid_rx[i]);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x",
-				sta->ampdu_mlme.tid_active_rx[i] ?
+				sta->ampdu_mlme.tid_rx[i] ?
 				sta->ampdu_mlme.tid_rx[i]->dialog_token : 0);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x",
-				sta->ampdu_mlme.tid_active_rx[i] ?
+				sta->ampdu_mlme.tid_rx[i] ?
 				sta->ampdu_mlme.tid_rx[i]->ssn : 0);
 
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",



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

* [PATCH 11/23] mac80211: use RCU for TX aggregation
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (9 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 10/23] mac80211: use RCU for RX aggregation Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 12/23] mac80211: remove non-irqsafe aggregation callbacks Johannes Berg
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Currently we allocate some memory for each TX
aggregation session and additionally keep a
state bitmap indicating the state it is in.
By using RCU to protect the pointer, moving
the state into the structure and some locking
trickery we can avoid locking when the TX agg
session is fully operational.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c              |  229 ++++++++++++++++++-------------------
 net/mac80211/debugfs_sta.c         |    8 -
 net/mac80211/ht.c                  |    9 -
 net/mac80211/ieee80211_i.h         |    2 
 net/mac80211/rc80211_minstrel_ht.c |    2 
 net/mac80211/sta_info.c            |   12 -
 net/mac80211/sta_info.h            |   33 +++--
 net/mac80211/tx.c                  |   88 +++++++++-----
 8 files changed, 206 insertions(+), 177 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:11.000000000 +0200
@@ -125,25 +125,42 @@ void ieee80211_send_bar(struct ieee80211
 	ieee80211_tx_skb(sdata, skb);
 }
 
-int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
-				    enum ieee80211_back_parties initiator)
+static void kfree_tid_tx(struct rcu_head *rcu_head)
+{
+	struct tid_ampdu_tx *tid_tx =
+	    container_of(rcu_head, struct tid_ampdu_tx, rcu_head);
+
+	kfree(tid_tx);
+}
+
+static int ___ieee80211_stop_tx_ba_session(
+		struct sta_info *sta, u16 tid,
+		enum ieee80211_back_parties initiator)
 {
 	struct ieee80211_local *local = sta->local;
+	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
 	int ret;
-	u8 *state;
+
+	lockdep_assert_held(&sta->lock);
+
+	if (WARN_ON(!tid_tx))
+		return -ENOENT;
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n",
 	       sta->sta.addr, tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
 
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
+	set_bit(HT_AGG_STATE_STOPPING, &tid_tx->state);
 
-	if (*state == HT_AGG_STATE_OPERATIONAL)
-		sta->ampdu_mlme.addba_req_num[tid] = 0;
+	/*
+	 * After this packets are no longer handed right through
+	 * to the driver but are put onto tid_tx->pending instead,
+	 * with locking to ensure proper access.
+	 */
+	clear_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state);
 
-	*state = HT_AGG_STATE_REQ_STOP_BA_MSK |
-		(initiator << HT_AGG_STATE_INITIATOR_SHIFT);
+	tid_tx->stop_initiator = initiator;
 
 	ret = drv_ampdu_action(local, sta->sdata,
 			       IEEE80211_AMPDU_TX_STOP,
@@ -174,15 +191,13 @@ static void sta_addba_resp_timer_expired
 	u16 tid = *(u8 *)data;
 	struct sta_info *sta = container_of((void *)data,
 		struct sta_info, timer_to_tid[tid]);
-	u8 *state;
-
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
+	struct tid_ampdu_tx *tid_tx;
 
 	/* check if the TID waits for addBA response */
 	spin_lock_bh(&sta->lock);
-	if ((*state & (HT_ADDBA_REQUESTED_MSK | HT_ADDBA_RECEIVED_MSK |
-		       HT_AGG_STATE_REQ_STOP_BA_MSK)) !=
-						HT_ADDBA_REQUESTED_MSK) {
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
+	if (!tid_tx ||
+	    test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) {
 		spin_unlock_bh(&sta->lock);
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "timer expired on tid %d but we are not "
@@ -210,7 +225,7 @@ int ieee80211_start_tx_ba_session(struct
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	struct ieee80211_local *local = sdata->local;
-	u8 *state;
+	struct tid_ampdu_tx *tid_tx;
 	int ret = 0;
 	u16 start_seq_num;
 
@@ -256,9 +271,9 @@ int ieee80211_start_tx_ba_session(struct
 		goto err_unlock_sta;
 	}
 
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 	/* check if the TID is not in aggregation flow already */
-	if (*state != HT_AGG_STATE_IDLE) {
+	if (tid_tx) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "BA request denied - session is not "
 				 "idle on tid %u\n", tid);
@@ -279,9 +294,8 @@ int ieee80211_start_tx_ba_session(struct
 		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
 
 	/* prepare A-MPDU MLME for Tx aggregation */
-	sta->ampdu_mlme.tid_tx[tid] =
-			kmalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC);
-	if (!sta->ampdu_mlme.tid_tx[tid]) {
+	tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC);
+	if (!tid_tx) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		if (net_ratelimit())
 			printk(KERN_ERR "allocate tx mlme to tid %d failed\n",
@@ -291,33 +305,27 @@ int ieee80211_start_tx_ba_session(struct
 		goto err_wake_queue;
 	}
 
-	skb_queue_head_init(&sta->ampdu_mlme.tid_tx[tid]->pending);
+	skb_queue_head_init(&tid_tx->pending);
 
 	/* Tx timer */
-	sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.function =
-			sta_addba_resp_timer_expired;
-	sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.data =
-			(unsigned long)&sta->timer_to_tid[tid];
-	init_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
-
-	/* Ok, the Addba frame hasn't been sent yet, but if the driver calls the
-	 * call back right away, it must see that the flow has begun */
-	*state |= HT_ADDBA_REQUESTED_MSK;
+	tid_tx->addba_resp_timer.function = sta_addba_resp_timer_expired;
+	tid_tx->addba_resp_timer.data = (unsigned long)&sta->timer_to_tid[tid];
+	init_timer(&tid_tx->addba_resp_timer);
 
 	start_seq_num = sta->tid_seq[tid] >> 4;
 
 	ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
 			       pubsta, tid, &start_seq_num);
-
 	if (ret) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "BA request denied - HW unavailable for"
 					" tid %d\n", tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
-		*state = HT_AGG_STATE_IDLE;
 		goto err_free;
 	}
 
+	rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
+
 	/* Driver vetoed or OKed, but we can take packets again now */
 	ieee80211_wake_queue_by_reason(
 		&local->hw, ieee80211_ac_from_tid(tid),
@@ -325,32 +333,30 @@ int ieee80211_start_tx_ba_session(struct
 
 	spin_unlock(&local->ampdu_lock);
 
+	/* activate the timer for the recipient's addBA response */
+	tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL;
+	add_timer(&tid_tx->addba_resp_timer);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
+#endif
+
 	/* prepare tid data */
 	sta->ampdu_mlme.dialog_token_allocator++;
-	sta->ampdu_mlme.tid_tx[tid]->dialog_token =
-			sta->ampdu_mlme.dialog_token_allocator;
-	sta->ampdu_mlme.tid_tx[tid]->ssn = start_seq_num;
+	tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator;
+	tid_tx->ssn = start_seq_num;
+
+	sta->ampdu_mlme.addba_req_num[tid]++;
 
 	spin_unlock_bh(&sta->lock);
 
 	/* send AddBA request */
 	ieee80211_send_addba_request(sdata, pubsta->addr, tid,
-			 sta->ampdu_mlme.tid_tx[tid]->dialog_token,
-			 sta->ampdu_mlme.tid_tx[tid]->ssn,
+			 tid_tx->dialog_token, tid_tx->ssn,
 			 0x40, 5000);
-	sta->ampdu_mlme.addba_req_num[tid]++;
-	/* activate the timer for the recipient's addBA response */
-	sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.expires =
-				jiffies + ADDBA_RESP_INTERVAL;
-	add_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
-#ifdef CONFIG_MAC80211_HT_DEBUG
-	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
-#endif
 	return 0;
 
  err_free:
-	kfree(sta->ampdu_mlme.tid_tx[tid]);
-	sta->ampdu_mlme.tid_tx[tid] = NULL;
+	kfree(tid_tx);
  err_wake_queue:
 	ieee80211_wake_queue_by_reason(
 		&local->hw, ieee80211_ac_from_tid(tid),
@@ -368,7 +374,8 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_sess
  * local->ampdu_lock across both calls.
  */
 static void ieee80211_agg_splice_packets(struct ieee80211_local *local,
-					 struct sta_info *sta, u16 tid)
+					 struct tid_ampdu_tx *tid_tx,
+					 u16 tid)
 {
 	unsigned long flags;
 	u16 queue = ieee80211_ac_from_tid(tid);
@@ -377,31 +384,23 @@ static void ieee80211_agg_splice_packets
 		&local->hw, queue,
 		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
 
-	if (!(sta->ampdu_mlme.tid_state_tx[tid] & HT_ADDBA_REQUESTED_MSK))
-		return;
-
-	if (WARN(!sta->ampdu_mlme.tid_tx[tid],
-		 "TID %d gone but expected when splicing aggregates from"
-		 "the pending queue\n", tid))
+	if (WARN(!tid_tx, "TID %d gone but expected when splicing aggregates"
+			  " from the pending queue\n", tid))
 		return;
 
-	if (!skb_queue_empty(&sta->ampdu_mlme.tid_tx[tid]->pending)) {
+	if (!skb_queue_empty(&tid_tx->pending)) {
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 		/* copy over remaining packets */
-		skb_queue_splice_tail_init(
-			&sta->ampdu_mlme.tid_tx[tid]->pending,
-			&local->pending[queue]);
+		skb_queue_splice_tail_init(&tid_tx->pending,
+					   &local->pending[queue]);
 		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 	}
 }
 
-static void ieee80211_agg_splice_finish(struct ieee80211_local *local,
-					struct sta_info *sta, u16 tid)
+static void ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid)
 {
-	u16 queue = ieee80211_ac_from_tid(tid);
-
 	ieee80211_wake_queue_by_reason(
-		&local->hw, queue,
+		&local->hw, ieee80211_ac_from_tid(tid),
 		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
 }
 
@@ -409,19 +408,21 @@ static void ieee80211_agg_splice_finish(
 static void ieee80211_agg_tx_operational(struct ieee80211_local *local,
 					 struct sta_info *sta, u16 tid)
 {
+	lockdep_assert_held(&sta->lock);
+
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid);
 #endif
 
 	spin_lock(&local->ampdu_lock);
-	ieee80211_agg_splice_packets(local, sta, tid);
+	ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid);
 	/*
-	 * NB: we rely on sta->lock being taken in the TX
-	 * processing here when adding to the pending queue,
-	 * otherwise we could only change the state of the
-	 * session to OPERATIONAL _here_.
+	 * Now mark as operational. This will be visible
+	 * in the TX path, and lets it go lock-free in
+	 * the common case.
 	 */
-	ieee80211_agg_splice_finish(local, sta, tid);
+	set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state);
+	ieee80211_agg_splice_finish(local, tid);
 	spin_unlock(&local->ampdu_lock);
 
 	drv_ampdu_action(local, sta->sdata,
@@ -434,7 +435,7 @@ void ieee80211_start_tx_ba_cb(struct iee
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
 	struct ieee80211_local *local = sdata->local;
 	struct sta_info *sta;
-	u8 *state;
+	struct tid_ampdu_tx *tid_tx;
 
 	trace_api_start_tx_ba_cb(sdata, ra, tid);
 
@@ -456,25 +457,22 @@ void ieee80211_start_tx_ba_cb(struct iee
 		return;
 	}
 
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
 	spin_lock_bh(&sta->lock);
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
-	if (WARN_ON(!(*state & HT_ADDBA_REQUESTED_MSK))) {
+	if (WARN_ON(!tid_tx)) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
-		printk(KERN_DEBUG "addBA was not requested yet, state is %d\n",
-				*state);
+		printk(KERN_DEBUG "addBA was not requested!\n");
 #endif
 		spin_unlock_bh(&sta->lock);
 		rcu_read_unlock();
 		return;
 	}
 
-	if (WARN_ON(*state & HT_ADDBA_DRV_READY_MSK))
+	if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state)))
 		goto out;
 
-	*state |= HT_ADDBA_DRV_READY_MSK;
-
-	if (*state == HT_AGG_STATE_OPERATIONAL)
+	if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state))
 		ieee80211_agg_tx_operational(local, sta, tid);
 
  out:
@@ -512,14 +510,14 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_i
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator)
 {
-	u8 *state;
+	struct tid_ampdu_tx *tid_tx;
 	int ret;
 
-	/* check if the TID is in aggregation */
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
 	spin_lock_bh(&sta->lock);
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
-	if (*state != HT_AGG_STATE_OPERATIONAL) {
+	/* check if the TID is in aggregation */
+	if (!tid_tx || !test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -554,7 +552,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
 	struct ieee80211_local *local = sdata->local;
 	struct sta_info *sta;
-	u8 *state;
+	struct tid_ampdu_tx *tid_tx;
 
 	trace_api_stop_tx_ba_cb(sdata, ra, tid);
 
@@ -580,39 +578,45 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 		rcu_read_unlock();
 		return;
 	}
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
 
-	/* NOTE: no need to use sta->lock in this state check, as
-	 * ieee80211_stop_tx_ba_session will let only one stop call to
-	 * pass through per sta/tid
-	 */
-	if ((*state & HT_AGG_STATE_REQ_STOP_BA_MSK) == 0) {
+	spin_lock_bh(&sta->lock);
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
+
+	if (!tid_tx || !test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n");
 #endif
+		spin_unlock_bh(&sta->lock);
 		rcu_read_unlock();
 		return;
 	}
 
-	if (*state & HT_AGG_STATE_INITIATOR_MSK)
+	if (tid_tx->stop_initiator == WLAN_BACK_INITIATOR)
 		ieee80211_send_delba(sta->sdata, ra, tid,
 			WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);
 
-	spin_lock_bh(&sta->lock);
-	spin_lock(&local->ampdu_lock);
+	/*
+	 * When we get here, the TX path will not be lockless any more wrt.
+	 * aggregation, since the OPERATIONAL bit has long been cleared.
+	 * Thus it will block on getting the lock, if it occurs. So if we
+	 * stop the queue now, we will not get any more packets, and any
+	 * that might be being processed will wait for us here, thereby
+	 * guaranteeing that no packets go to the tid_tx pending queue any
+	 * more.
+	 */
 
-	ieee80211_agg_splice_packets(local, sta, tid);
+	spin_lock(&local->ampdu_lock);
+	ieee80211_agg_splice_packets(local, tid_tx, tid);
 
-	*state = HT_AGG_STATE_IDLE;
-	/* from now on packets are no longer put onto sta->pending */
-	kfree(sta->ampdu_mlme.tid_tx[tid]);
-	sta->ampdu_mlme.tid_tx[tid] = NULL;
+	/* future packets must not find the tid_tx struct any more */
+	rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
 
-	ieee80211_agg_splice_finish(local, sta, tid);
+	ieee80211_agg_splice_finish(local, tid);
 
+	call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 	spin_unlock(&local->ampdu_lock);
-	spin_unlock_bh(&sta->lock);
 
+	spin_unlock_bh(&sta->lock);
 	rcu_read_unlock();
 }
 EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb);
@@ -649,40 +653,41 @@ void ieee80211_process_addba_resp(struct
 				  struct ieee80211_mgmt *mgmt,
 				  size_t len)
 {
+	struct tid_ampdu_tx *tid_tx;
 	u16 capab, tid;
-	u8 *state;
 
 	capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab);
 	tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
 
-	state = &sta->ampdu_mlme.tid_state_tx[tid];
-
 	spin_lock_bh(&sta->lock);
 
-	if (!(*state & HT_ADDBA_REQUESTED_MSK))
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
+
+	if (!tid_tx)
 		goto out;
 
-	if (mgmt->u.action.u.addba_resp.dialog_token !=
-		sta->ampdu_mlme.tid_tx[tid]->dialog_token) {
+	if (mgmt->u.action.u.addba_resp.dialog_token != tid_tx->dialog_token) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "wrong addBA response token, tid %d\n", tid);
-#endif /* CONFIG_MAC80211_HT_DEBUG */
+#endif
 		goto out;
 	}
 
-	del_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
+	del_timer(&tid_tx->addba_resp_timer);
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "switched off addBA timer for tid %d\n", tid);
-#endif /* CONFIG_MAC80211_HT_DEBUG */
+#endif
 
 	if (le16_to_cpu(mgmt->u.action.u.addba_resp.status)
 			== WLAN_STATUS_SUCCESS) {
-		u8 curstate = *state;
-
-		*state |= HT_ADDBA_RECEIVED_MSK;
+		if (test_and_set_bit(HT_AGG_STATE_RESPONSE_RECEIVED,
+				     &tid_tx->state)) {
+			/* ignore duplicate response */
+			goto out;
+		}
 
-		if (*state != curstate && *state == HT_AGG_STATE_OPERATIONAL)
+		if (test_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state))
 			ieee80211_agg_tx_operational(local, sta, tid);
 
 		sta->ampdu_mlme.addba_req_num[tid] = 0;
--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:11.000000000 +0200
@@ -61,33 +61,40 @@ enum ieee80211_sta_info_flags {
 
 #define STA_TID_NUM 16
 #define ADDBA_RESP_INTERVAL HZ
-#define HT_AGG_MAX_RETRIES		(0x3)
+#define HT_AGG_MAX_RETRIES		0x3
 
-#define HT_AGG_STATE_INITIATOR_SHIFT	(4)
-
-#define HT_ADDBA_REQUESTED_MSK		BIT(0)
-#define HT_ADDBA_DRV_READY_MSK		BIT(1)
-#define HT_ADDBA_RECEIVED_MSK		BIT(2)
-#define HT_AGG_STATE_REQ_STOP_BA_MSK	BIT(3)
-#define HT_AGG_STATE_INITIATOR_MSK      BIT(HT_AGG_STATE_INITIATOR_SHIFT)
-#define HT_AGG_STATE_IDLE		(0x0)
-#define HT_AGG_STATE_OPERATIONAL	(HT_ADDBA_REQUESTED_MSK |	\
-					 HT_ADDBA_DRV_READY_MSK |	\
-					 HT_ADDBA_RECEIVED_MSK)
+#define HT_AGG_STATE_DRV_READY		0
+#define HT_AGG_STATE_RESPONSE_RECEIVED	1
+#define HT_AGG_STATE_OPERATIONAL	2
+#define HT_AGG_STATE_STOPPING		3
 
 /**
  * struct tid_ampdu_tx - TID aggregation information (Tx).
  *
+ * @rcu_head: rcu head for freeing structure
  * @addba_resp_timer: timer for peer's response to addba request
  * @pending: pending frames queue -- use sta's spinlock to protect
  * @ssn: Starting Sequence Number expected to be aggregated.
  * @dialog_token: dialog token for aggregation session
+ * @state: session state (see above)
+ * @stop_initiator: initiator of a session stop
+ *
+ * This structure is protected by RCU and the per-station
+ * spinlock. Assignments to the array holding it must hold
+ * the spinlock, only the TX path can access it under RCU
+ * lock-free if, and only if, the state has  the flag
+ * %HT_AGG_STATE_OPERATIONAL set. Otherwise, the TX path
+ * must also acquire the spinlock and re-check the state,
+ * see comments in the tx code touching it.
  */
 struct tid_ampdu_tx {
+	struct rcu_head rcu_head;
 	struct timer_list addba_resp_timer;
 	struct sk_buff_head pending;
+	unsigned long state;
 	u16 ssn;
 	u8 dialog_token;
+	u8 stop_initiator;
 };
 
 /**
@@ -129,7 +136,6 @@ struct tid_ampdu_rx {
  * struct sta_ampdu_mlme - STA aggregation information.
  *
  * @tid_rx: aggregation info for Rx per TID -- RCU protected
- * @tid_state_tx: TID's state in Tx session state machine.
  * @tid_tx: aggregation info for Tx per TID
  * @addba_req_num: number of times addBA request has been sent.
  * @dialog_token_allocator: dialog token enumerator for each new session;
@@ -138,7 +144,6 @@ struct sta_ampdu_mlme {
 	/* rx */
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
 	/* tx */
-	u8 tid_state_tx[STA_TID_NUM];
 	struct tid_ampdu_tx *tid_tx[STA_TID_NUM];
 	u8 addba_req_num[STA_TID_NUM];
 	u8 dialog_token_allocator;
--- wireless-testing.orig/net/mac80211/tx.c	2010-06-09 17:20:48.000000000 +0200
+++ wireless-testing/net/mac80211/tx.c	2010-06-09 17:21:11.000000000 +0200
@@ -1092,6 +1092,54 @@ static bool __ieee80211_parse_tx_radiota
 	return true;
 }
 
+static bool ieee80211_tx_prep_agg(struct ieee80211_tx_data *tx,
+				  struct sk_buff *skb,
+				  struct ieee80211_tx_info *info,
+				  struct tid_ampdu_tx *tid_tx,
+				  int tid)
+{
+	bool queued = false;
+
+	if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
+		info->flags |= IEEE80211_TX_CTL_AMPDU;
+	} else {
+		spin_lock(&tx->sta->lock);
+		/*
+		 * Need to re-check now, because we may get here
+		 *
+		 *  1) in the window during which the setup is actually
+		 *     already done, but not marked yet because not all
+		 *     packets are spliced over to the driver pending
+		 *     queue yet -- if this happened we acquire the lock
+		 *     either before or after the splice happens, but
+		 *     need to recheck which of these cases happened.
+		 *
+		 *  2) during session teardown, if the OPERATIONAL bit
+		 *     was cleared due to the teardown but the pointer
+		 *     hasn't been assigned NULL yet (or we loaded it
+		 *     before it was assigned) -- in this case it may
+		 *     now be NULL which means we should just let the
+		 *     packet pass through because splicing the frames
+		 *     back is already done.
+		 */
+		tid_tx = tx->sta->ampdu_mlme.tid_tx[tid];
+
+		if (!tid_tx) {
+			/* do nothing, let packet pass through */
+		} else if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
+			info->flags |= IEEE80211_TX_CTL_AMPDU;
+		} else {
+			queued = true;
+			info->control.vif = &tx->sdata->vif;
+			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
+			__skb_queue_tail(&tid_tx->pending, skb);
+		}
+		spin_unlock(&tx->sta->lock);
+	}
+
+	return queued;
+}
+
 /*
  * initialises @tx
  */
@@ -1104,8 +1152,7 @@ ieee80211_tx_prepare(struct ieee80211_su
 	struct ieee80211_hdr *hdr;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	int hdrlen, tid;
-	u8 *qc, *state;
-	bool queued = false;
+	u8 *qc;
 
 	memset(tx, 0, sizeof(*tx));
 	tx->skb = skb;
@@ -1157,35 +1204,16 @@ ieee80211_tx_prepare(struct ieee80211_su
 		qc = ieee80211_get_qos_ctl(hdr);
 		tid = *qc & IEEE80211_QOS_CTL_TID_MASK;
 
-		spin_lock(&tx->sta->lock);
-		/*
-		 * XXX: This spinlock could be fairly expensive, but see the
-		 *	comment in agg-tx.c:ieee80211_agg_tx_operational().
-		 *	One way to solve this would be to do something RCU-like
-		 *	for managing the tid_tx struct and using atomic bitops
-		 *	for the actual state -- by introducing an actual
-		 *	'operational' bit that would be possible. It would
-		 *	require changing ieee80211_agg_tx_operational() to
-		 *	set that bit, and changing the way tid_tx is managed
-		 *	everywhere, including races between that bit and
-		 *	tid_tx going away (tid_tx being added can be easily
-		 *	committed to memory before the 'operational' bit).
-		 */
-		tid_tx = tx->sta->ampdu_mlme.tid_tx[tid];
-		state = &tx->sta->ampdu_mlme.tid_state_tx[tid];
-		if (*state == HT_AGG_STATE_OPERATIONAL) {
-			info->flags |= IEEE80211_TX_CTL_AMPDU;
-		} else if (*state != HT_AGG_STATE_IDLE) {
-			/* in progress */
-			queued = true;
-			info->control.vif = &sdata->vif;
-			info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
-			__skb_queue_tail(&tid_tx->pending, skb);
-		}
-		spin_unlock(&tx->sta->lock);
+		tid_tx = rcu_dereference(tx->sta->ampdu_mlme.tid_tx[tid]);
+		if (tid_tx) {
+			bool queued;
+
+			queued = ieee80211_tx_prep_agg(tx, skb, info,
+						       tid_tx, tid);
 
-		if (unlikely(queued))
-			return TX_QUEUED;
+			if (unlikely(queued))
+				return TX_QUEUED;
+		}
 	}
 
 	if (is_multicast_ether_addr(hdr->addr1)) {
--- wireless-testing.orig/net/mac80211/sta_info.c	2010-06-09 17:20:48.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.c	2010-06-09 17:21:11.000000000 +0200
@@ -246,14 +246,12 @@ struct sta_info *sta_info_alloc(struct i
 	}
 
 	for (i = 0; i < STA_TID_NUM; i++) {
-		/* timer_to_tid must be initialized with identity mapping to
-		 * enable session_timer's data differentiation. refer to
-		 * sta_rx_agg_session_timer_expired for useage */
+		/*
+		 * timer_to_tid must be initialized with identity mapping
+		 * to enable session_timer's data differentiation. See
+		 * sta_rx_agg_session_timer_expired for usage.
+		 */
 		sta->timer_to_tid[i] = i;
-		/* tx */
-		sta->ampdu_mlme.tid_state_tx[i] = HT_AGG_STATE_IDLE;
-		sta->ampdu_mlme.tid_tx[i] = NULL;
-		sta->ampdu_mlme.addba_req_num[i] = 0;
 	}
 	skb_queue_head_init(&sta->ps_tx_buf);
 	skb_queue_head_init(&sta->tx_filtered);
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:20:48.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:11.000000000 +0200
@@ -176,13 +176,8 @@ void ieee80211_process_delba(struct ieee
 
 	if (initiator == WLAN_BACK_INITIATOR)
 		__ieee80211_stop_rx_ba_session(sta, tid, WLAN_BACK_INITIATOR, 0);
-	else { /* WLAN_BACK_RECIPIENT */
-		spin_lock_bh(&sta->lock);
-		if (sta->ampdu_mlme.tid_state_tx[tid] & HT_ADDBA_REQUESTED_MSK)
-			___ieee80211_stop_tx_ba_session(sta, tid,
-							WLAN_BACK_RECIPIENT);
-		spin_unlock_bh(&sta->lock);
-	}
+	else
+		__ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_RECIPIENT);
 }
 
 int ieee80211_send_smps_action(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/debugfs_sta.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/debugfs_sta.c	2010-06-09 17:21:11.000000000 +0200
@@ -134,15 +134,15 @@ static ssize_t sta_agg_status_read(struc
 				sta->ampdu_mlme.tid_rx[i]->ssn : 0);
 
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",
-				sta->ampdu_mlme.tid_state_tx[i]);
+				!!sta->ampdu_mlme.tid_tx[i]);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x",
-				sta->ampdu_mlme.tid_state_tx[i] ?
+				sta->ampdu_mlme.tid_tx[i] ?
 				sta->ampdu_mlme.tid_tx[i]->dialog_token : 0);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x",
-				sta->ampdu_mlme.tid_state_tx[i] ?
+				sta->ampdu_mlme.tid_tx[i] ?
 				sta->ampdu_mlme.tid_tx[i]->ssn : 0);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%03d",
-				sta->ampdu_mlme.tid_state_tx[i] ?
+				sta->ampdu_mlme.tid_tx[i] ?
 				skb_queue_len(&sta->ampdu_mlme.tid_tx[i]->pending) : 0);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\n");
 	}
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:11.000000000 +0200
@@ -1119,8 +1119,6 @@ void ieee80211_process_addba_request(str
 
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator);
-int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
-				    enum ieee80211_back_parties initiator);
 
 /* Spectrum management */
 void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/rc80211_minstrel_ht.c	2010-06-09 17:20:48.000000000 +0200
+++ wireless-testing/net/mac80211/rc80211_minstrel_ht.c	2010-06-09 17:21:11.000000000 +0200
@@ -365,7 +365,7 @@ minstrel_aggr_check(struct minstrel_priv
 		return;
 
 	tid = *ieee80211_get_qos_ctl(hdr) & IEEE80211_QOS_CTL_TID_MASK;
-	if (likely(sta->ampdu_mlme.tid_state_tx[tid] != HT_AGG_STATE_IDLE))
+	if (likely(sta->ampdu_mlme.tid_tx[tid]))
 		return;
 
 	ieee80211_start_tx_ba_session(pubsta, tid);



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

* [PATCH 12/23] mac80211: remove non-irqsafe aggregation callbacks
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (10 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 11/23] mac80211: use RCU for TX aggregation Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 13/23] mac80211: refcount aggregation queue stop Johannes Berg
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

The non-irqsafe aggregation start/stop done
callbacks are currently only used by ath9k_htc,
and can cause callbacks into the driver again.
This might lead to locking issues, which will
only get worse as we modify locking. To avoid
trouble, remove the non-irqsafe versions and
change ath9k_htc to use those instead.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/ath/ath9k/htc_drv_main.c |    6 ++--
 include/net/mac80211.h                        |   32 ++++----------------------
 net/mac80211/agg-tx.c                         |    2 -
 net/mac80211/ieee80211_i.h                    |    2 +
 4 files changed, 10 insertions(+), 32 deletions(-)

--- wireless-testing.orig/include/net/mac80211.h	2010-06-09 17:21:02.000000000 +0200
+++ wireless-testing/include/net/mac80211.h	2010-06-09 17:21:12.000000000 +0200
@@ -1445,7 +1445,7 @@ enum ieee80211_filter_flags {
  *
  * Note that drivers MUST be able to deal with a TX aggregation
  * session being stopped even before they OK'ed starting it by
- * calling ieee80211_start_tx_ba_cb(_irqsafe), because the peer
+ * calling ieee80211_start_tx_ba_cb_irqsafe, because the peer
  * might receive the addBA frame and send a delBA right away!
  *
  * @IEEE80211_AMPDU_RX_START: start Rx aggregation
@@ -2314,25 +2314,14 @@ void ieee80211_queue_delayed_work(struct
 int ieee80211_start_tx_ba_session(struct ieee80211_sta *sta, u16 tid);
 
 /**
- * ieee80211_start_tx_ba_cb - low level driver ready to aggregate.
- * @vif: &struct ieee80211_vif pointer from the add_interface callback
- * @ra: receiver address of the BA session recipient.
- * @tid: the TID to BA on.
- *
- * This function must be called by low level driver once it has
- * finished with preparations for the BA session.
- */
-void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid);
-
-/**
  * ieee80211_start_tx_ba_cb_irqsafe - low level driver ready to aggregate.
  * @vif: &struct ieee80211_vif pointer from the add_interface callback
  * @ra: receiver address of the BA session recipient.
  * @tid: the TID to BA on.
  *
  * This function must be called by low level driver once it has
- * finished with preparations for the BA session.
- * This version of the function is IRQ-safe.
+ * finished with preparations for the BA session. It can be called
+ * from any context.
  */
 void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra,
 				      u16 tid);
@@ -2351,25 +2340,14 @@ void ieee80211_start_tx_ba_cb_irqsafe(st
 int ieee80211_stop_tx_ba_session(struct ieee80211_sta *sta, u16 tid);
 
 /**
- * ieee80211_stop_tx_ba_cb - low level driver ready to stop aggregate.
- * @vif: &struct ieee80211_vif pointer from the add_interface callback
- * @ra: receiver address of the BA session recipient.
- * @tid: the desired TID to BA on.
- *
- * This function must be called by low level driver once it has
- * finished with preparations for the BA session tear down.
- */
-void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid);
-
-/**
  * ieee80211_stop_tx_ba_cb_irqsafe - low level driver ready to stop aggregate.
  * @vif: &struct ieee80211_vif pointer from the add_interface callback
  * @ra: receiver address of the BA session recipient.
  * @tid: the desired TID to BA on.
  *
  * This function must be called by low level driver once it has
- * finished with preparations for the BA session tear down.
- * This version of the function is IRQ-safe.
+ * finished with preparations for the BA session tear down. It
+ * can be called from any context.
  */
 void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif, const u8 *ra,
 				     u16 tid);
--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:12.000000000 +0200
@@ -479,7 +479,6 @@ void ieee80211_start_tx_ba_cb(struct iee
 	spin_unlock_bh(&sta->lock);
 	rcu_read_unlock();
 }
-EXPORT_SYMBOL(ieee80211_start_tx_ba_cb);
 
 void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
 				      const u8 *ra, u16 tid)
@@ -619,7 +618,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	spin_unlock_bh(&sta->lock);
 	rcu_read_unlock();
 }
-EXPORT_SYMBOL(ieee80211_stop_tx_ba_cb);
 
 void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
 				     const u8 *ra, u16 tid)
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:12.000000000 +0200
@@ -1119,6 +1119,8 @@ void ieee80211_process_addba_request(str
 
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator);
+void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid);
+void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid);
 
 /* Spectrum management */
 void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/htc_drv_main.c	2010-06-09 17:20:48.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath/ath9k/htc_drv_main.c	2010-06-09 17:21:12.000000000 +0200
@@ -510,13 +510,13 @@ void ath9k_htc_aggr_work(struct work_str
 		ret = ath9k_htc_aggr_oper(priv, wk->vif, wk->sta_addr,
 					  wk->tid, true);
 		if (!ret)
-			ieee80211_start_tx_ba_cb(wk->vif, wk->sta_addr,
-						 wk->tid);
+			ieee80211_start_tx_ba_cb_irqsafe(wk->vif, wk->sta_addr,
+							 wk->tid);
 		break;
 	case IEEE80211_AMPDU_TX_STOP:
 		ath9k_htc_aggr_oper(priv, wk->vif, wk->sta_addr,
 				    wk->tid, false);
-		ieee80211_stop_tx_ba_cb(wk->vif, wk->sta_addr, wk->tid);
+		ieee80211_stop_tx_ba_cb_irqsafe(wk->vif, wk->sta_addr, wk->tid);
 		break;
 	default:
 		ath_print(ath9k_hw_common(priv->ah), ATH_DBG_FATAL,



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

* [PATCH 13/23] mac80211: refcount aggregation queue stop
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (11 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 12/23] mac80211: remove non-irqsafe aggregation callbacks Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 14/23] mac80211: make TX aggregation start/stop request async Johannes Berg
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

mac80211 currently maintains the ampdu_lock to
avoid starting a queue due to one aggregation
session while another aggregation session needs
the queue stopped.

We can do better, however, and instead refcount
the queue stops for this particular purpose,
thus removing the need for the lock. This will
help making ampdu_action able to sleep.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c      |   77 +++++++++++++++++++++++++++------------------
 net/mac80211/ieee80211_i.h |    8 ----
 net/mac80211/main.c        |    6 +--
 3 files changed, 51 insertions(+), 40 deletions(-)

--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:12.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:13.000000000 +0200
@@ -723,13 +723,7 @@ struct ieee80211_local {
 	struct sk_buff_head pending[IEEE80211_MAX_QUEUES];
 	struct tasklet_struct tx_pending_tasklet;
 
-	/*
-	 * This lock is used to prevent concurrent A-MPDU
-	 * session start/stop processing, this thus also
-	 * synchronises the ->ampdu_action() callback to
-	 * drivers and limits it to one at a time.
-	 */
-	spinlock_t ampdu_lock;
+	atomic_t agg_queue_stop[IEEE80211_MAX_QUEUES];
 
 	/* number of interfaces with corresponding IFF_ flags */
 	atomic_t iff_allmultis, iff_promiscs;
--- wireless-testing.orig/net/mac80211/main.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/main.c	2010-06-09 17:21:13.000000000 +0200
@@ -460,8 +460,10 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 
 	sta_info_init(local);
 
-	for (i = 0; i < IEEE80211_MAX_QUEUES; i++)
+	for (i = 0; i < IEEE80211_MAX_QUEUES; i++) {
 		skb_queue_head_init(&local->pending[i]);
+		atomic_set(&local->agg_queue_stop[i], 0);
+	}
 	tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending,
 		     (unsigned long)local);
 
@@ -472,8 +474,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(
 	skb_queue_head_init(&local->skb_queue);
 	skb_queue_head_init(&local->skb_queue_unreliable);
 
-	spin_lock_init(&local->ampdu_lock);
-
 	return local_to_hw(local);
 }
 EXPORT_SYMBOL(ieee80211_alloc_hw);
--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:12.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:13.000000000 +0200
@@ -220,6 +220,41 @@ static inline int ieee80211_ac_from_tid(
 	return ieee802_1d_to_ac[tid & 7];
 }
 
+/*
+ * When multiple aggregation sessions on multiple stations
+ * are being created/destroyed simultaneously, we need to
+ * refcount the global queue stop caused by that in order
+ * to not get into a situation where one of the aggregation
+ * setup or teardown re-enables queues before the other is
+ * ready to handle that.
+ *
+ * These two functions take care of this issue by keeping
+ * a global "agg_queue_stop" refcount.
+ */
+static void __acquires(agg_queue)
+ieee80211_stop_queue_agg(struct ieee80211_local *local, int tid)
+{
+	int queue = ieee80211_ac_from_tid(tid);
+
+	if (atomic_inc_return(&local->agg_queue_stop[queue]) == 1)
+		ieee80211_stop_queue_by_reason(
+			&local->hw, queue,
+			IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	__acquire(agg_queue);
+}
+
+static void __releases(agg_queue)
+ieee80211_wake_queue_agg(struct ieee80211_local *local, int tid)
+{
+	int queue = ieee80211_ac_from_tid(tid);
+
+	if (atomic_dec_return(&local->agg_queue_stop[queue]) == 0)
+		ieee80211_wake_queue_by_reason(
+			&local->hw, queue,
+			IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	__release(agg_queue);
+}
+
 int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
 {
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
@@ -263,7 +298,6 @@ int ieee80211_start_tx_ba_session(struct
 	}
 
 	spin_lock_bh(&sta->lock);
-	spin_lock(&local->ampdu_lock);
 
 	/* we have tried too many times, receiver does not want A-MPDU */
 	if (sta->ampdu_mlme.addba_req_num[tid] > HT_AGG_MAX_RETRIES) {
@@ -289,9 +323,7 @@ int ieee80211_start_tx_ba_session(struct
 	 * which would require us to put them to the AC pending
 	 * afterwards which just makes the code more complex.
 	 */
-	ieee80211_stop_queue_by_reason(
-		&local->hw, ieee80211_ac_from_tid(tid),
-		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	ieee80211_stop_queue_agg(local, tid);
 
 	/* prepare A-MPDU MLME for Tx aggregation */
 	tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC);
@@ -327,11 +359,7 @@ int ieee80211_start_tx_ba_session(struct
 	rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
 
 	/* Driver vetoed or OKed, but we can take packets again now */
-	ieee80211_wake_queue_by_reason(
-		&local->hw, ieee80211_ac_from_tid(tid),
-		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
-
-	spin_unlock(&local->ampdu_lock);
+	ieee80211_wake_queue_agg(local, tid);
 
 	/* activate the timer for the recipient's addBA response */
 	tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL;
@@ -358,11 +386,8 @@ int ieee80211_start_tx_ba_session(struct
  err_free:
 	kfree(tid_tx);
  err_wake_queue:
-	ieee80211_wake_queue_by_reason(
-		&local->hw, ieee80211_ac_from_tid(tid),
-		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	ieee80211_wake_queue_agg(local, tid);
  err_unlock_sta:
-	spin_unlock(&local->ampdu_lock);
 	spin_unlock_bh(&sta->lock);
 	return ret;
 }
@@ -370,19 +395,16 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_sess
 
 /*
  * splice packets from the STA's pending to the local pending,
- * requires a call to ieee80211_agg_splice_finish and holding
- * local->ampdu_lock across both calls.
+ * requires a call to ieee80211_agg_splice_finish later
  */
-static void ieee80211_agg_splice_packets(struct ieee80211_local *local,
-					 struct tid_ampdu_tx *tid_tx,
-					 u16 tid)
+static void __acquires(agg_queue)
+ieee80211_agg_splice_packets(struct ieee80211_local *local,
+			     struct tid_ampdu_tx *tid_tx, u16 tid)
 {
+	int queue = ieee80211_ac_from_tid(tid);
 	unsigned long flags;
-	u16 queue = ieee80211_ac_from_tid(tid);
 
-	ieee80211_stop_queue_by_reason(
-		&local->hw, queue,
-		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	ieee80211_stop_queue_agg(local, tid);
 
 	if (WARN(!tid_tx, "TID %d gone but expected when splicing aggregates"
 			  " from the pending queue\n", tid))
@@ -397,11 +419,10 @@ static void ieee80211_agg_splice_packets
 	}
 }
 
-static void ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid)
+static void __releases(agg_queue)
+ieee80211_agg_splice_finish(struct ieee80211_local *local, u16 tid)
 {
-	ieee80211_wake_queue_by_reason(
-		&local->hw, ieee80211_ac_from_tid(tid),
-		IEEE80211_QUEUE_STOP_REASON_AGGREGATION);
+	ieee80211_wake_queue_agg(local, tid);
 }
 
 /* caller must hold sta->lock */
@@ -414,7 +435,6 @@ static void ieee80211_agg_tx_operational
 	printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid);
 #endif
 
-	spin_lock(&local->ampdu_lock);
 	ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid);
 	/*
 	 * Now mark as operational. This will be visible
@@ -423,7 +443,6 @@ static void ieee80211_agg_tx_operational
 	 */
 	set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state);
 	ieee80211_agg_splice_finish(local, tid);
-	spin_unlock(&local->ampdu_lock);
 
 	drv_ampdu_action(local, sta->sdata,
 			 IEEE80211_AMPDU_TX_OPERATIONAL,
@@ -604,7 +623,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	 * more.
 	 */
 
-	spin_lock(&local->ampdu_lock);
 	ieee80211_agg_splice_packets(local, tid_tx, tid);
 
 	/* future packets must not find the tid_tx struct any more */
@@ -613,7 +631,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	ieee80211_agg_splice_finish(local, tid);
 
 	call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
-	spin_unlock(&local->ampdu_lock);
 
 	spin_unlock_bh(&sta->lock);
 	rcu_read_unlock();



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

* [PATCH 14/23] mac80211: make TX aggregation start/stop request async
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (12 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 13/23] mac80211: refcount aggregation queue stop Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 15/23] mac80211: move BA session work Johannes Berg
                   ` (8 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

When the driver or rate control requests starting
or stopping an aggregation session, that currently
causes a direct callback into the driver, which
could potentially cause locking problems. Also,
the functions need to be callable from contexts
that cannot sleep, and thus will interfere with
making the ampdu_action callback sleeping.

To address these issues, add a new work item for
each station that will process any start or stop
requests out of line.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c      |  177 +++++++++++++++++++++++++++++++--------------
 net/mac80211/debugfs_sta.c |    5 -
 net/mac80211/ht.c          |    2 
 net/mac80211/ieee80211_i.h |    1 
 net/mac80211/sta_info.c    |    1 
 net/mac80211/sta_info.h    |    6 +
 net/mac80211/tx.c          |    5 +
 7 files changed, 139 insertions(+), 58 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:13.000000000 +0200
@@ -146,6 +146,13 @@ static int ___ieee80211_stop_tx_ba_sessi
 	if (WARN_ON(!tid_tx))
 		return -ENOENT;
 
+	if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) {
+		/* not even started yet! */
+		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
+		return 0;
+	}
+
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n",
 	       sta->sta.addr, tid);
@@ -255,6 +262,94 @@ ieee80211_wake_queue_agg(struct ieee8021
 	__release(agg_queue);
 }
 
+static void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
+{
+	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
+	struct ieee80211_local *local = sta->local;
+	struct ieee80211_sub_if_data *sdata = sta->sdata;
+	u16 start_seq_num;
+	int ret;
+
+	/*
+	 * While we're asking the driver about the aggregation,
+	 * stop the AC queue so that we don't have to worry
+	 * about frames that came in while we were doing that,
+	 * which would require us to put them to the AC pending
+	 * afterwards which just makes the code more complex.
+	 */
+	ieee80211_stop_queue_agg(local, tid);
+
+	clear_bit(HT_AGG_STATE_WANT_START, &tid_tx->state);
+
+	/*
+	 * This might be off by one due to a race that we can't
+	 * really prevent here without synchronize_net() which
+	 * can't be called now.
+	 */
+	start_seq_num = sta->tid_seq[tid] >> 4;
+
+	ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
+			       &sta->sta, tid, &start_seq_num);
+	if (ret) {
+#ifdef CONFIG_MAC80211_HT_DEBUG
+		printk(KERN_DEBUG "BA request denied - HW unavailable for"
+					" tid %d\n", tid);
+#endif
+		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		ieee80211_wake_queue_agg(local, tid);
+		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
+		return;
+	}
+
+	/* we can take packets again now */
+	ieee80211_wake_queue_agg(local, tid);
+
+	/* activate the timer for the recipient's addBA response */
+	mod_timer(&tid_tx->addba_resp_timer, jiffies + ADDBA_RESP_INTERVAL);
+#ifdef CONFIG_MAC80211_HT_DEBUG
+	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
+#endif
+
+	sta->ampdu_mlme.addba_req_num[tid]++;
+
+	/* send AddBA request */
+	ieee80211_send_addba_request(sdata, sta->sta.addr, tid,
+				     tid_tx->dialog_token, start_seq_num,
+				     0x40, 5000);
+}
+
+void ieee80211_tx_ba_session_work(struct work_struct *work)
+{
+	struct sta_info *sta =
+		container_of(work, struct sta_info, ampdu_mlme.work);
+	struct tid_ampdu_tx *tid_tx;
+	int tid;
+
+	/*
+	 * When this flag is set, new sessions should be
+	 * blocked, and existing sessions will be torn
+	 * down by the code that set the flag, so this
+	 * need not run.
+	 */
+	if (test_sta_flags(sta, WLAN_STA_BLOCK_BA))
+		return;
+
+	spin_lock_bh(&sta->lock);
+	for (tid = 0; tid < STA_TID_NUM; tid++) {
+		tid_tx = sta->ampdu_mlme.tid_tx[tid];
+		if (!tid_tx)
+			continue;
+
+		if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state))
+			ieee80211_tx_ba_session_handle_start(sta, tid);
+		else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP,
+					    &tid_tx->state))
+			___ieee80211_stop_tx_ba_session(sta, tid,
+							WLAN_BACK_INITIATOR);
+	}
+	spin_unlock_bh(&sta->lock);
+}
+
 int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
 {
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
@@ -262,7 +357,6 @@ int ieee80211_start_tx_ba_session(struct
 	struct ieee80211_local *local = sdata->local;
 	struct tid_ampdu_tx *tid_tx;
 	int ret = 0;
-	u16 start_seq_num;
 
 	trace_api_start_tx_ba_session(pubsta, tid);
 
@@ -316,15 +410,6 @@ int ieee80211_start_tx_ba_session(struct
 		goto err_unlock_sta;
 	}
 
-	/*
-	 * While we're asking the driver about the aggregation,
-	 * stop the AC queue so that we don't have to worry
-	 * about frames that came in while we were doing that,
-	 * which would require us to put them to the AC pending
-	 * afterwards which just makes the code more complex.
-	 */
-	ieee80211_stop_queue_agg(local, tid);
-
 	/* prepare A-MPDU MLME for Tx aggregation */
 	tid_tx = kzalloc(sizeof(struct tid_ampdu_tx), GFP_ATOMIC);
 	if (!tid_tx) {
@@ -334,59 +419,27 @@ int ieee80211_start_tx_ba_session(struct
 					tid);
 #endif
 		ret = -ENOMEM;
-		goto err_wake_queue;
+		goto err_unlock_sta;
 	}
 
 	skb_queue_head_init(&tid_tx->pending);
+	__set_bit(HT_AGG_STATE_WANT_START, &tid_tx->state);
 
 	/* Tx timer */
 	tid_tx->addba_resp_timer.function = sta_addba_resp_timer_expired;
 	tid_tx->addba_resp_timer.data = (unsigned long)&sta->timer_to_tid[tid];
 	init_timer(&tid_tx->addba_resp_timer);
 
-	start_seq_num = sta->tid_seq[tid] >> 4;
-
-	ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
-			       pubsta, tid, &start_seq_num);
-	if (ret) {
-#ifdef CONFIG_MAC80211_HT_DEBUG
-		printk(KERN_DEBUG "BA request denied - HW unavailable for"
-					" tid %d\n", tid);
-#endif /* CONFIG_MAC80211_HT_DEBUG */
-		goto err_free;
-	}
-
-	rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
-
-	/* Driver vetoed or OKed, but we can take packets again now */
-	ieee80211_wake_queue_agg(local, tid);
-
-	/* activate the timer for the recipient's addBA response */
-	tid_tx->addba_resp_timer.expires = jiffies + ADDBA_RESP_INTERVAL;
-	add_timer(&tid_tx->addba_resp_timer);
-#ifdef CONFIG_MAC80211_HT_DEBUG
-	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
-#endif
-
-	/* prepare tid data */
+	/* assign a dialog token */
 	sta->ampdu_mlme.dialog_token_allocator++;
 	tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator;
-	tid_tx->ssn = start_seq_num;
 
-	sta->ampdu_mlme.addba_req_num[tid]++;
+	/* finally, assign it to the array */
+	rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
 
-	spin_unlock_bh(&sta->lock);
+	ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
 
-	/* send AddBA request */
-	ieee80211_send_addba_request(sdata, pubsta->addr, tid,
-			 tid_tx->dialog_token, tid_tx->ssn,
-			 0x40, 5000);
-	return 0;
-
- err_free:
-	kfree(tid_tx);
- err_wake_queue:
-	ieee80211_wake_queue_agg(local, tid);
+	/* this flow continues off the work */
  err_unlock_sta:
 	spin_unlock_bh(&sta->lock);
 	return ret;
@@ -534,8 +587,7 @@ int __ieee80211_stop_tx_ba_session(struc
 	spin_lock_bh(&sta->lock);
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
-	/* check if the TID is in aggregation */
-	if (!tid_tx || !test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
+	if (!tid_tx) {
 		ret = -ENOENT;
 		goto unlock;
 	}
@@ -552,6 +604,8 @@ int ieee80211_stop_tx_ba_session(struct
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	struct ieee80211_local *local = sdata->local;
+	struct tid_ampdu_tx *tid_tx;
+	int ret = 0;
 
 	trace_api_stop_tx_ba_session(pubsta, tid);
 
@@ -561,7 +615,26 @@ int ieee80211_stop_tx_ba_session(struct
 	if (tid >= STA_TID_NUM)
 		return -EINVAL;
 
-	return __ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR);
+	spin_lock_bh(&sta->lock);
+	tid_tx = sta->ampdu_mlme.tid_tx[tid];
+
+	if (!tid_tx) {
+		ret = -ENOENT;
+		goto unlock;
+	}
+
+	if (test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) {
+		/* already in progress stopping it */
+		ret = 0;
+		goto unlock;
+	}
+
+	set_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state);
+	ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
+
+ unlock:
+	spin_unlock_bh(&sta->lock);
+	return ret;
 }
 EXPORT_SYMBOL(ieee80211_stop_tx_ba_session);
 
--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:13.000000000 +0200
@@ -67,6 +67,8 @@ enum ieee80211_sta_info_flags {
 #define HT_AGG_STATE_RESPONSE_RECEIVED	1
 #define HT_AGG_STATE_OPERATIONAL	2
 #define HT_AGG_STATE_STOPPING		3
+#define HT_AGG_STATE_WANT_START		4
+#define HT_AGG_STATE_WANT_STOP		5
 
 /**
  * struct tid_ampdu_tx - TID aggregation information (Tx).
@@ -74,7 +76,6 @@ enum ieee80211_sta_info_flags {
  * @rcu_head: rcu head for freeing structure
  * @addba_resp_timer: timer for peer's response to addba request
  * @pending: pending frames queue -- use sta's spinlock to protect
- * @ssn: Starting Sequence Number expected to be aggregated.
  * @dialog_token: dialog token for aggregation session
  * @state: session state (see above)
  * @stop_initiator: initiator of a session stop
@@ -92,7 +93,6 @@ struct tid_ampdu_tx {
 	struct timer_list addba_resp_timer;
 	struct sk_buff_head pending;
 	unsigned long state;
-	u16 ssn;
 	u8 dialog_token;
 	u8 stop_initiator;
 };
@@ -139,11 +139,13 @@ struct tid_ampdu_rx {
  * @tid_tx: aggregation info for Tx per TID
  * @addba_req_num: number of times addBA request has been sent.
  * @dialog_token_allocator: dialog token enumerator for each new session;
+ * @work: work struct for starting/stopping aggregation
  */
 struct sta_ampdu_mlme {
 	/* rx */
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
 	/* tx */
+	struct work_struct work;
 	struct tid_ampdu_tx *tid_tx[STA_TID_NUM];
 	u8 addba_req_num[STA_TID_NUM];
 	u8 dialog_token_allocator;
--- wireless-testing.orig/net/mac80211/debugfs_sta.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/debugfs_sta.c	2010-06-09 17:21:13.000000000 +0200
@@ -121,7 +121,7 @@ static ssize_t sta_agg_status_read(struc
 	p += scnprintf(p, sizeof(buf) + buf - p, "next dialog_token: %#02x\n",
 			sta->ampdu_mlme.dialog_token_allocator + 1);
 	p += scnprintf(p, sizeof(buf) + buf - p,
-		       "TID\t\tRX active\tDTKN\tSSN\t\tTX\tDTKN\tSSN\tpending\n");
+		       "TID\t\tRX active\tDTKN\tSSN\t\tTX\tDTKN\tpending\n");
 	for (i = 0; i < STA_TID_NUM; i++) {
 		p += scnprintf(p, sizeof(buf) + buf - p, "%02d", i);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t\t%x",
@@ -138,9 +138,6 @@ static ssize_t sta_agg_status_read(struc
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.2x",
 				sta->ampdu_mlme.tid_tx[i] ?
 				sta->ampdu_mlme.tid_tx[i]->dialog_token : 0);
-		p += scnprintf(p, sizeof(buf) + buf - p, "\t%#.3x",
-				sta->ampdu_mlme.tid_tx[i] ?
-				sta->ampdu_mlme.tid_tx[i]->ssn : 0);
 		p += scnprintf(p, sizeof(buf) + buf - p, "\t%03d",
 				sta->ampdu_mlme.tid_tx[i] ?
 				skb_queue_len(&sta->ampdu_mlme.tid_tx[i]->pending) : 0);
--- wireless-testing.orig/net/mac80211/tx.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/tx.c	2010-06-09 17:21:13.000000000 +0200
@@ -1102,6 +1102,11 @@ static bool ieee80211_tx_prep_agg(struct
 
 	if (test_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state)) {
 		info->flags |= IEEE80211_TX_CTL_AMPDU;
+	} else if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) {
+		/*
+		 * nothing -- this aggregation session is being started
+		 * but that might still fail with the driver
+		 */
 	} else {
 		spin_lock(&tx->sta->lock);
 		/*
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:13.000000000 +0200
@@ -1115,6 +1115,7 @@ int __ieee80211_stop_tx_ba_session(struc
 				   enum ieee80211_back_parties initiator);
 void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid);
 void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid);
+void ieee80211_tx_ba_session_work(struct work_struct *work);
 
 /* Spectrum management */
 void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/sta_info.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.c	2010-06-09 17:21:13.000000000 +0200
@@ -235,6 +235,7 @@ struct sta_info *sta_info_alloc(struct i
 	spin_lock_init(&sta->lock);
 	spin_lock_init(&sta->flaglock);
 	INIT_WORK(&sta->drv_unblock_wk, sta_unblock);
+	INIT_WORK(&sta->ampdu_mlme.work, ieee80211_tx_ba_session_work);
 
 	memcpy(sta->sta.addr, addr, ETH_ALEN);
 	sta->local = local;
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:13.000000000 +0200
@@ -105,6 +105,8 @@ void ieee80211_sta_tear_down_BA_sessions
 {
 	int i;
 
+	cancel_work_sync(&sta->ampdu_mlme.work);
+
 	for (i = 0; i <  STA_TID_NUM; i++) {
 		__ieee80211_stop_tx_ba_session(sta, i, WLAN_BACK_INITIATOR);
 		__ieee80211_stop_rx_ba_session(sta, i, WLAN_BACK_RECIPIENT,



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

* [PATCH 15/23] mac80211: move BA session work
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (13 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 14/23] mac80211: make TX aggregation start/stop request async Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 16/23] mac80211: defer RX agg session teardown to work Johannes Berg
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Move the block-ack session works into common
code, since it will be needed for RX agg too
in the next patches.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c      |   39 +++------------------------------------
 net/mac80211/ht.c          |   32 ++++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |    5 ++++-
 net/mac80211/sta_info.c    |    2 +-
 4 files changed, 40 insertions(+), 38 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:14.000000000 +0200
@@ -133,9 +133,8 @@ static void kfree_tid_tx(struct rcu_head
 	kfree(tid_tx);
 }
 
-static int ___ieee80211_stop_tx_ba_session(
-		struct sta_info *sta, u16 tid,
-		enum ieee80211_back_parties initiator)
+int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
+				    enum ieee80211_back_parties initiator)
 {
 	struct ieee80211_local *local = sta->local;
 	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
@@ -262,7 +261,7 @@ ieee80211_wake_queue_agg(struct ieee8021
 	__release(agg_queue);
 }
 
-static void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
+void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
 {
 	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
 	struct ieee80211_local *local = sta->local;
@@ -318,38 +317,6 @@ static void ieee80211_tx_ba_session_hand
 				     0x40, 5000);
 }
 
-void ieee80211_tx_ba_session_work(struct work_struct *work)
-{
-	struct sta_info *sta =
-		container_of(work, struct sta_info, ampdu_mlme.work);
-	struct tid_ampdu_tx *tid_tx;
-	int tid;
-
-	/*
-	 * When this flag is set, new sessions should be
-	 * blocked, and existing sessions will be torn
-	 * down by the code that set the flag, so this
-	 * need not run.
-	 */
-	if (test_sta_flags(sta, WLAN_STA_BLOCK_BA))
-		return;
-
-	spin_lock_bh(&sta->lock);
-	for (tid = 0; tid < STA_TID_NUM; tid++) {
-		tid_tx = sta->ampdu_mlme.tid_tx[tid];
-		if (!tid_tx)
-			continue;
-
-		if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state))
-			ieee80211_tx_ba_session_handle_start(sta, tid);
-		else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP,
-					    &tid_tx->state))
-			___ieee80211_stop_tx_ba_session(sta, tid,
-							WLAN_BACK_INITIATOR);
-	}
-	spin_unlock_bh(&sta->lock);
-}
-
 int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
 {
 	struct sta_info *sta = container_of(pubsta, struct sta_info, sta);
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:14.000000000 +0200
@@ -114,6 +114,38 @@ void ieee80211_sta_tear_down_BA_sessions
 	}
 }
 
+void ieee80211_ba_session_work(struct work_struct *work)
+{
+	struct sta_info *sta =
+		container_of(work, struct sta_info, ampdu_mlme.work);
+	struct tid_ampdu_tx *tid_tx;
+	int tid;
+
+	/*
+	 * When this flag is set, new sessions should be
+	 * blocked, and existing sessions will be torn
+	 * down by the code that set the flag, so this
+	 * need not run.
+	 */
+	if (test_sta_flags(sta, WLAN_STA_BLOCK_BA))
+		return;
+
+	spin_lock_bh(&sta->lock);
+	for (tid = 0; tid < STA_TID_NUM; tid++) {
+		tid_tx = sta->ampdu_mlme.tid_tx[tid];
+		if (!tid_tx)
+			continue;
+
+		if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state))
+			ieee80211_tx_ba_session_handle_start(sta, tid);
+		else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP,
+					    &tid_tx->state))
+			___ieee80211_stop_tx_ba_session(sta, tid,
+							WLAN_BACK_INITIATOR);
+	}
+	spin_unlock_bh(&sta->lock);
+}
+
 void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata,
 			  const u8 *da, u16 tid,
 			  u16 initiator, u16 reason_code)
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:14.000000000 +0200
@@ -1113,9 +1113,12 @@ void ieee80211_process_addba_request(str
 
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator);
+int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
+				    enum ieee80211_back_parties initiator);
 void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid);
 void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid);
-void ieee80211_tx_ba_session_work(struct work_struct *work);
+void ieee80211_ba_session_work(struct work_struct *work);
+void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid);
 
 /* Spectrum management */
 void ieee80211_process_measurement_req(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/sta_info.c	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.c	2010-06-09 17:21:14.000000000 +0200
@@ -235,7 +235,7 @@ struct sta_info *sta_info_alloc(struct i
 	spin_lock_init(&sta->lock);
 	spin_lock_init(&sta->flaglock);
 	INIT_WORK(&sta->drv_unblock_wk, sta_unblock);
-	INIT_WORK(&sta->ampdu_mlme.work, ieee80211_tx_ba_session_work);
+	INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work);
 
 	memcpy(sta->sta.addr, addr, ETH_ALEN);
 	sta->local = local;



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

* [PATCH 16/23] mac80211: defer RX agg session teardown to work
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (14 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 15/23] mac80211: move BA session work Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 17/23] mac80211: fix RX aggregation timer Johannes Berg
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Since we want the code to be able to sleep
in the future, it must not be called from
the timer directly. To prepare, move it out
into the aggregation work.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-rx.c      |   24 ++++++++++--------------
 net/mac80211/ht.c          |    5 +++++
 net/mac80211/ieee80211_i.h |    2 ++
 net/mac80211/sta_info.h    |    3 +++
 4 files changed, 20 insertions(+), 14 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-rx.c	2010-06-09 17:21:11.000000000 +0200
+++ wireless-testing/net/mac80211/agg-rx.c	2010-06-09 17:21:15.000000000 +0200
@@ -32,21 +32,18 @@ static void ieee80211_free_tid_rx(struct
 	kfree(tid_rx);
 }
 
-static void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
-					    u16 initiator, u16 reason,
-					    bool from_timer)
+void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
+				     u16 initiator, u16 reason)
 {
 	struct ieee80211_local *local = sta->local;
 	struct tid_ampdu_rx *tid_rx;
 
-	spin_lock_bh(&sta->lock);
+	lockdep_assert_held(&sta->lock);
 
 	tid_rx = sta->ampdu_mlme.tid_rx[tid];
 
-	if (!tid_rx) {
-		spin_unlock_bh(&sta->lock);
+	if (!tid_rx)
 		return;
-	}
 
 	rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], NULL);
 
@@ -65,10 +62,7 @@ static void ___ieee80211_stop_rx_ba_sess
 		ieee80211_send_delba(sta->sdata, sta->sta.addr,
 				     tid, 0, reason);
 
-	spin_unlock_bh(&sta->lock);
-
-	if (!from_timer)
-		del_timer_sync(&tid_rx->session_timer);
+	del_timer_sync(&tid_rx->session_timer);
 
 	call_rcu(&tid_rx->rcu_head, ieee80211_free_tid_rx);
 }
@@ -76,7 +70,9 @@ static void ___ieee80211_stop_rx_ba_sess
 void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
 				    u16 initiator, u16 reason)
 {
-	___ieee80211_stop_rx_ba_session(sta, tid, initiator, reason, false);
+	spin_lock_bh(&sta->lock);
+	___ieee80211_stop_rx_ba_session(sta, tid, initiator, reason);
+	spin_unlock_bh(&sta->lock);
 }
 
 /*
@@ -97,8 +93,8 @@ static void sta_rx_agg_session_timer_exp
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "rx session timer expired on tid %d\n", (u16)*ptid);
 #endif
-	___ieee80211_stop_rx_ba_session(sta, *ptid, WLAN_BACK_RECIPIENT,
-					WLAN_REASON_QSTA_TIMEOUT, true);
+	set_bit(*ptid, sta->ampdu_mlme.tid_rx_timer_expired);
+	ieee80211_queue_work(&sta->local->hw, &sta->ampdu_mlme.work);
 }
 
 static void ieee80211_send_addba_resp(struct ieee80211_sub_if_data *sdata, u8 *da, u16 tid,
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:21:14.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:15.000000000 +0200
@@ -132,6 +132,11 @@ void ieee80211_ba_session_work(struct wo
 
 	spin_lock_bh(&sta->lock);
 	for (tid = 0; tid < STA_TID_NUM; tid++) {
+		if (test_and_clear_bit(tid, sta->ampdu_mlme.tid_rx_timer_expired))
+			___ieee80211_stop_rx_ba_session(
+				sta, tid, WLAN_BACK_RECIPIENT,
+				WLAN_REASON_QSTA_TIMEOUT);
+
 		tid_tx = sta->ampdu_mlme.tid_tx[tid];
 		if (!tid_tx)
 			continue;
--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:21:13.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:15.000000000 +0200
@@ -140,10 +140,13 @@ struct tid_ampdu_rx {
  * @addba_req_num: number of times addBA request has been sent.
  * @dialog_token_allocator: dialog token enumerator for each new session;
  * @work: work struct for starting/stopping aggregation
+ * @tid_rx_timer_expired: bitmap indicating on which TIDs the
+ *	RX timer expired until the work for it runs
  */
 struct sta_ampdu_mlme {
 	/* rx */
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
+	unsigned long tid_rx_timer_expired[BITS_TO_LONGS(STA_TID_NUM)];
 	/* tx */
 	struct work_struct work;
 	struct tid_ampdu_tx *tid_tx[STA_TID_NUM];
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-06-09 17:21:14.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-06-09 17:21:15.000000000 +0200
@@ -1096,6 +1096,8 @@ int ieee80211_send_smps_action(struct ie
 			       enum ieee80211_smps_mode smps, const u8 *da,
 			       const u8 *bssid);
 
+void ___ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
+				     u16 initiator, u16 reason);
 void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
 				    u16 initiator, u16 reason);
 void ieee80211_sta_tear_down_BA_sessions(struct sta_info *sta);



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

* [PATCH 17/23] mac80211: fix RX aggregation timer
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (15 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 16/23] mac80211: defer RX agg session teardown to work Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 18/23] mac80211: change RX aggregation locking Johannes Berg
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

I noticed that when there was _no_ traffic at
all on a given aggregation session, it would
never time out. This won't happen unless you
forced creating a session, but fix it anyway.

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

--- wireless-testing.orig/net/mac80211/agg-rx.c	2010-06-09 17:21:15.000000000 +0200
+++ wireless-testing/net/mac80211/agg-rx.c	2010-06-09 17:21:15.000000000 +0200
@@ -274,6 +274,10 @@ void ieee80211_process_addba_request(str
 
 	/* activate it for RX */
 	rcu_assign_pointer(sta->ampdu_mlme.tid_rx[tid], tid_agg_rx);
+
+	if (timeout)
+		mod_timer(&tid_agg_rx->session_timer, TU_TO_EXP_TIME(timeout));
+
 end:
 	spin_unlock_bh(&sta->lock);
 



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

* [PATCH 18/23] mac80211: change RX aggregation locking
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (16 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 17/23] mac80211: fix RX aggregation timer Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 19/23] mac80211: defer TX agg session teardown to work Johannes Berg
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

To prepare for allowing drivers to sleep in
ampdu_action, change the locking in the RX
aggregation code to use a mutex, so that it
would already allow drivers to sleep. But
explicitly disable BHs around the callback
for now since the TX part cannot yet sleep,
and drivers' locking might require it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-rx.c     |   12 ++++++------
 net/mac80211/driver-ops.h |    2 ++
 net/mac80211/ht.c         |    6 +++++-
 net/mac80211/iface.c      |    8 ++++----
 net/mac80211/sta_info.c   |    1 +
 net/mac80211/sta_info.h   |    3 +++
 6 files changed, 21 insertions(+), 11 deletions(-)

--- wireless-testing.orig/net/mac80211/iface.c	2010-06-09 17:21:10.000000000 +0200
+++ wireless-testing/net/mac80211/iface.c	2010-06-09 17:21:16.000000000 +0200
@@ -740,7 +740,7 @@ static void ieee80211_iface_work(struct
 			   mgmt->u.action.category == WLAN_CATEGORY_BACK) {
 			int len = skb->len;
 
-			rcu_read_lock();
+			mutex_lock(&local->sta_mtx);
 			sta = sta_info_get(sdata, mgmt->sa);
 			if (sta) {
 				switch (mgmt->u.action.u.addba_req.action_code) {
@@ -761,7 +761,7 @@ static void ieee80211_iface_work(struct
 					break;
 				}
 			}
-			rcu_read_unlock();
+			mutex_unlock(&local->sta_mtx);
 		} else if (ieee80211_is_data_qos(mgmt->frame_control)) {
 			struct ieee80211_hdr *hdr = (void *)mgmt;
 			/*
@@ -781,7 +781,7 @@ static void ieee80211_iface_work(struct
 			 * a block-ack session was active. That cannot be
 			 * right, so terminate the session.
 			 */
-			rcu_read_lock();
+			mutex_lock(&local->sta_mtx);
 			sta = sta_info_get(sdata, mgmt->sa);
 			if (sta) {
 				u16 tid = *ieee80211_get_qos_ctl(hdr) &
@@ -791,7 +791,7 @@ static void ieee80211_iface_work(struct
 					sta, tid, WLAN_BACK_RECIPIENT,
 					WLAN_REASON_QSTA_REQUIRE_SETUP);
 			}
-			rcu_read_unlock();
+			mutex_unlock(&local->sta_mtx);
 		} else switch (sdata->vif.type) {
 		case NL80211_IFTYPE_STATION:
 			ieee80211_sta_rx_queued_mgmt(sdata, skb);
--- wireless-testing.orig/net/mac80211/driver-ops.h	2010-06-09 17:20:46.000000000 +0200
+++ wireless-testing/net/mac80211/driver-ops.h	2010-06-09 17:21:16.000000000 +0200
@@ -349,9 +349,11 @@ static inline int drv_ampdu_action(struc
 				   u16 *ssn)
 {
 	int ret = -EOPNOTSUPP;
+	local_bh_disable();
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action,
 					       sta, tid, ssn);
+	local_bh_enable();
 	trace_drv_ampdu_action(local, sdata, action, sta, tid, ssn, ret);
 	return ret;
 }
--- wireless-testing.orig/net/mac80211/sta_info.c	2010-06-09 17:21:14.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.c	2010-06-09 17:21:16.000000000 +0200
@@ -236,6 +236,7 @@ struct sta_info *sta_info_alloc(struct i
 	spin_lock_init(&sta->flaglock);
 	INIT_WORK(&sta->drv_unblock_wk, sta_unblock);
 	INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work);
+	mutex_init(&sta->ampdu_mlme.mtx);
 
 	memcpy(sta->sta.addr, addr, ETH_ALEN);
 	sta->local = local;
--- wireless-testing.orig/net/mac80211/sta_info.h	2010-06-09 17:21:15.000000000 +0200
+++ wireless-testing/net/mac80211/sta_info.h	2010-06-09 17:21:16.000000000 +0200
@@ -142,8 +142,11 @@ struct tid_ampdu_rx {
  * @work: work struct for starting/stopping aggregation
  * @tid_rx_timer_expired: bitmap indicating on which TIDs the
  *	RX timer expired until the work for it runs
+ * @mtx: mutex to protect all TX data (except non-NULL assignments
+ *	to tid_tx[idx], which are protected by the sta spinlock)
  */
 struct sta_ampdu_mlme {
+	struct mutex mtx;
 	/* rx */
 	struct tid_ampdu_rx *tid_rx[STA_TID_NUM];
 	unsigned long tid_rx_timer_expired[BITS_TO_LONGS(STA_TID_NUM)];
--- wireless-testing.orig/net/mac80211/agg-rx.c	2010-06-09 17:21:15.000000000 +0200
+++ wireless-testing/net/mac80211/agg-rx.c	2010-06-09 17:21:16.000000000 +0200
@@ -6,7 +6,7 @@
  * Copyright 2005-2006, Devicescape Software, Inc.
  * Copyright 2006-2007	Jiri Benc <jbenc@suse.cz>
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
- * Copyright 2007-2008, Intel Corporation
+ * Copyright 2007-2010, Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -38,7 +38,7 @@ void ___ieee80211_stop_rx_ba_session(str
 	struct ieee80211_local *local = sta->local;
 	struct tid_ampdu_rx *tid_rx;
 
-	lockdep_assert_held(&sta->lock);
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
 
 	tid_rx = sta->ampdu_mlme.tid_rx[tid];
 
@@ -70,9 +70,9 @@ void ___ieee80211_stop_rx_ba_session(str
 void __ieee80211_stop_rx_ba_session(struct sta_info *sta, u16 tid,
 				    u16 initiator, u16 reason)
 {
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	___ieee80211_stop_rx_ba_session(sta, tid, initiator, reason);
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
 
 /*
@@ -205,7 +205,7 @@ void ieee80211_process_addba_request(str
 
 
 	/* examine state machine */
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 
 	if (sta->ampdu_mlme.tid_rx[tid]) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
@@ -279,7 +279,7 @@ void ieee80211_process_addba_request(str
 		mod_timer(&tid_agg_rx->session_timer, TU_TO_EXP_TIME(timeout));
 
 end:
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 
 end_no_lock:
 	ieee80211_send_addba_resp(sta->sdata, sta->sta.addr, tid,
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:21:15.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:16.000000000 +0200
@@ -130,13 +130,17 @@ void ieee80211_ba_session_work(struct wo
 	if (test_sta_flags(sta, WLAN_STA_BLOCK_BA))
 		return;
 
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	for (tid = 0; tid < STA_TID_NUM; tid++) {
 		if (test_and_clear_bit(tid, sta->ampdu_mlme.tid_rx_timer_expired))
 			___ieee80211_stop_rx_ba_session(
 				sta, tid, WLAN_BACK_RECIPIENT,
 				WLAN_REASON_QSTA_TIMEOUT);
+	}
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 
+	spin_lock_bh(&sta->lock);
+	for (tid = 0; tid < STA_TID_NUM; tid++) {
 		tid_tx = sta->ampdu_mlme.tid_tx[tid];
 		if (!tid_tx)
 			continue;



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

* [PATCH 19/23] mac80211: defer TX agg session teardown to work
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (17 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 18/23] mac80211: change RX aggregation locking Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 20/23] mac80211: change TX aggregation locking Johannes Berg
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Since we want the code to be able to sleep
in the future, it must not be called from
the timer directly. To achieve that, simply
call the function drivers would call, and
also use RCU in the timer to get the struct
so we don't need to rely on the spinlock in
the future.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:14.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:16.000000000 +0200
@@ -200,11 +200,11 @@ static void sta_addba_resp_timer_expired
 	struct tid_ampdu_tx *tid_tx;
 
 	/* check if the TID waits for addBA response */
-	spin_lock_bh(&sta->lock);
-	tid_tx = sta->ampdu_mlme.tid_tx[tid];
+	rcu_read_lock();
+	tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]);
 	if (!tid_tx ||
 	    test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state)) {
-		spin_unlock_bh(&sta->lock);
+		rcu_read_unlock();
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "timer expired on tid %d but we are not "
 				"(or no longer) expecting addBA response there\n",
@@ -217,8 +217,8 @@ static void sta_addba_resp_timer_expired
 	printk(KERN_DEBUG "addBA response timer expired on tid %d\n", tid);
 #endif
 
-	___ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR);
-	spin_unlock_bh(&sta->lock);
+	ieee80211_stop_tx_ba_session(&sta->sta, tid);
+	rcu_read_unlock();
 }
 
 static inline int ieee80211_ac_from_tid(int tid)



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

* [PATCH 20/23] mac80211: change TX aggregation locking
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (18 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 19/23] mac80211: defer TX agg session teardown to work Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 21/23] mac80211: allow drivers to sleep in ampdu_action Johannes Berg
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

To prepare for allowing drivers to sleep in
ampdu_action, change the locking in the TX
aggregation code to use the mutex the RX part
already uses. The spinlock is still necessary
around some code to avoid races with TX, but
now we can also synchronize_net() to avoid
getting an inconsistent sequence number.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c     |   94 +++++++++++++++++++++++++---------------------
 net/mac80211/driver-ops.h |    3 +
 net/mac80211/ht.c         |    8 ---
 3 files changed, 58 insertions(+), 47 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:16.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:17.000000000 +0200
@@ -6,7 +6,7 @@
  * Copyright 2005-2006, Devicescape Software, Inc.
  * Copyright 2006-2007	Jiri Benc <jbenc@suse.cz>
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
- * Copyright 2007-2009, Intel Corporation
+ * Copyright 2007-2010, Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -140,18 +140,23 @@ int ___ieee80211_stop_tx_ba_session(stru
 	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
 	int ret;
 
-	lockdep_assert_held(&sta->lock);
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
 
-	if (WARN_ON(!tid_tx))
+	if (!tid_tx)
 		return -ENOENT;
 
+	spin_lock_bh(&sta->lock);
+
 	if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) {
 		/* not even started yet! */
 		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		spin_unlock_bh(&sta->lock);
 		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 		return 0;
 	}
 
+	spin_unlock_bh(&sta->lock);
+
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n",
 	       sta->sta.addr, tid);
@@ -269,6 +274,8 @@ void ieee80211_tx_ba_session_handle_star
 	u16 start_seq_num;
 	int ret;
 
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
+
 	/*
 	 * While we're asking the driver about the aggregation,
 	 * stop the AC queue so that we don't have to worry
@@ -281,10 +288,11 @@ void ieee80211_tx_ba_session_handle_star
 	clear_bit(HT_AGG_STATE_WANT_START, &tid_tx->state);
 
 	/*
-	 * This might be off by one due to a race that we can't
-	 * really prevent here without synchronize_net() which
-	 * can't be called now.
+	 * make sure no packets are being processed to get
+	 * valid starting sequence number
 	 */
+	synchronize_net();
+
 	start_seq_num = sta->tid_seq[tid] >> 4;
 
 	ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
@@ -294,7 +302,10 @@ void ieee80211_tx_ba_session_handle_star
 		printk(KERN_DEBUG "BA request denied - HW unavailable for"
 					" tid %d\n", tid);
 #endif
+		spin_lock_bh(&sta->lock);
 		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		spin_unlock_bh(&sta->lock);
+
 		ieee80211_wake_queue_agg(local, tid);
 		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 		return;
@@ -309,7 +320,9 @@ void ieee80211_tx_ba_session_handle_star
 	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
 #endif
 
+	spin_lock_bh(&sta->lock);
 	sta->ampdu_mlme.addba_req_num[tid]++;
+	spin_unlock_bh(&sta->lock);
 
 	/* send AddBA request */
 	ieee80211_send_addba_request(sdata, sta->sta.addr, tid,
@@ -445,16 +458,25 @@ ieee80211_agg_splice_finish(struct ieee8
 	ieee80211_wake_queue_agg(local, tid);
 }
 
-/* caller must hold sta->lock */
 static void ieee80211_agg_tx_operational(struct ieee80211_local *local,
 					 struct sta_info *sta, u16 tid)
 {
-	lockdep_assert_held(&sta->lock);
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid);
 #endif
 
+	drv_ampdu_action(local, sta->sdata,
+			 IEEE80211_AMPDU_TX_OPERATIONAL,
+			 &sta->sta, tid, NULL);
+
+	/*
+	 * synchronize with TX path, while splicing the TX path
+	 * should block so it won't put more packets onto pending.
+	 */
+	spin_lock_bh(&sta->lock);
+
 	ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid);
 	/*
 	 * Now mark as operational. This will be visible
@@ -464,9 +486,7 @@ static void ieee80211_agg_tx_operational
 	set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state);
 	ieee80211_agg_splice_finish(local, tid);
 
-	drv_ampdu_action(local, sta->sdata,
-			 IEEE80211_AMPDU_TX_OPERATIONAL,
-			 &sta->sta, tid, NULL);
+	spin_unlock_bh(&sta->lock);
 }
 
 void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid)
@@ -486,37 +506,35 @@ void ieee80211_start_tx_ba_cb(struct iee
 		return;
 	}
 
-	rcu_read_lock();
+	mutex_lock(&local->sta_mtx);
 	sta = sta_info_get(sdata, ra);
 	if (!sta) {
-		rcu_read_unlock();
+		mutex_unlock(&local->sta_mtx);
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "Could not find station: %pM\n", ra);
 #endif
 		return;
 	}
 
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
 	if (WARN_ON(!tid_tx)) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "addBA was not requested!\n");
 #endif
-		spin_unlock_bh(&sta->lock);
-		rcu_read_unlock();
-		return;
+		goto unlock;
 	}
 
 	if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state)))
-		goto out;
+		goto unlock;
 
 	if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state))
 		ieee80211_agg_tx_operational(local, sta, tid);
 
- out:
-	spin_unlock_bh(&sta->lock);
-	rcu_read_unlock();
+ unlock:
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+	mutex_unlock(&local->sta_mtx);
 }
 
 void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
@@ -548,21 +566,14 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_i
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator)
 {
-	struct tid_ampdu_tx *tid_tx;
 	int ret;
 
-	spin_lock_bh(&sta->lock);
-	tid_tx = sta->ampdu_mlme.tid_tx[tid];
-
-	if (!tid_tx) {
-		ret = -ENOENT;
-		goto unlock;
-	}
+	mutex_lock(&sta->ampdu_mlme.mtx);
 
 	ret = ___ieee80211_stop_tx_ba_session(sta, tid, initiator);
 
- unlock:
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+
 	return ret;
 }
 
@@ -627,16 +638,17 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	       ra, tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
 
-	rcu_read_lock();
+	mutex_lock(&local->sta_mtx);
+
 	sta = sta_info_get(sdata, ra);
 	if (!sta) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "Could not find station: %pM\n", ra);
 #endif
-		rcu_read_unlock();
-		return;
+		goto unlock;
 	}
 
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	spin_lock_bh(&sta->lock);
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
@@ -644,9 +656,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n");
 #endif
-		spin_unlock_bh(&sta->lock);
-		rcu_read_unlock();
-		return;
+		goto unlock_sta;
 	}
 
 	if (tid_tx->stop_initiator == WLAN_BACK_INITIATOR)
@@ -672,8 +682,11 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 
 	call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 
+ unlock_sta:
 	spin_unlock_bh(&sta->lock);
-	rcu_read_unlock();
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+ unlock:
+	mutex_unlock(&local->sta_mtx);
 }
 
 void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
@@ -714,10 +727,9 @@ void ieee80211_process_addba_resp(struct
 	capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab);
 	tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
 
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
-
 	if (!tid_tx)
 		goto out;
 
@@ -751,5 +763,5 @@ void ieee80211_process_addba_resp(struct
 	}
 
  out:
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-09 17:21:16.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-09 17:21:17.000000000 +0200
@@ -6,7 +6,7 @@
  * Copyright 2005-2006, Devicescape Software, Inc.
  * Copyright 2006-2007	Jiri Benc <jbenc@suse.cz>
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
- * Copyright 2007-2008, Intel Corporation
+ * Copyright 2007-2010, Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -136,11 +136,7 @@ void ieee80211_ba_session_work(struct wo
 			___ieee80211_stop_rx_ba_session(
 				sta, tid, WLAN_BACK_RECIPIENT,
 				WLAN_REASON_QSTA_TIMEOUT);
-	}
-	mutex_unlock(&sta->ampdu_mlme.mtx);
 
-	spin_lock_bh(&sta->lock);
-	for (tid = 0; tid < STA_TID_NUM; tid++) {
 		tid_tx = sta->ampdu_mlme.tid_tx[tid];
 		if (!tid_tx)
 			continue;
@@ -152,7 +148,7 @@ void ieee80211_ba_session_work(struct wo
 			___ieee80211_stop_tx_ba_session(sta, tid,
 							WLAN_BACK_INITIATOR);
 	}
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
 
 void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/driver-ops.h	2010-06-09 17:21:16.000000000 +0200
+++ wireless-testing/net/mac80211/driver-ops.h	2010-06-09 17:21:17.000000000 +0200
@@ -349,6 +349,9 @@ static inline int drv_ampdu_action(struc
 				   u16 *ssn)
 {
 	int ret = -EOPNOTSUPP;
+
+	might_sleep();
+
 	local_bh_disable();
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action,



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

* [PATCH 21/23] mac80211: allow drivers to sleep in ampdu_action
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (19 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 20/23] mac80211: change TX aggregation locking Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 22/23] mac80211: update aggregation documentation Johannes Berg
  2010-06-10  8:21 ` [PATCH 23/23] mac80211: fix mgmt frame accounting Johannes Berg
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Allow drivers to sleep, and indicate this in
the documentation. ath9k has some locking I
don't understand, so keep it safe and disable
BHs in it, all other drivers look fine with
the context change.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    4 ++++
 include/net/mac80211.h                |    2 +-
 net/mac80211/driver-ops.h             |    3 +--
 3 files changed, 6 insertions(+), 3 deletions(-)

--- wireless-testing.orig/include/net/mac80211.h	2010-06-09 17:21:12.000000000 +0200
+++ wireless-testing/include/net/mac80211.h	2010-06-09 17:21:17.000000000 +0200
@@ -1640,7 +1640,7 @@ enum ieee80211_ampdu_mlme_action {
  * 	is the first frame we expect to perform the action on. Notice
  * 	that TX/RX_STOP can pass NULL for this parameter.
  *	Returns a negative error code on failure.
- *	The callback must be atomic.
+ *	The callback can sleep.
  *
  * @get_survey: Return per-channel survey information
  *
--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/main.c	2010-06-09 17:20:45.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath/ath9k/main.c	2010-06-09 17:21:17.000000000 +0200
@@ -1769,6 +1769,8 @@ static int ath9k_ampdu_action(struct iee
 	struct ath_softc *sc = aphy->sc;
 	int ret = 0;
 
+	local_bh_disable();
+
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
 		if (!(sc->sc_flags & SC_OP_RXAGGR))
@@ -1798,6 +1800,8 @@ static int ath9k_ampdu_action(struct iee
 			  "Unknown AMPDU action\n");
 	}
 
+	local_bh_enable();
+
 	return ret;
 }
 
--- wireless-testing.orig/net/mac80211/driver-ops.h	2010-06-09 17:21:17.000000000 +0200
+++ wireless-testing/net/mac80211/driver-ops.h	2010-06-09 17:21:17.000000000 +0200
@@ -352,11 +352,10 @@ static inline int drv_ampdu_action(struc
 
 	might_sleep();
 
-	local_bh_disable();
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action,
 					       sta, tid, ssn);
-	local_bh_enable();
+
 	trace_drv_ampdu_action(local, sdata, action, sta, tid, ssn, ret);
 	return ret;
 }



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

* [PATCH 22/23] mac80211: update aggregation documentation
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (20 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 21/23] mac80211: allow drivers to sleep in ampdu_action Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  2010-06-10  8:21 ` [PATCH 23/23] mac80211: fix mgmt frame accounting Johannes Berg
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

Even before the recent changes, the documentation
for TX aggregation was somewhat out of date. Update
it and also add documentation for the RX side.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-rx.c |   23 +++++++++++++++++++++++
 net/mac80211/agg-tx.c |   43 +++++++++++++++++++++++++++----------------
 2 files changed, 50 insertions(+), 16 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-09 17:21:17.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-09 17:21:18.000000000 +0200
@@ -21,28 +21,39 @@
 #include "wme.h"
 
 /**
- * DOC: TX aggregation
+ * DOC: TX A-MPDU aggregation
  *
  * Aggregation on the TX side requires setting the hardware flag
- * %IEEE80211_HW_AMPDU_AGGREGATION as well as, if present, the @ampdu_queues
- * hardware parameter to the number of hardware AMPDU queues. If there are no
- * hardware queues then the driver will (currently) have to do all frame
- * buffering.
+ * %IEEE80211_HW_AMPDU_AGGREGATION. The driver will then be handed
+ * packets with a flag indicating A-MPDU aggregation. The driver
+ * or device is responsible for actually aggregating the frames,
+ * as well as deciding how many and which to aggregate.
  *
- * When TX aggregation is started by some subsystem (usually the rate control
- * algorithm would be appropriate) by calling the
- * ieee80211_start_tx_ba_session() function, the driver will be notified via
- * its @ampdu_action function, with the %IEEE80211_AMPDU_TX_START action.
+ * When TX aggregation is started by some subsystem (usually the rate
+ * control algorithm would be appropriate) by calling the
+ * ieee80211_start_tx_ba_session() function, the driver will be
+ * notified via its @ampdu_action function, with the
+ * %IEEE80211_AMPDU_TX_START action.
  *
  * In response to that, the driver is later required to call the
- * ieee80211_start_tx_ba_cb() (or ieee80211_start_tx_ba_cb_irqsafe())
- * function, which will start the aggregation session.
+ * ieee80211_start_tx_ba_cb_irqsafe() function, which will really
+ * start the aggregation session after the peer has also responded.
+ * If the peer responds negatively, the session will be stopped
+ * again right away. Note that it is possible for the aggregation
+ * session to be stopped before the driver has indicated that it
+ * is done setting it up, in which case it must not indicate the
+ * setup completion.
  *
- * Similarly, when the aggregation session is stopped by
- * ieee80211_stop_tx_ba_session(), the driver's @ampdu_action function will
- * be called with the action %IEEE80211_AMPDU_TX_STOP. In this case, the
- * call must not fail, and the driver must later call ieee80211_stop_tx_ba_cb()
- * (or ieee80211_stop_tx_ba_cb_irqsafe()).
+ * Also note that, since we also need to wait for a response from
+ * the peer, the driver is notified of the completion of the
+ * handshake by the %IEEE80211_AMPDU_TX_OPERATIONAL action to the
+ * @ampdu_action callback.
+ *
+ * Similarly, when the aggregation session is stopped by the peer
+ * or something calling ieee80211_stop_tx_ba_session(), the driver's
+ * @ampdu_action function will be called with the action
+ * %IEEE80211_AMPDU_TX_STOP. In this case, the call must not fail,
+ * and the driver must later call ieee80211_stop_tx_ba_cb_irqsafe().
  */
 
 static void ieee80211_send_addba_request(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/agg-rx.c	2010-06-09 17:21:16.000000000 +0200
+++ wireless-testing/net/mac80211/agg-rx.c	2010-06-09 17:21:18.000000000 +0200
@@ -13,6 +13,29 @@
  * published by the Free Software Foundation.
  */
 
+/**
+ * DOC: RX A-MPDU aggregation
+ *
+ * Aggregation on the RX side requires only implementing the
+ * @ampdu_action callback that is invoked to start/stop any
+ * block-ack sessions for RX aggregation.
+ *
+ * When RX aggregation is started by the peer, the driver is
+ * notified via @ampdu_action function, with the
+ * %IEEE80211_AMPDU_RX_START action, and may reject the request
+ * in which case a negative response is sent to the peer, if it
+ * accepts it a positive response is sent.
+ *
+ * While the session is active, the device/driver are required
+ * to de-aggregate frames and pass them up one by one to mac80211,
+ * which will handle the reorder buffer.
+ *
+ * When the aggregation session is stopped again by the peer or
+ * ourselves, the driver's @ampdu_action function will be called
+ * with the action %IEEE80211_AMPDU_RX_STOP. In this case, the
+ * call must not fail.
+ */
+
 #include <linux/ieee80211.h>
 #include <linux/slab.h>
 #include <net/mac80211.h>



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

* [PATCH 23/23] mac80211: fix mgmt frame accounting
  2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
                   ` (21 preceding siblings ...)
  2010-06-10  8:21 ` [PATCH 22/23] mac80211: update aggregation documentation Johannes Berg
@ 2010-06-10  8:21 ` Johannes Berg
  22 siblings, 0 replies; 24+ messages in thread
From: Johannes Berg @ 2010-06-10  8:21 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless

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

The recent change to processing action frames from
the management frame queue had already broken action
frame accounting, and my rework didn't help either.
So add back accounting and simplify the code with a
label rather than duplicating it, and also add
accounting for management frames.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/rx.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

--- wireless-testing.orig/net/mac80211/rx.c	2010-06-10 08:37:20.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-06-10 08:41:02.000000000 +0200
@@ -1966,10 +1966,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 			goto invalid;
 		}
 
-		rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
-		skb_queue_tail(&sdata->skb_queue, rx->skb);
-		ieee80211_queue_work(&local->hw, &sdata->work);
-		return RX_QUEUED;
+		goto queue;
 	case WLAN_CATEGORY_SPECTRUM_MGMT:
 		if (local->hw.conf.channel->band != IEEE80211_BAND_5GHZ)
 			break;
@@ -1999,10 +1996,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 			if (memcmp(mgmt->bssid, sdata->u.mgd.bssid, ETH_ALEN))
 				break;
 
-			rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
-			skb_queue_tail(&sdata->skb_queue, rx->skb);
-			ieee80211_queue_work(&local->hw, &sdata->work);
-			return RX_QUEUED;
+			goto queue;
 		}
 		break;
 	case WLAN_CATEGORY_SA_QUERY:
@@ -2022,10 +2016,7 @@ ieee80211_rx_h_action(struct ieee80211_r
 	case WLAN_CATEGORY_MESH_PATH_SEL:
 		if (!ieee80211_vif_is_mesh(&sdata->vif))
 			break;
-		rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
-		skb_queue_tail(&sdata->skb_queue, rx->skb);
-		ieee80211_queue_work(&local->hw, &sdata->work);
-		return RX_QUEUED;
+		goto queue;
 	}
 
  invalid:
@@ -2076,6 +2067,14 @@ ieee80211_rx_h_action(struct ieee80211_r
 		rx->sta->rx_packets++;
 	dev_kfree_skb(rx->skb);
 	return RX_QUEUED;
+
+ queue:
+	rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
+	skb_queue_tail(&sdata->skb_queue, rx->skb);
+	ieee80211_queue_work(&local->hw, &sdata->work);
+	if (rx->sta)
+		rx->sta->rx_packets++;
+	return RX_QUEUED;
 }
 
 static ieee80211_rx_result debug_noinline
@@ -2131,6 +2130,8 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_
 	rx->skb->pkt_type = IEEE80211_SDATA_QUEUE_TYPE_FRAME;
 	skb_queue_tail(&sdata->skb_queue, rx->skb);
 	ieee80211_queue_work(&rx->local->hw, &sdata->work);
+	if (rx->sta)
+		rx->sta->rx_packets++;
 
 	return RX_QUEUED;
 }



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

end of thread, other threads:[~2010-06-10  8:26 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-06-10  8:21 [PATCH 00/23] aggregation locking rework Johannes Berg
2010-06-10  8:21 ` [PATCH 01/23] mac80211: simplify station/aggregation code Johannes Berg
2010-06-10  8:21 ` [PATCH 02/23] mac80211: use common skb queue Johannes Berg
2010-06-10  8:21 ` [PATCH 03/23] mac80211: use common work struct Johannes Berg
2010-06-10  8:21 ` [PATCH 04/23] mac80211: use common work function Johannes Berg
2010-06-10  8:21 ` [PATCH 05/23] mac80211: common work skb freeing Johannes Berg
2010-06-10  8:21 ` [PATCH 06/23] mac80211: pull mgmt frame rx into rx handler Johannes Berg
2010-06-10  8:21 ` [PATCH 07/23] mac80211: always process blockack action from workqueue Johannes Berg
2010-06-10  8:21 ` [PATCH 08/23] mac80211: move blockack stop due to fragmentation Johannes Berg
2010-06-10  8:21 ` [PATCH 09/23] mac80211: move aggregation callback processing Johannes Berg
2010-06-10  8:21 ` [PATCH 10/23] mac80211: use RCU for RX aggregation Johannes Berg
2010-06-10  8:21 ` [PATCH 11/23] mac80211: use RCU for TX aggregation Johannes Berg
2010-06-10  8:21 ` [PATCH 12/23] mac80211: remove non-irqsafe aggregation callbacks Johannes Berg
2010-06-10  8:21 ` [PATCH 13/23] mac80211: refcount aggregation queue stop Johannes Berg
2010-06-10  8:21 ` [PATCH 14/23] mac80211: make TX aggregation start/stop request async Johannes Berg
2010-06-10  8:21 ` [PATCH 15/23] mac80211: move BA session work Johannes Berg
2010-06-10  8:21 ` [PATCH 16/23] mac80211: defer RX agg session teardown to work Johannes Berg
2010-06-10  8:21 ` [PATCH 17/23] mac80211: fix RX aggregation timer Johannes Berg
2010-06-10  8:21 ` [PATCH 18/23] mac80211: change RX aggregation locking Johannes Berg
2010-06-10  8:21 ` [PATCH 19/23] mac80211: defer TX agg session teardown to work Johannes Berg
2010-06-10  8:21 ` [PATCH 20/23] mac80211: change TX aggregation locking Johannes Berg
2010-06-10  8:21 ` [PATCH 21/23] mac80211: allow drivers to sleep in ampdu_action Johannes Berg
2010-06-10  8:21 ` [PATCH 22/23] mac80211: update aggregation documentation Johannes Berg
2010-06-10  8:21 ` [PATCH 23/23] mac80211: fix mgmt frame accounting Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.