* [ath9k-devel] [PATCH 1/5] ath10k: use data_lock to protect {scan, rx}_channel
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
@ 2013-05-06 11:24 ` Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 2/5] ath10k: simplify scan event locking Michal Kazior
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Michal Kazior @ 2013-05-06 11:24 UTC (permalink / raw)
To: ath9k-devel
We used RCU in a funky way.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.h | 4 ++--
drivers/net/wireless/ath/ath10k/mac.c | 4 +++-
drivers/net/wireless/ath/ath10k/txrx.c | 9 ++++-----
drivers/net/wireless/ath/ath10k/wmi.c | 12 +++++++-----
4 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index fbf4d11..1110551 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -315,10 +315,10 @@ struct ath10k {
} mac;
/* should never be NULL; needed for regular htt rx */
- struct ieee80211_channel __rcu *rx_channel;
+ struct ieee80211_channel *rx_channel;
/* valid during scan; needed for mgmt rx during scan */
- struct ieee80211_channel __rcu *scan_channel;
+ struct ieee80211_channel *scan_channel;
int free_vdev_map;
int monitor_vdev_id;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b5cc3c8..3a27f35 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1589,7 +1589,9 @@ static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
ath10k_dbg(ATH10K_DBG_MAC, "Config channel %d mhz\n",
conf->chandef.chan->center_freq);
- rcu_assign_pointer(ar->rx_channel, conf->chandef.chan);
+ spin_lock_bh(&ar->data_lock);
+ ar->rx_channel = conf->chandef.chan;
+ spin_unlock_bh(&ar->data_lock);
}
if (changed & IEEE80211_CONF_CHANGE_PS) {
diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c
index 9d0b1e2..a32448c 100644
--- a/drivers/net/wireless/ath/ath10k/txrx.c
+++ b/drivers/net/wireless/ath/ath10k/txrx.c
@@ -261,13 +261,13 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
status->signal = info->signal;
- rcu_read_lock();
- ch = rcu_dereference(ar->scan_channel);
+ spin_lock_bh(&ar->data_lock);
+ ch = ar->scan_channel;
if (!ch)
- ch = rcu_dereference(ar->rx_channel);
+ ch = ar->rx_channel;
+ spin_unlock_bh(&ar->data_lock);
if (!ch) {
- rcu_read_unlock();
ath10k_warn("no channel configured; ignoring frame!\n");
dev_kfree_skb_any(info->skb);
return;
@@ -276,7 +276,6 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info)
process_rx_rates(ar, info, ch->band, status);
status->band = ch->band;
status->freq = ch->center_freq;
- rcu_read_unlock();
ath10k_dbg(ATH10K_DBG_HTT,
"rx skb %p len %u %s%s%s%s%s %srate_idx %u vht_nss %u freq %u band %u\n",
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 3b819c8..c44a2ff 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -177,9 +177,8 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
break;
}
- rcu_assign_pointer(ar->scan_channel, NULL);
-
spin_lock_bh(&ar->data_lock);
+ ar->scan_channel = NULL;
if (!ar->scan.in_progress) {
spin_unlock_bh(&ar->data_lock);
ath10k_warn("no scan requested, ignoring\n");
@@ -202,12 +201,15 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
break;
case WMI_SCAN_EVENT_BSS_CHANNEL:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_BSS_CHANNEL\n");
- rcu_assign_pointer(ar->scan_channel, NULL);
+ spin_lock_bh(&ar->data_lock);
+ ar->scan_channel = NULL;
+ spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_FOREIGN_CHANNEL:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_FOREIGN_CHANNEL\n");
- rcu_assign_pointer(ar->scan_channel,
- ieee80211_get_channel(ar->hw->wiphy, freq));
+ spin_lock_bh(&ar->data_lock);
+ ar->scan_channel = ieee80211_get_channel(ar->hw->wiphy, freq);
+ spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_DEQUEUED:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_DEQUEUED\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ath9k-devel] [PATCH 2/5] ath10k: simplify scan event locking
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 1/5] ath10k: use data_lock to protect {scan, rx}_channel Michal Kazior
@ 2013-05-06 11:24 ` Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 3/5] ath10k: increase scan timeout Michal Kazior
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Michal Kazior @ 2013-05-06 11:24 UTC (permalink / raw)
To: ath9k-devel
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/wmi.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index c44a2ff..efa72fc 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -147,16 +147,15 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
"scan_id %d vdev_id %d\n",
event_type, reason, freq, req_id, scan_id, vdev_id);
+ spin_lock_bh(&ar->data_lock);
+
switch (event_type) {
case WMI_SCAN_EVENT_STARTED:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_STARTED\n");
-
- spin_lock_bh(&ar->data_lock);
if (ar->scan.in_progress && ar->scan.is_roc)
ieee80211_ready_on_channel(ar->hw);
complete(&ar->scan.started);
- spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_COMPLETED:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_COMPLETED\n");
@@ -177,10 +176,8 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
break;
}
- spin_lock_bh(&ar->data_lock);
ar->scan_channel = NULL;
if (!ar->scan.in_progress) {
- spin_unlock_bh(&ar->data_lock);
ath10k_warn("no scan requested, ignoring\n");
break;
}
@@ -197,19 +194,14 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
del_timer(&ar->scan.timeout);
complete_all(&ar->scan.completed);
ar->scan.in_progress = false;
- spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_BSS_CHANNEL:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_BSS_CHANNEL\n");
- spin_lock_bh(&ar->data_lock);
ar->scan_channel = NULL;
- spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_FOREIGN_CHANNEL:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_FOREIGN_CHANNEL\n");
- spin_lock_bh(&ar->data_lock);
ar->scan_channel = ieee80211_get_channel(ar->hw->wiphy, freq);
- spin_unlock_bh(&ar->data_lock);
break;
case WMI_SCAN_EVENT_DEQUEUED:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_DEQUEUED\n");
@@ -224,6 +216,7 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
break;
}
+ spin_unlock_bh(&ar->data_lock);
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ath9k-devel] [PATCH 3/5] ath10k: increase scan timeout
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 1/5] ath10k: use data_lock to protect {scan, rx}_channel Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 2/5] ath10k: simplify scan event locking Michal Kazior
@ 2013-05-06 11:24 ` Michal Kazior
2013-05-07 13:52 ` Kalle Valo
2013-05-06 11:24 ` [ath9k-devel] [PATCH 4/5] ath10k: wait for remain on channel chan switch Michal Kazior
` (2 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Michal Kazior @ 2013-05-06 11:24 UTC (permalink / raw)
To: ath9k-devel
We can't use the max scan time as the timeout
itself. We must account for various overheads (ie.
command/event processing).
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 3a27f35..df57772 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1481,8 +1481,10 @@ static int ath10k_start_scan(struct ath10k *ar,
/* the scan can complete earlier, before we even
* start the timer. in that case the timer handler
* checks ar->scan.in_progress and bails out if its
- * false. */
- mod_timer(&ar->scan.timeout, jiffies + (arg->max_scan_time*HZ)/1000);
+ * false. Add a 200ms margin to account event/command
+ * processing. */
+ mod_timer(&ar->scan.timeout, jiffies +
+ ((arg->max_scan_time+200)*HZ)/1000);
return 0;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ath9k-devel] [PATCH 3/5] ath10k: increase scan timeout
2013-05-06 11:24 ` [ath9k-devel] [PATCH 3/5] ath10k: increase scan timeout Michal Kazior
@ 2013-05-07 13:52 ` Kalle Valo
0 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2013-05-07 13:52 UTC (permalink / raw)
To: ath9k-devel
Michal Kazior <michal.kazior@tieto.com> writes:
> We can't use the max scan time as the timeout
> itself. We must account for various overheads (ie.
> command/event processing).
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 3a27f35..df57772 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -1481,8 +1481,10 @@ static int ath10k_start_scan(struct ath10k *ar,
> /* the scan can complete earlier, before we even
> * start the timer. in that case the timer handler
> * checks ar->scan.in_progress and bails out if its
> - * false. */
> - mod_timer(&ar->scan.timeout, jiffies + (arg->max_scan_time*HZ)/1000);
> + * false. Add a 200ms margin to account event/command
> + * processing. */
> + mod_timer(&ar->scan.timeout, jiffies +
> + ((arg->max_scan_time+200)*HZ)/1000);
This patch is ok (no need to resend), but this code could use
msecs_to_jiffies().
--
Kalle Valo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [ath9k-devel] [PATCH 4/5] ath10k: wait for remain on channel chan switch
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
` (2 preceding siblings ...)
2013-05-06 11:24 ` [ath9k-devel] [PATCH 3/5] ath10k: increase scan timeout Michal Kazior
@ 2013-05-06 11:24 ` Michal Kazior
2013-05-06 11:24 ` [ath9k-devel] [PATCH 5/5] ath10k: abort scan if start completion never came Michal Kazior
2013-05-07 13:53 ` [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Kalle Valo
5 siblings, 0 replies; 8+ messages in thread
From: Michal Kazior @ 2013-05-06 11:24 UTC (permalink / raw)
To: ath9k-devel
We didn't wait for scan channel change event so we
could end up sending frames to a different
channel.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/core.c | 1 +
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/mac.c | 12 ++++++++++++
drivers/net/wireless/ath/ath10k/wmi.c | 4 ++++
4 files changed, 19 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 2be47f3..5ed04af 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -466,6 +466,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
init_completion(&ar->scan.started);
init_completion(&ar->scan.completed);
+ init_completion(&ar->scan.on_channel);
init_completion(&ar->install_key_done);
init_completion(&ar->vdev_setup_done);
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 1110551..6c681cf 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -303,11 +303,13 @@ struct ath10k {
struct {
struct completion started;
struct completion completed;
+ struct completion on_channel;
struct timer_list timeout;
bool is_roc;
bool in_progress;
bool aborting;
int vdev_id;
+ int roc_freq;
} scan;
struct {
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index df57772..72ac899 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2294,10 +2294,12 @@ static int ath10k_remain_on_channel(struct ieee80211_hw *hw,
INIT_COMPLETION(ar->scan.started);
INIT_COMPLETION(ar->scan.completed);
+ INIT_COMPLETION(ar->scan.on_channel);
ar->scan.in_progress = true;
ar->scan.aborting = false;
ar->scan.is_roc = true;
ar->scan.vdev_id = arvif->vdev_id;
+ ar->scan.roc_freq = chan->center_freq;
spin_unlock_bh(&ar->data_lock);
memset(&arg, 0, sizeof(arg));
@@ -2318,8 +2320,18 @@ static int ath10k_remain_on_channel(struct ieee80211_hw *hw,
spin_lock_bh(&ar->data_lock);
ar->scan.in_progress = false;
spin_unlock_bh(&ar->data_lock);
+ goto exit;
+ }
+
+ ret = wait_for_completion_timeout(&ar->scan.on_channel, 3*HZ);
+ if (ret == 0) {
+ ath10k_warn("could not switch to channel for roc scan\n");
+ ath10k_abort_scan(ar);
+ ret = -ETIMEDOUT;
+ goto exit;
}
+ ret = 0;
exit:
mutex_unlock(&ar->conf_mutex);
return ret;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index efa72fc..404976d 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -202,6 +202,10 @@ static int ath10k_wmi_event_scan(struct ath10k *ar, struct sk_buff *skb)
case WMI_SCAN_EVENT_FOREIGN_CHANNEL:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_FOREIGN_CHANNEL\n");
ar->scan_channel = ieee80211_get_channel(ar->hw->wiphy, freq);
+ if (ar->scan.in_progress && ar->scan.is_roc &&
+ ar->scan.roc_freq == freq) {
+ complete(&ar->scan.on_channel);
+ }
break;
case WMI_SCAN_EVENT_DEQUEUED:
ath10k_dbg(ATH10K_DBG_WMI, "SCAN_EVENT_DEQUEUED\n");
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ath9k-devel] [PATCH 5/5] ath10k: abort scan if start completion never came
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
` (3 preceding siblings ...)
2013-05-06 11:24 ` [ath9k-devel] [PATCH 4/5] ath10k: wait for remain on channel chan switch Michal Kazior
@ 2013-05-06 11:24 ` Michal Kazior
2013-05-07 13:53 ` [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Kalle Valo
5 siblings, 0 replies; 8+ messages in thread
From: Michal Kazior @ 2013-05-06 11:24 UTC (permalink / raw)
To: ath9k-devel
Abort the scan just in case FW has hung
temporarily.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 72ac899..23af4c5 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1475,8 +1475,10 @@ static int ath10k_start_scan(struct ath10k *ar,
ath10k_wmi_flush_tx(ar);
ret = wait_for_completion_timeout(&ar->scan.started, 1*HZ);
- if (ret == 0)
+ if (ret == 0) {
+ ath10k_abort_scan(ar);
return ret;
+ }
/* the scan can complete earlier, before we even
* start the timer. in that case the timer handler
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes
2013-05-06 11:24 [ath9k-devel] [PATCH 0/5] ath10k: scan/roc fixes Michal Kazior
` (4 preceding siblings ...)
2013-05-06 11:24 ` [ath9k-devel] [PATCH 5/5] ath10k: abort scan if start completion never came Michal Kazior
@ 2013-05-07 13:53 ` Kalle Valo
5 siblings, 0 replies; 8+ messages in thread
From: Kalle Valo @ 2013-05-07 13:53 UTC (permalink / raw)
To: ath9k-devel
Michal Kazior <michal.kazior@tieto.com> writes:
> Michal Kazior (5):
> ath10k: use data_lock to protect {scan,rx}_channel
> ath10k: simplify scan event locking
> ath10k: increase scan timeout
> ath10k: wait for remain on channel chan switch
> ath10k: abort scan if start completion never came
Thanks, applied.
--
Kalle Valo
^ permalink raw reply [flat|nested] 8+ messages in thread