From: Johannes Berg <johannes@sipsolutions.net> To: "Toke Høiland-Jørgensen" <toke@toke.dk>, "Grant Grundler" <grundler@google.com>, wgong@qti.qualcomm.com Cc: wgong@codeaurora.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org Subject: Re: [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips Date: Mon, 03 Sep 2018 13:47:20 +0200 [thread overview] Message-ID: <1535975240.3437.61.camel@sipsolutions.net> (raw) In-Reply-To: <87in3m25uu.fsf@toke.dk> On Mon, 2018-09-03 at 13:11 +0200, Toke Høiland-Jørgensen wrote: > > 6 vs. 8, I think? But I didn't follow the full discussion. Err, I just realized that I was completely wrong - the default, of course, is 10. So smaller values mean more buffering. Most of my argumentation was thus utter garbage :-) > > I also think that we shouldn't necessarily restrict this to "for the > > ath10k". Is there any reason to think that this would be different for > > different chips? > > No, I also think it should be possible to select a value that will work > for all drivers. There's a strong diminishing returns effect here after > a certain point, and I believe we should strongly push back against just > adding more buffering to chase (single-flow) throughput numbers; and I'm > not convinced that having different values for different drivers is > worth the complexity. I think I can see some point in it if the driver requires some buffering for some specific reason? But you'd have to be able to state that reason, I guess, I could imagine having a firmware limitation to need to build two aggregates, or something around MU-MIMO, etc. > As far as I'm concerned, just showing an increase in maximum throughput > under ideal conditions (as this initial patch submission did) is not > sufficient argument for adding more buffering. At a minimum, the > evaluation should also include: > > - Impact on latency and throughput for competing flows in the same test. > - Impact on latency for the TCP flow itself. > - A full evaluation at low rates and in scenarios with more than one > client. That seems reasonable. > As far as the actual value, I *think* it may be that the default shift > should be 7 (8 ms) rather than 8 (4 ms) as it currently is. Going back > and looking at my data from when I submitted the original patch, it > looks like the point of diminishing returns is somewhere between those > two with ath9k (where I did most of my testing), and it seems reasonable > that it could be slightly higher (i.e., more buffering) for ath10k. Grant's data shows a significant difference between 6 and 7 for both latency and throughput: * median tpt - ~241 vs ~201 (both 1 and 5 streams) * median latency - 7.5 vs 6 (1 stream) - 17.3 vs. 16.6 (5 streams) A 20% throughput improvement at <= 1.5ms latency cost seems like a pretty reasonable trade-off? johannes
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Berg <johannes@sipsolutions.net> To: "Toke Høiland-Jørgensen" <toke@toke.dk>, "Grant Grundler" <grundler@google.com>, wgong@qti.qualcomm.com Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, wgong@codeaurora.org Subject: Re: [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips Date: Mon, 03 Sep 2018 13:47:20 +0200 [thread overview] Message-ID: <1535975240.3437.61.camel@sipsolutions.net> (raw) In-Reply-To: <87in3m25uu.fsf@toke.dk> On Mon, 2018-09-03 at 13:11 +0200, Toke Høiland-Jørgensen wrote: > > 6 vs. 8, I think? But I didn't follow the full discussion. Err, I just realized that I was completely wrong - the default, of course, is 10. So smaller values mean more buffering. Most of my argumentation was thus utter garbage :-) > > I also think that we shouldn't necessarily restrict this to "for the > > ath10k". Is there any reason to think that this would be different for > > different chips? > > No, I also think it should be possible to select a value that will work > for all drivers. There's a strong diminishing returns effect here after > a certain point, and I believe we should strongly push back against just > adding more buffering to chase (single-flow) throughput numbers; and I'm > not convinced that having different values for different drivers is > worth the complexity. I think I can see some point in it if the driver requires some buffering for some specific reason? But you'd have to be able to state that reason, I guess, I could imagine having a firmware limitation to need to build two aggregates, or something around MU-MIMO, etc. > As far as I'm concerned, just showing an increase in maximum throughput > under ideal conditions (as this initial patch submission did) is not > sufficient argument for adding more buffering. At a minimum, the > evaluation should also include: > > - Impact on latency and throughput for competing flows in the same test. > - Impact on latency for the TCP flow itself. > - A full evaluation at low rates and in scenarios with more than one > client. That seems reasonable. > As far as the actual value, I *think* it may be that the default shift > should be 7 (8 ms) rather than 8 (4 ms) as it currently is. Going back > and looking at my data from when I submitted the original patch, it > looks like the point of diminishing returns is somewhere between those > two with ath9k (where I did most of my testing), and it seems reasonable > that it could be slightly higher (i.e., more buffering) for ath10k. Grant's data shows a significant difference between 6 and 7 for both latency and throughput: * median tpt - ~241 vs ~201 (both 1 and 5 streams) * median latency - 7.5 vs 6 (1 stream) - 17.3 vs. 16.6 (5 streams) A 20% throughput improvement at <= 1.5ms latency cost seems like a pretty reasonable trade-off? johannes _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k
next prev parent reply other threads:[~2018-09-03 16:07 UTC|newest] Thread overview: 97+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-08 10:40 [PATCH v2 0/2] Change sk_pacing_shift in ieee80211_hw for best tx throughput Wen Gong 2018-08-08 10:40 ` Wen Gong 2018-08-08 10:40 ` [PATCH v2 1/2] mac80211: Change sk_pacing_shift saved to ieee80211_hw Wen Gong 2018-08-08 10:40 ` Wen Gong 2018-08-08 10:40 ` [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips Wen Gong 2018-08-08 10:40 ` Wen Gong 2018-08-08 10:43 ` Toke Høiland-Jørgensen 2018-08-08 10:43 ` Toke Høiland-Jørgensen 2018-08-10 8:05 ` Wen Gong 2018-08-10 8:05 ` Wen Gong 2018-08-10 13:17 ` Toke Høiland-Jørgensen 2018-08-10 13:17 ` Toke Høiland-Jørgensen 2018-08-13 5:37 ` Wen Gong 2018-08-13 5:37 ` Wen Gong 2018-08-13 11:18 ` Toke Høiland-Jørgensen 2018-08-13 11:18 ` Toke Høiland-Jørgensen 2018-08-14 5:55 ` Wen Gong 2018-08-14 5:55 ` Wen Gong 2018-08-17 11:32 ` Toke Høiland-Jørgensen 2018-08-17 11:32 ` Toke Høiland-Jørgensen 2018-08-30 23:25 ` Peter Oh 2018-08-30 23:25 ` Peter Oh 2018-08-31 15:36 ` Toke Høiland-Jørgensen 2018-08-31 15:36 ` Toke Høiland-Jørgensen 2018-08-30 23:32 ` Grant Grundler 2018-09-03 9:38 ` Johannes Berg 2018-09-03 9:38 ` Johannes Berg 2018-09-03 11:11 ` Toke Høiland-Jørgensen 2018-09-03 11:11 ` Toke Høiland-Jørgensen 2018-09-03 11:47 ` Johannes Berg [this message] 2018-09-03 11:47 ` Johannes Berg 2018-09-03 13:35 ` Toke Høiland-Jørgensen 2018-09-03 13:35 ` Toke Høiland-Jørgensen 2018-09-03 14:57 ` Dave Taht 2018-09-03 14:57 ` Dave Taht 2018-09-03 15:35 ` Dave Taht 2018-09-03 15:35 ` Dave Taht 2018-09-04 23:43 ` Grant Grundler 2018-09-04 23:43 ` Grant Grundler 2018-09-05 7:23 ` Wen Gong 2018-09-05 7:23 ` Wen Gong 2018-09-06 10:18 ` Toke Høiland-Jørgensen 2018-09-06 10:18 ` Toke Høiland-Jørgensen 2019-02-20 19:15 ` Grant Grundler 2019-02-20 19:15 ` Grant Grundler 2019-02-21 4:39 ` Kalle Valo 2019-02-21 4:39 ` Kalle Valo 2019-02-21 15:42 ` Toke Høiland-Jørgensen 2019-02-21 15:42 ` Toke Høiland-Jørgensen 2019-02-21 16:10 ` Kalle Valo 2019-02-21 16:10 ` Kalle Valo 2019-02-21 16:22 ` Ben Greear 2019-02-21 16:22 ` Ben Greear 2019-02-21 16:37 ` Toke Høiland-Jørgensen 2019-02-21 16:37 ` Toke Høiland-Jørgensen 2019-02-21 16:57 ` Ben Greear 2019-02-21 16:57 ` Ben Greear 2019-02-21 17:15 ` Toke Høiland-Jørgensen 2019-02-21 17:15 ` Toke Høiland-Jørgensen 2019-02-21 17:29 ` [PATCH] mac80211: Change default tx_sk_pacing_shift to 7 Toke Høiland-Jørgensen 2019-02-21 17:29 ` Toke Høiland-Jørgensen 2019-02-22 12:29 ` Johannes Berg 2019-02-22 13:06 ` Toke Høiland-Jørgensen 2019-02-22 13:06 ` Toke Høiland-Jørgensen 2019-02-22 13:07 ` Johannes Berg 2019-02-22 13:07 ` Johannes Berg 2019-02-22 13:40 ` Toke Høiland-Jørgensen 2019-02-22 13:40 ` Toke Høiland-Jørgensen 2019-02-22 19:10 ` Johannes Berg 2019-02-22 19:10 ` Johannes Berg 2019-02-23 11:49 ` Toke Høiland-Jørgensen 2019-02-23 11:49 ` Toke Høiland-Jørgensen 2019-02-21 17:29 ` [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips Ben Greear 2019-02-21 17:29 ` Ben Greear 2019-02-21 22:50 ` Toke Høiland-Jørgensen 2019-02-21 22:50 ` Toke Høiland-Jørgensen 2019-02-21 16:28 ` Toke Høiland-Jørgensen 2019-02-21 16:28 ` Toke Høiland-Jørgensen 2020-04-23 6:31 ` Kalle Valo 2020-04-23 6:31 ` Kalle Valo 2018-08-08 19:00 ` [PATCH v2 0/2] Change sk_pacing_shift in ieee80211_hw for best tx throughput Peter Oh 2018-08-08 19:00 ` Peter Oh 2018-08-09 9:32 ` Arend van Spriel 2018-08-09 9:32 ` Arend van Spriel 2018-08-10 13:20 ` Toke Høiland-Jørgensen 2018-08-10 13:20 ` Toke Høiland-Jørgensen 2018-08-10 19:28 ` Arend van Spriel 2018-08-10 19:28 ` Arend van Spriel 2018-08-10 19:52 ` Ben Greear 2018-08-10 19:52 ` Ben Greear 2018-08-11 19:21 ` Arend van Spriel 2018-08-11 19:21 ` Arend van Spriel 2018-08-20 12:46 ` Toke Høiland-Jørgensen 2018-08-20 12:46 ` Toke Høiland-Jørgensen 2018-08-20 15:14 ` Ben Greear 2018-08-20 15:14 ` Ben Greear 2018-09-03 18:36 [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips Kan Yan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1535975240.3437.61.camel@sipsolutions.net \ --to=johannes@sipsolutions.net \ --cc=ath10k@lists.infradead.org \ --cc=grundler@google.com \ --cc=linux-wireless@vger.kernel.org \ --cc=toke@toke.dk \ --cc=wgong@codeaurora.org \ --cc=wgong@qti.qualcomm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.