From 8ade00824607fcfa8842572012d4393b40a74a94 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 18 Nov 2009 17:15:06 +0100 Subject: mac80211: fix addba timer (again...) commit 2171abc58644e09dbba546d91366b12743115396 Author: Johannes Berg Date: Thu Oct 29 08:34:00 2009 +0100 mac80211: fix addba timer left a problem in there, even if the timer was never started it could be deleted and then added. Linus pointed out that del_timer_sync() isn't actually needed if we make the timer able to deal with no longer being needed when it gets queued _while_ we're in the locked section that also deletes it. For that the timer function only needs to check the HT_ADDBA_RECEIVED_MSK bit as well as the HT_ADDBA_REQUESTED_MSK bit, only if the former is clear should it do anything. Cc: Linus Torvalds Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'net/mac80211') diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index b09948ceec4..206fd82f0c7 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -173,12 +173,14 @@ static void sta_addba_resp_timer_expired(unsigned long data) /* check if the TID waits for addBA response */ spin_lock_bh(&sta->lock); - if (!(*state & HT_ADDBA_REQUESTED_MSK)) { + if ((*state & (HT_ADDBA_REQUESTED_MSK | HT_ADDBA_RECEIVED_MSK)) != + HT_ADDBA_REQUESTED_MSK) { spin_unlock_bh(&sta->lock); *state = HT_AGG_STATE_IDLE; #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "timer expired on tid %d but we are not " - "expecting addBA response there", tid); + "(or no longer) expecting addBA response there", + tid); #endif return; } @@ -666,21 +668,21 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, state = &sta->ampdu_mlme.tid_state_tx[tid]; - del_timer_sync(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer); - spin_lock_bh(&sta->lock); if (!(*state & HT_ADDBA_REQUESTED_MSK)) - goto timer_still_needed; + goto out; if (mgmt->u.action.u.addba_resp.dialog_token != sta->ampdu_mlme.tid_tx[tid]->dialog_token) { #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "wrong addBA response token, tid %d\n", tid); #endif /* CONFIG_MAC80211_HT_DEBUG */ - goto timer_still_needed; + goto out; } + del_timer(&sta->ampdu_mlme.tid_tx[tid]->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 */ @@ -699,10 +701,6 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, ___ieee80211_stop_tx_ba_session(sta, tid, WLAN_BACK_INITIATOR); } - goto out; - - timer_still_needed: - add_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer); out: spin_unlock_bh(&sta->lock); } -- cgit v1.2.3 From ceb99fe071eb688255798d89be337affffa2b103 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 19 Nov 2009 14:29:39 +0100 Subject: mac80211: fix resume When mac80211 resumes, it currently first sets suspended to false so the driver can start doing things and we can receive frames. However, if we actually receive frames then it can end up starting some work which adds timers and then later runs into a BUG_ON in the timer code because it tries add_timer() on a pending timer. Fix this by keeping track of the resuming process by introducing a new variable 'resuming' which gets set to true early on instead of setting 'suspended' to false, and allow queueing work but not receiving frames while resuming. Reported-by: Maxim Levitsky Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/ieee80211_i.h | 8 ++++++++ net/mac80211/util.c | 19 +++++++++---------- 2 files changed, 17 insertions(+), 10 deletions(-) (limited to 'net/mac80211') diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 588005c84a6..a910bf1f092 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -661,6 +661,14 @@ struct ieee80211_local { */ bool suspended; + /* + * Resuming is true while suspended, but when we're reprogramming the + * hardware -- at that time it's allowed to use ieee80211_queue_work() + * again even though some other parts of the stack are still suspended + * and we still drop received frames to avoid waking the stack. + */ + bool resuming; + /* * quiescing is true during the suspend process _only_ to * ease timer cancelling etc. diff --git a/net/mac80211/util.c b/net/mac80211/util.c index aeb65b3d229..e6c08da8da2 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -520,9 +520,9 @@ EXPORT_SYMBOL_GPL(ieee80211_iterate_active_interfaces_atomic); */ static bool ieee80211_can_queue_work(struct ieee80211_local *local) { - if (WARN(local->suspended, "queueing ieee80211 work while " - "going to suspend\n")) - return false; + if (WARN(local->suspended && !local->resuming, + "queueing ieee80211 work while going to suspend\n")) + return false; return true; } @@ -1025,13 +1025,9 @@ int ieee80211_reconfig(struct ieee80211_local *local) struct sta_info *sta; unsigned long flags; int res; - bool from_suspend = local->suspended; - /* - * We're going to start the hardware, at that point - * we are no longer suspended and can RX frames. - */ - local->suspended = false; + if (local->suspended) + local->resuming = true; /* restart hardware */ if (local->open_count) { @@ -1129,11 +1125,14 @@ int ieee80211_reconfig(struct ieee80211_local *local) * If this is for hw restart things are still running. * We may want to change that later, however. */ - if (!from_suspend) + if (!local->suspended) return 0; #ifdef CONFIG_PM + /* first set suspended false, then resuming */ local->suspended = false; + mb(); + local->resuming = false; list_for_each_entry(sdata, &local->interfaces, list) { switch(sdata->vif.type) { -- cgit v1.2.3