The command logic shouldn't really care about
arvif structure.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/wmi-ops.h | 14 ++++++++++----
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 20 +++++++++++---------
drivers/net/wireless/ath/ath10k/wmi.c | 27 +++++++++++++++++----------
3 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 0dd49a7..129cec4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -102,7 +102,10 @@ struct wmi_ops {
u32 value);
struct sk_buff *(*gen_scan_chan_list)(struct ath10k *ar,
const struct wmi_scan_chan_list_arg *arg);
- struct sk_buff *(*gen_beacon_dma)(struct ath10k_vif *arvif);
+ struct sk_buff *(*gen_beacon_dma)(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab);
struct sk_buff *(*gen_pdev_set_wmm)(struct ath10k *ar,
const struct wmi_pdev_set_wmm_params_arg *arg);
struct sk_buff *(*gen_request_stats)(struct ath10k *ar,
@@ -724,16 +727,19 @@ ath10k_wmi_peer_assoc(struct ath10k *ar,
}
static inline int
-ath10k_wmi_beacon_send_ref_nowait(struct ath10k_vif *arvif)
+ath10k_wmi_beacon_send_ref_nowait(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
{
- struct ath10k *ar = arvif->ar;
struct sk_buff *skb;
int ret;
if (!ar->wmi.ops->gen_beacon_dma)
return -EOPNOTSUPP;
- skb = ar->wmi.ops->gen_beacon_dma(arvif);
+ skb = ar->wmi.ops->gen_beacon_dma(ar, vdev_id, bcn, bcn_len, bcn_paddr,
+ dtim_zero, deliver_cab);
if (IS_ERR(skb))
return PTR_ERR(skb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 6f34fc7..f44bbf0 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1728,13 +1728,15 @@ ath10k_wmi_tlv_op_gen_scan_chan_list(struct ath10k *ar,
}
static struct sk_buff *
-ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k_vif *arvif)
+ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
+
{
- struct ath10k *ar = arvif->ar;
struct wmi_bcn_tx_ref_cmd *cmd;
struct wmi_tlv *tlv;
struct sk_buff *skb;
- struct sk_buff *beacon = arvif->beacon;
struct ieee80211_hdr *hdr;
u16 fc;
@@ -1742,24 +1744,24 @@ ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k_vif *arvif)
if (!skb)
return ERR_PTR(-ENOMEM);
- hdr = (struct ieee80211_hdr *)beacon->data;
+ hdr = (struct ieee80211_hdr *)bcn;
fc = le16_to_cpu(hdr->frame_control);
tlv = (void *)skb->data;
tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_BCN_SEND_FROM_HOST_CMD);
tlv->len = __cpu_to_le16(sizeof(*cmd));
cmd = (void *)tlv->value;
- cmd->vdev_id = __cpu_to_le32(arvif->vdev_id);
- cmd->data_len = __cpu_to_le32(beacon->len);
- cmd->data_ptr = __cpu_to_le32(ATH10K_SKB_CB(beacon)->paddr);
+ cmd->vdev_id = __cpu_to_le32(vdev_id);
+ cmd->data_len = __cpu_to_le32(bcn_len);
+ cmd->data_ptr = __cpu_to_le32(bcn_paddr);
cmd->msdu_id = 0;
cmd->frame_control = __cpu_to_le32(fc);
cmd->flags = 0;
- if (ATH10K_SKB_CB(beacon)->bcn.dtim_zero)
+ if (dtim_zero)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DTIM_ZERO);
- if (ATH10K_SKB_CB(beacon)->bcn.deliver_cab)
+ if (deliver_cab)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DELIVER_CAB);
ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv beacon dma\n");
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 5fe17e8..5cc611f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -956,6 +956,8 @@ err_pull:
static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
{
+ struct sk_buff *bcn;
+ struct ath10k_skb_cb *cb;
int ret;
lockdep_assert_held(&arvif->ar->data_lock);
@@ -966,7 +968,12 @@ static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
if (arvif->beacon_sent)
return;
- ret = ath10k_wmi_beacon_send_ref_nowait(arvif);
+ bcn = arvif->beacon;
+ cb = ATH10K_SKB_CB(bcn);
+ ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar, arvif->vdev_id,
+ bcn->data, bcn->len, cb->paddr,
+ cb->bcn.dtim_zero,
+ cb->bcn.deliver_cab);
if (ret)
return;
@@ -4683,12 +4690,12 @@ ath10k_wmi_10_2_op_gen_pdev_get_temperature(struct ath10k *ar)
/* This function assumes the beacon is already DMA mapped */
static struct sk_buff *
-ath10k_wmi_op_gen_beacon_dma(struct ath10k_vif *arvif)
+ath10k_wmi_op_gen_beacon_dma(struct ath10k *ar, u32 vdev_id, const void *bcn,
+ size_t bcn_len, u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
{
- struct ath10k *ar = arvif->ar;
struct wmi_bcn_tx_ref_cmd *cmd;
struct sk_buff *skb;
- struct sk_buff *beacon = arvif->beacon;
struct ieee80211_hdr *hdr;
u16 fc;
@@ -4696,22 +4703,22 @@ ath10k_wmi_op_gen_beacon_dma(struct ath10k_vif *arvif)
if (!skb)
return ERR_PTR(-ENOMEM);
- hdr = (struct ieee80211_hdr *)beacon->data;
+ hdr = (struct ieee80211_hdr *)bcn;
fc = le16_to_cpu(hdr->frame_control);
cmd = (struct wmi_bcn_tx_ref_cmd *)skb->data;
- cmd->vdev_id = __cpu_to_le32(arvif->vdev_id);
- cmd->data_len = __cpu_to_le32(beacon->len);
- cmd->data_ptr = __cpu_to_le32(ATH10K_SKB_CB(beacon)->paddr);
+ cmd->vdev_id = __cpu_to_le32(vdev_id);
+ cmd->data_len = __cpu_to_le32(bcn_len);
+ cmd->data_ptr = __cpu_to_le32(bcn_paddr);
cmd->msdu_id = 0;
cmd->frame_control = __cpu_to_le32(fc);
cmd->flags = 0;
cmd->antenna_mask = __cpu_to_le32(WMI_BCN_TX_REF_DEF_ANTENNA);
- if (ATH10K_SKB_CB(beacon)->bcn.dtim_zero)
+ if (dtim_zero)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DTIM_ZERO);
- if (ATH10K_SKB_CB(beacon)->bcn.deliver_cab)
+ if (deliver_cab)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DELIVER_CAB);
return skb;
--
1.8.5.3
This should fix a very rare occurrence of the following deadlock:
[<ffffffffa018265e>] ath10k_wmi_tx_beacons_nowait+0x1e/0x50 [ath10k_core]
[<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
[<ffffffffa017d685>] ath10k_htc_send+0x285/0x3d0 [ath10k_core]
[<ffffffffa0184b81>] ath10k_wmi_cmd_send_nowait+0x81/0x110 [ath10k_core]
[<ffffffffa0184c61>] ath10k_wmi_tx_beacon_nowait.part.33+0x51/0x90 [ath10k_core]
[<ffffffffa0184cd0>] ath10k_wmi_tx_beacons_iter+0x30/0x40 [ath10k_core]
[<ffffffff81882246>] __iterate_active_interfaces+0xa6/0x100
[<ffffffffa0184ca0>] ? ath10k_wmi_tx_beacon_nowait.part.33+0x90/0x90 [ath10k_core]
[<ffffffff818822ae>] ieee80211_iterate_active_interfaces_atomic+0xe/0x10
[<ffffffffa0182676>] ath10k_wmi_tx_beacons_nowait+0x36/0x50 [ath10k_core]
[<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
[<ffffffffa017d140>] ath10k_htc_rx+0x280/0x410 [ath10k_core]
[<ffffffffa01bcbf0>] ? ath10k_ce_completed_recv_next+0x60/0x80 [ath10k_pci]
[<ffffffffa01bc6ab>] ath10k_pci_ce_recv_data+0x11b/0x1d0 [ath10k_pci]
[<ffffffffa01bcf44>] ath10k_ce_per_engine_service+0x64/0xc0 [ath10k_pci]
[<ffffffffa01bcfc2>] ath10k_ce_per_engine_service_any+0x22/0x50 [ath10k_pci]
[<ffffffffa01bc4d0>] ath10k_pci_tasklet+0x30/0x90 [ath10k_pci]
[<ffffffff81055a55>] tasklet_action+0xc5/0x100
To prevent this make sure to release ar->data_lock
while calling to ath10k_wmi_beacon_send_ref_nowait().
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 8 +++-
drivers/net/wireless/ath/ath10k/mac.c | 6 ++-
drivers/net/wireless/ath/ath10k/wmi.c | 68 +++++++++++++++++++++++-----------
3 files changed, 58 insertions(+), 24 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 2d9f871..67e6b46 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -262,6 +262,12 @@ struct ath10k_sta {
#define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
+enum ath10k_beacon_state {
+ ATH10K_BEACON_SCHEDULED = 0,
+ ATH10K_BEACON_SENDING,
+ ATH10K_BEACON_SENT,
+};
+
struct ath10k_vif {
struct list_head list;
@@ -272,7 +278,7 @@ struct ath10k_vif {
u32 dtim_period;
struct sk_buff *beacon;
/* protected by data_lock */
- bool beacon_sent;
+ enum ath10k_beacon_state beacon_state;
void *beacon_buf;
dma_addr_t beacon_paddr;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 4ff6dd0..2a51f0e 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -521,10 +521,14 @@ void ath10k_mac_vif_beacon_free(struct ath10k_vif *arvif)
dma_unmap_single(ar->dev, ATH10K_SKB_CB(arvif->beacon)->paddr,
arvif->beacon->len, DMA_TO_DEVICE);
+ if (WARN_ON(arvif->beacon_state != ATH10K_BEACON_SCHEDULED &&
+ arvif->beacon_state != ATH10K_BEACON_SENT))
+ return;
+
dev_kfree_skb_any(arvif->beacon);
arvif->beacon = NULL;
- arvif->beacon_sent = false;
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
}
static void ath10k_mac_vif_beacon_cleanup(struct ath10k_vif *arvif)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 459b203..a398193 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -956,30 +956,45 @@ err_pull:
static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
{
- struct sk_buff *bcn;
+ struct ath10k *ar = arvif->ar;
struct ath10k_skb_cb *cb;
+ struct sk_buff *bcn;
int ret;
- lockdep_assert_held(&arvif->ar->data_lock);
+ spin_lock_bh(&ar->data_lock);
- if (arvif->beacon == NULL)
- return;
+ bcn = arvif->beacon;
- if (arvif->beacon_sent)
- return;
+ if (!bcn)
+ goto unlock;
- bcn = arvif->beacon;
cb = ATH10K_SKB_CB(bcn);
- ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar, arvif->vdev_id,
- bcn->data, bcn->len, cb->paddr,
- cb->bcn.dtim_zero,
- cb->bcn.deliver_cab);
- if (ret)
- return;
- /* We need to retain the arvif->beacon reference for DMA unmapping and
- * freeing the skbuff later. */
- arvif->beacon_sent = true;
+ switch (arvif->beacon_state) {
+ case ATH10K_BEACON_SENDING:
+ case ATH10K_BEACON_SENT:
+ break;
+ case ATH10K_BEACON_SCHEDULED:
+ arvif->beacon_state = ATH10K_BEACON_SENDING;
+ spin_unlock_bh(&ar->data_lock);
+
+ ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar,
+ arvif->vdev_id,
+ bcn->data, bcn->len,
+ cb->paddr,
+ cb->bcn.dtim_zero,
+ cb->bcn.deliver_cab);
+
+ spin_lock_bh(&ar->data_lock);
+
+ if (ret == 0)
+ arvif->beacon_state = ATH10K_BEACON_SENT;
+ else
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
+ }
+
+unlock:
+ spin_unlock_bh(&ar->data_lock);
}
static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
@@ -992,12 +1007,10 @@ static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
static void ath10k_wmi_tx_beacons_nowait(struct ath10k *ar)
{
- spin_lock_bh(&ar->data_lock);
ieee80211_iterate_active_interfaces_atomic(ar->hw,
IEEE80211_IFACE_ITER_NORMAL,
ath10k_wmi_tx_beacons_iter,
NULL);
- spin_unlock_bh(&ar->data_lock);
}
static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
@@ -2459,9 +2472,19 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
spin_lock_bh(&ar->data_lock);
if (arvif->beacon) {
- if (!arvif->beacon_sent)
- ath10k_warn(ar, "SWBA overrun on vdev %d\n",
+ switch (arvif->beacon_state) {
+ case ATH10K_BEACON_SENT:
+ break;
+ case ATH10K_BEACON_SCHEDULED:
+ ath10k_warn(ar, "SWBA overrun on vdev %d, skipped old beacon\n",
+ arvif->vdev_id);
+ break;
+ case ATH10K_BEACON_SENDING:
+ ath10k_warn(ar, "SWBA overrun on vdev %d, skipped new beacon\n",
arvif->vdev_id);
+ dev_kfree_skb(bcn);
+ goto skip;
+ }
ath10k_mac_vif_beacon_free(arvif);
}
@@ -2489,15 +2512,16 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
}
arvif->beacon = bcn;
- arvif->beacon_sent = false;
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
trace_ath10k_tx_hdr(ar, bcn->data, bcn->len);
trace_ath10k_tx_payload(ar, bcn->data, bcn->len);
- ath10k_wmi_tx_beacon_nowait(arvif);
skip:
spin_unlock_bh(&ar->data_lock);
}
+
+ ath10k_wmi_tx_beacons_nowait(ar);
}
void ath10k_wmi_event_tbttoffset_update(struct ath10k *ar, struct sk_buff *skb)
--
1.8.5.3
The command logic shouldn't really care about
arvif structure.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/wmi-ops.h | 14 ++++++++++----
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 20 +++++++++++---------
drivers/net/wireless/ath/ath10k/wmi.c | 27 +++++++++++++++++----------
3 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 80bd28a..06062cf 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -102,7 +102,10 @@ struct wmi_ops {
u32 value);
struct sk_buff *(*gen_scan_chan_list)(struct ath10k *ar,
const struct wmi_scan_chan_list_arg *arg);
- struct sk_buff *(*gen_beacon_dma)(struct ath10k_vif *arvif);
+ struct sk_buff *(*gen_beacon_dma)(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab);
struct sk_buff *(*gen_pdev_set_wmm)(struct ath10k *ar,
const struct wmi_pdev_set_wmm_params_arg *arg);
struct sk_buff *(*gen_request_stats)(struct ath10k *ar,
@@ -749,16 +752,19 @@ ath10k_wmi_peer_assoc(struct ath10k *ar,
}
static inline int
-ath10k_wmi_beacon_send_ref_nowait(struct ath10k_vif *arvif)
+ath10k_wmi_beacon_send_ref_nowait(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
{
- struct ath10k *ar = arvif->ar;
struct sk_buff *skb;
int ret;
if (!ar->wmi.ops->gen_beacon_dma)
return -EOPNOTSUPP;
- skb = ar->wmi.ops->gen_beacon_dma(arvif);
+ skb = ar->wmi.ops->gen_beacon_dma(ar, vdev_id, bcn, bcn_len, bcn_paddr,
+ dtim_zero, deliver_cab);
if (IS_ERR(skb))
return PTR_ERR(skb);
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index d3cf91dc..c3d077f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1906,13 +1906,15 @@ ath10k_wmi_tlv_op_gen_scan_chan_list(struct ath10k *ar,
}
static struct sk_buff *
-ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k_vif *arvif)
+ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k *ar, u32 vdev_id,
+ const void *bcn, size_t bcn_len,
+ u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
+
{
- struct ath10k *ar = arvif->ar;
struct wmi_bcn_tx_ref_cmd *cmd;
struct wmi_tlv *tlv;
struct sk_buff *skb;
- struct sk_buff *beacon = arvif->beacon;
struct ieee80211_hdr *hdr;
u16 fc;
@@ -1920,24 +1922,24 @@ ath10k_wmi_tlv_op_gen_beacon_dma(struct ath10k_vif *arvif)
if (!skb)
return ERR_PTR(-ENOMEM);
- hdr = (struct ieee80211_hdr *)beacon->data;
+ hdr = (struct ieee80211_hdr *)bcn;
fc = le16_to_cpu(hdr->frame_control);
tlv = (void *)skb->data;
tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_BCN_SEND_FROM_HOST_CMD);
tlv->len = __cpu_to_le16(sizeof(*cmd));
cmd = (void *)tlv->value;
- cmd->vdev_id = __cpu_to_le32(arvif->vdev_id);
- cmd->data_len = __cpu_to_le32(beacon->len);
- cmd->data_ptr = __cpu_to_le32(ATH10K_SKB_CB(beacon)->paddr);
+ cmd->vdev_id = __cpu_to_le32(vdev_id);
+ cmd->data_len = __cpu_to_le32(bcn_len);
+ cmd->data_ptr = __cpu_to_le32(bcn_paddr);
cmd->msdu_id = 0;
cmd->frame_control = __cpu_to_le32(fc);
cmd->flags = 0;
- if (ATH10K_SKB_CB(beacon)->bcn.dtim_zero)
+ if (dtim_zero)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DTIM_ZERO);
- if (ATH10K_SKB_CB(beacon)->bcn.deliver_cab)
+ if (deliver_cab)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DELIVER_CAB);
ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi tlv beacon dma\n");
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index d340361..459b203 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -956,6 +956,8 @@ err_pull:
static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
{
+ struct sk_buff *bcn;
+ struct ath10k_skb_cb *cb;
int ret;
lockdep_assert_held(&arvif->ar->data_lock);
@@ -966,7 +968,12 @@ static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
if (arvif->beacon_sent)
return;
- ret = ath10k_wmi_beacon_send_ref_nowait(arvif);
+ bcn = arvif->beacon;
+ cb = ATH10K_SKB_CB(bcn);
+ ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar, arvif->vdev_id,
+ bcn->data, bcn->len, cb->paddr,
+ cb->bcn.dtim_zero,
+ cb->bcn.deliver_cab);
if (ret)
return;
@@ -4856,12 +4863,12 @@ ath10k_wmi_10_2_op_gen_pdev_get_temperature(struct ath10k *ar)
/* This function assumes the beacon is already DMA mapped */
static struct sk_buff *
-ath10k_wmi_op_gen_beacon_dma(struct ath10k_vif *arvif)
+ath10k_wmi_op_gen_beacon_dma(struct ath10k *ar, u32 vdev_id, const void *bcn,
+ size_t bcn_len, u32 bcn_paddr, bool dtim_zero,
+ bool deliver_cab)
{
- struct ath10k *ar = arvif->ar;
struct wmi_bcn_tx_ref_cmd *cmd;
struct sk_buff *skb;
- struct sk_buff *beacon = arvif->beacon;
struct ieee80211_hdr *hdr;
u16 fc;
@@ -4869,22 +4876,22 @@ ath10k_wmi_op_gen_beacon_dma(struct ath10k_vif *arvif)
if (!skb)
return ERR_PTR(-ENOMEM);
- hdr = (struct ieee80211_hdr *)beacon->data;
+ hdr = (struct ieee80211_hdr *)bcn;
fc = le16_to_cpu(hdr->frame_control);
cmd = (struct wmi_bcn_tx_ref_cmd *)skb->data;
- cmd->vdev_id = __cpu_to_le32(arvif->vdev_id);
- cmd->data_len = __cpu_to_le32(beacon->len);
- cmd->data_ptr = __cpu_to_le32(ATH10K_SKB_CB(beacon)->paddr);
+ cmd->vdev_id = __cpu_to_le32(vdev_id);
+ cmd->data_len = __cpu_to_le32(bcn_len);
+ cmd->data_ptr = __cpu_to_le32(bcn_paddr);
cmd->msdu_id = 0;
cmd->frame_control = __cpu_to_le32(fc);
cmd->flags = 0;
cmd->antenna_mask = __cpu_to_le32(WMI_BCN_TX_REF_DEF_ANTENNA);
- if (ATH10K_SKB_CB(beacon)->bcn.dtim_zero)
+ if (dtim_zero)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DTIM_ZERO);
- if (ATH10K_SKB_CB(beacon)->bcn.deliver_cab)
+ if (deliver_cab)
cmd->flags |= __cpu_to_le32(WMI_BCN_TX_REF_FLAG_DELIVER_CAB);
return skb;
--
1.8.5.3
This should fix a very rare occurrence of the following deadlock:
[<ffffffffa018265e>] ath10k_wmi_tx_beacons_nowait+0x1e/0x50 [ath10k_core]
[<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
[<ffffffffa017d685>] ath10k_htc_send+0x285/0x3d0 [ath10k_core]
[<ffffffffa0184b81>] ath10k_wmi_cmd_send_nowait+0x81/0x110 [ath10k_core]
[<ffffffffa0184c61>] ath10k_wmi_tx_beacon_nowait.part.33+0x51/0x90 [ath10k_core]
[<ffffffffa0184cd0>] ath10k_wmi_tx_beacons_iter+0x30/0x40 [ath10k_core]
[<ffffffff81882246>] __iterate_active_interfaces+0xa6/0x100
[<ffffffffa0184ca0>] ? ath10k_wmi_tx_beacon_nowait.part.33+0x90/0x90 [ath10k_core]
[<ffffffff818822ae>] ieee80211_iterate_active_interfaces_atomic+0xe/0x10
[<ffffffffa0182676>] ath10k_wmi_tx_beacons_nowait+0x36/0x50 [ath10k_core]
[<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
[<ffffffffa017d140>] ath10k_htc_rx+0x280/0x410 [ath10k_core]
[<ffffffffa01bcbf0>] ? ath10k_ce_completed_recv_next+0x60/0x80 [ath10k_pci]
[<ffffffffa01bc6ab>] ath10k_pci_ce_recv_data+0x11b/0x1d0 [ath10k_pci]
[<ffffffffa01bcf44>] ath10k_ce_per_engine_service+0x64/0xc0 [ath10k_pci]
[<ffffffffa01bcfc2>] ath10k_ce_per_engine_service_any+0x22/0x50 [ath10k_pci]
[<ffffffffa01bc4d0>] ath10k_pci_tasklet+0x30/0x90 [ath10k_pci]
[<ffffffff81055a55>] tasklet_action+0xc5/0x100
To prevent this make sure to release ar->data_lock
while calling to ath10k_wmi_beacon_send_ref_nowait().
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/core.h | 8 +++-
drivers/net/wireless/ath/ath10k/mac.c | 6 ++-
drivers/net/wireless/ath/ath10k/wmi.c | 69 ++++++++++++++++++++++------------
3 files changed, 58 insertions(+), 25 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index c5686122..a338b7b 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -248,6 +248,12 @@ struct ath10k_sta {
#define ATH10K_VDEV_SETUP_TIMEOUT_HZ (5*HZ)
+enum ath10k_beacon_state {
+ ATH10K_BEACON_SCHEDULED = 0,
+ ATH10K_BEACON_SENDING,
+ ATH10K_BEACON_SENT,
+};
+
struct ath10k_vif {
struct list_head list;
@@ -258,7 +264,7 @@ struct ath10k_vif {
u32 dtim_period;
struct sk_buff *beacon;
/* protected by data_lock */
- bool beacon_sent;
+ enum ath10k_beacon_state beacon_state;
void *beacon_buf;
dma_addr_t beacon_paddr;
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9524bc5..ef0c0c4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -524,10 +524,14 @@ void ath10k_mac_vif_beacon_free(struct ath10k_vif *arvif)
dma_unmap_single(ar->dev, ATH10K_SKB_CB(arvif->beacon)->paddr,
arvif->beacon->len, DMA_TO_DEVICE);
+ if (WARN_ON(arvif->beacon_state != ATH10K_BEACON_SCHEDULED &&
+ arvif->beacon_state != ATH10K_BEACON_SENT))
+ return;
+
dev_kfree_skb_any(arvif->beacon);
arvif->beacon = NULL;
- arvif->beacon_sent = false;
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
}
static void ath10k_mac_vif_beacon_cleanup(struct ath10k_vif *arvif)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 5cc611f..530ffef 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -956,30 +956,44 @@ err_pull:
static void ath10k_wmi_tx_beacon_nowait(struct ath10k_vif *arvif)
{
- struct sk_buff *bcn;
+ struct ath10k *ar = arvif->ar;
struct ath10k_skb_cb *cb;
+ struct sk_buff *bcn;
int ret;
- lockdep_assert_held(&arvif->ar->data_lock);
-
- if (arvif->beacon == NULL)
- return;
-
- if (arvif->beacon_sent)
- return;
+ spin_lock_bh(&ar->data_lock);
bcn = arvif->beacon;
cb = ATH10K_SKB_CB(bcn);
- ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar, arvif->vdev_id,
- bcn->data, bcn->len, cb->paddr,
- cb->bcn.dtim_zero,
- cb->bcn.deliver_cab);
- if (ret)
- return;
- /* We need to retain the arvif->beacon reference for DMA unmapping and
- * freeing the skbuff later. */
- arvif->beacon_sent = true;
+ if (!bcn)
+ goto unlock;
+
+ switch (arvif->beacon_state) {
+ case ATH10K_BEACON_SENDING:
+ case ATH10K_BEACON_SENT:
+ break;
+ case ATH10K_BEACON_SCHEDULED:
+ arvif->beacon_state = ATH10K_BEACON_SENDING;
+ spin_unlock_bh(&ar->data_lock);
+
+ ret = ath10k_wmi_beacon_send_ref_nowait(arvif->ar,
+ arvif->vdev_id,
+ bcn->data, bcn->len,
+ cb->paddr,
+ cb->bcn.dtim_zero,
+ cb->bcn.deliver_cab);
+
+ spin_lock_bh(&ar->data_lock);
+
+ if (ret == 0)
+ arvif->beacon_state = ATH10K_BEACON_SENT;
+ else
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
+ }
+
+unlock:
+ spin_unlock_bh(&ar->data_lock);
}
static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
@@ -992,12 +1006,10 @@ static void ath10k_wmi_tx_beacons_iter(void *data, u8 *mac,
static void ath10k_wmi_tx_beacons_nowait(struct ath10k *ar)
{
- spin_lock_bh(&ar->data_lock);
ieee80211_iterate_active_interfaces_atomic(ar->hw,
IEEE80211_IFACE_ITER_NORMAL,
ath10k_wmi_tx_beacons_iter,
NULL);
- spin_unlock_bh(&ar->data_lock);
}
static void ath10k_wmi_op_ep_tx_credits(struct ath10k *ar)
@@ -2286,9 +2298,19 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
spin_lock_bh(&ar->data_lock);
if (arvif->beacon) {
- if (!arvif->beacon_sent)
- ath10k_warn(ar, "SWBA overrun on vdev %d\n",
+ switch (arvif->beacon_state) {
+ case ATH10K_BEACON_SENT:
+ break;
+ case ATH10K_BEACON_SCHEDULED:
+ ath10k_warn(ar, "SWBA overrun on vdev %d, skipped old beacon\n",
+ arvif->vdev_id);
+ break;
+ case ATH10K_BEACON_SENDING:
+ ath10k_warn(ar, "SWBA overrun on vdev %d, skipped new beacon\n",
arvif->vdev_id);
+ dev_kfree_skb(bcn);
+ goto skip;
+ }
ath10k_mac_vif_beacon_free(arvif);
}
@@ -2316,15 +2338,16 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
}
arvif->beacon = bcn;
- arvif->beacon_sent = false;
+ arvif->beacon_state = ATH10K_BEACON_SCHEDULED;
trace_ath10k_tx_hdr(ar, bcn->data, bcn->len);
trace_ath10k_tx_payload(ar, bcn->data, bcn->len);
- ath10k_wmi_tx_beacon_nowait(arvif);
skip:
spin_unlock_bh(&ar->data_lock);
}
+
+ ath10k_wmi_tx_beacons_nowait(ar);
}
void ath10k_wmi_event_tbttoffset_update(struct ath10k *ar, struct sk_buff *skb)
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> This should fix a very rare occurrence of the following deadlock:
>
> [<ffffffffa018265e>] ath10k_wmi_tx_beacons_nowait+0x1e/0x50 [ath10k_core]
> [<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
> [<ffffffffa017d685>] ath10k_htc_send+0x285/0x3d0 [ath10k_core]
> [<ffffffffa0184b81>] ath10k_wmi_cmd_send_nowait+0x81/0x110 [ath10k_core]
> [<ffffffffa0184c61>] ath10k_wmi_tx_beacon_nowait.part.33+0x51/0x90 [ath10k_core]
> [<ffffffffa0184cd0>] ath10k_wmi_tx_beacons_iter+0x30/0x40 [ath10k_core]
> [<ffffffff81882246>] __iterate_active_interfaces+0xa6/0x100
> [<ffffffffa0184ca0>] ? ath10k_wmi_tx_beacon_nowait.part.33+0x90/0x90 [ath10k_core]
> [<ffffffff818822ae>] ieee80211_iterate_active_interfaces_atomic+0xe/0x10
> [<ffffffffa0182676>] ath10k_wmi_tx_beacons_nowait+0x36/0x50 [ath10k_core]
> [<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
> [<ffffffffa017d140>] ath10k_htc_rx+0x280/0x410 [ath10k_core]
> [<ffffffffa01bcbf0>] ? ath10k_ce_completed_recv_next+0x60/0x80 [ath10k_pci]
> [<ffffffffa01bc6ab>] ath10k_pci_ce_recv_data+0x11b/0x1d0 [ath10k_pci]
> [<ffffffffa01bcf44>] ath10k_ce_per_engine_service+0x64/0xc0 [ath10k_pci]
> [<ffffffffa01bcfc2>] ath10k_ce_per_engine_service_any+0x22/0x50 [ath10k_pci]
> [<ffffffffa01bc4d0>] ath10k_pci_tasklet+0x30/0x90 [ath10k_pci]
> [<ffffffff81055a55>] tasklet_action+0xc5/0x100
>
> To prevent this make sure to release ar->data_lock
> while calling to ath10k_wmi_beacon_send_ref_nowait().
>
> Signed-off-by: Michal Kazior <[email protected]>
This introduces a new warning from kbuild:
tree: git://github.com/kvalo/ath ath-next-test
head: 59030a70b61c41268383d1406bb49fc559e0da4e
commit: 60845cf1bc104bbec7509eb4b6c9f09a1559bfdc [104/113] ath10k: fix beacon deadlock
New smatch warnings:
drivers/net/wireless/ath/ath10k/wmi.c:969 ath10k_wmi_tx_beacon_nowait() warn: variable dereferenced before check 'bcn' (see line 967)
Old smatch warnings:
drivers/net/wireless/ath/ath10k/wmi.c:3982 ath10k_wmi_start_scan_verify() warn: this array is probably non-NULL. 'arg->ie'
drivers/net/wireless/ath/ath10k/wmi.c:3984 ath10k_wmi_start_scan_verify() warn: this array is probably non-NULL. 'arg->channels'
drivers/net/wireless/ath/ath10k/wmi.c:3986 ath10k_wmi_start_scan_verify() warn: this array is probably non-NULL. 'arg->ssids'
drivers/net/wireless/ath/ath10k/wmi.c:3988 ath10k_wmi_start_scan_verify() warn: this array is probably non-NULL. 'arg->bssids'
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> The command logic shouldn't really care about
> arvif structure.
>
> Signed-off-by: Michal Kazior <[email protected]>
There was a conflict but 3-way merge automatically solved. Please check
the result any in the pending branch.
--
Kalle Valo
On 24 January 2015 at 17:24, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> This should fix a very rare occurrence of the following deadlock:
>>
>> [<ffffffffa018265e>] ath10k_wmi_tx_beacons_nowait+0x1e/0x50 [ath10k_core]
>> [<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
>> [<ffffffffa017d685>] ath10k_htc_send+0x285/0x3d0 [ath10k_core]
>> [<ffffffffa0184b81>] ath10k_wmi_cmd_send_nowait+0x81/0x110 [ath10k_core]
>> [<ffffffffa0184c61>] ath10k_wmi_tx_beacon_nowait.part.33+0x51/0x90 [ath10k_core]
>> [<ffffffffa0184cd0>] ath10k_wmi_tx_beacons_iter+0x30/0x40 [ath10k_core]
>> [<ffffffff81882246>] __iterate_active_interfaces+0xa6/0x100
>> [<ffffffffa0184ca0>] ? ath10k_wmi_tx_beacon_nowait.part.33+0x90/0x90 [ath10k_core]
>> [<ffffffff818822ae>] ieee80211_iterate_active_interfaces_atomic+0xe/0x10
>> [<ffffffffa0182676>] ath10k_wmi_tx_beacons_nowait+0x36/0x50 [ath10k_core]
>> [<ffffffffa01829b6>] ath10k_wmi_op_ep_tx_credits+0x16/0x40 [ath10k_core]
>> [<ffffffffa017d140>] ath10k_htc_rx+0x280/0x410 [ath10k_core]
>> [<ffffffffa01bcbf0>] ? ath10k_ce_completed_recv_next+0x60/0x80 [ath10k_pci]
>> [<ffffffffa01bc6ab>] ath10k_pci_ce_recv_data+0x11b/0x1d0 [ath10k_pci]
>> [<ffffffffa01bcf44>] ath10k_ce_per_engine_service+0x64/0xc0 [ath10k_pci]
>> [<ffffffffa01bcfc2>] ath10k_ce_per_engine_service_any+0x22/0x50 [ath10k_pci]
>> [<ffffffffa01bc4d0>] ath10k_pci_tasklet+0x30/0x90 [ath10k_pci]
>> [<ffffffff81055a55>] tasklet_action+0xc5/0x100
>>
>> To prevent this make sure to release ar->data_lock
>> while calling to ath10k_wmi_beacon_send_ref_nowait().
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> This introduces a new warning from kbuild:
>
> tree: git://github.com/kvalo/ath ath-next-test
> head: 59030a70b61c41268383d1406bb49fc559e0da4e
> commit: 60845cf1bc104bbec7509eb4b6c9f09a1559bfdc [104/113] ath10k: fix beacon deadlock
>
> New smatch warnings:
> drivers/net/wireless/ath/ath10k/wmi.c:969 ath10k_wmi_tx_beacon_nowait() warn: variable dereferenced before check 'bcn' (see line 967)
>From a practical standpoint there is no dereference because control
buffer is inlined in sk_buff and it ends up just as an offset to the
sk_buff. I guess smatch couldn't figure this out due to macros.
When I run smatch (tip) locally it doesn't complain about this. Maybe
I'm missing some parameters?
I'll post a v2 later.
Michał
Michal Kazior <[email protected]> writes:
> The command logic shouldn't really care about
> arvif structure.
>
> Signed-off-by: Michal Kazior <[email protected]>
Thanks, both patches applied to ath.git.
--
Kalle Valo