summaryrefslogtreecommitdiffstats
path: root/net/mac80211/agg-tx.c
diff options
context:
space:
mode:
authorJohannes Berg <johannes.berg@intel.com>2011-05-13 13:35:40 +0200
committerJohn W. Linville <linville@tuxdriver.com>2011-05-16 14:10:40 -0400
commitec034b208dc8aa5dc73ec46c3f27e34c5efbf113 (patch)
tree56140fc3a4115441822b5cc2aa9de5224bcb8654 /net/mac80211/agg-tx.c
parent7527a782e187d1214a5b3dc2897ce441033bb4ef (diff)
mac80211: fix TX a-MPDU locking
During my quest to make mac80211 not have any RCU warnings from sparse, I came across the a-MPDU code again and it wasn't quite clear why it isn't racy. So instead of assigning the tid_tx array with just the spinlock held in ieee80211_start_tx_ba_session use a separate temporary array protected only by the spinlock and protect all assignments to the "live" array by both the spinlock and the mutex so that other code is easily verified to be correct. Due to pointer assignment atomicity I don't think this is a real issue, but I'm not sure, especially on Alpha the current code might be problematic. Signed-off-by: Johannes Berg <johannes.berg@intel.com> Signed-off-by: John W. Linville <linville@tuxdriver.com>
Diffstat (limited to 'net/mac80211/agg-tx.c')
-rw-r--r--net/mac80211/agg-tx.c23
1 files changed, 17 insertions, 6 deletions
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 63d852cb4ca..f614ee60208 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -136,6 +136,14 @@ void ieee80211_send_bar(struct ieee80211_sub_if_data *sdata, u8 *ra, u16 tid, u1
ieee80211_tx_skb(sdata, skb);
}
+void ieee80211_assign_tid_tx(struct sta_info *sta, int tid,
+ struct tid_ampdu_tx *tid_tx)
+{
+ lockdep_assert_held(&sta->ampdu_mlme.mtx);
+ lockdep_assert_held(&sta->lock);
+ rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
+}
+
static void kfree_tid_tx(struct rcu_head *rcu_head)
{
struct tid_ampdu_tx *tid_tx =
@@ -161,7 +169,7 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
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);
+ ieee80211_assign_tid_tx(sta, tid, NULL);
spin_unlock_bh(&sta->lock);
call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
return 0;
@@ -318,7 +326,7 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
" tid %d\n", tid);
#endif
spin_lock_bh(&sta->lock);
- rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+ ieee80211_assign_tid_tx(sta, tid, NULL);
spin_unlock_bh(&sta->lock);
ieee80211_wake_queue_agg(local, tid);
@@ -398,7 +406,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
tid_tx = sta->ampdu_mlme.tid_tx[tid];
/* check if the TID is not in aggregation flow already */
- if (tid_tx) {
+ if (tid_tx || sta->ampdu_mlme.tid_start_tx[tid]) {
#ifdef CONFIG_MAC80211_HT_DEBUG
printk(KERN_DEBUG "BA request denied - session is not "
"idle on tid %u\n", tid);
@@ -433,8 +441,11 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
sta->ampdu_mlme.dialog_token_allocator++;
tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator;
- /* finally, assign it to the array */
- rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
+ /*
+ * Finally, assign it to the start array; the work item will
+ * collect it and move it to the normal array.
+ */
+ sta->ampdu_mlme.tid_start_tx[tid] = tid_tx;
ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
@@ -697,7 +708,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid)
ieee80211_agg_splice_packets(local, tid_tx, tid);
/* future packets must not find the tid_tx struct any more */
- rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+ ieee80211_assign_tid_tx(sta, tid, NULL);
ieee80211_agg_splice_finish(local, tid);