Skip to content

Commit

Permalink
mac80211: refcount aggregation queue stop
Browse files Browse the repository at this point in the history 
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>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
  • Loading branch information
Johannes Berg authored and John W. Linville committed Jun 14, 2010
1 parent 5d22c89 commit a6a67db
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 40 deletions.
77 changes: 47 additions & 30 deletions net/mac80211/agg-tx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,41 @@ static inline int ieee80211_ac_from_tid(int 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);
Expand Down Expand Up @@ -263,7 +298,6 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
}

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) {
Expand All @@ -289,9 +323,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
* 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);
Expand Down Expand Up @@ -327,11 +359,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
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;
Expand All @@ -358,31 +386,25 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid)
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;
}
EXPORT_SYMBOL(ieee80211_start_tx_ba_session);

/*
* 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))
Expand All @@ -397,11 +419,10 @@ static void ieee80211_agg_splice_packets(struct ieee80211_local *local,
}
}

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 */
Expand All @@ -414,7 +435,6 @@ static void ieee80211_agg_tx_operational(struct ieee80211_local *local,
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
Expand All @@ -423,7 +443,6 @@ static void ieee80211_agg_tx_operational(struct ieee80211_local *local,
*/
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,
Expand Down Expand Up @@ -604,7 +623,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 tid)
* 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 */
Expand All @@ -613,7 +631,6 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u8 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);
rcu_read_unlock();
Expand Down
8 changes: 1 addition & 7 deletions net/mac80211/ieee80211_i.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions net/mac80211/main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,8 +463,10 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,

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

Expand All @@ -475,8 +477,6 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
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);
Expand Down

0 comments on commit a6a67db

Please sign in to comment.