| Message ID | 20171109071945.11033-1-toke@toke.dk (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
> 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. -Rajkumar [1] - http://lists.infradead.org/pipermail/ath10k/2016-April/007266.html
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
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.
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.
> > > > 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
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
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);
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(-)