Some setups suffer performance regressions with
current wake_tx_queue implementation.
Signed-off-by: Michal Kazior <[email protected]>
---
Hi Roman,
Can you give this patch a try and see if it helps
with your performance problems, please?
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 24 ++++++++++++++++++++++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 1379054000f9..44e7c2cd2e9b 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -657,6 +657,7 @@ struct ath10k_fw_components {
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
+ struct ieee80211_ops *ops;
struct device *dev;
u8 mac_addr[ETH_ALEN];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0e24f9ee8bff..5fa3888afa41 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7396,21 +7396,32 @@ static const struct ieee80211_channel ath10k_5ghz_channels[] = {
struct ath10k *ath10k_mac_create(size_t priv_size)
{
struct ieee80211_hw *hw;
+ struct ieee80211_ops *ops;
struct ath10k *ar;
- hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, &ath10k_ops);
- if (!hw)
+ ops = kmemdup(&ath10k_ops, sizeof(ath10k_ops), GFP_KERNEL);
+ if (!ops)
return NULL;
+ hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, ops);
+ if (!hw) {
+ kfree(ops);
+ return NULL;
+ }
+
ar = hw->priv;
ar->hw = hw;
+ ar->ops = ops;
return ar;
}
void ath10k_mac_destroy(struct ath10k *ar)
{
+ struct ieee80211_ops *ops = ar->ops;
+
ieee80211_free_hw(ar->hw);
+ kfree(ops);
}
static const struct ieee80211_iface_limit ath10k_if_limits[] = {
@@ -7838,6 +7849,15 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_warn(ar, "failed to initialise DFS pattern detector\n");
}
+ /* Current wake_tx_queue implementation imposes a significant
+ * performance penalty in some setups. The tx scheduling code needs
+ * more work anyway so disable the wake_tx_queue unless firmware
+ * supports the pull-push mechanism.
+ */
+ if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL,
+ ar->running_fw->fw_file.fw_features))
+ ar->ops->wake_tx_queue = NULL;
+
ret = ath_regd_init(&ar->ath_common.regulatory, ar->hw->wiphy,
ath10k_reg_notifier);
if (ret) {
--
2.1.4
On 2 June 2016 at 16:14, Valo, Kalle <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> Some setups suffer performance regressions with
>> current wake_tx_queue implementation.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>> ---
>> Hi Roman,
>>
>> Can you give this patch a try and see if it helps
>> with your performance problems, please?
>
> Did anyone seeing the performance regression manage to test this? It
> would be good to get feedback, good or bad, before I push this to 4.7.
>
> Full patch here:
>
> https://patchwork.kernel.org/patch/9111981/
Sorry, didn't try yet, bad timing for me :(
Will try to test by the end of the week
Regards,
Roman
Ideally wake_tx_queue should be used regardless as
it is a requirement for reducing bufferbloat and
implementing airtime fairness in the future.
However some setups (typically low-end platforms
hosting QCA988X) suffer performance regressions
with the current wake_tx_queue implementation.
Therefore disable it unless it is really
beneficial with current codebase (which is when
firmware supports smart pull-push tx scheduling).
Signed-off-by: Michal Kazior <[email protected]>
---
Notes:
v1:
- improve commit log
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 24 ++++++++++++++++++++++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 3da18c9dbd7a..de580af2e5ef 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -667,6 +667,7 @@ struct ath10k_fw_components {
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
+ struct ieee80211_ops *ops;
struct device *dev;
u8 mac_addr[ETH_ALEN];
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index d4b7a168f7c0..958bc6c91aac 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -7471,21 +7471,32 @@ static const struct ieee80211_channel ath10k_5ghz_channels[] = {
struct ath10k *ath10k_mac_create(size_t priv_size)
{
struct ieee80211_hw *hw;
+ struct ieee80211_ops *ops;
struct ath10k *ar;
- hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, &ath10k_ops);
- if (!hw)
+ ops = kmemdup(&ath10k_ops, sizeof(ath10k_ops), GFP_KERNEL);
+ if (!ops)
return NULL;
+ hw = ieee80211_alloc_hw(sizeof(struct ath10k) + priv_size, ops);
+ if (!hw) {
+ kfree(ops);
+ return NULL;
+ }
+
ar = hw->priv;
ar->hw = hw;
+ ar->ops = ops;
return ar;
}
void ath10k_mac_destroy(struct ath10k *ar)
{
+ struct ieee80211_ops *ops = ar->ops;
+
ieee80211_free_hw(ar->hw);
+ kfree(ops);
}
static const struct ieee80211_iface_limit ath10k_if_limits[] = {
@@ -7919,6 +7930,15 @@ int ath10k_mac_register(struct ath10k *ar)
ath10k_warn(ar, "failed to initialise DFS pattern detector\n");
}
+ /* Current wake_tx_queue implementation imposes a significant
+ * performance penalty in some setups. The tx scheduling code needs
+ * more work anyway so disable the wake_tx_queue unless firmware
+ * supports the pull-push mechanism.
+ */
+ if (!test_bit(ATH10K_FW_FEATURE_PEER_FLOW_CONTROL,
+ ar->running_fw->fw_file.fw_features))
+ ar->ops->wake_tx_queue = NULL;
+
ret = ath_regd_init(&ar->ath_common.regulatory, ar->hw->wiphy,
ath10k_reg_notifier);
if (ret) {
--
2.1.4
Roman Yeryomin <[email protected]> writes:
> On 2 June 2016 at 16:14, Valo, Kalle <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> Some setups suffer performance regressions with
>>> current wake_tx_queue implementation.
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>> ---
>>> Hi Roman,
>>>
>>> Can you give this patch a try and see if it helps
>>> with your performance problems, please?
>>
>> Did anyone seeing the performance regression manage to test this? It
>> would be good to get feedback, good or bad, before I push this to 4.7.
>>
>> Full patch here:
>>
>> https://patchwork.kernel.org/patch/9111981/
>
> Sorry, didn't try yet, bad timing for me :(
> Will try to test by the end of the week
Roman, did you have a chance to test this?
Michal, could you submit this patch as a formal patch, please? I think
we should take it even if Roman hasn't tested it.
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> Some setups suffer performance regressions with
> current wake_tx_queue implementation.
>
> Signed-off-by: Michal Kazior <[email protected]>
> ---
> Hi Roman,
>
> Can you give this patch a try and see if it helps
> with your performance problems, please?
Did anyone seeing the performance regression manage to test this? It
would be good to get feedback, good or bad, before I push this to 4.7.
Full patch here:
https://patchwork.kernel.org/patch/9111981/
--
Kalle Valo
Michal Kazior <[email protected]> wrote:
> Ideally wake_tx_queue should be used regardless as
> it is a requirement for reducing bufferbloat and
> implementing airtime fairness in the future.
>
> However some setups (typically low-end platforms
> hosting QCA988X) suffer performance regressions
> with the current wake_tx_queue implementation.
> Therefore disable it unless it is really
> beneficial with current codebase (which is when
> firmware supports smart pull-push tx scheduling).
>
> Signed-off-by: Michal Kazior <[email protected]>
Thanks, 1 patch applied to ath-next branch of ath.git:
4ca1807815aa ath10k: disable wake_tx_queue for older devices
--
Sent by pwcli
https://patchwork.kernel.org/patch/9204997/
Michal Kazior <[email protected]> writes:
> Ideally wake_tx_queue should be used regardless as
> it is a requirement for reducing bufferbloat and
> implementing airtime fairness in the future.
>
> However some setups (typically low-end platforms
> hosting QCA988X) suffer performance regressions
> with the current wake_tx_queue implementation.
> Therefore disable it unless it is really
> beneficial with current codebase (which is when
> firmware supports smart pull-push tx scheduling).
>
> Signed-off-by: Michal Kazior <[email protected]>
I think it's too late to send this to 4.7 anymore (and this due to my
vacation). So I'm planning to queue this to 4.8, but if the feedback is
positive we can always send this to a 4.7 stable release.
--
Kalle Valo
On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <[email protected]> wrote:
> On 7 July 2016 at 19:30, Valo, Kalle <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> Ideally wake_tx_queue should be used regardless as
>>> it is a requirement for reducing bufferbloat and
>>> implementing airtime fairness in the future.
>>>
>>> However some setups (typically low-end platforms
>>> hosting QCA988X) suffer performance regressions
>>> with the current wake_tx_queue implementation.
>>> Therefore disable it unless it is really
>>> beneficial with current codebase (which is when
>>> firmware supports smart pull-push tx scheduling).
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>
>> I think it's too late to send this to 4.7 anymore (and this due to my
>> vacation). So I'm planning to queue this to 4.8, but if the feedback is
>> positive we can always send this to a 4.7 stable release.
>>
>
> Sorry guys, drowned.
> So, yes, applying this patch does the job. That is gets me to the
> results similar to
> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html
>
> Going to try latest code on same system...
Can you try increasing the quantum to 1514, and reducing the codel
target to 5ms? (without this patch?)
>
> Regards,
> Roman
>
> _______________________________________________
> ath10k mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath10k
--
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org
On 1 August 2016 at 12:04, Dave Taht <[email protected]> wrote:
> On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <[email protected]> wrote:
>> On 7 July 2016 at 19:30, Valo, Kalle <[email protected]> wrote:
>>> Michal Kazior <[email protected]> writes:
>>>
>>>> Ideally wake_tx_queue should be used regardless as
>>>> it is a requirement for reducing bufferbloat and
>>>> implementing airtime fairness in the future.
>>>>
>>>> However some setups (typically low-end platforms
>>>> hosting QCA988X) suffer performance regressions
>>>> with the current wake_tx_queue implementation.
>>>> Therefore disable it unless it is really
>>>> beneficial with current codebase (which is when
>>>> firmware supports smart pull-push tx scheduling).
>>>>
>>>> Signed-off-by: Michal Kazior <[email protected]>
>>>
>>> I think it's too late to send this to 4.7 anymore (and this due to my
>>> vacation). So I'm planning to queue this to 4.8, but if the feedback is
>>> positive we can always send this to a 4.7 stable release.
>>>
>>
>> Sorry guys, drowned.
>> So, yes, applying this patch does the job. That is gets me to the
>> results similar to
>> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html
>>
>> Going to try latest code on same system...
>
> Can you try increasing the quantum to 1514, and reducing the codel
> target to 5ms? (without this patch?)
>
So it was 1514 already...
Regards,
Roman
On Thu, Aug 4, 2016 at 12:07 PM, Roman Yeryomin <[email protected]> wrote:
> On 1 August 2016 at 12:04, Dave Taht <[email protected]> wrote:
>> On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <[email protected]> wrote:
>>> On 7 July 2016 at 19:30, Valo, Kalle <[email protected]> wrote:
>>>> Michal Kazior <[email protected]> writes:
>>>>
>>>>> Ideally wake_tx_queue should be used regardless as
>>>>> it is a requirement for reducing bufferbloat and
>>>>> implementing airtime fairness in the future.
>>>>>
>>>>> However some setups (typically low-end platforms
>>>>> hosting QCA988X) suffer performance regressions
>>>>> with the current wake_tx_queue implementation.
>>>>> Therefore disable it unless it is really
>>>>> beneficial with current codebase (which is when
>>>>> firmware supports smart pull-push tx scheduling).
>>>>>
>>>>> Signed-off-by: Michal Kazior <[email protected]>
>>>>
>>>> I think it's too late to send this to 4.7 anymore (and this due to my
>>>> vacation). So I'm planning to queue this to 4.8, but if the feedback is
>>>> positive we can always send this to a 4.7 stable release.
>>>>
>>>
>>> Sorry guys, drowned.
>>> So, yes, applying this patch does the job. That is gets me to the
>>> results similar to
>>> https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html
>>>
>>> Going to try latest code on same system...
>>
>> Can you try increasing the quantum to 1514, and reducing the codel
>> target to 5ms? (without this patch?)
>>
>
> So it was 1514 already...
based on some testing of 20, codel target should be 5ms and isn't.
https://github.com/torvalds/linux/commit/5caa328e3811b7cfa33fd02c93280ffa622deb0e
> Regards,
> Roman
>
> _______________________________________________
> ath10k mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath10k
--
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org
On 7 July 2016 at 19:30, Valo, Kalle <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> Ideally wake_tx_queue should be used regardless as
>> it is a requirement for reducing bufferbloat and
>> implementing airtime fairness in the future.
>>
>> However some setups (typically low-end platforms
>> hosting QCA988X) suffer performance regressions
>> with the current wake_tx_queue implementation.
>> Therefore disable it unless it is really
>> beneficial with current codebase (which is when
>> firmware supports smart pull-push tx scheduling).
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> I think it's too late to send this to 4.7 anymore (and this due to my
> vacation). So I'm planning to queue this to 4.8, but if the feedback is
> positive we can always send this to a 4.7 stable release.
>
Sorry guys, drowned.
So, yes, applying this patch does the job. That is gets me to the
results similar to
https://lists.openwrt.org/pipermail/openwrt-devel/2016-May/041448.html
Going to try latest code on same system...
Regards,
Roman