* [PATCH] rt2x00: Fix PCI interrupt processing race on SMP systems
@ 2011-08-06 11:13 Ivo van Doorn
0 siblings, 0 replies; only message in thread
From: Ivo van Doorn @ 2011-08-06 11:13 UTC (permalink / raw)
To: John W. Linville; +Cc: linux-wireless, users
From: Helmut Schaa <helmut.schaa@googlemail.com>
When toggle_irq is called for PCI devices to disable device interrupts
it used tasklet_disable to wait for a possibly running tasklet to finish.
However, on SMP systems the tasklet might still be scheduled on another CPU.
Instead, use tasklet_kill to ensure that all scheduled tasklets are finished
before returning from toggle_irq.
Furthermore, it was possible that a tasklet reenabled its interrupt even
though interrupts have been disabled already. Fix this by checking the
DEVICE_STATE_ENABLED_RADIO flag before reenabling single interrupts
during tasklet processing.
While at it also enable/kill the TBTT and PRETBTT tasklets in the
toggle_irq callback and only use tasklet_kill in stop_queue to wait
for a currently scheduled beacon update before returning.
Signed-off-by: Helmut Schaa <helmut.schaa@googlemail.com>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
drivers/net/wireless/rt2x00/rt2400pci.c | 39 +++++++++++---------------
drivers/net/wireless/rt2x00/rt2500pci.c | 39 +++++++++++---------------
drivers/net/wireless/rt2x00/rt2800pci.c | 46 +++++++++++++------------------
drivers/net/wireless/rt2x00/rt2x00dev.c | 1 -
drivers/net/wireless/rt2x00/rt61pci.c | 34 +++++++++--------------
5 files changed, 64 insertions(+), 95 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 76bcc354..daa32fc 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -645,11 +645,6 @@ static void rt2400pci_start_queue(struct data_queue *queue)
rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
break;
case QID_BEACON:
- /*
- * Allow the tbtt tasklet to be scheduled.
- */
- tasklet_enable(&rt2x00dev->tbtt_tasklet);
-
rt2x00pci_register_read(rt2x00dev, CSR14, ®);
rt2x00_set_field32(®, CSR14_TSF_COUNT, 1);
rt2x00_set_field32(®, CSR14_TBCN, 1);
@@ -715,7 +710,7 @@ static void rt2400pci_stop_queue(struct data_queue *queue)
/*
* Wait for possibly running tbtt tasklets.
*/
- tasklet_disable(&rt2x00dev->tbtt_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
break;
default:
break;
@@ -982,12 +977,6 @@ static void rt2400pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
if (state == STATE_RADIO_IRQ_ON) {
rt2x00pci_register_read(rt2x00dev, CSR7, ®);
rt2x00pci_register_write(rt2x00dev, CSR7, reg);
-
- /*
- * Enable tasklets.
- */
- tasklet_enable(&rt2x00dev->txstatus_tasklet);
- tasklet_enable(&rt2x00dev->rxdone_tasklet);
}
/*
@@ -1011,8 +1000,9 @@ static void rt2400pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
* Ensure that all tasklets are finished before
* disabling the interrupts.
*/
- tasklet_disable(&rt2x00dev->txstatus_tasklet);
- tasklet_disable(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->txstatus_tasklet);
+ tasklet_kill(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
}
}
@@ -1347,22 +1337,25 @@ static void rt2400pci_txstatus_tasklet(unsigned long data)
/*
* Enable all TXDONE interrupts again.
*/
- spin_lock_irq(&rt2x00dev->irqmask_lock);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags)) {
+ spin_lock_irq(&rt2x00dev->irqmask_lock);
- rt2x00pci_register_read(rt2x00dev, CSR8, ®);
- rt2x00_set_field32(®, CSR8_TXDONE_TXRING, 0);
- rt2x00_set_field32(®, CSR8_TXDONE_ATIMRING, 0);
- rt2x00_set_field32(®, CSR8_TXDONE_PRIORING, 0);
- rt2x00pci_register_write(rt2x00dev, CSR8, reg);
+ rt2x00pci_register_read(rt2x00dev, CSR8, ®);
+ rt2x00_set_field32(®, CSR8_TXDONE_TXRING, 0);
+ rt2x00_set_field32(®, CSR8_TXDONE_ATIMRING, 0);
+ rt2x00_set_field32(®, CSR8_TXDONE_PRIORING, 0);
+ rt2x00pci_register_write(rt2x00dev, CSR8, reg);
- spin_unlock_irq(&rt2x00dev->irqmask_lock);
+ spin_unlock_irq(&rt2x00dev->irqmask_lock);
+ }
}
static void rt2400pci_tbtt_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2x00lib_beacondone(rt2x00dev);
- rt2400pci_enable_interrupt(rt2x00dev, CSR8_TBCN_EXPIRE);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt2400pci_enable_interrupt(rt2x00dev, CSR8_TBCN_EXPIRE);
}
static void rt2400pci_rxdone_tasklet(unsigned long data)
@@ -1370,7 +1363,7 @@ static void rt2400pci_rxdone_tasklet(unsigned long data)
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
if (rt2x00pci_rxdone(rt2x00dev))
tasklet_schedule(&rt2x00dev->rxdone_tasklet);
- else
+ else if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
rt2400pci_enable_interrupt(rt2x00dev, CSR8_RXDONE);
}
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index c288d95..b46c3b8 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -735,11 +735,6 @@ static void rt2500pci_start_queue(struct data_queue *queue)
rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
break;
case QID_BEACON:
- /*
- * Allow the tbtt tasklet to be scheduled.
- */
- tasklet_enable(&rt2x00dev->tbtt_tasklet);
-
rt2x00pci_register_read(rt2x00dev, CSR14, ®);
rt2x00_set_field32(®, CSR14_TSF_COUNT, 1);
rt2x00_set_field32(®, CSR14_TBCN, 1);
@@ -805,7 +800,7 @@ static void rt2500pci_stop_queue(struct data_queue *queue)
/*
* Wait for possibly running tbtt tasklets.
*/
- tasklet_disable(&rt2x00dev->tbtt_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
break;
default:
break;
@@ -1137,12 +1132,6 @@ static void rt2500pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
if (state == STATE_RADIO_IRQ_ON) {
rt2x00pci_register_read(rt2x00dev, CSR7, ®);
rt2x00pci_register_write(rt2x00dev, CSR7, reg);
-
- /*
- * Enable tasklets.
- */
- tasklet_enable(&rt2x00dev->txstatus_tasklet);
- tasklet_enable(&rt2x00dev->rxdone_tasklet);
}
/*
@@ -1165,8 +1154,9 @@ static void rt2500pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
/*
* Ensure that all tasklets are finished.
*/
- tasklet_disable(&rt2x00dev->txstatus_tasklet);
- tasklet_disable(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->txstatus_tasklet);
+ tasklet_kill(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
}
}
@@ -1479,22 +1469,25 @@ static void rt2500pci_txstatus_tasklet(unsigned long data)
/*
* Enable all TXDONE interrupts again.
*/
- spin_lock_irq(&rt2x00dev->irqmask_lock);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags)) {
+ spin_lock_irq(&rt2x00dev->irqmask_lock);
- rt2x00pci_register_read(rt2x00dev, CSR8, ®);
- rt2x00_set_field32(®, CSR8_TXDONE_TXRING, 0);
- rt2x00_set_field32(®, CSR8_TXDONE_ATIMRING, 0);
- rt2x00_set_field32(®, CSR8_TXDONE_PRIORING, 0);
- rt2x00pci_register_write(rt2x00dev, CSR8, reg);
+ rt2x00pci_register_read(rt2x00dev, CSR8, ®);
+ rt2x00_set_field32(®, CSR8_TXDONE_TXRING, 0);
+ rt2x00_set_field32(®, CSR8_TXDONE_ATIMRING, 0);
+ rt2x00_set_field32(®, CSR8_TXDONE_PRIORING, 0);
+ rt2x00pci_register_write(rt2x00dev, CSR8, reg);
- spin_unlock_irq(&rt2x00dev->irqmask_lock);
+ spin_unlock_irq(&rt2x00dev->irqmask_lock);
+ }
}
static void rt2500pci_tbtt_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2x00lib_beacondone(rt2x00dev);
- rt2500pci_enable_interrupt(rt2x00dev, CSR8_TBCN_EXPIRE);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt2500pci_enable_interrupt(rt2x00dev, CSR8_TBCN_EXPIRE);
}
static void rt2500pci_rxdone_tasklet(unsigned long data)
@@ -1502,7 +1495,7 @@ static void rt2500pci_rxdone_tasklet(unsigned long data)
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
if (rt2x00pci_rxdone(rt2x00dev))
tasklet_schedule(&rt2x00dev->rxdone_tasklet);
- else
+ else if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
rt2500pci_enable_interrupt(rt2x00dev, CSR8_RXDONE);
}
diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 5319ed9..7a0fd0e 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -200,13 +200,6 @@ static void rt2800pci_start_queue(struct data_queue *queue)
rt2x00pci_register_write(rt2x00dev, MAC_SYS_CTRL, reg);
break;
case QID_BEACON:
- /*
- * Allow beacon tasklets to be scheduled for periodic
- * beacon updates.
- */
- tasklet_enable(&rt2x00dev->tbtt_tasklet);
- tasklet_enable(&rt2x00dev->pretbtt_tasklet);
-
rt2x00pci_register_read(rt2x00dev, BCN_TIME_CFG, ®);
rt2x00_set_field32(®, BCN_TIME_CFG_TSF_TICKING, 1);
rt2x00_set_field32(®, BCN_TIME_CFG_TBTT_ENABLE, 1);
@@ -269,10 +262,13 @@ static void rt2800pci_stop_queue(struct data_queue *queue)
rt2x00pci_register_write(rt2x00dev, INT_TIMER_EN, reg);
/*
- * Wait for tbtt tasklets to finish.
+ * Wait for current invocation to finish. The tasklet
+ * won't be scheduled anymore afterwards since we disabled
+ * the TBTT and PRE TBTT timer.
*/
- tasklet_disable(&rt2x00dev->tbtt_tasklet);
- tasklet_disable(&rt2x00dev->pretbtt_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
+ tasklet_kill(&rt2x00dev->pretbtt_tasklet);
+
break;
default:
break;
@@ -437,14 +433,6 @@ static void rt2800pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
if (state == STATE_RADIO_IRQ_ON) {
rt2x00pci_register_read(rt2x00dev, INT_SOURCE_CSR, ®);
rt2x00pci_register_write(rt2x00dev, INT_SOURCE_CSR, reg);
-
- /*
- * Enable tasklets. The beacon related tasklets are
- * enabled when the beacon queue is started.
- */
- tasklet_enable(&rt2x00dev->txstatus_tasklet);
- tasklet_enable(&rt2x00dev->rxdone_tasklet);
- tasklet_enable(&rt2x00dev->autowake_tasklet);
}
spin_lock_irqsave(&rt2x00dev->irqmask_lock, flags);
@@ -472,12 +460,13 @@ static void rt2800pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
if (state == STATE_RADIO_IRQ_OFF) {
/*
- * Ensure that all tasklets are finished before
- * disabling the interrupts.
+ * Wait for possibly running tasklets to finish.
*/
- tasklet_disable(&rt2x00dev->txstatus_tasklet);
- tasklet_disable(&rt2x00dev->rxdone_tasklet);
- tasklet_disable(&rt2x00dev->autowake_tasklet);
+ tasklet_kill(&rt2x00dev->txstatus_tasklet);
+ tasklet_kill(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->autowake_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
+ tasklet_kill(&rt2x00dev->pretbtt_tasklet);
}
}
@@ -813,14 +802,16 @@ static void rt2800pci_pretbtt_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2x00lib_pretbtt(rt2x00dev);
- rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_PRE_TBTT);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_PRE_TBTT);
}
static void rt2800pci_tbtt_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2x00lib_beacondone(rt2x00dev);
- rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_TBTT);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_TBTT);
}
static void rt2800pci_rxdone_tasklet(unsigned long data)
@@ -828,7 +819,7 @@ static void rt2800pci_rxdone_tasklet(unsigned long data)
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
if (rt2x00pci_rxdone(rt2x00dev))
tasklet_schedule(&rt2x00dev->rxdone_tasklet);
- else
+ else if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_RX_DONE);
}
@@ -836,7 +827,8 @@ static void rt2800pci_autowake_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2800pci_wakeup(rt2x00dev);
- rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_AUTO_WAKEUP);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt2800pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_AUTO_WAKEUP);
}
static void rt2800pci_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 0955c94..92ff6a7 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -946,7 +946,6 @@ static int rt2x00lib_probe_hw(struct rt2x00_dev *rt2x00dev)
tasklet_init(&rt2x00dev->taskletname, \
rt2x00dev->ops->lib->taskletname, \
(unsigned long)rt2x00dev); \
- tasklet_disable(&rt2x00dev->taskletname); \
}
RT2X00_TASKLET_INIT(txstatus_tasklet);
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index 53110b8..058ef4b 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1142,11 +1142,6 @@ static void rt61pci_start_queue(struct data_queue *queue)
rt2x00pci_register_write(rt2x00dev, TXRX_CSR0, reg);
break;
case QID_BEACON:
- /*
- * Allow the tbtt tasklet to be scheduled.
- */
- tasklet_enable(&rt2x00dev->tbtt_tasklet);
-
rt2x00pci_register_read(rt2x00dev, TXRX_CSR9, ®);
rt2x00_set_field32(®, TXRX_CSR9_TSF_TICKING, 1);
rt2x00_set_field32(®, TXRX_CSR9_TBTT_ENABLE, 1);
@@ -1230,7 +1225,7 @@ static void rt61pci_stop_queue(struct data_queue *queue)
/*
* Wait for possibly running tbtt tasklets.
*/
- tasklet_disable(&rt2x00dev->tbtt_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
break;
default:
break;
@@ -1731,13 +1726,6 @@ static void rt61pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
rt2x00pci_register_read(rt2x00dev, MCU_INT_SOURCE_CSR, ®);
rt2x00pci_register_write(rt2x00dev, MCU_INT_SOURCE_CSR, reg);
-
- /*
- * Enable tasklets.
- */
- tasklet_enable(&rt2x00dev->txstatus_tasklet);
- tasklet_enable(&rt2x00dev->rxdone_tasklet);
- tasklet_enable(&rt2x00dev->autowake_tasklet);
}
/*
@@ -1772,9 +1760,10 @@ static void rt61pci_toggle_irq(struct rt2x00_dev *rt2x00dev,
/*
* Ensure that all tasklets are finished.
*/
- tasklet_disable(&rt2x00dev->txstatus_tasklet);
- tasklet_disable(&rt2x00dev->rxdone_tasklet);
- tasklet_disable(&rt2x00dev->autowake_tasklet);
+ tasklet_kill(&rt2x00dev->txstatus_tasklet);
+ tasklet_kill(&rt2x00dev->rxdone_tasklet);
+ tasklet_kill(&rt2x00dev->autowake_tasklet);
+ tasklet_kill(&rt2x00dev->tbtt_tasklet);
}
}
@@ -2300,22 +2289,24 @@ static void rt61pci_txstatus_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt61pci_txdone(rt2x00dev);
- rt61pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_TXDONE);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt61pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_TXDONE);
}
static void rt61pci_tbtt_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
rt2x00lib_beacondone(rt2x00dev);
- rt61pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_BEACON_DONE);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt61pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_BEACON_DONE);
}
static void rt61pci_rxdone_tasklet(unsigned long data)
{
struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
if (rt2x00pci_rxdone(rt2x00dev))
- rt2x00pci_rxdone(rt2x00dev);
- else
+ tasklet_schedule(&rt2x00dev->rxdone_tasklet);
+ else if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
rt61pci_enable_interrupt(rt2x00dev, INT_MASK_CSR_RXDONE);
}
@@ -2325,7 +2316,8 @@ static void rt61pci_autowake_tasklet(unsigned long data)
rt61pci_wakeup(rt2x00dev);
rt2x00pci_register_write(rt2x00dev,
M2H_CMD_DONE_CSR, 0xffffffff);
- rt61pci_enable_mcu_interrupt(rt2x00dev, MCU_INT_MASK_CSR_TWAKEUP);
+ if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+ rt61pci_enable_mcu_interrupt(rt2x00dev, MCU_INT_MASK_CSR_TWAKEUP);
}
static irqreturn_t rt61pci_interrupt(int irq, void *dev_instance)
--
1.7.2.3
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2011-08-06 11:13 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-06 11:13 [PATCH] rt2x00: Fix PCI interrupt processing race on SMP systems Ivo van Doorn
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.