* [ath9k-devel] [RFC/RFT] ath9k_htc: fix race conditions when stop device
@ 2011-01-21 6:12 ` Stanislaw Gruszka
0 siblings, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2011-01-21 6:12 UTC (permalink / raw)
To: ath9k-devel
Similar fix I already posted for ath9k. When stopping device, disable
interrupts, kill tasklets and then works, in correct order. Patch drop
mutex in them middle of a function, which I don't like and can possibly
not be correct. Perhaps this can be arranged differently. Also
there is no synchronize_irq, which IMHO should be added somewhere in
htc/wmi stop.
RFC/RTC for now (note I'm not able to test patch).
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 3 ---
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 21 +++++++++++++++------
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index 8e04586..a7bc26d 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -142,9 +142,6 @@ static void ath9k_deinit_priv(struct ath9k_htc_priv *priv)
{
ath9k_htc_exit_debug(priv->ah);
ath9k_hw_deinit(priv->ah);
- tasklet_kill(&priv->swba_tasklet);
- tasklet_kill(&priv->rx_tasklet);
- tasklet_kill(&priv->tx_tasklet);
kfree(priv->ah);
priv->ah = NULL;
}
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index f14f37d..a702089 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1026,12 +1026,6 @@ static void ath9k_htc_stop(struct ieee80211_hw *hw)
int ret = 0;
u8 cmd_rsp;
- /* Cancel all the running timers/work .. */
- cancel_work_sync(&priv->fatal_work);
- cancel_work_sync(&priv->ps_work);
- cancel_delayed_work_sync(&priv->ath9k_led_blink_work);
- ath9k_led_stop_brightness(priv);
-
mutex_lock(&priv->mutex);
if (priv->op_flags & OP_INVALID) {
@@ -1045,8 +1039,23 @@ static void ath9k_htc_stop(struct ieee80211_hw *hw)
WMI_CMD(WMI_DISABLE_INTR_CMDID);
WMI_CMD(WMI_DRAIN_TXQ_ALL_CMDID);
WMI_CMD(WMI_STOP_RECV_CMDID);
+
+ tasklet_kill(&priv->swba_tasklet);
+ tasklet_kill(&priv->rx_tasklet);
+ tasklet_kill(&priv->tx_tasklet);
+
skb_queue_purge(&priv->tx_queue);
+ mutex_unlock(&priv->mutex);
+
+ /* Cancel all the running timers/work .. */
+ cancel_work_sync(&priv->fatal_work);
+ cancel_work_sync(&priv->ps_work);
+ cancel_delayed_work_sync(&priv->ath9k_led_blink_work);
+ ath9k_led_stop_brightness(priv);
+
+ mutex_lock(&priv->mutex);
+
/* Remove monitor interface here */
if (ah->opmode == NL80211_IFTYPE_MONITOR) {
if (ath9k_htc_remove_monitor_interface(priv))
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC/RFT] ath9k_htc: fix race conditions when stop device
@ 2011-01-21 6:12 ` Stanislaw Gruszka
0 siblings, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2011-01-21 6:12 UTC (permalink / raw)
To: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan,
Senthil Balasubramanian
Cc: ath9k-devel, linux-wireless
Similar fix I already posted for ath9k. When stopping device, disable
interrupts, kill tasklets and then works, in correct order. Patch drop
mutex in them middle of a function, which I don't like and can possibly
not be correct. Perhaps this can be arranged differently. Also
there is no synchronize_irq, which IMHO should be added somewhere in
htc/wmi stop.
RFC/RTC for now (note I'm not able to test patch).
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/ath/ath9k/htc_drv_init.c | 3 ---
drivers/net/wireless/ath/ath9k/htc_drv_main.c | 21 +++++++++++++++------
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index 8e04586..a7bc26d 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -142,9 +142,6 @@ static void ath9k_deinit_priv(struct ath9k_htc_priv *priv)
{
ath9k_htc_exit_debug(priv->ah);
ath9k_hw_deinit(priv->ah);
- tasklet_kill(&priv->swba_tasklet);
- tasklet_kill(&priv->rx_tasklet);
- tasklet_kill(&priv->tx_tasklet);
kfree(priv->ah);
priv->ah = NULL;
}
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index f14f37d..a702089 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1026,12 +1026,6 @@ static void ath9k_htc_stop(struct ieee80211_hw *hw)
int ret = 0;
u8 cmd_rsp;
- /* Cancel all the running timers/work .. */
- cancel_work_sync(&priv->fatal_work);
- cancel_work_sync(&priv->ps_work);
- cancel_delayed_work_sync(&priv->ath9k_led_blink_work);
- ath9k_led_stop_brightness(priv);
-
mutex_lock(&priv->mutex);
if (priv->op_flags & OP_INVALID) {
@@ -1045,8 +1039,23 @@ static void ath9k_htc_stop(struct ieee80211_hw *hw)
WMI_CMD(WMI_DISABLE_INTR_CMDID);
WMI_CMD(WMI_DRAIN_TXQ_ALL_CMDID);
WMI_CMD(WMI_STOP_RECV_CMDID);
+
+ tasklet_kill(&priv->swba_tasklet);
+ tasklet_kill(&priv->rx_tasklet);
+ tasklet_kill(&priv->tx_tasklet);
+
skb_queue_purge(&priv->tx_queue);
+ mutex_unlock(&priv->mutex);
+
+ /* Cancel all the running timers/work .. */
+ cancel_work_sync(&priv->fatal_work);
+ cancel_work_sync(&priv->ps_work);
+ cancel_delayed_work_sync(&priv->ath9k_led_blink_work);
+ ath9k_led_stop_brightness(priv);
+
+ mutex_lock(&priv->mutex);
+
/* Remove monitor interface here */
if (ah->opmode == NL80211_IFTYPE_MONITOR) {
if (ath9k_htc_remove_monitor_interface(priv))
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [ath9k-devel] [RFC/RFT] ath9k_htc: fix race conditions when stop device
2011-01-21 6:12 ` Stanislaw Gruszka
@ 2011-01-21 10:21 ` Sujith
-1 siblings, 0 replies; 6+ messages in thread
From: Sujith @ 2011-01-21 10:21 UTC (permalink / raw)
To: ath9k-devel
Stanislaw Gruszka wrote:
> Similar fix I already posted for ath9k. When stopping device, disable
> interrupts, kill tasklets and then works, in correct order. Patch drop
> mutex in them middle of a function, which I don't like and can possibly
> not be correct. Perhaps this can be arranged differently. Also
> there is no synchronize_irq, which IMHO should be added somewhere in
> htc/wmi stop.
It should be okay if the work instances are canceled first before the mutex
is acquired. And I don't think synchronize_irq() is required since this is a USB driver.
But when the driver is loaded and unloaded without bringing the interface up,
stop() wouldn't be called at all, but I guess it's alright since no tasklet would
have been scheduled. So just moving the tasklet_kill() functions to stop() would
be fine. I tested with ath9k_htc and saw no issues.
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC/RFT] ath9k_htc: fix race conditions when stop device
@ 2011-01-21 10:21 ` Sujith
0 siblings, 0 replies; 6+ messages in thread
From: Sujith @ 2011-01-21 10:21 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan,
Senthil Balasubramanian, ath9k-devel, linux-wireless
Stanislaw Gruszka wrote:
> Similar fix I already posted for ath9k. When stopping device, disable
> interrupts, kill tasklets and then works, in correct order. Patch drop
> mutex in them middle of a function, which I don't like and can possibly
> not be correct. Perhaps this can be arranged differently. Also
> there is no synchronize_irq, which IMHO should be added somewhere in
> htc/wmi stop.
It should be okay if the work instances are canceled first before the mutex
is acquired. And I don't think synchronize_irq() is required since this is a USB driver.
But when the driver is loaded and unloaded without bringing the interface up,
stop() wouldn't be called at all, but I guess it's alright since no tasklet would
have been scheduled. So just moving the tasklet_kill() functions to stop() would
be fine. I tested with ath9k_htc and saw no issues.
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
* [ath9k-devel] [RFC/RFT] ath9k_htc: fix race conditions when stop device
2011-01-21 10:21 ` Sujith
@ 2011-01-21 12:51 ` Stanislaw Gruszka
-1 siblings, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2011-01-21 12:51 UTC (permalink / raw)
To: ath9k-devel
On Fri, 21 Jan 2011 15:51:51 +0530
Sujith <m.sujith@gmail.com> wrote:
> Stanislaw Gruszka wrote:
> > Similar fix I already posted for ath9k. When stopping device, disable
> > interrupts, kill tasklets and then works, in correct order. Patch drop
> > mutex in them middle of a function, which I don't like and can possibly
> > not be correct. Perhaps this can be arranged differently. Also
> > there is no synchronize_irq, which IMHO should be added somewhere in
> > htc/wmi stop.
>
> It should be okay if the work instances are canceled first before the mutex
> is acquired.
IIRC, there is possibility to schedule priv->ps_work from ath9k_rx_tasklet (when
receive beacon frame and being in power save mode), so we should cancel that work
after assure tasklet will not be executed.
> And I don't think synchronize_irq() is required since this is a USB driver.
>
> But when the driver is loaded and unloaded without bringing the interface up,
> stop() wouldn't be called at all, but I guess it's alright since no tasklet would
> have been scheduled.
I assumed ath9k_htc_rxep() and ath9k_htc_txep() can not be called before _start()
(and after _stop() of course), hence removing tasklet_kill's from ath9k_deinit_priv()
> I tested with ath9k_htc and saw no issues.
Thanks!
Stanislaw
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC/RFT] ath9k_htc: fix race conditions when stop device
@ 2011-01-21 12:51 ` Stanislaw Gruszka
0 siblings, 0 replies; 6+ messages in thread
From: Stanislaw Gruszka @ 2011-01-21 12:51 UTC (permalink / raw)
To: Sujith
Cc: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan,
Senthil Balasubramanian, ath9k-devel, linux-wireless
On Fri, 21 Jan 2011 15:51:51 +0530
Sujith <m.sujith@gmail.com> wrote:
> Stanislaw Gruszka wrote:
> > Similar fix I already posted for ath9k. When stopping device, disable
> > interrupts, kill tasklets and then works, in correct order. Patch drop
> > mutex in them middle of a function, which I don't like and can possibly
> > not be correct. Perhaps this can be arranged differently. Also
> > there is no synchronize_irq, which IMHO should be added somewhere in
> > htc/wmi stop.
>
> It should be okay if the work instances are canceled first before the mutex
> is acquired.
IIRC, there is possibility to schedule priv->ps_work from ath9k_rx_tasklet (when
receive beacon frame and being in power save mode), so we should cancel that work
after assure tasklet will not be executed.
> And I don't think synchronize_irq() is required since this is a USB driver.
>
> But when the driver is loaded and unloaded without bringing the interface up,
> stop() wouldn't be called at all, but I guess it's alright since no tasklet would
> have been scheduled.
I assumed ath9k_htc_rxep() and ath9k_htc_txep() can not be called before _start()
(and after _stop() of course), hence removing tasklet_kill's from ath9k_deinit_priv()
> I tested with ath9k_htc and saw no issues.
Thanks!
Stanislaw
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-21 12:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-21 6:12 [ath9k-devel] [RFC/RFT] ath9k_htc: fix race conditions when stop device Stanislaw Gruszka
2011-01-21 6:12 ` Stanislaw Gruszka
2011-01-21 10:21 ` [ath9k-devel] " Sujith
2011-01-21 10:21 ` Sujith
2011-01-21 12:51 ` [ath9k-devel] " Stanislaw Gruszka
2011-01-21 12:51 ` Stanislaw Gruszka
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.