diff mbox series

mac80211: Change default tx_sk_pacing_shift to 7

Message ID 20190221172936.21816-1-toke@redhat.com (mailing list archive)
State Accepted
Commit a41f56b9a17a50f22992dbdf0e8fcf0c3a43cf33
Delegated to: Johannes Berg
Headers show
Series mac80211: Change default tx_sk_pacing_shift to 7 | expand

Commit Message

Toke Høiland-Jørgensen Feb. 21, 2019, 5:29 p.m. UTC
When we did the original tests for the optimal value of sk_pacing_shift, we
came up with 6 ms of buffering as the default. Sadly, 6 is not a power of
two, so when picking the shift value I erred on the size of less buffering
and picked 4 ms instead of 8. This was probably wrong; those 2 ms of extra
buffering makes a larger difference than I thought.

So, change the default pacing shift to 7, which corresponds to 8 ms of
buffering. The point of diminishing returns really kicks in after 8 ms, and
so having this as a default should cut down on the need for extensive
per-device testing and overrides needed in the drivers.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 net/mac80211/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Johannes Berg Feb. 22, 2019, 12:29 p.m. UTC | #1
Toke Høiland-Jørgensen wrote:

> When we did the original tests for the optimal value of sk_pacing_shift, we
> came up with 6 ms of buffering as the default. Sadly, 6 is not a power of
> two, so when picking the shift value I erred on the size of less buffering
> and picked 4 ms instead of 8. This was probably wrong; those 2 ms of extra
> buffering makes a larger difference than I thought.
> 
> So, change the default pacing shift to 7, which corresponds to 8 ms of
> buffering. The point of diminishing returns really kicks in after 8 ms, and
> so having this as a default should cut down on the need for extensive
> per-device testing and overrides needed in the drivers.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

Patch applied to wireless.git, thanks.

a41f56b9a17a (HEAD -> mac80211) mac80211: Change default tx_sk_pacing_shift to 7
Toke Høiland-Jørgensen Feb. 22, 2019, 1:06 p.m. UTC | #2
Johannes Berg <johannes@sipsolutions.net> writes:

> Toke Høiland-Jørgensen wrote:
>
>> When we did the original tests for the optimal value of sk_pacing_shift, we
>> came up with 6 ms of buffering as the default. Sadly, 6 is not a power of
>> two, so when picking the shift value I erred on the size of less buffering
>> and picked 4 ms instead of 8. This was probably wrong; those 2 ms of extra
>> buffering makes a larger difference than I thought.
>> 
>> So, change the default pacing shift to 7, which corresponds to 8 ms of
>> buffering. The point of diminishing returns really kicks in after 8 ms, and
>> so having this as a default should cut down on the need for extensive
>> per-device testing and overrides needed in the drivers.
>> 
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>
> Patch applied to wireless.git, thanks.
>
> a41f56b9a17a (HEAD -> mac80211) mac80211: Change default
> tx_sk_pacing_shift to 7

Cool, thanks! What's the easiest way to backport this? I figure it's
easier to just update sk_pacing_shift_update() in tx.c for 4.19 (which
predates the addition of the driver override hook); shall I just send a
separate patch to stable for that? Or do we need to backport the driver
override hook as well?

-Toke
Johannes Berg Feb. 22, 2019, 1:07 p.m. UTC | #3
On Fri, 2019-02-22 at 14:06 +0100, Toke Høiland-Jørgensen wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > Toke Høiland-Jørgensen wrote:
> > 
> > > When we did the original tests for the optimal value of sk_pacing_shift, we
> > > came up with 6 ms of buffering as the default. Sadly, 6 is not a power of
> > > two, so when picking the shift value I erred on the size of less buffering
> > > and picked 4 ms instead of 8. This was probably wrong; those 2 ms of extra
> > > buffering makes a larger difference than I thought.
> > > 
> > > So, change the default pacing shift to 7, which corresponds to 8 ms of
> > > buffering. The point of diminishing returns really kicks in after 8 ms, and
> > > so having this as a default should cut down on the need for extensive
> > > per-device testing and overrides needed in the drivers.
> > > 
> > > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > 
> > Patch applied to wireless.git, thanks.
> > 
> > a41f56b9a17a (HEAD -> mac80211) mac80211: Change default
> > tx_sk_pacing_shift to 7

