All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <lrodriguez@atheros.com>
To: linville@tuxdriver.com
Cc: linux-wireless@vger.kernel.org,
	"Luis R. Rodriguez" <lrodriguez@atheros.com>,
	stable@kernel.org, Paul Stewart <pstew@google.com>,
	Amod Bodas <amod.bodas@atheros.com>
Subject: [PATCH v3 01/11] ath9k: fix power save race conditions
Date: Tue, 14 Sep 2010 19:40:14 -0400	[thread overview]
Message-ID: <1284507624-18176-2-git-send-email-lrodriguez@atheros.com> (raw)
In-Reply-To: <1284507624-18176-1-git-send-email-lrodriguez@atheros.com>

ath9k has a race on putting the chip into network sleep and
having registers read from hardware. The race occurs because
although ath9k_ps_restore() locks its own callers it makes use
of some variables which get altered in the driver at different
code paths. The variables are the ps_enabled and ps_flags.

This is easily reprodicible in large network environments when
roaming with the wpa_supplicant simple bgscan. You'd get some
0xdeadbeef read out on certain registers such as:

ath: timeout (100000 us) on reg 0x806c: 0xdeadbeef & 0x01f00000 != 0x00000000
ath: RX failed to go idle in 10 ms RXSM=0xdeadbeef

ath: timeout (100000 us) on reg 0x7000: 0xdeadbeef & 0x00000003 != 0x00000000
ath: Chip reset failed

The fix is to protect the ath9k_config(hw, IEEE80211_CONF_CHANGE_PS)
calls with a spin_lock_irqsave() which will disable contendors for
these variables from interrupt context, timers, re-entry from mac80211
on the same callback, and most importantly from ath9k_ps_restore()
which is the only call which will put the device into network sleep.

There are quite a few threads and bug reports on these a few of them are:

https://bugs.launchpad.net/ubuntu/karmic/+source/linux/+bug/407040
http://code.google.com/p/chromium-os/issues/detail?id=5709
http://code.google.com/p/chromium-os/issues/detail?id=5943

Stable fixes apply to [2.6.32+]

Cc: stable@kernel.org
Cc: Paul Stewart <pstew@google.com>
Cc: Amod Bodas <amod.bodas@atheros.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    5 ++++-
 drivers/net/wireless/ath/ath9k/recv.c |    3 +++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index a2f7eb2..826315d 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1554,6 +1554,8 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	 * IEEE80211_CONF_CHANGE_PS is only passed by mac80211 for STA mode.
 	 */
 	if (changed & IEEE80211_CONF_CHANGE_PS) {
+		unsigned long flags;
+		spin_lock_irqsave(&sc->sc_pm_lock, flags);
 		if (conf->flags & IEEE80211_CONF_PS) {
 			sc->ps_flags |= PS_ENABLED;
 			/*
@@ -1568,7 +1570,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 			sc->ps_enabled = false;
 			sc->ps_flags &= ~(PS_ENABLED |
 					  PS_NULLFUNC_COMPLETED);
-			ath9k_setpower(sc, ATH9K_PM_AWAKE);
+			ath9k_hw_setpower(sc->sc_ah, ATH9K_PM_AWAKE);
 			if (!(ah->caps.hw_caps &
 			      ATH9K_HW_CAP_AUTOSLEEP)) {
 				ath9k_hw_setrxabort(sc->sc_ah, 0);
@@ -1583,6 +1585,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 				}
 			}
 		}
+		spin_unlock_irqrestore(&sc->sc_pm_lock, flags);
 	}
 
 	if (changed & IEEE80211_CONF_CHANGE_MONITOR) {
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6fb3b45..7523f7d 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1640,6 +1640,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 	u8 rx_status_len = ah->caps.rx_status_len;
 	u64 tsf = 0;
 	u32 tsf_lower = 0;
+	unsigned long flags;
 
 	if (edma)
 		dma_type = DMA_BIDIRECTIONAL;
@@ -1748,11 +1749,13 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
 			sc->rx.rxotherant = 0;
 		}
 
+		spin_lock_irqsave(&sc->sc_pm_lock, flags);
 		if (unlikely(ath9k_check_auto_sleep(sc) ||
 			     (sc->ps_flags & (PS_WAIT_FOR_BEACON |
 					      PS_WAIT_FOR_CAB |
 					      PS_WAIT_FOR_PSPOLL_DATA))))
 			ath_rx_ps(sc, skb);
+		spin_unlock_irqrestore(&sc->sc_pm_lock, flags);
 
 		if (ah->caps.hw_caps & ATH9K_HW_CAP_ANT_DIV_COMB)
 			ath_ant_comb_scan(sc, &rs);
-- 
1.7.0.4


  reply	other threads:[~2010-09-14 23:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-14 23:40 [PATCH v3 00/11] ath9k / mac80211: power save fixes Luis R. Rodriguez
2010-09-14 23:40 ` Luis R. Rodriguez [this message]
2010-09-14 23:40 ` [PATCH v3 02/11] ath9k: fix regression on beacon loss after bgscan Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 03/11] ath9k: fix enabling ANI / tx monitor after bg scan Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 04/11] mac80211: add helper for reseting the connection monitor Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 05/11] mac80211: reset probe send counter upon connection timer reset Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 06/11] mac80211: reset connection idle when going offchannel Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 07/11] mac80211: make the beacon monitor available externally Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 08/11] mac80211: disable beacon monitor while going offchannel Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 09/11] mac80211: send last 3/5 probe requests as unicast Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 10/11] ath9k: fix regression which prevents chip sleep after CAB data Luis R. Rodriguez
2010-09-14 23:40 ` [PATCH v3 11/11] ath9k: fix regression which disabled ps on ath9k Luis R. Rodriguez
2010-09-15  7:23 ` [PATCH v3 00/11] ath9k / mac80211: power save fixes Luis R. Rodriguez
     [not found] ` <AANLkTinNA9Xb-bq17V_PcyLujaJM-mOWimvQzWMRKpk6@mail.gmail.com>
     [not found]   ` <10161E8E-4F96-4B3B-B5D3-27DF9236BF83@Atheros.com>
2010-09-15 18:31     ` Luis R. Rodriguez

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1284507624-18176-2-git-send-email-lrodriguez@atheros.com \
    --to=lrodriguez@atheros.com \
    --cc=amod.bodas@atheros.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=pstew@google.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.