| Message ID | 1454329450-31527-1-git-send-email-sven@open-mesh.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Johannes Berg |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }
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(-) -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html