[PATCH] ath10k: Re-enable TXQs for all devices

[PATCH] ath10k: Re-enable TXQs for all devices

[Toke Høiland-Jørgensen]

Commit 4ca1807815aa6801aaced7fdefa9edacc2521767 disables the use of the
mac80211 TXQs for some devices because of a theoretical throughput
regression. We have not seen this regression for a while now, so it
should be safe to re-enable TXQs.

Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
This has been in LEDE trunk for a couple of months now with good results.

 drivers/net/wireless/ath/ath10k/mac.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 0a947eef348d..ca596ecd2d64 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8329,15 +8329,6 @@ 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 = ath10k_mac_init_rd(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to derive regdom: %d\n", ret);
-- 
2.15.0

RE: [PATCH] ath10k: Re-enable TXQs for all devices

[Rajkumar Manoharan]

PiBDb21taXQgNGNhMTgwNzgxNWFhNjgwMWFhY2VkN2ZkZWZhOWVkYWNjMjUyMTc2NyBkaXNhYmxl
cyB0aGUgdXNlIG9mIHRoZQ0KPiBtYWM4MDIxMSBUWFFzIGZvciBzb21lIGRldmljZXMgYmVjYXVz
ZSBvZiBhIHRoZW9yZXRpY2FsIHRocm91Z2hwdXQNCj4gcmVncmVzc2lvbi4gV2UgaGF2ZSBub3Qg
c2VlbiB0aGlzIHJlZ3Jlc3Npb24gZm9yIGEgd2hpbGUgbm93LCBzbyBpdCBzaG91bGQgYmUNCj4g
c2FmZSB0byByZS1lbmFibGUgVFhRcy4NCj4gDQo+IFNpZ25lZC1vZmYtYnk6IFRva2UgSMO4aWxh
bmQtSsO4cmdlbnNlbiA8dG9rZUB0b2tlLmRrPg0KPiAtLS0NCj4gVGhpcyBoYXMgYmVlbiBpbiBM
RURFIHRydW5rIGZvciBhIGNvdXBsZSBvZiBtb250aHMgbm93IHdpdGggZ29vZCByZXN1bHRzLg0K
Pg0KVG9rZSwNCiANCkdvb2QgdG8ga25vdyB0aGF0IHRoZSBwZXJmb3JtYW5jZSBkcm9wIGlzIG5v
dCBzZWVuIHdpdGggdGhlIGNoaXBzIHRoYXQgZG9lcyBub3QNCmhhdmUgcHVzaC1wdWxsIHN1cHBv
cnQuIFRoZSBpc3N1ZSB3YXMgb3JpZ2luYWxseSByZXBvcnRlZCB3aXRoIGFwMTUyICsgcWNhOTg4
eA0KYnkgY29tbXVuaXR5IFsxXS4gSG9wZSB0aGlzIGNvbWJpbmF0aW9uIGlzIGFsc28gY29uc2lk
ZXJlZCBpbiBMRURFLg0KDQotUmFqa3VtYXINCg0KWzFdIC0gaHR0cDovL2xpc3RzLmluZnJhZGVh
ZC5vcmcvcGlwZXJtYWlsL2F0aDEway8yMDE2LUFwcmlsLzAwNzI2Ni5odG1sDQoNCg==

RE: [PATCH] ath10k: Re-enable TXQs for all devices

[Toke Høiland-Jørgensen]

Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:

>> Commit 4ca1807815aa6801aaced7fdefa9edacc2521767 disables the use of the
>> mac80211 TXQs for some devices because of a theoretical throughput
>> regression. We have not seen this regression for a while now, so it shou=
ld be
>> safe to re-enable TXQs.
>>=20
>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>> ---
>> This has been in LEDE trunk for a couple of months now with good results.
>>
> Toke,
>=20=20
> Good to know that the performance drop is not seen with the chips that do=
es not
> have push-pull support. The issue was originally reported with ap152 + qc=
a988x
> by community [1]. Hope this combination is also considered in LEDE.

Ah, was that the original bug report? Thank you, I have not been able to
find that anywhere!

The issue that seems to point to has been fixed a while ago; I'll send
and updated patch with a better commit message (also forgot to cc the
ath10k list, I see).

-Toke

Re: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Dave Taht]

