All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFT 0/3] ath10k: fixes
@ 2013-10-03 13:09 ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This patchset addresses recently spotted issue
with (yet another) scheduling while atomic bug
(the other being WEP key index setting). This one
is related to hw_config() and powersave settings.
This comes from recent changes I've do to HTC/WMI.
WMI commands can now block so it's illegal to call
them in an atomic context anymore.

ath10k needs to setup some settings per-vdev (i.e.
per-interface) such as powersave, rts, fragmentation.
Until now mac80211 iteration functions were used.
However using non-atomic iteration function variant
doesn't solve the problem as it introduces an
issue with iflist_mtx deadlock in some cases.

I briefly tried to reproduce the issue Kalle
reported but was unsuccessful thus the "/RFT".



Michal Kazior (3):
  ath10k: fix add_interface failure handling
  ath10k: track vif list internally
  ath10k: fix scheduling while atomic config bug

 drivers/net/wireless/ath/ath10k/core.c |    2 +
 drivers/net/wireless/ath/ath10k/core.h |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  199 ++++++++++++++++----------------
 3 files changed, 107 insertions(+), 97 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH/RFT 0/3] ath10k: fixes
@ 2013-10-03 13:09 ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This patchset addresses recently spotted issue
with (yet another) scheduling while atomic bug
(the other being WEP key index setting). This one
is related to hw_config() and powersave settings.
This comes from recent changes I've do to HTC/WMI.
WMI commands can now block so it's illegal to call
them in an atomic context anymore.

ath10k needs to setup some settings per-vdev (i.e.
per-interface) such as powersave, rts, fragmentation.
Until now mac80211 iteration functions were used.
However using non-atomic iteration function variant
doesn't solve the problem as it introduces an
issue with iflist_mtx deadlock in some cases.

I briefly tried to reproduce the issue Kalle
reported but was unsuccessful thus the "/RFT".



Michal Kazior (3):
  ath10k: fix add_interface failure handling
  ath10k: track vif list internally
  ath10k: fix scheduling while atomic config bug

 drivers/net/wireless/ath/ath10k/core.c |    2 +
 drivers/net/wireless/ath/ath10k/core.h |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  199 ++++++++++++++++----------------
 3 files changed, 107 insertions(+), 97 deletions(-)

-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH/RFT 1/3] ath10k: fix add_interface failure handling
  2013-10-03 13:09 ` Michal Kazior
@ 2013-10-03 13:09   ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.

This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   50 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8684e03..7f8b258 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2001,18 +2001,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	bit = ffs(ar->free_vdev_map);
 	if (bit == 0) {
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	arvif->vdev_id = bit - 1;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
 
 	if (ar->p2p)
 		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2048,26 +2047,32 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 				     arvif->vdev_subtype, vif->addr);
 	if (ret) {
 		ath10k_warn("WMI vdev create failed: ret %d\n", ret);
-		goto exit;
+		goto err;
 	}
 
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
 					arvif->def_wep_key_index);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	vdev_param = ar->wmi.vdev_param->tx_encap_type;
 	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
 					ATH10K_HW_TXRX_NATIVE_WIFI);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set TX encap: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
 		if (ret) {
 			ath10k_warn("Failed to create peer for AP: %d\n", ret);
-			goto exit;
+			goto err_vdev_delete;
 		}
 	}
 
@@ -2076,38 +2081,57 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
 		value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
 		value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+			goto err_peer_delete;
+		}
 	}
 
 	ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-exit:
+	mutex_unlock(&ar->conf_mutex);
+	return 0;
+
+err_peer_delete:
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+err_vdev_delete:
+	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT 1/3] ath10k: fix add_interface failure handling
@ 2013-10-03 13:09   ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.

This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   50 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8684e03..7f8b258 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2001,18 +2001,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	bit = ffs(ar->free_vdev_map);
 	if (bit == 0) {
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	arvif->vdev_id = bit - 1;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
 
 	if (ar->p2p)
 		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2048,26 +2047,32 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 				     arvif->vdev_subtype, vif->addr);
 	if (ret) {
 		ath10k_warn("WMI vdev create failed: ret %d\n", ret);
-		goto exit;
+		goto err;
 	}
 
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
 					arvif->def_wep_key_index);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	vdev_param = ar->wmi.vdev_param->tx_encap_type;
 	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
 					ATH10K_HW_TXRX_NATIVE_WIFI);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set TX encap: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
 		if (ret) {
 			ath10k_warn("Failed to create peer for AP: %d\n", ret);
-			goto exit;
+			goto err_vdev_delete;
 		}
 	}
 
@@ -2076,38 +2081,57 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
 		value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
 		value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+			goto err_peer_delete;
+		}
 	}
 
 	ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-exit:
+	mutex_unlock(&ar->conf_mutex);
+	return 0;
+
+err_peer_delete:
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+err_vdev_delete:
+	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT 2/3] ath10k: track vif list internally
  2013-10-03 13:09 ` Michal Kazior
@ 2013-10-03 13:09   ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    2 ++
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |    3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 7b5dd09..4d04dc8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -710,6 +710,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	mutex_init(&ar->conf_mutex);
 	spin_lock_init(&ar->data_lock);
 
+	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->peers);
 	init_waitqueue_head(&ar->peer_mapping_wq);
 
@@ -817,6 +818,7 @@ int ath10k_core_start(struct ath10k *ar)
 		goto err_disconnect_htc;
 
 	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+	INIT_LIST_HEAD(&ar->arvifs);
 
 	return 0;
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..de20ac0 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
 #define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
 
 struct ath10k_vif {
+	struct list_head list;
+
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
 	enum wmi_vdev_subtype vdev_subtype;
@@ -402,6 +404,7 @@ struct ath10k {
 	/* protects shared structure data */
 	spinlock_t data_lock;
 
+	struct list_head arvifs;
 	struct list_head peers;
 	wait_queue_head_t peer_mapping_wq;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7f8b258..292e5cd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2051,6 +2051,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2131,6 +2132,7 @@ err_peer_delete:
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_del(&arvif->list);
 err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
@@ -2153,6 +2155,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	ar->free_vdev_map |= 1 << (arvif->vdev_id);
+	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT 2/3] ath10k: track vif list internally
@ 2013-10-03 13:09   ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    2 ++
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |    3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 7b5dd09..4d04dc8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -710,6 +710,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	mutex_init(&ar->conf_mutex);
 	spin_lock_init(&ar->data_lock);
 
+	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->peers);
 	init_waitqueue_head(&ar->peer_mapping_wq);
 
