We include two cleanup patches in preparation for more 3945 porting
and a fix to kernel.org bug 13224. For this
bugfix we also include a patch that applies to 2.6.30.
[PATCH 1/3] iwlwifi: drop struct iwl3945_hw_key
[PATCH 2/3] iwlwifi: drop iwl3945_tid_data
[PATCH 3/3] iwlwifi: do not cancel delayed work inside spin_lock_irqsave
[PATCH v2.6.30] iwlwifi: do not cancel delayed work inside spin_lock_irqsave
Thank you
Reinette
From: Reinette Chatre <[email protected]>
Calling cancel_delayed_work() from inside
spin_lock_irqsave, introduces a potential deadlock.
As explained by Johannes Berg <[email protected]>
A - lock
T - timer
phase CPU 1 CPU 2
---------------------------------------------
some place that calls
cancel_timer_sync()
(which is the | code)
lock-irq(A)
| "lock-irq"(T)
| "unlock"(T)
| wait(T)
unlock(A)
timer softirq
"lock"(T)
run(T)
"unlock"(T)
irq handler
lock(A)
unlock(A)
Now all that again, interleaved, leading to deadlock:
lock-irq(A)
"lock"(T)
run(T)
IRQ during or maybe
before run(T) --> lock(A)
"lock-irq"(T)
wait(T)
We fix this by moving the call to cancel_delayed_work() into workqueue.
There are cases where the work may not actually be queued or running
at the time we are trying to cancel it, but cancel_delayed_work() is
able to deal with this.
Also cleanup iwl_set_mode related to this call. This function
(iwl_set_mode) is only called when bringing interface up and there will
thus not be any scanning done. No need to try to cancel scanning.
Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13224, which was also
reported at http://marc.info/?l=linux-wireless&m=124081921903223&w=2 .
Tested-by: Miles Lane <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
Acked-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn.c | 7 -------
drivers/net/wireless/iwlwifi/iwl-scan.c | 7 ++++---
drivers/net/wireless/iwlwifi/iwl3945-base.c | 9 ++-------
3 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 1ef4192..0e6e7ff 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -669,13 +669,6 @@ static int iwl_set_mode(struct iwl_priv *priv, int mode)
if (!iwl_is_ready_rf(priv))
return -EAGAIN;
- cancel_delayed_work(&priv->scan_check);
- if (iwl_scan_cancel_timeout(priv, 100)) {
- IWL_WARN(priv, "Aborted scan still in progress after 100ms\n");
- IWL_DEBUG_MAC80211(priv, "leaving - scan abort failed.\n");
- return -EAGAIN;
- }
-
iwl_commit_rxon(priv);
return 0;
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 23644cf..a879e9c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -227,9 +227,6 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
/* The HW is no longer scanning */
clear_bit(STATUS_SCAN_HW, &priv->status);
- /* The scan completion notification came in, so kill that timer... */
- cancel_delayed_work(&priv->scan_check);
-
IWL_DEBUG_INFO(priv, "Scan pass on %sGHz took %dms\n",
(priv->scan_bands & BIT(IEEE80211_BAND_2GHZ)) ?
"2.4" : "5.2",
@@ -712,6 +709,8 @@ static void iwl_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
@@ -925,6 +924,8 @@ void iwl_bg_scan_completed(struct work_struct *work)
IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
+ cancel_delayed_work(&priv->scan_check);
+
if (test_bit(STATUS_EXIT_PENDING, &priv->status))
return;
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 617c423..12a2d35 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -782,13 +782,6 @@ static int iwl3945_set_mode(struct iwl_priv *priv, int mode)
if (!iwl_is_ready_rf(priv))
return -EAGAIN;
- cancel_delayed_work(&priv->scan_check);
- if (iwl_scan_cancel_timeout(priv, 100)) {
- IWL_WARN(priv, "Aborted scan still in progress after 100ms\n");
- IWL_DEBUG_MAC80211(priv, "leaving - scan abort failed.\n");
- return -EAGAIN;
- }
-
iwl3945_commit_rxon(priv);
return 0;
@@ -3300,6 +3293,8 @@ static void iwl3945_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
--
1.5.6.3
From: Reinette Chatre <[email protected]>
Calling cancel_delayed_work() from inside
spin_lock_irqsave, introduces a potential deadlock.
As explained by Johannes Berg <[email protected]>
A - lock
T - timer
phase CPU 1 CPU 2
---------------------------------------------
some place that calls
cancel_timer_sync()
(which is the | code)
lock-irq(A)
| "lock-irq"(T)
| "unlock"(T)
| wait(T)
unlock(A)
timer softirq
"lock"(T)
run(T)
"unlock"(T)
irq handler
lock(A)
unlock(A)
Now all that again, interleaved, leading to deadlock:
lock-irq(A)
"lock"(T)
run(T)
IRQ during or maybe
before run(T) --> lock(A)
"lock-irq"(T)
wait(T)
We fix this by moving the call to cancel_delayed_work() into workqueue.
There are cases where the work may not actually be queued or running
at the time we are trying to cancel it, but cancel_delayed_work() is
able to deal with this.
Also cleanup iwl_set_mode related to this call. This function
(iwl_set_mode) is only called when bringing interface up and there will
thus not be any scanning done. No need to try to cancel scanning.
Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13224, which was also
reported at http://marc.info/?l=linux-wireless&m=124081921903223&w=2 .
Tested-by: Miles Lane <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
Acked-by: Zhu Yi <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-core.c | 7 -------
drivers/net/wireless/iwlwifi/iwl-scan.c | 7 ++++---
drivers/net/wireless/iwlwifi/iwl3945-base.c | 2 ++
3 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 5393fb3..967343a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -2437,13 +2437,6 @@ int iwl_set_mode(struct iwl_priv *priv, int mode)
if (!iwl_is_ready_rf(priv))
return -EAGAIN;
- cancel_delayed_work(&priv->scan_check);
- if (iwl_scan_cancel_timeout(priv, 100)) {
- IWL_WARN(priv, "Aborted scan still in progress after 100ms\n");
- IWL_DEBUG_MAC80211(priv, "leaving - scan abort failed.\n");
- return -EAGAIN;
- }
-
iwlcore_commit_rxon(priv);
return 0;
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 8029206..41358cf 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -227,9 +227,6 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
/* The HW is no longer scanning */
clear_bit(STATUS_SCAN_HW, &priv->status);
- /* The scan completion notification came in, so kill that timer... */
- cancel_delayed_work(&priv->scan_check);
-
IWL_DEBUG_INFO(priv, "Scan pass on %sGHz took %dms\n",
(priv->scan_bands & BIT(IEEE80211_BAND_2GHZ)) ?
"2.4" : "5.2",
@@ -591,6 +588,8 @@ static void iwl_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
@@ -816,6 +815,8 @@ void iwl_bg_scan_completed(struct work_struct *work)
IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
+ cancel_delayed_work(&priv->scan_check);
+
priv->scan_request = NULL;
ieee80211_scan_completed(priv->hw, false);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 5d52f22..ded7fea 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2995,6 +2995,8 @@ static void iwl3945_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
--
1.5.6.3
From: Tomas Winkler <[email protected]>
This patch is one of the incremental steps for unifying iwl_station_entry
for all HWs, i.e. removing of iwl3945_station_entry
This patch drops iwl3945_tid_data and use iwl_tid_data instead.
Signed-off-by: Tomas Winkler <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-dev.h | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 427489a..b6b8327 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -499,14 +499,10 @@ struct iwl_qos_info {
#define STA_PS_STATUS_WAKE 0
#define STA_PS_STATUS_SLEEP 1
-struct iwl3945_tid_data {
- u16 seq_number;
-};
-
struct iwl3945_station_entry {
struct iwl3945_addsta_cmd sta;
- struct iwl3945_tid_data tid[MAX_TID_COUNT];
+ struct iwl_tid_data tid[MAX_TID_COUNT];
u8 used;
u8 ps_status;
struct iwl_hw_key keyinfo;
--
1.5.6.3
Hi John,
On Mon, 2009-05-18 at 12:22 -0700, John W. Linville wrote:
> From: Reinette Chatre <[email protected]>
>
> Calling cancel_delayed_work() from inside
> spin_lock_irqsave, introduces a potential deadlock.
>
> As explained by Johannes Berg <[email protected]>
>
> A - lock
> T - timer
>
> phase CPU 1 CPU 2
> ---------------------------------------------
>
> some place that calls
> cancel_timer_sync()
> (which is the | code)
> lock-irq(A)
> | "lock-irq"(T)
> | "unlock"(T)
> | wait(T)
> unlock(A)
>
> timer softirq
> "lock"(T)
> run(T)
> "unlock"(T)
>
> irq handler
> lock(A)
> unlock(A)
>
> Now all that again, interleaved, leading to deadlock:
>
> lock-irq(A)
> "lock"(T)
> run(T)
> IRQ during or maybe
> before run(T) --> lock(A)
> "lock-irq"(T)
> wait(T)
>
> We fix this by moving the call to cancel_delayed_work() into workqueue.
> There are cases where the work may not actually be queued or running
> at the time we are trying to cancel it, but cancel_delayed_work() is
> able to deal with this.
>
> Also cleanup iwl_set_mode related to this call. This function
> (iwl_set_mode) is only called when bringing interface up and there will
> thus not be any scanning done. No need to try to cancel scanning.
>
> Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13224, which was also
> reported at http://marc.info/?l=linux-wireless&m=124081921903223&w=2 .
>
> Tested-by: Miles Lane <[email protected]>
> Signed-off-by: Reinette Chatre <[email protected]>
> Acked-by: Zhu Yi <[email protected]>
> Signed-off-by: John W. Linville <[email protected]>
> ---
> Did not apply to 2.6.30-rc6 as posted -- please verify my fix-up!
I am very sorry - this patch was created against and tested with an
earlier rc release. Thank you very much for fixing it up. It looks good.
Reinette
From: Tomas Winkler <[email protected]>
This patch replaces struct iwl3945_hw_key by struct iwl_hw_key.
It's not used directly with any host command therefore removal is trivial
Signed-off-by: Tomas Winkler <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-dev.h | 7 +------
drivers/net/wireless/iwlwifi/iwl3945-base.c | 5 ++---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 3049ba2..427489a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -503,18 +503,13 @@ struct iwl3945_tid_data {
u16 seq_number;
};
-struct iwl3945_hw_key {
- enum ieee80211_key_alg alg;
- int keylen;
- u8 key[32];
-};
struct iwl3945_station_entry {
struct iwl3945_addsta_cmd sta;
struct iwl3945_tid_data tid[MAX_TID_COUNT];
u8 used;
u8 ps_status;
- struct iwl3945_hw_key keyinfo;
+ struct iwl_hw_key keyinfo;
};
struct iwl_station_entry {
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index c32ec80..5d52f22 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -340,7 +340,7 @@ static int iwl3945_clear_sta_key_info(struct iwl_priv *priv, u8 sta_id)
unsigned long flags;
spin_lock_irqsave(&priv->sta_lock, flags);
- memset(&priv->stations_39[sta_id].keyinfo, 0, sizeof(struct iwl3945_hw_key));
+ memset(&priv->stations_39[sta_id].keyinfo, 0, sizeof(struct iwl_hw_key));
memset(&priv->stations_39[sta_id].sta.key, 0,
sizeof(struct iwl4965_keyinfo));
priv->stations_39[sta_id].sta.key.key_flags = STA_KEY_FLG_NO_ENC;
@@ -578,8 +578,7 @@ static void iwl3945_build_tx_cmd_hwcrypto(struct iwl_priv *priv,
int sta_id)
{
struct iwl3945_tx_cmd *tx = (struct iwl3945_tx_cmd *)cmd->cmd.payload;
- struct iwl3945_hw_key *keyinfo =
- &priv->stations_39[sta_id].keyinfo;
+ struct iwl_hw_key *keyinfo = &priv->stations_39[sta_id].keyinfo;
switch (keyinfo->alg) {
case ALG_CCMP:
--
1.5.6.3
From: Reinette Chatre <[email protected]>
Calling cancel_delayed_work() from inside
spin_lock_irqsave, introduces a potential deadlock.
As explained by Johannes Berg <[email protected]>
A - lock
T - timer
phase CPU 1 CPU 2
---------------------------------------------
some place that calls
cancel_timer_sync()
(which is the | code)
lock-irq(A)
| "lock-irq"(T)
| "unlock"(T)
| wait(T)
unlock(A)
timer softirq
"lock"(T)
run(T)
"unlock"(T)
irq handler
lock(A)
unlock(A)
Now all that again, interleaved, leading to deadlock:
lock-irq(A)
"lock"(T)
run(T)
IRQ during or maybe
before run(T) --> lock(A)
"lock-irq"(T)
wait(T)
We fix this by moving the call to cancel_delayed_work() into workqueue.
There are cases where the work may not actually be queued or running
at the time we are trying to cancel it, but cancel_delayed_work() is
able to deal with this.
Also cleanup iwl_set_mode related to this call. This function
(iwl_set_mode) is only called when bringing interface up and there will
thus not be any scanning done. No need to try to cancel scanning.
Fixes http://bugzilla.kernel.org/show_bug.cgi?id=13224, which was also
reported at http://marc.info/?l=linux-wireless&m=124081921903223&w=2 .
Tested-by: Miles Lane <[email protected]>
Signed-off-by: Reinette Chatre <[email protected]>
Acked-by: Zhu Yi <[email protected]>
Signed-off-by: John W. Linville <[email protected]>
---
Did not apply to 2.6.30-rc6 as posted -- please verify my fix-up!
drivers/net/wireless/iwlwifi/iwl-agn.c | 7 -------
drivers/net/wireless/iwlwifi/iwl-scan.c | 7 ++++---
drivers/net/wireless/iwlwifi/iwl3945-base.c | 9 ++-------
3 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 3bb28db..f46ba24 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -669,13 +669,6 @@ static int iwl_set_mode(struct iwl_priv *priv, int mode)
if (!iwl_is_ready_rf(priv))
return -EAGAIN;
- cancel_delayed_work(&priv->scan_check);
- if (iwl_scan_cancel_timeout(priv, 100)) {
- IWL_WARN(priv, "Aborted scan still in progress after 100ms\n");
- IWL_DEBUG_MAC80211(priv, "leaving - scan abort failed.\n");
- return -EAGAIN;
- }
-
iwl_commit_rxon(priv);
return 0;
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index e7c65c4..6330b91 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -227,9 +227,6 @@ static void iwl_rx_scan_complete_notif(struct iwl_priv *priv,
/* The HW is no longer scanning */
clear_bit(STATUS_SCAN_HW, &priv->status);
- /* The scan completion notification came in, so kill that timer... */
- cancel_delayed_work(&priv->scan_check);
-
IWL_DEBUG_INFO(priv, "Scan pass on %sGHz took %dms\n",
(priv->scan_bands & BIT(IEEE80211_BAND_2GHZ)) ?
"2.4" : "5.2",
@@ -712,6 +709,8 @@ static void iwl_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
@@ -925,6 +924,8 @@ void iwl_bg_scan_completed(struct work_struct *work)
IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
+ cancel_delayed_work(&priv->scan_check);
+
ieee80211_scan_completed(priv->hw, false);
if (test_bit(STATUS_EXIT_PENDING, &priv->status))
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index 4cce661..ff4d0e4 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -782,13 +782,6 @@ static int iwl3945_set_mode(struct iwl_priv *priv, int mode)
if (!iwl_is_ready_rf(priv))
return -EAGAIN;
- cancel_delayed_work(&priv->scan_check);
- if (iwl_scan_cancel_timeout(priv, 100)) {
- IWL_WARN(priv, "Aborted scan still in progress after 100ms\n");
- IWL_DEBUG_MAC80211(priv, "leaving - scan abort failed.\n");
- return -EAGAIN;
- }
-
iwl3945_commit_rxon(priv);
return 0;
@@ -3298,6 +3291,8 @@ static void iwl3945_bg_request_scan(struct work_struct *data)
mutex_lock(&priv->mutex);
+ cancel_delayed_work(&priv->scan_check);
+
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
--
1.6.0.6