On Thu, Nov 9, 2017 at 4:10 PM, Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke=
.dk> wrote:
> Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:
>
>>> Commit 4ca1807815aa6801aaced7fdefa9edacc2521767 disables the use of the
>>> mac80211 TXQs for some devices because of a theoretical throughput
>>> regression. We have not seen this regression for a while now, so it sho=
uld be
>>> safe to re-enable TXQs.
>>>
>>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>>> ---
>>> This has been in LEDE trunk for a couple of months now with good result=
s.
>>>
>> Toke,
>>
>> Good to know that the performance drop is not seen with the chips that d=
oes not
>> have push-pull support. The issue was originally reported with ap152 + q=
ca988x
>> by community [1]. Hope this combination is also considered in LEDE.
>
> Ah, was that the original bug report? Thank you, I have not been able to
> find that anywhere!
>
> The issue that seems to point to has been fixed a while ago; I'll send
> and updated patch with a better commit message (also forgot to cc the
> ath10k list, I see).
>
> -Toke

Hmm. I remember that thread. I thought we'd basically resolved that
issue (45% of the time spent in fq_codel_drop under udp flood),
back then, with eric adding the batch drop fix to fq_codel itself:

See commit: https://osdn.net/projects/android-x86/scm/git/kernel/commits/9d=
18562a227874289fda8ca5d117d8f503f1dcca

which fixed up the problem beautifully:

https://lists.bufferbloat.net/pipermail/make-wifi-fast/2016-May/000590.html

So if we've been carrying this darn patch for the ath10k vs something
that we'd actually fixed elsewhere in the stack, for over a year,
sigh.

--=20

Dave T=C3=A4ht
CEO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-669-226-2619

Re: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Kalle Valo]

Adding ath10k list to get this discussion to the list archive.

Dave Taht <dave.taht@gmail.com> writes:

> On Thu, Nov 9, 2017 at 4:10 PM, Toke H=C3=B8iland-J=C3=B8rgensen <toke@to=
ke.dk> wrote:
>> Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:
>>
>>>> Commit 4ca1807815aa6801aaced7fdefa9edacc2521767 disables the use of the
>>>> mac80211 TXQs for some devices because of a theoretical throughput
>>>> regression. We have not seen this regression for a while now, so it sh=
ould be
>>>> safe to re-enable TXQs.
>>>>
>>>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>>>> ---
>>>> This has been in LEDE trunk for a couple of months now with good resul=
ts.
>>>>
>>> Toke,
>>>
>>> Good to know that the performance drop is not seen with the chips that =
does not
>>> have push-pull support. The issue was originally reported with ap152 + =
qca988x
>>> by community [1]. Hope this combination is also considered in LEDE.
>>
>> Ah, was that the original bug report? Thank you, I have not been able to
>> find that anywhere!
>>
>> The issue that seems to point to has been fixed a while ago; I'll send
>> and updated patch with a better commit message (also forgot to cc the
>> ath10k list, I see).
>>
>> -Toke
>
> Hmm. I remember that thread. I thought we'd basically resolved that
> issue (45% of the time spent in fq_codel_drop under udp flood),
> back then, with eric adding the batch drop fix to fq_codel itself:
>
> See commit: https://osdn.net/projects/android-x86/scm/git/kernel/commits/=
9d18562a227874289fda8ca5d117d8f503f1dcca
>
> which fixed up the problem beautifully:
>
> https://lists.bufferbloat.net/pipermail/make-wifi-fast/2016-May/000590.ht=
ml
>
> So if we've been carrying this darn patch for the ath10k vs something
> that we'd actually fixed elsewhere in the stack, for over a year,
> sigh.