@@ -817,6 +818,7 @@ int ath10k_core_start(struct ath10k *ar)
 		goto err_disconnect_htc;
 
 	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+	INIT_LIST_HEAD(&ar->arvifs);
 
 	return 0;
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..de20ac0 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
 #define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
 
 struct ath10k_vif {
+	struct list_head list;
+
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
 	enum wmi_vdev_subtype vdev_subtype;
@@ -402,6 +404,7 @@ struct ath10k {
 	/* protects shared structure data */
 	spinlock_t data_lock;
 
+	struct list_head arvifs;
 	struct list_head peers;
 	wait_queue_head_t peer_mapping_wq;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7f8b258..292e5cd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2051,6 +2051,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2131,6 +2132,7 @@ err_peer_delete:
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_del(&arvif->list);
 err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
@@ -2153,6 +2155,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	ar->free_vdev_map |= 1 << (arvif->vdev_id);
+	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT 3/3] ath10k: fix scheduling while atomic config bug
  2013-10-03 13:09 ` Michal Kazior
@ 2013-10-03 13:09   ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.

mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.

The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.

Reported-By: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |  146 ++++++++++++++-------------------
 1 file changed, 62 insertions(+), 84 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 292e5cd..df6f973 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
 /*
  * Review this when mac80211 gains per-interface powersave support.
  */
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct ath10k *ar = arvif->ar;
+	struct ieee80211_conf *conf = &ar->hw->conf;
 	enum wmi_sta_powersave_param param;
 	enum wmi_sta_ps_mode psmode;
 	int ret;
 
 	lockdep_assert_held(&arvif->ar->conf_mutex);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return;
+	if (arvif->vif->type != NL80211_IFTYPE_STATION)
+		return 0;
 
 	if (conf->flags & IEEE80211_CONF_PS) {
 		psmode = WMI_STA_PS_MODE_ENABLED;
 		param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
 
-		ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
-						  arvif->vdev_id,
-						  param,
+		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
 						  conf->dynamic_ps_timeout);
 		if (ret) {
 			ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
 				    arvif->vdev_id);
-			return;
+			return ret;
 		}
-
-		ar_iter->ret = ret;
 	} else {
 		psmode = WMI_STA_PS_MODE_DISABLED;
 	}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
 		   arvif->vdev_id, psmode ? "enable" : "disable");
 
-	ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
-					     psmode);
-	if (ar_iter->ret)
+	ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+	if (ret) {
 		ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
 			    psmode, arvif->vdev_id);
+		return ret;
+	}
+
+	return 0;
 }
 
 /**********************/
@@ -1919,9 +1917,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	cancel_work_sync(&ar->restart_work);
 }
 
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
 {
-	struct ath10k_generic_iter ar_iter;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1930,17 +1929,17 @@ static void ath10k_config_ps(struct ath10k *ar)
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
 	if (ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+		return 0;
 
-	ieee80211_iterate_active_interfaces_atomic(
-		ar->hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_ps_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ret = ath10k_mac_vif_setup_ps(arvif);
+		if (ret) {
+			ath10k_warn("could not setup powersave (%d)\n", ret);
+			break;
+		}
+	}
 
-	if (ar_iter.ret)
-		ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+	return ret;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2834,86 +2833,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
  * Both RTS and Fragmentation threshold are interface-specific
  * in ath10k, but device-specific in mac80211.
  */
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
-		   arvif->vdev_id, rts);
-
-	ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_rts_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
-		   arvif->vdev_id, frag);
-
-	ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_frag_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
 static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT 3/3] ath10k: fix scheduling while atomic config bug
@ 2013-10-03 13:09   ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-03 13:09 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.

mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.

The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.

Reported-By: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |  146 ++++++++++++++-------------------
 1 file changed, 62 insertions(+), 84 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 292e5cd..df6f973 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
 /*
  * Review this when mac80211 gains per-interface powersave support.
  */
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct ath10k *ar = arvif->ar;
+	struct ieee80211_conf *conf = &ar->hw->conf;
 	enum wmi_sta_powersave_param param;
 	enum wmi_sta_ps_mode psmode;
 	int ret;
 
 	lockdep_assert_held(&arvif->ar->conf_mutex);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return;
+	if (arvif->vif->type != NL80211_IFTYPE_STATION)
+		return 0;
 
 	if (conf->flags & IEEE80211_CONF_PS) {
 		psmode = WMI_STA_PS_MODE_ENABLED;
 		param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
 
-		ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
-						  arvif->vdev_id,
-						  param,
+		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
 						  conf->dynamic_ps_timeout);
 		if (ret) {
 			ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
 				    arvif->vdev_id);
-			return;
+			return ret;
 		}
-
-		ar_iter->ret = ret;
 	} else {
 		psmode = WMI_STA_PS_MODE_DISABLED;
 	}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
 		   arvif->vdev_id, psmode ? "enable" : "disable");
 
-	ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
-					     psmode);
-	if (ar_iter->ret)
+	ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+	if (ret) {
 		ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
 			    psmode, arvif->vdev_id);
+		return ret;
+	}
+
+	return 0;
 }
 
 /**********************/
@@ -1919,9 +1917,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	cancel_work_sync(&ar->restart_work);
 }
 
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
 {
-	struct ath10k_generic_iter ar_iter;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1930,17 +1929,17 @@ static void ath10k_config_ps(struct ath10k *ar)
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
 	if (ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+		return 0;
 
-	ieee80211_iterate_active_interfaces_atomic(
-		ar->hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_ps_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ret = ath10k_mac_vif_setup_ps(arvif);
+		if (ret) {
+			ath10k_warn("could not setup powersave (%d)\n", ret);
+			break;
+		}
+	}
 
-	if (ar_iter.ret)
-		ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+	return ret;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2834,86 +2833,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
  * Both RTS and Fragmentation threshold are interface-specific
  * in ath10k, but device-specific in mac80211.
  */
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
-		   arvif->vdev_id, rts);
-
-	ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_rts_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
-		   arvif->vdev_id, frag);
-
-	ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_frag_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
 static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 0/4] ath10k: fixes
  2013-10-03 13:09 ` Michal Kazior
@ 2013-10-04  5:43   ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This patchset addresses recently spotted issue
with (yet another) scheduling while atomic bug
(the other being WEP key index setting). This one
is related to hw_config() and powersave settings.
This comes from recent changes I've done to
HTC/WMI. WMI commands can block now so it's
illegal to call them in an atomic context anymore.

ath10k needs to setup some settings per-vdev (i.e.
per-interface) such as powersave, rts, fragmentation.
Until now mac80211 iteration functions were used.
However using non-atomic iteration function variant
doesn't solve the problem as it introduces an
issue with iflist_mtx deadlock in some cases.

I briefly tried to reproduce the issue Kalle
reported but was unsuccessful thus the "/RFT".

v2:
 * fix kbuild test robot warning
   (uninitialized `ret`)
 * add patch #4 that removes code that becomes
   unnecessary after patch #3


Michal Kazior (4):
  ath10k: fix add_interface failure handling
  ath10k: track vif list internally
  ath10k: fix scheduling while atomic config bug
  ath10k: remove unnecessary checks

 drivers/net/wireless/ath/ath10k/core.c |    2 +
 drivers/net/wireless/ath/ath10k/core.h |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  200 +++++++++++++++-----------------
 3 files changed, 97 insertions(+), 108 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 0/4] ath10k: fixes
