* [PATCH 1/2] wifi: ath11k: move update channel list from update reg worker to reg notifier
2023-03-29 9:12 [PATCH 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Wen Gong
@ 2023-03-29 9:12 ` Wen Gong
2023-03-29 9:12 ` [PATCH 2/2] wifi: ath11k: move udpate channel list to worker for wait flag Wen Gong
2023-07-10 3:03 ` [PATCH 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Wen Gong
2 siblings, 0 replies; 6+ messages in thread
From: Wen Gong @ 2023-03-29 9:12 UTC (permalink / raw)
To: ath11k; +Cc: linux-wireless, quic_wgong
Currently ath11k call regulatory_set_wiphy_regd() in ath11k_regd_update()
to notify the reg domain change to cfg80211, and then ath11k update the
channel list to firmware by ath11k_reg_update_chan_list() immediately in
ath11k_regd_update(), they are running in two threads, it leads the channel
list data out of sync caused by muti-threads without synchronization.
cfg80211 calculate the flags of ieee80211_channel asynchronously in its
reg_work, it is calculated by the callstack below and ath11k get the
flags of ieee80211_channel in another thread without synchronization
for the channel list data.
callstack:
handle_channel_custom()
handle_band_custom()
reg_process_self_managed_hint()
reg_process_self_managed_hints()
reg_todo()
ath11k_reg_update_chan_list() need to use the flags calculated by the
reg_work, they are two thread and have no synchronization for the flags,
then ath11k_reg_update_chan_list() maybe get the wrong flags value before
the calculation finished.
The correct flow is ath11k_reg_update_chan_list() should be called after
the flags finished calculation by reg_work. reg_call_notifier() with
initiator type NL80211_REGDOM_SET_BY_DRIVER will be called by
reg_process_self_managed_hint() after the calculation finished.
So change to update channel list in ath11k_reg_notifier() for initiator
type NL80211_REGDOM_SET_BY_DRIVER, then the calculation of flags of
ieee80211_channel has already finished and ath11k_reg_update_chan_list()
will use the correct value of flags.
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
Fixes: f45cb6b29cd3 ("wifi: ath11k: avoid deadlock during regulatory update in ath11k_regd_update()")
Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
drivers/net/wireless/ath/ath11k/reg.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index 6fae4e61ede7..f50740219202 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -55,6 +55,17 @@ ath11k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
ath11k_dbg(ar->ab, ATH11K_DBG_REG,
"Regulatory Notification received for %s\n", wiphy_name(wiphy));
+ if ((request->initiator == NL80211_REGDOM_SET_BY_DRIVER) &&
+ (ar->state == ATH11K_STATE_ON)) {
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "dynamically updated by driver\n");
+ ret = ath11k_reg_update_chan_list(ar, true);
+ if (ret)
+ ath11k_warn(ar->ab, "failed to update channel list: %d\n", ret);
+
+ return;
+ }
+
/* Currently supporting only General User Hints. Cell base user
* hints to be handled later.
* Hints from other sources like Core, Beacons are not expected for
@@ -294,12 +305,6 @@ int ath11k_regd_update(struct ath11k *ar)
if (ret)
goto err;
- if (ar->state == ATH11K_STATE_ON) {
- ret = ath11k_reg_update_chan_list(ar, true);
- if (ret)
- goto err;
- }
-
return 0;
err:
ath11k_warn(ab, "failed to perform regd update : %d\n", ret);
@@ -743,6 +748,7 @@ void ath11k_regd_update_work(struct work_struct *work)
void ath11k_reg_init(struct ath11k *ar)
{
ar->hw->wiphy->regulatory_flags = REGULATORY_WIPHY_SELF_MANAGED;
+ ar->hw->wiphy->flags |= WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER;
ar->hw->wiphy->reg_notifier = ath11k_reg_notifier;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] wifi: ath11k: move udpate channel list to worker for wait flag
2023-03-29 9:12 [PATCH 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Wen Gong
2023-03-29 9:12 ` [PATCH 1/2] wifi: ath11k: move update channel list from update reg worker to reg notifier Wen Gong
@ 2023-03-29 9:12 ` Wen Gong
2023-07-10 3:03 ` [PATCH 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Wen Gong
2 siblings, 0 replies; 6+ messages in thread
From: Wen Gong @ 2023-03-29 9:12 UTC (permalink / raw)
To: ath11k; +Cc: linux-wireless, quic_wgong
When wait flag is set for ath11k_reg_update_chan_list(), it maybe wait
the completion of 11d/hw scan if 11d/hw scan are running, and now after
the previous patch "wifi: ath11k: move update channel list from update
reg worker to reg notifier", ath11k_reg_update_chan_list() is called by
ath11k_reg_notifier() which is running in the reg_work of cfg80211, the
reg_work is running with rtnl_lock() which is a global lock, if the wait
of completion of 11d/hw scan happened in ath11k_reg_update_chan_list(),
it will increase the time of occupy the rtnl_lock by reg_work, and then
increase the wait time of the rtnl_lock for other threads.
Move update channel list operation in ath11k_reg_update_chan_list() to
a worker of ath11k, then the wait of completion of 11d/hw scan will not
happen in reg_work and not increase the time of occupy the rtnl_lock by
reg_work.
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
drivers/net/wireless/ath/ath11k/core.c | 1 +
drivers/net/wireless/ath/ath11k/core.h | 4 ++
drivers/net/wireless/ath/ath11k/mac.c | 13 ++++
drivers/net/wireless/ath/ath11k/reg.c | 82 +++++++++++++++++---------
drivers/net/wireless/ath/ath11k/reg.h | 1 +
drivers/net/wireless/ath/ath11k/wmi.h | 1 +
6 files changed, 74 insertions(+), 28 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index 75fdbe4ef83a..650e6744923c 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1702,6 +1702,7 @@ void ath11k_core_halt(struct ath11k *ar)
ath11k_mac_scan_finish(ar);
ath11k_mac_peer_cleanup_all(ar);
cancel_delayed_work_sync(&ar->scan.timeout);
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
cancel_work_sync(&ab->update_11d_work);
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 0830276e5028..97712814e4cf 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -694,6 +694,10 @@ struct ath11k {
struct completion bss_survey_done;
struct work_struct regd_update_work;
+ struct work_struct channel_update_work;
+ struct list_head channel_update_queue;
+ /* protects channel_update_queue data */
+ spinlock_t channel_update_lock;
struct work_struct wmi_mgmt_tx_work;
struct sk_buff_head wmi_mgmt_tx_queue;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 110a38cce0a7..1a0c597e03bc 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -5962,6 +5962,7 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw)
{
struct ath11k *ar = hw->priv;
struct htt_ppdu_stats_info *ppdu_stats, *tmp;
+ struct scan_chan_list_params *params, *tmp_ch;
int ret;
ath11k_mac_drain_tx(ar);
@@ -5977,6 +5978,7 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw)
mutex_unlock(&ar->conf_mutex);
cancel_delayed_work_sync(&ar->scan.timeout);
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
cancel_work_sync(&ar->ab->update_11d_work);
@@ -5992,6 +5994,13 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw)
}
spin_unlock_bh(&ar->data_lock);
+ spin_lock_bh(&ar->channel_update_lock);
+ list_for_each_entry_safe(params, tmp_ch, &ar->channel_update_queue, list) {
+ list_del(¶ms->list);
+ kfree(params);
+ }
+ spin_unlock_bh(&ar->channel_update_lock);
+
rcu_assign_pointer(ar->ab->pdevs_active[ar->pdev_idx], NULL);
synchronize_rcu();
@@ -8924,6 +8933,7 @@ static const struct wiphy_iftype_ext_capab ath11k_iftypes_ext_capa[] = {
static void __ath11k_mac_unregister(struct ath11k *ar)
{
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
ieee80211_unregister_hw(ar->hw);
@@ -9315,6 +9325,9 @@ int ath11k_mac_allocate(struct ath11k_base *ab)
INIT_DELAYED_WORK(&ar->scan.timeout, ath11k_scan_timeout_work);
INIT_WORK(&ar->regd_update_work, ath11k_regd_update_work);
+ INIT_WORK(&ar->channel_update_work, ath11k_regd_update_chan_list_work);
+ INIT_LIST_HEAD(&ar->channel_update_queue);
+ spin_lock_init(&ar->channel_update_lock);
INIT_WORK(&ar->wmi_mgmt_tx_work, ath11k_mgmt_over_wmi_tx_work);
skb_queue_head_init(&ar->wmi_mgmt_tx_queue);
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index f50740219202..79d074455196 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -123,32 +123,7 @@ int ath11k_reg_update_chan_list(struct ath11k *ar, bool wait)
struct channel_param *ch;
enum nl80211_band band;
int num_channels = 0;
- int i, ret, left;
-
- if (wait && ar->state_11d != ATH11K_11D_IDLE) {
- left = wait_for_completion_timeout(&ar->completed_11d_scan,
- ATH11K_SCAN_TIMEOUT_HZ);
- if (!left) {
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "failed to receive 11d scan complete: timed out\n");
- ar->state_11d = ATH11K_11D_IDLE;
- }
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "reg 11d scan wait left time %d\n", left);
- }
-
- if (wait &&
- (ar->scan.state == ATH11K_SCAN_STARTING ||
- ar->scan.state == ATH11K_SCAN_RUNNING)) {
- left = wait_for_completion_timeout(&ar->scan.completed,
- ATH11K_SCAN_TIMEOUT_HZ);
- if (!left)
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "failed to receive hw scan complete: timed out\n");
-
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "reg hw scan wait left time %d\n", left);
- }
+ int i, ret = 0;
if (ar->state == ATH11K_STATE_RESTARTING)
return 0;
@@ -230,8 +205,15 @@ int ath11k_reg_update_chan_list(struct ath11k *ar, bool wait)
}
}
- ret = ath11k_wmi_send_scan_chan_list_cmd(ar, params);
- kfree(params);
+ if (wait) {
+ spin_lock_bh(&ar->channel_update_lock);
+ list_add_tail(¶ms->list, &ar->channel_update_queue);
+ spin_unlock_bh(&ar->channel_update_lock);
+ queue_work(ar->ab->workqueue, &ar->channel_update_work);
+ } else {
+ ret = ath11k_wmi_send_scan_chan_list_cmd(ar, params);
+ kfree(params);
+ }
return ret;
}
@@ -728,6 +710,50 @@ ath11k_reg_build_regd(struct ath11k_base *ab,
return new_regd;
}
+void ath11k_regd_update_chan_list_work(struct work_struct *work)
+{
+ struct ath11k *ar = container_of(work, struct ath11k,
+ channel_update_work);
+ struct scan_chan_list_params *params, *tmp;
+ int left;
+
+ spin_lock_bh(&ar->channel_update_lock);
+
+ list_for_each_entry_safe(params, tmp, &ar->channel_update_queue, list) {
+ list_del(¶ms->list);
+ spin_unlock_bh(&ar->channel_update_lock);
+
+ if (ar->state_11d != ATH11K_11D_IDLE) {
+ left = wait_for_completion_timeout(&ar->completed_11d_scan,
+ ATH11K_SCAN_TIMEOUT_HZ);
+ if (!left) {
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "failed to receive 11d scan complete: timed out\n");
+ ar->state_11d = ATH11K_11D_IDLE;
+ }
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "reg 11d scan wait left time %d\n", left);
+ }
+
+ if ((ar->scan.state == ATH11K_SCAN_STARTING ||
+ ar->scan.state == ATH11K_SCAN_RUNNING)) {
+ left = wait_for_completion_timeout(&ar->scan.completed,
+ ATH11K_SCAN_TIMEOUT_HZ);
+ if (!left)
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "failed to receive hw scan complete: timed out\n");
+
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "reg hw scan wait left time %d\n", left);
+ }
+
+ ath11k_wmi_send_scan_chan_list_cmd(ar, params);
+ kfree(params);
+ spin_lock_bh(&ar->channel_update_lock);
+ }
+ spin_unlock_bh(&ar->channel_update_lock);
+}
+
void ath11k_regd_update_work(struct work_struct *work)
{
struct ath11k *ar = container_of(work, struct ath11k,
diff --git a/drivers/net/wireless/ath/ath11k/reg.h b/drivers/net/wireless/ath/ath11k/reg.h
index 2f284f26378d..13751fab06c4 100644
--- a/drivers/net/wireless/ath/ath11k/reg.h
+++ b/drivers/net/wireless/ath/ath11k/reg.h
@@ -28,6 +28,7 @@ enum ath11k_dfs_region {
void ath11k_reg_init(struct ath11k *ar);
void ath11k_reg_free(struct ath11k_base *ab);
void ath11k_regd_update_work(struct work_struct *work);
+void ath11k_regd_update_chan_list_work(struct work_struct *work);
struct ieee80211_regdomain *
ath11k_reg_build_regd(struct ath11k_base *ab,
struct cur_regulatory_info *reg_info, bool intersect);
diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
index 2273d885f7fa..be324be080be 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.h
+++ b/drivers/net/wireless/ath/ath11k/wmi.h
@@ -3685,6 +3685,7 @@ struct wmi_stop_scan_cmd {
};
struct scan_chan_list_params {
+ struct list_head list;
u32 pdev_id;
u16 nallchans;
struct channel_param ch_param[];
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread