Hi,
Here are some fixes for ath10k. The rts threshold
patch addresses my mistake in commit
9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
caused rts to be always enabled causing throughput
issues in some cases.
Michal Kazior (3):
ath10k: prevent HTC from being used after stopping
ath10k: fix memleak in mac setup
ath10k: fix rts/fragmentation threshold setup
drivers/net/wireless/ath/ath10k/htc.c | 28 ++++++------
drivers/net/wireless/ath/ath10k/htc.h | 4 +-
drivers/net/wireless/ath/ath10k/mac.c | 80 ++++++++++++++++++---------------
3 files changed, 58 insertions(+), 54 deletions(-)
--
1.7.9.5
If RTS and fragmentation threshold values are
0xFFFFFFFF they should be considered disabled and
no min/max limits must be applied.
This fixes some issues with throughput issues,
especially with VHT.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 54 +++++++++++++++++----------------
1 file changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 3d0f876..019c6b8 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -331,6 +331,29 @@ static int ath10k_peer_create(struct ath10k *ar, u32 vdev_id, const u8 *addr)
return 0;
}
+static int ath10k_mac_set_rts(struct ath10k_vif *arvif, u32 value)
+{
+ if (value != 0xFFFFFFFF)
+ value = min_t(u32, arvif->ar->hw->wiphy->rts_threshold,
+ ATH10K_RTS_MAX);
+
+ return ath10k_wmi_vdev_set_param(arvif->ar, arvif->vdev_id,
+ WMI_VDEV_PARAM_RTS_THRESHOLD,
+ value);
+}
+
+static int ath10k_mac_set_frag(struct ath10k_vif *arvif, u32 value)
+{
+ if (value != 0xFFFFFFFF)
+ value = clamp_t(u32, arvif->ar->hw->wiphy->frag_threshold,
+ ATH10K_FRAGMT_THRESHOLD_MIN,
+ ATH10K_FRAGMT_THRESHOLD_MAX);
+
+ return ath10k_wmi_vdev_set_param(arvif->ar, arvif->vdev_id,
+ WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
+ value);
+}
+
static int ath10k_peer_delete(struct ath10k *ar, u32 vdev_id, const u8 *addr)
{
int ret;
@@ -1803,7 +1826,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
enum wmi_sta_powersave_param param;
int ret = 0;
- u32 value, rts, frag;
+ u32 value;
int bit;
mutex_lock(&ar->conf_mutex);
@@ -1906,20 +1929,12 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
}
- rts = min_t(u32, ar->hw->wiphy->rts_threshold, ATH10K_RTS_MAX);
- ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
- WMI_VDEV_PARAM_RTS_THRESHOLD,
- rts);
+ ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
if (ret)
ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
- frag = clamp_t(u32, ar->hw->wiphy->frag_threshold,
- ATH10K_FRAGMT_THRESHOLD_MIN,
- ATH10K_FRAGMT_THRESHOLD_MAX);
- ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
- WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
- frag);
+ ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
if (ret)
ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
@@ -2627,11 +2642,7 @@ static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
lockdep_assert_held(&arvif->ar->conf_mutex);
- rts = min_t(u32, rts, ATH10K_RTS_MAX);
-
- ar_iter->ret = ath10k_wmi_vdev_set_param(ar_iter->ar, arvif->vdev_id,
- WMI_VDEV_PARAM_RTS_THRESHOLD,
- 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);
@@ -2663,19 +2674,10 @@ static void ath10k_set_frag_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 frag = ar_iter->ar->hw->wiphy->frag_threshold;
- int ret;
lockdep_assert_held(&arvif->ar->conf_mutex);
- frag = clamp_t(u32, frag,
- ATH10K_FRAGMT_THRESHOLD_MIN,
- ATH10K_FRAGMT_THRESHOLD_MAX);
-
- ret = ath10k_wmi_vdev_set_param(ar_iter->ar, arvif->vdev_id,
- WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
- frag);
-
- ar_iter->ret = ret;
+ 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);
--
1.7.9.5
It was possible to submit new HTC commands
after/while HTC stopped. This led to memory
corruption in some rare cases.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htc.c | 28 +++++++++++++---------------
drivers/net/wireless/ath/ath10k/htc.h | 4 ++--
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 7d5a366..f5610f1 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -251,10 +251,14 @@ int ath10k_htc_send(struct ath10k_htc *htc,
return -ENOENT;
}
- skb_push(skb, sizeof(struct ath10k_htc_hdr));
-
spin_lock_bh(&htc->tx_lock);
+ if (htc->stopped) {
+ spin_unlock_bh(&htc->tx_lock);
+ return -ESHUTDOWN;
+ }
+
__skb_queue_tail(&ep->tx_queue, skb);
+ skb_push(skb, sizeof(struct ath10k_htc_hdr));
spin_unlock_bh(&htc->tx_lock);
queue_work(htc->ar->workqueue, &ep->send_work);
@@ -267,23 +271,17 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
{
struct ath10k_htc *htc = &ar->htc;
struct ath10k_htc_ep *ep = &htc->endpoint[eid];
- bool stopping;
ath10k_htc_notify_tx_completion(ep, skb);
/* the skb now belongs to the completion handler */
+ /* note: when using TX credit flow, the re-checking of queues happens
+ * when credits flow back from the target. in the non-TX credit case,
+ * we recheck after the packet completes */
spin_lock_bh(&htc->tx_lock);
- stopping = htc->stopping;
- spin_unlock_bh(&htc->tx_lock);
-
- if (!ep->tx_credit_flow_enabled && !stopping)
- /*
- * note: when using TX credit flow, the re-checking of
- * queues happens when credits flow back from the target.
- * in the non-TX credit case, we recheck after the packet
- * completes
- */
+ if (!ep->tx_credit_flow_enabled && !htc->stopped)
queue_work(ar->workqueue, &ep->send_work);
+ spin_unlock_bh(&htc->tx_lock);
return 0;
}
@@ -948,7 +946,7 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
struct ath10k_htc_ep *ep;
spin_lock_bh(&htc->tx_lock);
- htc->stopping = true;
+ htc->stopped = true;
spin_unlock_bh(&htc->tx_lock);
for (i = ATH10K_HTC_EP_0; i < ATH10K_HTC_EP_COUNT; i++) {
@@ -957,7 +955,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
}
ath10k_hif_stop(htc->ar);
- ath10k_htc_reset_endpoint_states(htc);
}
/* registered target arrival callback from the HIF layer */
@@ -969,6 +966,7 @@ int ath10k_htc_init(struct ath10k *ar)
spin_lock_init(&htc->tx_lock);
+ htc->stopped = false;
ath10k_htc_reset_endpoint_states(htc);
/* setup HIF layer callbacks */
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 1606c9f..e1dd8c7 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -335,7 +335,7 @@ struct ath10k_htc {
struct ath10k *ar;
struct ath10k_htc_ep endpoint[ATH10K_HTC_EP_COUNT];
- /* protects endpoint and stopping fields */
+ /* protects endpoint and stopped fields */
spinlock_t tx_lock;
struct ath10k_htc_ops htc_ops;
@@ -349,7 +349,7 @@ struct ath10k_htc {
struct ath10k_htc_svc_tx_credits service_tx_alloc[ATH10K_HTC_EP_COUNT];
int target_credit_size;
- bool stopping;
+ bool stopped;
};
int ath10k_htc_init(struct ath10k *ar);
--
1.7.9.5
On 19 July 2013 13:07, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> Hi,
>>
>> Here are some fixes for ath10k. The rts threshold
>> patch addresses my mistake in commit
>> 9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
>> caused rts to be always enabled causing throughput
>> issues in some cases.
>>
>>
>> Michal Kazior (3):
>> ath10k: prevent HTC from being used after stopping
>> ath10k: fix memleak in mac setup
>> ath10k: fix rts/fragmentation threshold setup
>
> Doesn't apply:
>
> Applying: ath10k: prevent HTC from being used after stopping
> Applying: ath10k: fix memleak in mac setup
> Applying: ath10k: fix rts/fragmentation threshold setup
> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/mac.c).
> Repository lacks necessary blobs to fall back on 3-way merge.
> Cannot fall back to three-way merge.
> Patch failed at 0003 ath10k: fix rts/fragmentation threshold setup
Eww. Sorry about that (I did warn you though). Perhaps that's because
you skipped one of the recovery patches that git is confused now.
I'll resend the whole patchset rebased and with the HTC patch fixed.
Pozdrawiam / Best regards,
Micha? Kazior.
Michal Kazior <[email protected]> writes:
> Hi,
>
> Here are some fixes for ath10k. The rts threshold
> patch addresses my mistake in commit
> 9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
> caused rts to be always enabled causing throughput
> issues in some cases.
>
>
> Michal Kazior (3):
> ath10k: prevent HTC from being used after stopping
> ath10k: fix memleak in mac setup
> ath10k: fix rts/fragmentation threshold setup
Doesn't apply:
Applying: ath10k: prevent HTC from being used after stopping
Applying: ath10k: fix memleak in mac setup
Applying: ath10k: fix rts/fragmentation threshold setup
fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/mac.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0003 ath10k: fix rts/fragmentation threshold setup
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> On 19 July 2013 13:07, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> Hi,
>>>
>>> Here are some fixes for ath10k. The rts threshold
>>> patch addresses my mistake in commit
>>> 9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
>>> caused rts to be always enabled causing throughput
>>> issues in some cases.
>>>
>>>
>>> Michal Kazior (3):
>>> ath10k: prevent HTC from being used after stopping
>>> ath10k: fix memleak in mac setup
>>> ath10k: fix rts/fragmentation threshold setup
>>
>> Doesn't apply:
>>
>> Applying: ath10k: prevent HTC from being used after stopping
>> Applying: ath10k: fix memleak in mac setup
>> Applying: ath10k: fix rts/fragmentation threshold setup
>> fatal: sha1 information is lacking or useless (drivers/net/wireless/ath/ath10k/mac.c).
>> Repository lacks necessary blobs to fall back on 3-way merge.
>> Cannot fall back to three-way merge.
>> Patch failed at 0003 ath10k: fix rts/fragmentation threshold setup
>
> Eww. Sorry about that (I did warn you though). Perhaps that's because
> you skipped one of the recovery patches that git is confused now.
No worries. Most likely I could have fixed the conflict myself, but if
3-way merge doesn't work it's too much work.
> I'll resend the whole patchset rebased and with the HTC patch fixed.
Thanks.
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> Endpoints are re-initialized upon HTC start anyway
> so there's no need to do that twice in case of
> restarting HTC (i.e. in case of hardware
> recovery).
>
> Signed-off-by: Michal Kazior <[email protected]>
This is much better, thanks.
--
Kalle Valo
In some cases channel arrays were never freed.
The patch also unifies error handling in the mac
setup function.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 07e5f7d..6144b3b 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3225,8 +3225,10 @@ int ath10k_mac_register(struct ath10k *ar)
channels = kmemdup(ath10k_2ghz_channels,
sizeof(ath10k_2ghz_channels),
GFP_KERNEL);
- if (!channels)
- return -ENOMEM;
+ if (!channels) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
band->n_channels = ARRAY_SIZE(ath10k_2ghz_channels);
@@ -3245,11 +3247,8 @@ int ath10k_mac_register(struct ath10k *ar)
sizeof(ath10k_5ghz_channels),
GFP_KERNEL);
if (!channels) {
- if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY) {
- band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
- kfree(band->channels);
- }
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_free;
}
band = &ar->mac.sbands[IEEE80211_BAND_5GHZ];
@@ -3313,25 +3312,30 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_reg_notifier);
if (ret) {
ath10k_err("Regulatory initialization failed\n");
- return ret;
+ goto err_free;
}
ret = ieee80211_register_hw(ar->hw);
if (ret) {
ath10k_err("ieee80211 registration failed: %d\n", ret);
- return ret;
+ goto err_free;
}
if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
ret = regulatory_hint(ar->hw->wiphy,
ar->ath_common.regulatory.alpha2);
if (ret)
- goto exit;
+ goto err_unregister;
}
return 0;
-exit:
+
+err_unregister:
ieee80211_unregister_hw(ar->hw);
+err_free:
+ kfree(ar->mac.sbands[IEEE80211_BAND_2GHZ].channels);
+ kfree(ar->mac.sbands[IEEE80211_BAND_5GHZ].channels);
+
return ret;
}
--
1.7.9.5
If RTS and fragmentation threshold values are
0xFFFFFFFF they should be considered disabled and
no min/max limits must be applied.
This fixes some issues with throughput issues,
especially with VHT.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 54 +++++++++++++++++----------------
1 file changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 6144b3b..d0a7761 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -332,6 +332,29 @@ static int ath10k_peer_create(struct ath10k *ar, u32 vdev_id, const u8 *addr)
return 0;
}
+static int ath10k_mac_set_rts(struct ath10k_vif *arvif, u32 value)
+{
+ if (value != 0xFFFFFFFF)
+ value = min_t(u32, arvif->ar->hw->wiphy->rts_threshold,
+ ATH10K_RTS_MAX);
+
+ return ath10k_wmi_vdev_set_param(arvif->ar, arvif->vdev_id,
+ WMI_VDEV_PARAM_RTS_THRESHOLD,
+ value);
+}
+
+static int ath10k_mac_set_frag(struct ath10k_vif *arvif, u32 value)
+{
+ if (value != 0xFFFFFFFF)
+ value = clamp_t(u32, arvif->ar->hw->wiphy->frag_threshold,
+ ATH10K_FRAGMT_THRESHOLD_MIN,
+ ATH10K_FRAGMT_THRESHOLD_MAX);
+
+ return ath10k_wmi_vdev_set_param(arvif->ar, arvif->vdev_id,
+ WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
+ value);
+}
+
static int ath10k_peer_delete(struct ath10k *ar, u32 vdev_id, const u8 *addr)
{
int ret;
@@ -1897,7 +1920,7 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
enum wmi_sta_powersave_param param;
int ret = 0;
- u32 value, rts, frag;
+ u32 value;
int bit;
mutex_lock(&ar->conf_mutex);
@@ -2000,20 +2023,12 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
ath10k_warn("Failed to set PSPOLL count: %d\n", ret);
}
- rts = min_t(u32, ar->hw->wiphy->rts_threshold, ATH10K_RTS_MAX);
- ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
- WMI_VDEV_PARAM_RTS_THRESHOLD,
- rts);
+ ret = ath10k_mac_set_rts(arvif, ar->hw->wiphy->rts_threshold);
if (ret)
ath10k_warn("failed to set rts threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
- frag = clamp_t(u32, ar->hw->wiphy->frag_threshold,
- ATH10K_FRAGMT_THRESHOLD_MIN,
- ATH10K_FRAGMT_THRESHOLD_MAX);
- ret = ath10k_wmi_vdev_set_param(ar, arvif->vdev_id,
- WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
- frag);
+ ret = ath10k_mac_set_frag(arvif, ar->hw->wiphy->frag_threshold);
if (ret)
ath10k_warn("failed to set frag threshold for vdev %d (%d)\n",
arvif->vdev_id, ret);
@@ -2728,11 +2743,7 @@ static void ath10k_set_rts_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
return;
- rts = min_t(u32, rts, ATH10K_RTS_MAX);
-
- ar_iter->ret = ath10k_wmi_vdev_set_param(ar_iter->ar, arvif->vdev_id,
- WMI_VDEV_PARAM_RTS_THRESHOLD,
- 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);
@@ -2764,7 +2775,6 @@ static void ath10k_set_frag_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 frag = ar_iter->ar->hw->wiphy->frag_threshold;
- int ret;
lockdep_assert_held(&arvif->ar->conf_mutex);
@@ -2775,15 +2785,7 @@ static void ath10k_set_frag_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
if (ar_iter->ar->state == ATH10K_STATE_RESTARTED)
return;
- frag = clamp_t(u32, frag,
- ATH10K_FRAGMT_THRESHOLD_MIN,
- ATH10K_FRAGMT_THRESHOLD_MAX);
-
- ret = ath10k_wmi_vdev_set_param(ar_iter->ar, arvif->vdev_id,
- WMI_VDEV_PARAM_FRAGMENTATION_THRESHOLD,
- frag);
-
- ar_iter->ret = ret;
+ 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);
--
1.7.9.5
Michal Kazior <[email protected]> writes:
> On 19 July 2013 13:04, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> It was possible to submit new HTC commands
>>> after/while HTC stopped. This led to memory
>>> corruption in some rare cases.
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>
>> [...]
>>
>>> @@ -957,7 +955,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
>>> }
>>>
>>> ath10k_hif_stop(htc->ar);
>>> - ath10k_htc_reset_endpoint_states(htc);
>>> }
>>
>> Is this on purpose? I can't fit it to the description.
>
> You're right. I should've mentioned that in the commit message.
>
> This line is simply unnecessary. Do you prefer fixing the commit
> message accordingly or should I post it as a separate patch?
A separate patch, please. Makes it easier to bisect issues later on.
--
Kalle Valo
It was possible to submit new HTC commands
after/while HTC stopped. This led to memory
corruption in some rare cases.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htc.c | 27 +++++++++++++--------------
drivers/net/wireless/ath/ath10k/htc.h | 4 ++--
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 72e072c..47b7752 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -254,10 +254,14 @@ int ath10k_htc_send(struct ath10k_htc *htc,
return -ENOENT;
}
- skb_push(skb, sizeof(struct ath10k_htc_hdr));
-
spin_lock_bh(&htc->tx_lock);
+ if (htc->stopped) {
+ spin_unlock_bh(&htc->tx_lock);
+ return -ESHUTDOWN;
+ }
+
__skb_queue_tail(&ep->tx_queue, skb);
+ skb_push(skb, sizeof(struct ath10k_htc_hdr));
spin_unlock_bh(&htc->tx_lock);
queue_work(htc->ar->workqueue, &ep->send_work);
@@ -270,23 +274,17 @@ static int ath10k_htc_tx_completion_handler(struct ath10k *ar,
{
struct ath10k_htc *htc = &ar->htc;
struct ath10k_htc_ep *ep = &htc->endpoint[eid];
- bool stopping;
ath10k_htc_notify_tx_completion(ep, skb);
/* the skb now belongs to the completion handler */
+ /* note: when using TX credit flow, the re-checking of queues happens
+ * when credits flow back from the target. in the non-TX credit case,
+ * we recheck after the packet completes */
spin_lock_bh(&htc->tx_lock);
- stopping = htc->stopping;
- spin_unlock_bh(&htc->tx_lock);
-
- if (!ep->tx_credit_flow_enabled && !stopping)
- /*
- * note: when using TX credit flow, the re-checking of
- * queues happens when credits flow back from the target.
- * in the non-TX credit case, we recheck after the packet
- * completes
- */
+ if (!ep->tx_credit_flow_enabled && !htc->stopped)
queue_work(ar->workqueue, &ep->send_work);
+ spin_unlock_bh(&htc->tx_lock);
return 0;
}
@@ -951,7 +949,7 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
struct ath10k_htc_ep *ep;
spin_lock_bh(&htc->tx_lock);
- htc->stopping = true;
+ htc->stopped = true;
spin_unlock_bh(&htc->tx_lock);
for (i = ATH10K_HTC_EP_0; i < ATH10K_HTC_EP_COUNT; i++) {
@@ -972,6 +970,7 @@ int ath10k_htc_init(struct ath10k *ar)
spin_lock_init(&htc->tx_lock);
+ htc->stopped = false;
ath10k_htc_reset_endpoint_states(htc);
/* setup HIF layer callbacks */
diff --git a/drivers/net/wireless/ath/ath10k/htc.h b/drivers/net/wireless/ath/ath10k/htc.h
index 1606c9f..e1dd8c7 100644
--- a/drivers/net/wireless/ath/ath10k/htc.h
+++ b/drivers/net/wireless/ath/ath10k/htc.h
@@ -335,7 +335,7 @@ struct ath10k_htc {
struct ath10k *ar;
struct ath10k_htc_ep endpoint[ATH10K_HTC_EP_COUNT];
- /* protects endpoint and stopping fields */
+ /* protects endpoint and stopped fields */
spinlock_t tx_lock;
struct ath10k_htc_ops htc_ops;
@@ -349,7 +349,7 @@ struct ath10k_htc {
struct ath10k_htc_svc_tx_credits service_tx_alloc[ATH10K_HTC_EP_COUNT];
int target_credit_size;
- bool stopping;
+ bool stopped;
};
int ath10k_htc_init(struct ath10k *ar);
--
1.7.9.5
Michal Kazior <[email protected]> writes:
> Hi,
>
> Here are some fixes for ath10k. The rts threshold
> patch addresses my mistake in commit
> 9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
> caused rts to be always enabled causing throughput
> issues in some cases.
>
> v2: first patch is split now into two as requested
> rebased on top of latest github master branch
>
>
> Michal Kazior (4):
> ath10k: prevent HTC from being used after stopping
> ath10k: don't reset HTC endpoints unnecessarily
> ath10k: fix memleak in mac setup
> ath10k: fix rts/fragmentation threshold setup
All four applied, thanks.
--
Kalle Valo
Hi,
Here are some fixes for ath10k. The rts threshold
patch addresses my mistake in commit
9aeb6fe53d1f0d6e58658484ce9ad7b59f0ef16b which
caused rts to be always enabled causing throughput
issues in some cases.
v2: first patch is split now into two as requested
rebased on top of latest github master branch
Michal Kazior (4):
ath10k: prevent HTC from being used after stopping
ath10k: don't reset HTC endpoints unnecessarily
ath10k: fix memleak in mac setup
ath10k: fix rts/fragmentation threshold setup
drivers/net/wireless/ath/ath10k/htc.c | 28 ++++++------
drivers/net/wireless/ath/ath10k/htc.h | 4 +-
drivers/net/wireless/ath/ath10k/mac.c | 80 ++++++++++++++++++---------------
3 files changed, 58 insertions(+), 54 deletions(-)
--
1.7.9.5
In some cases channel arrays were never freed.
The patch also unifies error handling in the mac
setup function.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 5082503..3d0f876 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3025,8 +3025,10 @@ int ath10k_mac_register(struct ath10k *ar)
channels = kmemdup(ath10k_2ghz_channels,
sizeof(ath10k_2ghz_channels),
GFP_KERNEL);
- if (!channels)
- return -ENOMEM;
+ if (!channels) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
band->n_channels = ARRAY_SIZE(ath10k_2ghz_channels);
@@ -3045,11 +3047,8 @@ int ath10k_mac_register(struct ath10k *ar)
sizeof(ath10k_5ghz_channels),
GFP_KERNEL);
if (!channels) {
- if (ar->phy_capability & WHAL_WLAN_11G_CAPABILITY) {
- band = &ar->mac.sbands[IEEE80211_BAND_2GHZ];
- kfree(band->channels);
- }
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_free;
}
band = &ar->mac.sbands[IEEE80211_BAND_5GHZ];
@@ -3113,25 +3112,30 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_reg_notifier);
if (ret) {
ath10k_err("Regulatory initialization failed\n");
- return ret;
+ goto err_free;
}
ret = ieee80211_register_hw(ar->hw);
if (ret) {
ath10k_err("ieee80211 registration failed: %d\n", ret);
- return ret;
+ goto err_free;
}
if (!ath_is_world_regd(&ar->ath_common.regulatory)) {
ret = regulatory_hint(ar->hw->wiphy,
ar->ath_common.regulatory.alpha2);
if (ret)
- goto exit;
+ goto err_unregister;
}
return 0;
-exit:
+
+err_unregister:
ieee80211_unregister_hw(ar->hw);
+err_free:
+ kfree(ar->mac.sbands[IEEE80211_BAND_2GHZ].channels);
+ kfree(ar->mac.sbands[IEEE80211_BAND_5GHZ].channels);
+
return ret;
}
--
1.7.9.5
On 19 July 2013 13:04, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> It was possible to submit new HTC commands
>> after/while HTC stopped. This led to memory
>> corruption in some rare cases.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> [...]
>
>> @@ -957,7 +955,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
>> }
>>
>> ath10k_hif_stop(htc->ar);
>> - ath10k_htc_reset_endpoint_states(htc);
>> }
>
> Is this on purpose? I can't fit it to the description.
You're right. I should've mentioned that in the commit message.
This line is simply unnecessary. Do you prefer fixing the commit
message accordingly or should I post it as a separate patch?
Pozdrawiam / Best regards,
Michał Kazior.
Michal Kazior <[email protected]> writes:
> It was possible to submit new HTC commands
> after/while HTC stopped. This led to memory
> corruption in some rare cases.
>
> Signed-off-by: Michal Kazior <[email protected]>
[...]
> @@ -957,7 +955,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
> }
>
> ath10k_hif_stop(htc->ar);
> - ath10k_htc_reset_endpoint_states(htc);
> }
Is this on purpose? I can't fit it to the description.
--
Kalle Valo
Endpoints are re-initialized upon HTC start anyway
so there's no need to do that twice in case of
restarting HTC (i.e. in case of hardware
recovery).
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htc.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index 47b7752..ef3329e 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -958,7 +958,6 @@ void ath10k_htc_stop(struct ath10k_htc *htc)
}
ath10k_hif_stop(htc->ar);
- ath10k_htc_reset_endpoint_states(htc);
}
/* registered target arrival callback from the HIF layer */
--
1.7.9.5
"Luis R. Rodriguez" <[email protected]> writes:
>> I disagree. The point of linux-stable is _not_ that we send all possible
>> fixes to stable. Instead we should send fixes only which really matter
>> to users and for which we have received bug reports. I haven't yet seen
>> any fix for ath10k which should be a candidate for stable releases.
>
> You don't need to wait for an issue to happen to consider it serious,
> the description of the symptoms seem pretty bad to me, but its your
> call in the end.
>
>> If we start sending all ath10k fixes to stable it's just extra churn for
>> both Greg and people working on ath10k.
>
> I'm not asking for anything that has the word "fix" to be sent, I'm
> asking them to be reviewed for stable consideration.
I think a good rule is that we should send fixes to stable only if a
user (!= developer) has reported the problem.
--
Kalle Valo
On 26 August 2013 22:20, Luis R. Rodriguez <[email protected]> wrote:
> On Mon, Aug 26, 2013 at 1:53 AM, Michal Kazior <[email protected]> wrote:
>> ath10k: fix issues on non-preemptible systems
>
> This patch looks like a stable candidate fix. Please annotate as such
> if you confirm. Also, I reviewed other ath10k "fixes" and I see no
> practice of propagating any patches to stable yet. Can you please
> start doing that? If there were patches which are already merged
> upstream that should be propagated to stable then they can be
> submitted as stable candidate patches.
Good point.
If this patch is to be propagated to stable the patch(set) needs some
adjustments to avoid conflicts. Right now the patch touches a place
that was introduced with "ath10k: move htt rx processing to worker" so
the offending patch hunk should be moved to that patch.
Actually the "ath10k: make the workqueue multithreaded" could also be
a candidate for stable as it fixes AP beaconing during heavy TX
traffic. I should've mentioned that in the commit log.
I'll resend an updated patchset later today.
Michał.
On Tue, Aug 27, 2013 at 07:42:39AM +0200, Michal Kazior wrote:
> On 26 August 2013 22:20, Luis R. Rodriguez <[email protected]> wrote:
> > On Mon, Aug 26, 2013 at 1:53 AM, Michal Kazior <[email protected]> wrote:
> >> ath10k: fix issues on non-preemptible systems
> >
> > This patch looks like a stable candidate fix. Please annotate as such
> > if you confirm. Also, I reviewed other ath10k "fixes" and I see no
> > practice of propagating any patches to stable yet. Can you please
> > start doing that? If there were patches which are already merged
> > upstream that should be propagated to stable then they can be
> > submitted as stable candidate patches.
>
> Good point.
>
> If this patch is to be propagated to stable the patch(set) needs some
> adjustments to avoid conflicts. Right now the patch touches a place
> that was introduced with "ath10k: move htt rx processing to worker" so
> the offending patch hunk should be moved to that patch.
>
> Actually the "ath10k: make the workqueue multithreaded" could also be
> a candidate for stable as it fixes AP beaconing during heavy TX
> traffic. I should've mentioned that in the commit log.
>
> I'll resend an updated patchset later today.
That one doesn't seem like a stable fix, it does quite a big change.
Luis