@ 2013-10-04  5:43   ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Hi,

This patchset addresses recently spotted issue
with (yet another) scheduling while atomic bug
(the other being WEP key index setting). This one
is related to hw_config() and powersave settings.
This comes from recent changes I've done to
HTC/WMI. WMI commands can block now so it's
illegal to call them in an atomic context anymore.

ath10k needs to setup some settings per-vdev (i.e.
per-interface) such as powersave, rts, fragmentation.
Until now mac80211 iteration functions were used.
However using non-atomic iteration function variant
doesn't solve the problem as it introduces an
issue with iflist_mtx deadlock in some cases.

I briefly tried to reproduce the issue Kalle
reported but was unsuccessful thus the "/RFT".

v2:
 * fix kbuild test robot warning
   (uninitialized `ret`)
 * add patch #4 that removes code that becomes
   unnecessary after patch #3


Michal Kazior (4):
  ath10k: fix add_interface failure handling
  ath10k: track vif list internally
  ath10k: fix scheduling while atomic config bug
  ath10k: remove unnecessary checks

 drivers/net/wireless/ath/ath10k/core.c |    2 +
 drivers/net/wireless/ath/ath10k/core.h |    3 +
 drivers/net/wireless/ath/ath10k/mac.c  |  200 +++++++++++++++-----------------
 3 files changed, 97 insertions(+), 108 deletions(-)

-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
  2013-10-04  5:43   ` Michal Kazior
@ 2013-10-04  5:43     ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.

This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   50 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8684e03..7f8b258 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2001,18 +2001,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	bit = ffs(ar->free_vdev_map);
 	if (bit == 0) {
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	arvif->vdev_id = bit - 1;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
 
 	if (ar->p2p)
 		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2048,26 +2047,32 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 				     arvif->vdev_subtype, vif->addr);
 	if (ret) {
 		ath10k_warn("WMI vdev create failed: ret %d\n", ret);
-		goto exit;
+		goto err;
 	}
 
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
 					arvif->def_wep_key_index);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	vdev_param = ar->wmi.vdev_param->tx_encap_type;
 	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
 					ATH10K_HW_TXRX_NATIVE_WIFI);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set TX encap: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
 		if (ret) {
 			ath10k_warn("Failed to create peer for AP: %d\n", ret);
-			goto exit;
+			goto err_vdev_delete;
 		}
 	}
 
@@ -2076,38 +2081,57 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
 		value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
 		value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+			goto err_peer_delete;
+		}
 	}
 
 	ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-exit:
+	mutex_unlock(&ar->conf_mutex);
+	return 0;
+
+err_peer_delete:
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+err_vdev_delete:
+	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
@ 2013-10-04  5:43     ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

If something failed along add_interface() setup it
was possible to leak a vdev id, vdev and peer.

This could end up with leaked FW state or FW crash
(assuming add_interface() failure wasn't a result of
a crash).

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   50 ++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 8684e03..7f8b258 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2001,18 +2001,17 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	if ((vif->type == NL80211_IFTYPE_MONITOR) && ar->monitor_present) {
 		ath10k_warn("Only one monitor interface allowed\n");
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	bit = ffs(ar->free_vdev_map);
 	if (bit == 0) {
 		ret = -EBUSY;
-		goto exit;
+		goto err;
 	}
 
 	arvif->vdev_id = bit - 1;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
-	ar->free_vdev_map &= ~(1 << arvif->vdev_id);
 
 	if (ar->p2p)
 		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
@@ -2048,26 +2047,32 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 				     arvif->vdev_subtype, vif->addr);
 	if (ret) {
 		ath10k_warn("WMI vdev create failed: ret %d\n", ret);
-		goto exit;
+		goto err;
 	}
 
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
 					arvif->def_wep_key_index);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set default keyid: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	vdev_param = ar->wmi.vdev_param->tx_encap_type;
 	ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id, vdev_param,
 					ATH10K_HW_TXRX_NATIVE_WIFI);
-	if (ret)
+	if (ret) {
 		ath10k_warn("Failed to set TX encap: %d\n", ret);
+		goto err_vdev_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_create(ar, arvif->vdev_id, vif->addr);
 		if (ret) {
 			ath10k_warn("Failed to create peer for AP: %d\n", ret);
-			goto exit;
+			goto err_vdev_delete;
 		}
 	}
 
@@ -2076,38 +2081,57 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 		value = WMI_STA_PS_RX_WAKE_POLICY_WAKE;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set RX wake policy: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_TX_WAKE_THRESHOLD;
 		value = WMI_STA_PS_TX_WAKE_THRESHOLD_ALWAYS;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set TX wake thresh: %d\n", ret);
+			goto err_peer_delete;
+		}
 
 		param = WMI_STA_PS_PARAM_PSPOLL_COUNT;
 		value = WMI_STA_PS_PSPOLL_COUNT_NO_MAX;
 		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id,
 						  param, value);
-		if (ret)
+		if (ret) {
 			ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
+			goto err_peer_delete;
+		}
 	}
 
 	ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
-	if (ret)
+	if (ret) {
 		ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
 			    arvif->vdev_id, ret);
+		goto err_peer_delete;
+	}
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR)
 		ar->monitor_present = true;
 
-exit:
+	mutex_unlock(&ar->conf_mutex);
+	return 0;
+
+err_peer_delete:
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP)
+		ath10k_wmi_peer_delete(ar, arvif->vdev_id, vif->addr);
+err_vdev_delete:
+	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
+	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 2/4] ath10k: track vif list internally
  2013-10-04  5:43   ` Michal Kazior
