ath9k: fix race condition in enabling/disabling IRQs

BugLink: http://bugs.launchpad.net/bugs/1673538
commit 3a5e969bb2f6692a256352649355d56d018d6b88 upstream.

The code currently relies on refcounting to disable IRQs from within the
IRQ handler and re-enabling them again after the tasklet has run.

However, due to race conditions sometimes the IRQ handler might be
called twice, or the tasklet may not run at all (if interrupted in the
middle of a reset).

This can cause nasty imbalances in the irq-disable refcount which will
get the driver permanently stuck until the entire radio has been stopped
and started again (ath_reset will not recover from this).

Instead of using this fragile logic, change the code to ensure that
running the irq handler during tasklet processing is safe, and leave the
refcount untouched.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index b42f4a963ef40ff0e6e29997099ad7cd66a779c1..a660e40f2df1e31ed3c5f6ed7459db0801536e97 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -959,6 +959,7 @@ struct ath_softc {
        struct survey_info *cur_survey;
        struct survey_info survey[ATH9K_NUM_CHANNELS];
 
+       spinlock_t intr_lock;
        struct tasklet_struct intr_tq;
        struct tasklet_struct bcon_tasklet;
        struct ath_hw *sc_ah;

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index bc70ce62bc03ce5e82844afc11ad268426a0cd3a..0f5672f5c9bade1b9d17bebeab49ae4595d8dac8 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -619,6 +619,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
                common->bt_ant_diversity = 1;
 
        spin_lock_init(&common->cc_lock);
+       spin_lock_init(&sc->intr_lock);
        spin_lock_init(&sc->sc_serial_rw);
        spin_lock_init(&sc->sc_pm_lock);
        spin_lock_init(&sc->chan_lock);

diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index bba85d1a6cd1bc44852d1845bede4918bc2b5ec7..d937c39b3a0b3569a7e4a67ec24973e17c9e4211 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -805,21 +805,12 @@ void ath9k_hw_disable_interrupts(struct ath_hw *ah)
 }
 EXPORT_SYMBOL(ath9k_hw_disable_interrupts);
 
-void ath9k_hw_enable_interrupts(struct ath_hw *ah)
+static void __ath9k_hw_enable_interrupts(struct ath_hw *ah)
 {
        struct ath_common *common = ath9k_hw_common(ah);
        u32 sync_default = AR_INTR_SYNC_DEFAULT;
        u32 async_mask;
 
-       if (!(ah->imask & ATH9K_INT_GLOBAL))
-               return;
-
-       if (!atomic_inc_and_test(&ah->intr_ref_cnt)) {
-               ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
-                       atomic_read(&ah->intr_ref_cnt));
-               return;
-       }
-
        if (AR_SREV_9340(ah) || AR_SREV_9550(ah) || AR_SREV_9531(ah) ||
            AR_SREV_9561(ah))
                sync_default &= ~AR_INTR_SYNC_HOST1_FATAL;
@@ -841,6 +832,39 @@ void ath9k_hw_enable_interrupts(struct ath_hw *ah)
        ath_dbg(common, INTERRUPT, "AR_IMR 0x%x IER 0x%x\n",
                REG_READ(ah, AR_IMR), REG_READ(ah, AR_IER));
 }
+
+void ath9k_hw_resume_interrupts(struct ath_hw *ah)
+{
+       struct ath_common *common = ath9k_hw_common(ah);
+
+       if (!(ah->imask & ATH9K_INT_GLOBAL))
+               return;
+
+       if (atomic_read(&ah->intr_ref_cnt) != 0) {
+               ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
+                       atomic_read(&ah->intr_ref_cnt));
+               return;
+       }
+
+       __ath9k_hw_enable_interrupts(ah);
+}
+EXPORT_SYMBOL(ath9k_hw_resume_interrupts);
+
+void ath9k_hw_enable_interrupts(struct ath_hw *ah)
+{
+       struct ath_common *common = ath9k_hw_common(ah);
+
+       if (!(ah->imask & ATH9K_INT_GLOBAL))
+               return;
+
+       if (!atomic_inc_and_test(&ah->intr_ref_cnt)) {
+               ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
+                       atomic_read(&ah->intr_ref_cnt));
+               return;
+       }
+
+       __ath9k_hw_enable_interrupts(ah);
+}
 EXPORT_SYMBOL(ath9k_hw_enable_interrupts);
 
 void ath9k_hw_set_interrupts(struct ath_hw *ah)

diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index 7fbf7f965f61c21bbc22371a1ddbf847252379df..1b63d26f30ce8f588cc3bece253bbaeb9f31fe12 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -748,6 +748,7 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah);
 void ath9k_hw_enable_interrupts(struct ath_hw *ah);
 void ath9k_hw_disable_interrupts(struct ath_hw *ah);
 void ath9k_hw_kill_interrupts(struct ath_hw *ah);
+void ath9k_hw_resume_interrupts(struct ath_hw *ah);
 
 void ar9002_hw_attach_mac_ops(struct ath_hw *ah);
 

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 8c5d2cf9c97981c42c4b792b878cb87589f1a120..b114e57a823fd2fb79a8d2195785f3f51bc86d29 100644 (file)
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -373,21 +373,20 @@ void ath9k_tasklet(unsigned long data)
        struct ath_common *common = ath9k_hw_common(ah);
        enum ath_reset_type type;
        unsigned long flags;
-       u32 status = sc->intrstatus;
+       u32 status;
        u32 rxmask;
 
+       spin_lock_irqsave(&sc->intr_lock, flags);
+       status = sc->intrstatus;
+       sc->intrstatus = 0;
+       spin_unlock_irqrestore(&sc->intr_lock, flags);
+
        ath9k_ps_wakeup(sc);
        spin_lock(&sc->sc_pcu_lock);
 
        if (status & ATH9K_INT_FATAL) {
                type = RESET_TYPE_FATAL_INT;
                ath9k_queue_reset(sc, type);
-
-               /*
-                * Increment the ref. counter here so that
-                * interrupts are enabled in the reset routine.
-                */
-               atomic_inc(&ah->intr_ref_cnt);
                ath_dbg(common, RESET, "FATAL: Skipping interrupts\n");
                goto out;
        }
@@ -403,11 +402,6 @@ void ath9k_tasklet(unsigned long data)
                        type = RESET_TYPE_BB_WATCHDOG;
                        ath9k_queue_reset(sc, type);
 
-                       /*
-                        * Increment the ref. counter here so that
-                        * interrupts are enabled in the reset routine.
-                        */
-                       atomic_inc(&ah->intr_ref_cnt);
                        ath_dbg(common, RESET,
                                "BB_WATCHDOG: Skipping interrupts\n");
                        goto out;
@@ -420,7 +414,6 @@ void ath9k_tasklet(unsigned long data)
                if ((sc->gtt_cnt >= MAX_GTT_CNT) && !ath9k_hw_check_alive(ah)) {
                        type = RESET_TYPE_TX_GTT;
                        ath9k_queue_reset(sc, type);
-                       atomic_inc(&ah->intr_ref_cnt);
                        ath_dbg(common, RESET,
                                "GTT: Skipping interrupts\n");
                        goto out;
@@ -477,7 +470,7 @@ void ath9k_tasklet(unsigned long data)
        ath9k_btcoex_handle_interrupt(sc, status);
 
        /* re-enable hardware interrupt */
-       ath9k_hw_enable_interrupts(ah);
+       ath9k_hw_resume_interrupts(ah);
 out:
        spin_unlock(&sc->sc_pcu_lock);
        ath9k_ps_restore(sc);
@@ -541,7 +534,9 @@ irqreturn_t ath_isr(int irq, void *dev)
                return IRQ_NONE;
 
        /* Cache the status */
-       sc->intrstatus = status;
+       spin_lock(&sc->intr_lock);
+       sc->intrstatus |= status;
+       spin_unlock(&sc->intr_lock);
 
        if (status & SCHED_INTR)
                sched = true;
@@ -587,7 +582,7 @@ chip_reset:
 
        if (sched) {
                /* turn off every interrupt */
-               ath9k_hw_disable_interrupts(ah);
+               ath9k_hw_kill_interrupts(ah);
                tasklet_schedule(&sc->intr_tq);
        }