--=20
Kalle Valo

Re: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Kalle Valo]

Adding ath10k list to get this discussion to the list archive.

Dave Taht <dave.taht@gmail.com> writes:

> On Thu, Nov 9, 2017 at 4:10 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
>> Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:
>>
>>>> Commit 4ca1807815aa6801aaced7fdefa9edacc2521767 disables the use of the
>>>> mac80211 TXQs for some devices because of a theoretical throughput
>>>> regression. We have not seen this regression for a while now, so it should be
>>>> safe to re-enable TXQs.
>>>>
>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>>>> ---
>>>> This has been in LEDE trunk for a couple of months now with good results.
>>>>
>>> Toke,
>>>
>>> Good to know that the performance drop is not seen with the chips that does not
>>> have push-pull support. The issue was originally reported with ap152 + qca988x
>>> by community [1]. Hope this combination is also considered in LEDE.
>>
>> Ah, was that the original bug report? Thank you, I have not been able to
>> find that anywhere!
>>
>> The issue that seems to point to has been fixed a while ago; I'll send
>> and updated patch with a better commit message (also forgot to cc the
>> ath10k list, I see).
>>
>> -Toke
>
> Hmm. I remember that thread. I thought we'd basically resolved that
> issue (45% of the time spent in fq_codel_drop under udp flood),
> back then, with eric adding the batch drop fix to fq_codel itself:
>
> See commit: https://osdn.net/projects/android-x86/scm/git/kernel/commits/9d18562a227874289fda8ca5d117d8f503f1dcca
>
> which fixed up the problem beautifully:
>
> https://lists.bufferbloat.net/pipermail/make-wifi-fast/2016-May/000590.html
>
> So if we've been carrying this darn patch for the ath10k vs something
> that we'd actually fixed elsewhere in the stack, for over a year,
> sigh.

-- 
Kalle Valo

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

RE: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Rajkumar Manoharan]

PiA+DQo+ID4gVGhlIGlzc3VlIHRoYXQgc2VlbXMgdG8gcG9pbnQgdG8gaGFzIGJlZW4gZml4ZWQg
YSB3aGlsZSBhZ287IEknbGwgc2VuZA0KPiA+IGFuZCB1cGRhdGVkIHBhdGNoIHdpdGggYSBiZXR0
ZXIgY29tbWl0IG1lc3NhZ2UgKGFsc28gZm9yZ290IHRvIGNjIHRoZQ0KPiA+IGF0aDEwayBsaXN0
LCBJIHNlZSkuDQo+ID4NCj4gPiAtVG9rZQ0KPiANCj4gSG1tLiBJIHJlbWVtYmVyIHRoYXQgdGhy
ZWFkLiBJIHRob3VnaHQgd2UnZCBiYXNpY2FsbHkgcmVzb2x2ZWQgdGhhdCBpc3N1ZSAoNDUlDQo+
IG9mIHRoZSB0aW1lIHNwZW50IGluIGZxX2NvZGVsX2Ryb3AgdW5kZXIgdWRwIGZsb29kKSwgYmFj
ayB0aGVuLCB3aXRoIGVyaWMgYWRkaW5nDQo+IHRoZSBiYXRjaCBkcm9wIGZpeCB0byBmcV9jb2Rl
bCBpdHNlbGY6DQo+IA0KPiBTZWUgY29tbWl0OiBodHRwczovL29zZG4ubmV0L3Byb2plY3RzL2Fu
ZHJvaWQtDQo+IHg4Ni9zY20vZ2l0L2tlcm5lbC9jb21taXRzLzlkMTg1NjJhMjI3ODc0Mjg5ZmRh
OGNhNWQxMTdkOGY1MDNmMWRjY2ENCj4gDQo+IHdoaWNoIGZpeGVkIHVwIHRoZSBwcm9ibGVtIGJl
YXV0aWZ1bGx5Og0KPiANCj4gaHR0cHM6Ly9saXN0cy5idWZmZXJibG9hdC5uZXQvcGlwZXJtYWls
L21ha2Utd2lmaS1mYXN0LzIwMTYtTWF5LzAwMDU5MC5odG1sDQo+IA0KPiBTbyBpZiB3ZSd2ZSBi
ZWVuIGNhcnJ5aW5nIHRoaXMgZGFybiBwYXRjaCBmb3IgdGhlIGF0aDEwayB2cyBzb21ldGhpbmcg
dGhhdCB3ZSdkDQo+IGFjdHVhbGx5IGZpeGVkIGVsc2V3aGVyZSBpbiB0aGUgc3RhY2ssIGZvciBv
dmVyIGEgeWVhciwgc2lnaC4NCj4NCkRhdmUsDQoNCkFncmVlLi4gRXZlbiBJIGFtIG5vdCBmYW4g
b2YgdGhhdCBwYXRjaCBpbiBhdGgxMGsuIElJUkMgTWljaGFsIHB1c2hlZCB0aGlzDQpjaGFuZ2Ug
YXMgdGVtcCBvbmUgYW5kIHBsYW5uZWQgdG8gcmV2ZXJ0IGl0IG9uY2UgaXQgaXMgZml4ZWQgaW4g
c3RhY2sgb3IgaW4gZHJpdmVyLg0KDQpJIHRob3VnaHQgRXJpYyBjaGFuZ2UgaW4gZnFfY29kZWwg
aXMgbWVhbnQgb25seSBmb3IgZmF0dHkgdWRwIGZsb3dzLiBGb3JnaXZlIG15IGlnbm9yYW5jZS4N
Cg0KLVJhamt1bWFyDQoNCg==

