123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197 |
- From: Felix Fietkau <nbd@nbd.name>
- Date: Wed, 25 Jan 2017 15:10:37 +0100
- Subject: [PATCH] ath9k: fix race condition in enabling/disabling IRQs
- 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.
- Cc: stable@vger.kernel.org
- Signed-off-by: Felix Fietkau <nbd@nbd.name>
- ---
- --- a/drivers/net/wireless/ath/ath9k/ath9k.h
- +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
- @@ -998,6 +998,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;
- --- a/drivers/net/wireless/ath/ath9k/init.c
- +++ b/drivers/net/wireless/ath/ath9k/init.c
- @@ -669,6 +669,7 @@ static int ath9k_init_softc(u16 devid, s
- 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);
- --- a/drivers/net/wireless/ath/ath9k/mac.c
- +++ b/drivers/net/wireless/ath/ath9k/mac.c
- @@ -810,21 +810,12 @@ void ath9k_hw_disable_interrupts(struct
- }
- 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;
- @@ -846,6 +837,39 @@ void ath9k_hw_enable_interrupts(struct a
- 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)
- --- a/drivers/net/wireless/ath/ath9k/mac.h
- +++ b/drivers/net/wireless/ath/ath9k/mac.h
- @@ -744,6 +744,7 @@ void ath9k_hw_set_interrupts(struct ath_
- 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);
-
- --- a/drivers/net/wireless/ath/ath9k/main.c
- +++ b/drivers/net/wireless/ath/ath9k/main.c
- @@ -374,21 +374,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;
- }
- @@ -404,11 +403,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;
- @@ -421,7 +415,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;
- @@ -478,7 +471,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);
- @@ -542,7 +535,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;
- @@ -588,7 +583,7 @@ chip_reset:
-
- if (sched) {
- /* turn off every interrupt */
- - ath9k_hw_disable_interrupts(ah);
- + ath9k_hw_kill_interrupts(ah);
- tasklet_schedule(&sc->intr_tq);
- }
-
|