@ 2013-10-04  5:43     ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    2 ++
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |    3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 7b5dd09..4d04dc8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -710,6 +710,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	mutex_init(&ar->conf_mutex);
 	spin_lock_init(&ar->data_lock);
 
+	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->peers);
 	init_waitqueue_head(&ar->peer_mapping_wq);
 
@@ -817,6 +818,7 @@ int ath10k_core_start(struct ath10k *ar)
 		goto err_disconnect_htc;
 
 	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+	INIT_LIST_HEAD(&ar->arvifs);
 
 	return 0;
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..de20ac0 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
 #define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
 
 struct ath10k_vif {
+	struct list_head list;
+
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
 	enum wmi_vdev_subtype vdev_subtype;
@@ -402,6 +404,7 @@ struct ath10k {
 	/* protects shared structure data */
 	spinlock_t data_lock;
 
+	struct list_head arvifs;
 	struct list_head peers;
 	wait_queue_head_t peer_mapping_wq;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7f8b258..292e5cd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2051,6 +2051,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2131,6 +2132,7 @@ err_peer_delete:
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_del(&arvif->list);
 err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
@@ -2153,6 +2155,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	ar->free_vdev_map |= 1 << (arvif->vdev_id);
+	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 2/4] ath10k: track vif list internally
@ 2013-10-04  5:43     ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface interations functions have
peculiar locking issues. This patch introduces
internal (to ath10k) vif list that will be used
for vif iteration purposes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/core.c |    2 ++
 drivers/net/wireless/ath/ath10k/core.h |    3 +++
 drivers/net/wireless/ath/ath10k/mac.c  |    3 +++
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 7b5dd09..4d04dc8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -710,6 +710,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
 	mutex_init(&ar->conf_mutex);
 	spin_lock_init(&ar->data_lock);
 
+	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->peers);
 	init_waitqueue_head(&ar->peer_mapping_wq);
 
@@ -817,6 +818,7 @@ int ath10k_core_start(struct ath10k *ar)
 		goto err_disconnect_htc;
 
 	ar->free_vdev_map = (1 << TARGET_NUM_VDEVS) - 1;