RE: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Rajkumar Manoharan]

> >
> > The issue that seems to point to has been fixed a while ago; I'll send
> > and updated patch with a better commit message (also forgot to cc the
> > ath10k list, I see).
> >
> > -Toke
> 
> Hmm. I remember that thread. I thought we'd basically resolved that issue (45%
> of the time spent in fq_codel_drop under udp flood), back then, with eric adding
> the batch drop fix to fq_codel itself:
> 
> See commit: https://osdn.net/projects/android-
> x86/scm/git/kernel/commits/9d18562a227874289fda8ca5d117d8f503f1dcca
> 
> which fixed up the problem beautifully:
> 
> https://lists.bufferbloat.net/pipermail/make-wifi-fast/2016-May/000590.html
> 
> So if we've been carrying this darn patch for the ath10k vs something that we'd
> actually fixed elsewhere in the stack, for over a year, sigh.
>
Dave,

Agree.. Even I am not fan of that patch in ath10k. IIRC Michal pushed this
change as temp one and planned to revert it once it is fixed in stack or in driver.

I thought Eric change in fq_codel is meant only for fatty udp flows. Forgive my ignorance.

-Rajkumar

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

RE: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Toke Høiland-Jørgensen]

Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:

> Agree.. Even I am not fan of that patch in ath10k. IIRC Michal pushed
> this change as temp one and planned to revert it once it is fixed in
> stack or in driver.
>
> I thought Eric change in fq_codel is meant only for fatty udp flows.
> Forgive my ignorance.

It is, mostly. There's a separate possible issue with TCP performance
that we're looking into, but that is unrelated to TXQs.

-Toke

RE: [Make-wifi-fast] [PATCH] ath10k: Re-enable TXQs for all devices

[Toke Høiland-Jørgensen]

Rajkumar Manoharan <rmanohar@qti.qualcomm.com> writes:

> Agree.. Even I am not fan of that patch in ath10k. IIRC Michal pushed
> this change as temp one and planned to revert it once it is fixed in
> stack or in driver.
>
> I thought Eric change in fq_codel is meant only for fatty udp flows.
> Forgive my ignorance.

It is, mostly. There's a separate possible issue with TCP performance
that we're looking into, but that is unrelated to TXQs.

-Toke

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