This mess came from Kalle's tool btw, so I can't really use it yet :-)

> Cool, thanks! What's the easiest way to backport this? I figure it's
> easier to just update sk_pacing_shift_update() in tx.c for 4.19 (which
> predates the addition of the driver override hook); shall I just send a
> separate patch to stable for that? Or do we need to backport the driver
> override hook as well?

Just update the value, no need to backport the hook.

johannes
Toke Høiland-Jørgensen Feb. 22, 2019, 1:40 p.m. UTC | #4
Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2019-02-22 at 14:06 +0100, Toke Høiland-Jørgensen wrote:
>> Johannes Berg <johannes@sipsolutions.net> writes:
>> 
>> > Toke Høiland-Jørgensen wrote:
>> > 
>> > > When we did the original tests for the optimal value of sk_pacing_shift, we
>> > > came up with 6 ms of buffering as the default. Sadly, 6 is not a power of
>> > > two, so when picking the shift value I erred on the size of less buffering
>> > > and picked 4 ms instead of 8. This was probably wrong; those 2 ms of extra
>> > > buffering makes a larger difference than I thought.
>> > > 
>> > > So, change the default pacing shift to 7, which corresponds to 8 ms of
>> > > buffering. The point of diminishing returns really kicks in after 8 ms, and
>> > > so having this as a default should cut down on the need for extensive
>> > > per-device testing and overrides needed in the drivers.
>> > > 
>> > > Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> > 
>> > Patch applied to wireless.git, thanks.
>> > 
>> > a41f56b9a17a (HEAD -> mac80211) mac80211: Change default
>> > tx_sk_pacing_shift to 7
>
> This mess came from Kalle's tool btw, so I can't really use it yet :-)
>
>> Cool, thanks! What's the easiest way to backport this? I figure it's
>> easier to just update sk_pacing_shift_update() in tx.c for 4.19 (which
>> predates the addition of the driver override hook); shall I just send a
>> separate patch to stable for that? Or do we need to backport the driver
>> override hook as well?
>
> Just update the value, no need to backport the hook.

And send it directly to stable, or does it need to go through you?

-Toke
Johannes Berg Feb. 22, 2019, 7:10 p.m. UTC | #5
On Fri, 2019-02-22 at 14:40 +0100, Toke Høiland-Jørgensen wrote:
> 
> And send it directly to stable, or does it need to go through you?

I added a Cc: stable tag. So all you really need to do is wait for the
emails telling you it failed to apply, and handle accordingly :)

johannes
Toke Høiland-Jørgensen Feb. 23, 2019, 11:49 a.m. UTC | #6
Johannes Berg <johannes@sipsolutions.net> writes:

> On Fri, 2019-02-22 at 14:40 +0100, Toke Høiland-Jørgensen wrote:
>> 
>> And send it directly to stable, or does it need to go through you?
>
> I added a Cc: stable tag. So all you really need to do is wait for the
> emails telling you it failed to apply, and handle accordingly :)

Cool, that I can do; thanks! :)

-Toke
diff mbox series

Patch

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 5055aeba5c5a..800e67615e2a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -617,13 +617,13 @@  struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
 	 * We need a bit of data queued to build aggregates properly, so
 	 * instruct the TCP stack to allow more than a single ms of data
 	 * to be queued in the stack. The value is a bit-shift of 1
-	 * second, so 8 is ~4ms of queued data. Only affects local TCP
+	 * second, so 7 is ~8ms of queued data. Only affects local TCP
 	 * sockets.
 	 * This is the default, anyhow - drivers may need to override it
 	 * for local reasons (longer buffers, longer completion time, or
 	 * similar).
 	 */
-	local->hw.tx_sk_pacing_shift = 8;
+	local->hw.tx_sk_pacing_shift = 7;
 
 	/* set up some defaults */
 	local->hw.queues = 1;