+	INIT_LIST_HEAD(&ar->arvifs);
 
 	return 0;
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index ce36daa..de20ac0 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -205,6 +205,8 @@ struct ath10k_peer {
 #define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
 
 struct ath10k_vif {
+	struct list_head list;
+
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
 	enum wmi_vdev_subtype vdev_subtype;
@@ -402,6 +404,7 @@ struct ath10k {
 	/* protects shared structure data */
 	spinlock_t data_lock;
 
+	struct list_head arvifs;
 	struct list_head peers;
 	wait_queue_head_t peer_mapping_wq;
 
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7f8b258..292e5cd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2051,6 +2051,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_add(&arvif->list, &ar->arvifs);
 
 	vdev_param = ar->wmi.vdev_param->def_keyid;
 	ret = ath10k_wmi_vdev_set_param(ar, 0, vdev_param,
@@ -2131,6 +2132,7 @@ err_peer_delete:
 err_vdev_delete:
 	ath10k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->free_vdev_map &= ~BIT(arvif->vdev_id);
+	list_del(&arvif->list);
 err:
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
@@ -2153,6 +2155,7 @@ static void ath10k_remove_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	ar->free_vdev_map |= 1 << (arvif->vdev_id);
+	list_del(&arvif->list);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
 		ret = ath10k_peer_delete(arvif->ar, arvif->vdev_id, vif->addr);
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 3/4] ath10k: fix scheduling while atomic config bug
  2013-10-04  5:43   ` Michal Kazior
@ 2013-10-04  5:43     ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.

mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.

The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.

Reported-By: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
v2:
 * fix kbuild test robot warning
   (uninitialized `ret`)

 drivers/net/wireless/ath/ath10k/mac.c |  146 ++++++++++++++-------------------
 1 file changed, 62 insertions(+), 84 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 292e5cd..583f065 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
 /*
  * Review this when mac80211 gains per-interface powersave support.
  */
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct ath10k *ar = arvif->ar;
+	struct ieee80211_conf *conf = &ar->hw->conf;
 	enum wmi_sta_powersave_param param;
 	enum wmi_sta_ps_mode psmode;
 	int ret;
 
 	lockdep_assert_held(&arvif->ar->conf_mutex);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return;
+	if (arvif->vif->type != NL80211_IFTYPE_STATION)
+		return 0;
 
 	if (conf->flags & IEEE80211_CONF_PS) {
 		psmode = WMI_STA_PS_MODE_ENABLED;
 		param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
 
-		ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
-						  arvif->vdev_id,
-						  param,
+		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
 						  conf->dynamic_ps_timeout);
 		if (ret) {
 			ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
 				    arvif->vdev_id);
-			return;
+			return ret;
 		}
-
-		ar_iter->ret = ret;
 	} else {
 		psmode = WMI_STA_PS_MODE_DISABLED;
 	}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
 		   arvif->vdev_id, psmode ? "enable" : "disable");
 
-	ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
-					     psmode);
-	if (ar_iter->ret)
+	ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+	if (ret) {
 		ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
 			    psmode, arvif->vdev_id);
+		return ret;
+	}
+
+	return 0;
 }
 
 /**********************/
@@ -1919,9 +1917,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	cancel_work_sync(&ar->restart_work);
 }
 
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
 {
-	struct ath10k_generic_iter ar_iter;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1930,17 +1929,17 @@ static void ath10k_config_ps(struct ath10k *ar)
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
 	if (ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+		return 0;
 
-	ieee80211_iterate_active_interfaces_atomic(
-		ar->hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_ps_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ret = ath10k_mac_vif_setup_ps(arvif);
+		if (ret) {
+			ath10k_warn("could not setup powersave (%d)\n", ret);
+			break;
+		}
+	}
 
-	if (ar_iter.ret)
-		ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+	return ret;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2834,86 +2833,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
  * Both RTS and Fragmentation threshold are interface-specific
  * in ath10k, but device-specific in mac80211.
  */
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
-		   arvif->vdev_id, rts);
-
-	ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_rts_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
-		   arvif->vdev_id, frag);
-
-	ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_frag_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
 static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 3/4] ath10k: fix scheduling while atomic config bug
@ 2013-10-04  5:43     ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

Recent HTC/WMI changes introduced the bug. ath10k
was using _atomic iteration function with
sleepable functions.

mac80211 provides another iteration function but
it cannot be safely called in hw_config() callback
due to local->iflist_mtx being possibly acquired
already.

The patch uses internal vif list for iteration
purposes and removes/refactors no longer necessary
_iter functions.

Reported-By: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
v2:
 * fix kbuild test robot warning
   (uninitialized `ret`)

 drivers/net/wireless/ath/ath10k/mac.c |  146 ++++++++++++++-------------------
 1 file changed, 62 insertions(+), 84 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 292e5cd..583f065 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -723,35 +723,30 @@ static void ath10k_control_ibss(struct ath10k_vif *arvif,
 /*
  * Review this when mac80211 gains per-interface powersave support.
  */
-static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ieee80211_conf *conf = &ar_iter->ar->hw->conf;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
+	struct ath10k *ar = arvif->ar;
+	struct ieee80211_conf *conf = &ar->hw->conf;
 	enum wmi_sta_powersave_param param;
 	enum wmi_sta_ps_mode psmode;
 	int ret;
 
 	lockdep_assert_held(&arvif->ar->conf_mutex);
 
-	if (vif->type != NL80211_IFTYPE_STATION)
-		return;
+	if (arvif->vif->type != NL80211_IFTYPE_STATION)
+		return 0;
 
 	if (conf->flags & IEEE80211_CONF_PS) {
 		psmode = WMI_STA_PS_MODE_ENABLED;
 		param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
 
-		ret = ath10k_wmi_set_sta_ps_param(ar_iter->ar,
-						  arvif->vdev_id,
-						  param,
+		ret = ath10k_wmi_set_sta_ps_param(ar, arvif->vdev_id, param,
 						  conf->dynamic_ps_timeout);
 		if (ret) {
 			ath10k_warn("Failed to set inactivity time for VDEV: %d\n",
 				    arvif->vdev_id);
-			return;
+			return ret;
 		}
-
-		ar_iter->ret = ret;
 	} else {
 		psmode = WMI_STA_PS_MODE_DISABLED;
 	}
@@ -759,11 +754,14 @@ static void ath10k_ps_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
 	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d psmode %s\n",
 		   arvif->vdev_id, psmode ? "enable" : "disable");
 
-	ar_iter->ret = ath10k_wmi_set_psmode(ar_iter->ar, arvif->vdev_id,
-					     psmode);
-	if (ar_iter->ret)
+	ret = ath10k_wmi_set_psmode(ar, arvif->vdev_id, psmode);
+	if (ret) {
 		ath10k_warn("Failed to set PS Mode: %d for VDEV: %d\n",
 			    psmode, arvif->vdev_id);
+		return ret;
+	}
+
+	return 0;
 }
 
 /**********************/
@@ -1919,9 +1917,10 @@ static void ath10k_stop(struct ieee80211_hw *hw)
 	cancel_work_sync(&ar->restart_work);
 }
 
-static void ath10k_config_ps(struct ath10k *ar)
+static int ath10k_config_ps(struct ath10k *ar)
 {
-	struct ath10k_generic_iter ar_iter;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	lockdep_assert_held(&ar->conf_mutex);
 
@@ -1930,17 +1929,17 @@ static void ath10k_config_ps(struct ath10k *ar)
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
 	if (ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+		return 0;
 
-	ieee80211_iterate_active_interfaces_atomic(
-		ar->hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_ps_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ret = ath10k_mac_vif_setup_ps(arvif);
+		if (ret) {
+			ath10k_warn("could not setup powersave (%d)\n", ret);
+			break;
+		}
+	}
 
-	if (ar_iter.ret)
-		ath10k_warn("failed to set ps config (%d)\n", ar_iter.ret);
+	return ret;
 }
 
 static int ath10k_config(struct ieee80211_hw *hw, u32 changed)
@@ -2834,86 +2833,65 @@ static int ath10k_cancel_remain_on_channel(struct ieee80211_hw *hw)
  * Both RTS and Fragmentation threshold are interface-specific
  * in ath10k, but device-specific in mac80211.
  */
-static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
-{
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 rts = ar_iter->ar->hw->wiphy->rts_threshold;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
+{
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts_threshold %d\n",
-		   arvif->vdev_id, rts);
-
-	ar_iter->ret = ath10k_mac_set_rts(arvif, rts);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set RTS threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_rts_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set rts threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
-static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
+static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 {
-	struct ath10k_generic_iter *ar_iter = data;
-	struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
-	u32 frag = ar_iter->ar->hw->wiphy->frag_threshold;
-
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	struct ath10k *ar = hw->priv;
+	struct ath10k_vif *arvif;
+	int ret = 0;
 
 	/* During HW reconfiguration mac80211 reports all interfaces that were
 	 * running until reconfiguration was started. Since FW doesn't have any
 	 * vdevs at this point we must not iterate over this interface list.
 	 * This setting will be updated upon add_interface(). */
-	if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
-		return;
-
-	ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation_threshold %d\n",
-		   arvif->vdev_id, frag);
-
-	ar_iter->ret = ath10k_mac_set_frag(arvif, frag);
-	if (ar_iter->ret)
-		ath10k_warn("Failed to set frag threshold for VDEV: %d\n",
-			    arvif->vdev_id);
-}
-
-static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
-{
-	struct ath10k_generic_iter ar_iter;
-	struct ath10k *ar = hw->priv;
-
-	memset(&ar_iter, 0, sizeof(struct ath10k_generic_iter));
-	ar_iter.ar = ar;
+	if (ar->state == ATH10K_STATE_RESTARTED)
+		return 0;
 
 	mutex_lock(&ar->conf_mutex);
-	ieee80211_iterate_active_interfaces_atomic(
-		hw, IEEE80211_IFACE_ITER_NORMAL,
-		ath10k_set_frag_iter, &ar_iter);
+	list_for_each_entry(arvif, &ar->arvifs, list) {
+		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
+			   arvif->vdev_id, value);
+
+		ret = ath10k_mac_set_rts(arvif, value);
+		if (ret) {
+			ath10k_warn("could not set fragmentation threshold for vdev %d (%d)\n",
+				    arvif->vdev_id, ret);
+			break;
+		}
+	}
 	mutex_unlock(&ar->conf_mutex);
 
-	return ar_iter.ret;
+	return ret;
 }
 
 static void ath10k_flush(struct ieee80211_hw *hw, u32 queues, bool drop)
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 4/4] ath10k: remove unnecessary checks
  2013-10-04  5:43   ` Michal Kazior
@ 2013-10-04  5:43     ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface iteration functions that were
used originally iterated over interfaces that
weren't re-added to the driver during recovery.

Since internal vif list is now used it's safe to
remove the safe-guard as internal vif list is
based on add/remove_interface function which
guarantees that vdev is created in FW before it is
iterated over.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 583f065..7567db0 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1924,13 +1924,6 @@ static int ath10k_config_ps(struct ath10k *ar)
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ret = ath10k_mac_vif_setup_ps(arvif);
 		if (ret) {
@@ -2840,13 +2833,6 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
@@ -2870,13 +2856,6 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH/RFT v2 4/4] ath10k: remove unnecessary checks
@ 2013-10-04  5:43     ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-04  5:43 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Michal Kazior

mac80211 interface iteration functions that were
used originally iterated over interfaces that
weren't re-added to the driver during recovery.

Since internal vif list is now used it's safe to
remove the safe-guard as internal vif list is
based on add/remove_interface function which
guarantees that vdev is created in FW before it is
iterated over.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c |   21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 583f065..7567db0 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1924,13 +1924,6 @@ static int ath10k_config_ps(struct ath10k *ar)
 
 	lockdep_assert_held(&ar->conf_mutex);
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ret = ath10k_mac_vif_setup_ps(arvif);
 		if (ret) {
@@ -2840,13 +2833,6 @@ static int ath10k_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d rts threshold %d\n",
@@ -2870,13 +2856,6 @@ static int ath10k_set_frag_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath10k_vif *arvif;
 	int ret = 0;
 
-	/* During HW reconfiguration mac80211 reports all interfaces that were
-	 * running until reconfiguration was started. Since FW doesn't have any
-	 * vdevs at this point we must not iterate over this interface list.
-	 * This setting will be updated upon add_interface(). */
-	if (ar->state == ATH10K_STATE_RESTARTED)
-		return 0;
-
 	mutex_lock(&ar->conf_mutex);
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath10k_dbg(ATH10K_DBG_MAC, "mac vdev %d fragmentation threshold %d\n",
-- 
1.7.9.5


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
  2013-10-04  5:43     ` Michal Kazior
@ 2013-10-08 12:54       ` Kalle Valo
  -1 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 12:54 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

Michal Kazior <michal.kazior@tieto.com> writes:

> If something failed along add_interface() setup it
> was possible to leak a vdev id, vdev and peer.
>
> This could end up with leaked FW state or FW crash
> (assuming add_interface() failure wasn't a result of
> a crash).
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Then I apply this patch on top of current ath.git master branch (commit
18fe0b53e76) interface up fails on the managed mode:

$ sudo ip link set wlan1 up
RTNETLINK answers: Invalid argument

Logs don't have anything special (the TX encap error was already
before):

[ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
[ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
[ 1259.820055] ath10k: MSI interrupt handling
[ 1260.747877] ath10k: UART prints disabled
[ 1260.828219] ath10k: firmware 10.1.389 booted
[ 1260.837585] ath10k: htt target version 2.1
[ 1260.838498] ath10k: Failed to set TX encap: -22

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
@ 2013-10-08 12:54       ` Kalle Valo
  0 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 12:54 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> If something failed along add_interface() setup it
> was possible to leak a vdev id, vdev and peer.
>
> This could end up with leaked FW state or FW crash
> (assuming add_interface() failure wasn't a result of
> a crash).
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Then I apply this patch on top of current ath.git master branch (commit
18fe0b53e76) interface up fails on the managed mode:

$ sudo ip link set wlan1 up
RTNETLINK answers: Invalid argument

Logs don't have anything special (the TX encap error was already
before):

[ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
[ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
[ 1259.820055] ath10k: MSI interrupt handling
[ 1260.747877] ath10k: UART prints disabled
[ 1260.828219] ath10k: firmware 10.1.389 booted
[ 1260.837585] ath10k: htt target version 2.1
[ 1260.838498] ath10k: Failed to set TX encap: -22

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
  2013-10-08 12:54       ` Kalle Valo
@ 2013-10-08 16:49         ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-08 16:49 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k, linux-wireless

On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> If something failed along add_interface() setup it
>> was possible to leak a vdev id, vdev and peer.
>>
>> This could end up with leaked FW state or FW crash
>> (assuming add_interface() failure wasn't a result of
>> a crash).
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Then I apply this patch on top of current ath.git master branch (commit
> 18fe0b53e76) interface up fails on the managed mode:
>
> $ sudo ip link set wlan1 up
> RTNETLINK answers: Invalid argument
>
> Logs don't have anything special (the TX encap error was already
> before):
>
> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
> [ 1259.820055] ath10k: MSI interrupt handling
> [ 1260.747877] ath10k: UART prints disabled
> [ 1260.828219] ath10k: firmware 10.1.389 booted
> [ 1260.837585] ath10k: htt target version 2.1
> [ 1260.838498] ath10k: Failed to set TX encap: -22

Ah. Right. I've tested this patch with main FW branch which supports
TX encap command. The TX encap issue should be dealth with first.


Michał

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
@ 2013-10-08 16:49         ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-08 16:49 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k

On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> If something failed along add_interface() setup it
>> was possible to leak a vdev id, vdev and peer.
>>
>> This could end up with leaked FW state or FW crash
>> (assuming add_interface() failure wasn't a result of
>> a crash).
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Then I apply this patch on top of current ath.git master branch (commit
> 18fe0b53e76) interface up fails on the managed mode:
>
> $ sudo ip link set wlan1 up
> RTNETLINK answers: Invalid argument
>
> Logs don't have anything special (the TX encap error was already
> before):
>
> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
> [ 1259.820055] ath10k: MSI interrupt handling
> [ 1260.747877] ath10k: UART prints disabled
> [ 1260.828219] ath10k: firmware 10.1.389 booted
> [ 1260.837585] ath10k: htt target version 2.1
> [ 1260.838498] ath10k: Failed to set TX encap: -22

Ah. Right. I've tested this patch with main FW branch which supports
TX encap command. The TX encap issue should be dealth with first.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
  2013-10-08 16:49         ` Michal Kazior
@ 2013-10-08 19:16           ` Kalle Valo
  -1 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 19:16 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> If something failed along add_interface() setup it
>>> was possible to leak a vdev id, vdev and peer.
>>>
>>> This could end up with leaked FW state or FW crash
>>> (assuming add_interface() failure wasn't a result of
>>> a crash).
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> Then I apply this patch on top of current ath.git master branch (commit
>> 18fe0b53e76) interface up fails on the managed mode:
>>
>> $ sudo ip link set wlan1 up
>> RTNETLINK answers: Invalid argument
>>
>> Logs don't have anything special (the TX encap error was already
>> before):
>>
>> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
>> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
>> [ 1259.820055] ath10k: MSI interrupt handling
>> [ 1260.747877] ath10k: UART prints disabled
>> [ 1260.828219] ath10k: firmware 10.1.389 booted
>> [ 1260.837585] ath10k: htt target version 2.1
>> [ 1260.838498] ath10k: Failed to set TX encap: -22
>
> Ah. Right. I've tested this patch with main FW branch which supports
> TX encap command. The TX encap issue should be dealth with first.

What do you mean? Did I miss a patch?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
@ 2013-10-08 19:16           ` Kalle Valo
  0 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 19:16 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> If something failed along add_interface() setup it
>>> was possible to leak a vdev id, vdev and peer.
>>>
>>> This could end up with leaked FW state or FW crash
>>> (assuming add_interface() failure wasn't a result of
>>> a crash).
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> Then I apply this patch on top of current ath.git master branch (commit
>> 18fe0b53e76) interface up fails on the managed mode:
>>
>> $ sudo ip link set wlan1 up
>> RTNETLINK answers: Invalid argument
>>
>> Logs don't have anything special (the TX encap error was already
>> before):
>>
>> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
>> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
>> [ 1259.820055] ath10k: MSI interrupt handling
>> [ 1260.747877] ath10k: UART prints disabled
>> [ 1260.828219] ath10k: firmware 10.1.389 booted
>> [ 1260.837585] ath10k: htt target version 2.1
>> [ 1260.838498] ath10k: Failed to set TX encap: -22
>
> Ah. Right. I've tested this patch with main FW branch which supports
> TX encap command. The TX encap issue should be dealth with first.

What do you mean? Did I miss a patch?

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 0/4] ath10k: fixes
  2013-10-04  5:43   ` Michal Kazior
@ 2013-10-08 19:17     ` Kalle Valo
  -1 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 19:17 UTC (permalink / raw)
  To: Michal Kazior; +Cc: ath10k, linux-wireless

Michal Kazior <michal.kazior@tieto.com> writes:

> Hi,
>
> This patchset addresses recently spotted issue
> with (yet another) scheduling while atomic bug
> (the other being WEP key index setting). This one
> is related to hw_config() and powersave settings.
> This comes from recent changes I've done to
> HTC/WMI. WMI commands can block now so it's
> illegal to call them in an atomic context anymore.
>
> ath10k needs to setup some settings per-vdev (i.e.
> per-interface) such as powersave, rts, fragmentation.
> Until now mac80211 iteration functions were used.
> However using non-atomic iteration function variant
> doesn't solve the problem as it introduces an
> issue with iflist_mtx deadlock in some cases.
>
> I briefly tried to reproduce the issue Kalle
> reported but was unsuccessful thus the "/RFT".
>
> v2:
>  * fix kbuild test robot warning
>    (uninitialized `ret`)
>  * add patch #4 that removes code that becomes
>    unnecessary after patch #3
>
>
> Michal Kazior (4):
>   ath10k: fix add_interface failure handling
>   ath10k: track vif list internally
>   ath10k: fix scheduling while atomic config bug
>   ath10k: remove unnecessary checks

Due to problems with patch 1 I'm dropping this patchset for now.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 0/4] ath10k: fixes
@ 2013-10-08 19:17     ` Kalle Valo
  0 siblings, 0 replies; 28+ messages in thread
From: Kalle Valo @ 2013-10-08 19:17 UTC (permalink / raw)
  To: Michal Kazior; +Cc: linux-wireless, ath10k

Michal Kazior <michal.kazior@tieto.com> writes:

> Hi,
>
> This patchset addresses recently spotted issue
> with (yet another) scheduling while atomic bug
> (the other being WEP key index setting). This one
> is related to hw_config() and powersave settings.
> This comes from recent changes I've done to
> HTC/WMI. WMI commands can block now so it's
> illegal to call them in an atomic context anymore.
>
> ath10k needs to setup some settings per-vdev (i.e.
> per-interface) such as powersave, rts, fragmentation.
> Until now mac80211 iteration functions were used.
> However using non-atomic iteration function variant
> doesn't solve the problem as it introduces an
> issue with iflist_mtx deadlock in some cases.
>
> I briefly tried to reproduce the issue Kalle
> reported but was unsuccessful thus the "/RFT".
>
> v2:
>  * fix kbuild test robot warning
>    (uninitialized `ret`)
>  * add patch #4 that removes code that becomes
>    unnecessary after patch #3
>
>
> Michal Kazior (4):
>   ath10k: fix add_interface failure handling
>   ath10k: track vif list internally
>   ath10k: fix scheduling while atomic config bug
>   ath10k: remove unnecessary checks

Due to problems with patch 1 I'm dropping this patchset for now.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
  2013-10-08 19:16           ` Kalle Valo
@ 2013-10-09  2:59             ` Michal Kazior
  -1 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-09  2:59 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k

On 8 October 2013 12:16, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>> Michal Kazior <michal.kazior@tieto.com> writes:
>>>
>>>> If something failed along add_interface() setup it
>>>> was possible to leak a vdev id, vdev and peer.
>>>>
>>>> This could end up with leaked FW state or FW crash
>>>> (assuming add_interface() failure wasn't a result of
>>>> a crash).
>>>>
>>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>>
>>> Then I apply this patch on top of current ath.git master branch (commit
>>> 18fe0b53e76) interface up fails on the managed mode:
>>>
>>> $ sudo ip link set wlan1 up
>>> RTNETLINK answers: Invalid argument
>>>
>>> Logs don't have anything special (the TX encap error was already
>>> before):
>>>
>>> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
>>> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
>>> [ 1259.820055] ath10k: MSI interrupt handling
>>> [ 1260.747877] ath10k: UART prints disabled
>>> [ 1260.828219] ath10k: firmware 10.1.389 booted
>>> [ 1260.837585] ath10k: htt target version 2.1
>>> [ 1260.838498] ath10k: Failed to set TX encap: -22
>>
>> Ah. Right. I've tested this patch with main FW branch which supports
>> TX encap command. The TX encap issue should be dealth with first.
>
> What do you mean? Did I miss a patch?

You didn't miss any patch.

I think you've already pointed out the TX encap warning in your other email.

It's the TX encap that most likely gets you "invalid argument" when
trying to bring up the interface with my patch as FW 10.1.389 doesn't
support the command.


Michał

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling
@ 2013-10-09  2:59             ` Michal Kazior
  0 siblings, 0 replies; 28+ messages in thread
From: Michal Kazior @ 2013-10-09  2:59 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, ath10k

On 8 October 2013 12:16, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 8 October 2013 05:54, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>> Michal Kazior <michal.kazior@tieto.com> writes:
>>>
>>>> If something failed along add_interface() setup it
>>>> was possible to leak a vdev id, vdev and peer.
>>>>
>>>> This could end up with leaked FW state or FW crash
>>>> (assuming add_interface() failure wasn't a result of
>>>> a crash).
>>>>
>>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>>
>>> Then I apply this patch on top of current ath.git master branch (commit
>>> 18fe0b53e76) interface up fails on the managed mode:
>>>
>>> $ sudo ip link set wlan1 up
>>> RTNETLINK answers: Invalid argument
>>>
>>> Logs don't have anything special (the TX encap error was already
>>> before):
>>>
>>> [ 1259.818863] ath10k: MSI-X didn't succeed (1), trying MSI
>>> [ 1259.819298] ath10k_pci 0000:02:00.0: irq 49 for MSI/MSI-X
>>> [ 1259.820055] ath10k: MSI interrupt handling
>>> [ 1260.747877] ath10k: UART prints disabled
>>> [ 1260.828219] ath10k: firmware 10.1.389 booted
>>> [ 1260.837585] ath10k: htt target version 2.1
>>> [ 1260.838498] ath10k: Failed to set TX encap: -22
>>
>> Ah. Right. I've tested this patch with main FW branch which supports
>> TX encap command. The TX encap issue should be dealth with first.
>
> What do you mean? Did I miss a patch?

You didn't miss any patch.

I think you've already pointed out the TX encap warning in your other email.

It's the TX encap that most likely gets you "invalid argument" when
trying to bring up the interface with my patch as FW 10.1.389 doesn't
support the command.


Michał

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 28+ messages in thread
end of thread, other threads:[~2013-10-09  2:59 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-03 13:09 [PATCH/RFT 0/3] ath10k: fixes Michal Kazior
2013-10-03 13:09 ` Michal Kazior
2013-10-03 13:09 ` [PATCH/RFT 1/3] ath10k: fix add_interface failure handling Michal Kazior
2013-10-03 13:09   ` Michal Kazior
2013-10-03 13:09 ` [PATCH/RFT 2/3] ath10k: track vif list internally Michal Kazior
2013-10-03 13:09   ` Michal Kazior
2013-10-03 13:09 ` [PATCH/RFT 3/3] ath10k: fix scheduling while atomic config bug Michal Kazior
2013-10-03 13:09   ` Michal Kazior
2013-10-04  5:43 ` [PATCH/RFT v2 0/4] ath10k: fixes Michal Kazior
2013-10-04  5:43   ` Michal Kazior
2013-10-04  5:43   ` [PATCH/RFT v2 1/4] ath10k: fix add_interface failure handling Michal Kazior
2013-10-04  5:43     ` Michal Kazior
2013-10-08 12:54     ` Kalle Valo
2013-10-08 12:54       ` Kalle Valo
2013-10-08 16:49       ` Michal Kazior
2013-10-08 16:49         ` Michal Kazior
2013-10-08 19:16         ` Kalle Valo
2013-10-08 19:16           ` Kalle Valo
2013-10-09  2:59           ` Michal Kazior
2013-10-09  2:59             ` Michal Kazior
2013-10-04  5:43   ` [PATCH/RFT v2 2/4] ath10k: track vif list internally Michal Kazior
2013-10-04  5:43     ` Michal Kazior
2013-10-04  5:43   ` [PATCH/RFT v2 3/4] ath10k: fix scheduling while atomic config bug Michal Kazior
2013-10-04  5:43     ` Michal Kazior
2013-10-04  5:43   ` [PATCH/RFT v2 4/4] ath10k: remove unnecessary checks Michal Kazior
2013-10-04  5:43     ` Michal Kazior
2013-10-08 19:17   ` [PATCH/RFT v2 0/4] ath10k: fixes Kalle Valo
2013-10-08 19:17     ` Kalle Valo

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.