aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJohannes Berg <johannes.berg@intel.com>2011-05-13 07:35:40 -0400
committerJohn W. Linville <linville@tuxdriver.com>2011-05-16 14:10:40 -0400
commitec034b208dc8aa5dc73ec46c3f27e34c5efbf113 (patch)
tree56140fc3a4115441822b5cc2aa9de5224bcb8654
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>
-rw-r--r--net/mac80211/agg-tx.c23
-rw-r--r--net/mac80211/ht.c27
-rw-r--r--net/mac80211/sta_info.h4
3 files changed, 42 insertions, 12 deletions
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 63d852cb4ca2..f614ee602089 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
136 ieee80211_tx_skb(sdata, skb); 136 ieee80211_tx_skb(sdata, skb);
137} 137}
138 138
139void ieee80211_assign_tid_tx(struct sta_info *sta, int tid,
140 struct tid_ampdu_tx *tid_tx)
141{
142 lockdep_assert_held(&sta->ampdu_mlme.mtx);
143 lockdep_assert_held(&sta->lock);
144 rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx);
145}
146
139static void kfree_tid_tx(struct rcu_head *rcu_head) 147static void kfree_tid_tx(struct rcu_head *rcu_head)
140{ 148{
141 struct tid_ampdu_tx *tid_tx = 149 struct tid_ampdu_tx *tid_tx =
@@ -161,7 +169,7 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
161 169
162 if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) { 170 if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) {
163 /* not even started yet! */ 171 /* not even started yet! */
164 rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); 172 ieee80211_assign_tid_tx(sta, tid, NULL);
165 spin_unlock_bh(&sta->lock); 173 spin_unlock_bh(&sta->lock);
166 call_rcu(&tid_tx->rcu_head, kfree_tid_tx); 174 call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
167 return 0; 175 return 0;
@@ -318,7 +326,7 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
318 " tid %d\n", tid); 326 " tid %d\n", tid);
319#endif 327#endif
320 spin_lock_bh(&sta->lock); 328 spin_lock_bh(&sta->lock);
321 rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); 329 ieee80211_assign_tid_tx(sta, tid, NULL);
322 spin_unlock_bh(&sta->lock); 330 spin_unlock_bh(&sta->lock);
323 331
324 ieee80211_wake_queue_agg(local, tid); 332 ieee80211_wake_queue_agg(local, tid);
@@ -398,7 +406,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
398 406
399 tid_tx = sta->ampdu_mlme.tid_tx[tid]; 407 tid_tx = sta->ampdu_mlme.tid_tx[tid];
400 /* check if the TID is not in aggregation flow already */ 408 /* check if the TID is not in aggregation flow already */
401 if (tid_tx) { 409 if (tid_tx || sta->ampdu_mlme.tid_start_tx[tid]) {
402#ifdef CONFIG_MAC80211_HT_DEBUG 410#ifdef CONFIG_MAC80211_HT_DEBUG
403 printk(KERN_DEBUG "BA request denied - session is not " 411 printk(KERN_DEBUG "BA request denied - session is not "
404 "idle on tid %u\n", tid); 412 "idle on tid %u\n", tid);
@@ -433,8 +441,11 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
433 sta->ampdu_mlme.dialog_token_allocator++; 441 sta->ampdu_mlme.dialog_token_allocator++;
434 tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator; 442 tid_tx->dialog_token = sta->ampdu_mlme.dialog_token_allocator;
435 443
436 /* finally, assign it to the array */ 444 /*
437 rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], tid_tx); 445 * Finally, assign it to the start array; the work item will
446 * collect it and move it to the normal array.
447 */
448 sta->ampdu_mlme.tid_start_tx[tid] = tid_tx;
438 449
439 ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work); 450 ieee80211_queue_work(&local->hw, &sta->ampdu_mlme.work);
440 451
@@ -697,7 +708,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid)
697 ieee80211_agg_splice_packets(local, tid_tx, tid); 708 ieee80211_agg_splice_packets(local, tid_tx, tid);
698 709
699 /* future packets must not find the tid_tx struct any more */ 710 /* future packets must not find the tid_tx struct any more */
700 rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL); 711 ieee80211_assign_tid_tx(sta, tid, NULL);
701 712
702 ieee80211_agg_splice_finish(local, tid); 713 ieee80211_agg_splice_finish(local, tid);
703 714
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index b9e4b9bd2179..9f5842a43111 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -140,14 +140,29 @@ void ieee80211_ba_session_work(struct work_struct *work)
140 sta, tid, WLAN_BACK_RECIPIENT, 140 sta, tid, WLAN_BACK_RECIPIENT,
141 WLAN_REASON_QSTA_TIMEOUT, true); 141 WLAN_REASON_QSTA_TIMEOUT, true);
142 142
143 tid_tx = sta->ampdu_mlme.tid_tx[tid]; 143 tid_tx = sta->ampdu_mlme.tid_start_tx[tid];
144 if (!tid_tx) 144 if (tid_tx) {
145 continue; 145 /*
146 * Assign it over to the normal tid_tx array
147 * where it "goes live".
148 */
149 spin_lock_bh(&sta->lock);
150
151 sta->ampdu_mlme.tid_start_tx[tid] = NULL;
152 /* could there be a race? */
153 if (sta->ampdu_mlme.tid_tx[tid])
154 kfree(tid_tx);
155 else
156 ieee80211_assign_tid_tx(sta, tid, tid_tx);
157 spin_unlock_bh(&sta->lock);
146 158
147 if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state))
148 ieee80211_tx_ba_session_handle_start(sta, tid); 159 ieee80211_tx_ba_session_handle_start(sta, tid);
149 else if (test_and_clear_bit(HT_AGG_STATE_WANT_STOP, 160 continue;
150 &tid_tx->state)) 161 }
162
163 tid_tx = sta->ampdu_mlme.tid_tx[tid];
164 if (tid_tx && test_and_clear_bit(HT_AGG_STATE_WANT_STOP,
165 &tid_tx->state))
151 ___ieee80211_stop_tx_ba_session(sta, tid, 166 ___ieee80211_stop_tx_ba_session(sta, tid,
152 WLAN_BACK_INITIATOR, 167 WLAN_BACK_INITIATOR,
153 true); 168 true);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index f00b4dcb49d7..55c51855ceb7 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -152,6 +152,7 @@ struct tid_ampdu_rx {
152 * 152 *
153 * @tid_rx: aggregation info for Rx per TID -- RCU protected 153 * @tid_rx: aggregation info for Rx per TID -- RCU protected
154 * @tid_tx: aggregation info for Tx per TID 154 * @tid_tx: aggregation info for Tx per TID
155 * @tid_start_tx: sessions where start was requested
155 * @addba_req_num: number of times addBA request has been sent. 156 * @addba_req_num: number of times addBA request has been sent.
156 * @dialog_token_allocator: dialog token enumerator for each new session; 157 * @dialog_token_allocator: dialog token enumerator for each new session;
157 * @work: work struct for starting/stopping aggregation 158 * @work: work struct for starting/stopping aggregation
@@ -168,6 +169,7 @@ struct sta_ampdu_mlme {
168 /* tx */ 169 /* tx */
169 struct work_struct work; 170 struct work_struct work;
170 struct tid_ampdu_tx *tid_tx[STA_TID_NUM]; 171 struct tid_ampdu_tx *tid_tx[STA_TID_NUM];
172 struct tid_ampdu_tx *tid_start_tx[STA_TID_NUM];
171 u8 addba_req_num[STA_TID_NUM]; 173 u8 addba_req_num[STA_TID_NUM];
172 u8 dialog_token_allocator; 174 u8 dialog_token_allocator;
173}; 175};
@@ -398,6 +400,8 @@ static inline u32 get_sta_flags(struct sta_info *sta)
398 return ret; 400 return ret;
399} 401}
400 402
403void ieee80211_assign_tid_tx(struct sta_info *sta, int tid,
404 struct tid_ampdu_tx *tid_tx);
401 405
402 406
403#define STA_HASH_SIZE 256 407#define STA_HASH_SIZE 256