* [PATCH] mac80211: minstrel: Scale expected_throughput by 10
@ 2016-02-01 12:24 Sven Eckelmann
2016-02-01 12:30 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2016-02-01 12:24 UTC (permalink / raw)
To: linux-wireless; +Cc: thomas, antonio, nbd, johannes, Sven Eckelmann
The change from cur_tp to the function
minstrel_get_tp_avg/minstrel_ht_get_tp_avg changed the unit used for the
current throughtput. For example in minstrel_ht the correct
conversion between them would be:
mrs->cur_tp / 10 == minstrel_ht_get_tp_avg(..).
This factor 10 must also be included in the calculation of
minstrel_get_expected_throughput and minstrel_ht_get_expected_throughput to
get similar results as before the change.
Fixes: 6a27b2c40b48 ("mac80211: restructure per-rate throughput calculation into function")
Signed-off-by: Sven Eckelmann <sven@open-mesh.com>
---
net/mac80211/rc80211_minstrel.c | 2 +-
net/mac80211/rc80211_minstrel_ht.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 3ece7d1..b54f398 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -711,7 +711,7 @@ static u32 minstrel_get_expected_throughput(void *priv_sta)
* computing cur_tp
*/
tmp_mrs = &mi->r[idx].stats;
- tmp_cur_tp = minstrel_get_tp_avg(&mi->r[idx], tmp_mrs->prob_ewma);
+ tmp_cur_tp = minstrel_get_tp_avg(&mi->r[idx], tmp_mrs->prob_ewma) * 10;
tmp_cur_tp = tmp_cur_tp * 1200 * 8 / 1024;
return tmp_cur_tp;
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 3928dbd..8928151 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1334,7 +1334,8 @@ static u32 minstrel_ht_get_expected_throughput(void *priv_sta)
prob = mi->groups[i].rates[j].prob_ewma;
/* convert tp_avg from pkt per second in kbps */
- tp_avg = minstrel_ht_get_tp_avg(mi, i, j, prob) * AVG_PKT_SIZE * 8 / 1024;
+ tp_avg = minstrel_ht_get_tp_avg(mi, i, j, prob) * 10;
+ tp_avg *= AVG_PKT_SIZE * 8 / 1024;
return tp_avg;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] mac80211: minstrel: Scale expected_throughput by 10
2016-02-01 12:24 [PATCH] mac80211: minstrel: Scale expected_throughput by 10 Sven Eckelmann
@ 2016-02-01 12:30 ` Johannes Berg
2016-02-01 12:56 ` Sven Eckelmann
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2016-02-01 12:30 UTC (permalink / raw)
To: Sven Eckelmann, linux-wireless; +Cc: thomas, antonio, nbd
On Mon, 2016-02-01 at 13:24 +0100, Sven Eckelmann wrote:
> The change from cur_tp to the function
> minstrel_get_tp_avg/minstrel_ht_get_tp_avg changed the unit used for
> the
> current throughtput. For example in minstrel_ht the correct
> conversion between them would be:
>
> mrs->cur_tp / 10 == minstrel_ht_get_tp_avg(..).
>
> This factor 10 must also be included in the calculation of
> minstrel_get_expected_throughput and
> minstrel_ht_get_expected_throughput to
> get similar results as before the change.
>
10 is a pretty expensive factor, perhaps that should use 16 instead?
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrel: Scale expected_throughput by 10
2016-02-01 12:30 ` Johannes Berg
@ 2016-02-01 12:56 ` Sven Eckelmann
2016-02-01 12:57 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2016-02-01 12:56 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, thomas, antonio, nbd
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
On Monday 01 February 2016 13:30:37 Johannes Berg wrote:
> On Mon, 2016-02-01 at 13:24 +0100, Sven Eckelmann wrote:
> > The change from cur_tp to the function
> > minstrel_get_tp_avg/minstrel_ht_get_tp_avg changed the unit used for
> > the
> > current throughtput. For example in minstrel_ht the correct
> > conversion between them would be:
> >
> > mrs->cur_tp / 10 == minstrel_ht_get_tp_avg(..).
> >
> > This factor 10 must also be included in the calculation of
> > minstrel_get_expected_throughput and
> > minstrel_ht_get_expected_throughput to
> > get similar results as before the change.
> >
>
> 10 is a pretty expensive factor, perhaps that should use 16 instead?
Not really funny but I will change the title.
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrel: Scale expected_throughput by 10
2016-02-01 12:56 ` Sven Eckelmann
@ 2016-02-01 12:57 ` Johannes Berg
2016-02-01 12:58 ` Antonio Quartulli
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2016-02-01 12:57 UTC (permalink / raw)
To: Sven Eckelmann; +Cc: linux-wireless, thomas, antonio, nbd
On Mon, 2016-02-01 at 13:56 +0100, Sven Eckelmann wrote:
> On Monday 01 February 2016 13:30:37 Johannes Berg wrote:
> > On Mon, 2016-02-01 at 13:24 +0100, Sven Eckelmann wrote:
> > > The change from cur_tp to the function
> > > minstrel_get_tp_avg/minstrel_ht_get_tp_avg changed the unit used
> > > for
> > > the
> > > current throughtput. For example in minstrel_ht the correct
> > > conversion between them would be:
> > >
> > > mrs->cur_tp / 10 == minstrel_ht_get_tp_avg(..).
> > >
> > > This factor 10 must also be included in the calculation of
> > > minstrel_get_expected_throughput and
> > > minstrel_ht_get_expected_throughput to
> > > get similar results as before the change.
> > >
> >
> > 10 is a pretty expensive factor, perhaps that should use 16
> > instead?
>
> Not really funny but I will change the title.
Huh? Not sure what you mean. I really meant to change 10 to 16 overall
as far as the factor is concerned, since division by/multiplication
with 16 is far easier in base 2 than by/with 10.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrel: Scale expected_throughput by 10
2016-02-01 12:57 ` Johannes Berg
@ 2016-02-01 12:58 ` Antonio Quartulli
2016-02-01 13:01 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2016-02-01 12:58 UTC (permalink / raw)
To: Johannes Berg; +Cc: Sven Eckelmann, linux-wireless, thomas, nbd
[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]
On Mon, Feb 01, 2016 at 01:57:53PM +0100, Johannes Berg wrote:
> On Mon, 2016-02-01 at 13:56 +0100, Sven Eckelmann wrote:
> > On Monday 01 February 2016 13:30:37 Johannes Berg wrote:
> > > On Mon, 2016-02-01 at 13:24 +0100, Sven Eckelmann wrote:
> > > > The change from cur_tp to the function
> > > > minstrel_get_tp_avg/minstrel_ht_get_tp_avg changed the unit used
> > > > for
> > > > the
> > > > current throughtput. For example in minstrel_ht the correct
> > > > conversion between them would be:
> > > >
> > > > mrs->cur_tp / 10 == minstrel_ht_get_tp_avg(..).
> > > >
> > > > This factor 10 must also be included in the calculation of
> > > > minstrel_get_expected_throughput and
> > > > minstrel_ht_get_expected_throughput to
> > > > get similar results as before the change.
> > > >
> > >
> > > 10 is a pretty expensive factor, perhaps that should use 16
> > > instead?
> >
> > Not really funny but I will change the title.
>
> Huh? Not sure what you mean. I really meant to change 10 to 16 overall
> as far as the factor is concerned, since division by/multiplication
> with 16 is far easier in base 2 than by/with 10.
Should we first fix the bug introduced by 6a27b2c40b48 and then (with another
patch) improve this by changing the factor from 10 to 16 ?
The latter is going to be a bigger change because we need to make sure that the
value exported via station_info gets still scaled to kbps.
Cheers,
--
Antonio Quartulli
Director of Embedded Software | Open Mesh, Inc.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrel: Scale expected_throughput by 10
2016-02-01 12:58 ` Antonio Quartulli
@ 2016-02-01 13:01 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2016-02-01 13:01 UTC (permalink / raw)
To: Antonio Quartulli; +Cc: Sven Eckelmann, linux-wireless, thomas, nbd
On Mon, 2016-02-01 at 20:58 +0800, Antonio Quartulli wrote:
>
> Should we first fix the bug introduced by 6a27b2c40b48 and then (with
> another
> patch) improve this by changing the factor from 10 to 16 ?
> The latter is going to be a bigger change because we need to make
> sure that the
> value exported via station_info gets still scaled to kbps.
>
Ok, I guess that makes sense.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-01 13:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-01 12:24 [PATCH] mac80211: minstrel: Scale expected_throughput by 10 Sven Eckelmann
2016-02-01 12:30 ` Johannes Berg
2016-02-01 12:56 ` Sven Eckelmann
2016-02-01 12:57 ` Johannes Berg
2016-02-01 12:58 ` Antonio Quartulli
2016-02-01 13:01 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).