ath10k: disable wake_tx_queue for older devices
diff mbox

Message ID 1467201146-6844-1-git-send-email-michal.kazior@tieto.com
State Accepted
Commit 4ca1807815aa6801aaced7fdefa9edacc2521767
Delegated to: Kalle Valo
Headers show

Commit Message

Michal Kazior June 29, 2016, 11:52 a.m. UTC
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 <michal.kazior@tieto.com>
---

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(-)

Comments

Kalle Valo July 7, 2016, 4:30 p.m. UTC | #1
Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>

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 July 19, 2016, 1:19 p.m. UTC | #2
Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>

Thanks, 1 patch applied to ath-next branch of ath.git:

4ca1807815aa ath10k: disable wake_tx_queue for older devices
Roman Yeryomin July 31, 2016, 11:35 p.m. UTC | #3
On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>
> 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
Dave Taht Aug. 1, 2016, 9:04 a.m. UTC | #4
On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>>
>> 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
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
Roman Yeryomin Aug. 4, 2016, 10:07 a.m. UTC | #5
On 1 August 2016 at 12:04, Dave Taht <dave.taht@gmail.com> wrote:
> On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
>> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>>> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>>>
>>> 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
Dave Taht Aug. 4, 2016, 12:02 p.m. UTC | #6
On Thu, Aug 4, 2016 at 12:07 PM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
> On 1 August 2016 at 12:04, Dave Taht <dave.taht@gmail.com> wrote:
>> On Mon, Aug 1, 2016 at 1:35 AM, Roman Yeryomin <leroi.lists@gmail.com> wrote:
>>> On 7 July 2016 at 19:30, Valo, Kalle <kvalo@qca.qualcomm.com> wrote:
>>>> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>>>>
>>>> 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
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k

Patch
diff mbox

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) {