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