diff mbox series

[1/6] mac80211: minstrel_ht: use bitfields to encode rate indexes

Message ID 20210124122812.49929-1-nbd@nbd.name
State Superseded
Headers show
Series [1/6] mac80211: minstrel_ht: use bitfields to encode rate indexes | expand

Commit Message

Felix Fietkau Jan. 24, 2021, 12:28 p.m. UTC
Get rid of a lot of divisions and modulo operations
Reduces code size and improves performance

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/rc80211_minstrel_ht.c         | 123 +++++++++++----------
 net/mac80211/rc80211_minstrel_ht.h         |  13 +++
 net/mac80211/rc80211_minstrel_ht_debugfs.c |   4 +-
 3 files changed, 78 insertions(+), 62 deletions(-)

Comments

kernel test robot Jan. 25, 2021, 7:25 p.m. UTC | #1
Hi Felix,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on next-20210125]
[cannot apply to mac80211/master v5.11-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Felix-Fietkau/mac80211-minstrel_ht-use-bitfields-to-encode-rate-indexes/20210125-210635
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: x86_64-randconfig-a015-20210125 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 12d0753aca22896fda2cf76781b0ee0524d55065)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/8a2d01887013f71806262345f71766d6518e3552
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Felix-Fietkau/mac80211-minstrel_ht-use-bitfields-to-encode-rate-indexes/20210125-210635
        git checkout 8a2d01887013f71806262345f71766d6518e3552
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/mac80211/rc80211_minstrel_ht.c:862:11: warning: variable 'group' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]

           else if (mi->supported[MINSTREL_OFDM_GROUP])
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mac80211/rc80211_minstrel_ht.c:865:18: note: uninitialized use occurs here
           index = MI_RATE(group, 0);
                           ^~~~~
   net/mac80211/rc80211_minstrel_ht.h:66:34: note: expanded from macro 'MI_RATE'
           (FIELD_PREP(MI_RATE_GROUP_MASK, _group) |       \
                                           ^~~~~~
   include/linux/bitfield.h:95:20: note: expanded from macro 'FIELD_PREP'
                   ((typeof(_mask))(_val) << __bf_shf(_mask)) & (_mask);   \
                                    ^~~~
   net/mac80211/rc80211_minstrel_ht.c:862:7: note: remove the 'if' if its condition is always true
           else if (mi->supported[MINSTREL_OFDM_GROUP])
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mac80211/rc80211_minstrel_ht.c:825:11: note: initialize the variable 'group' to silence this warning
           int group, i, j, cur_prob;
                    ^
                     = 0
   1 warning generated.


vim +862 net/mac80211/rc80211_minstrel_ht.c

cbda98c710d273b Felix Fietkau 2021-01-15  809  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  810  /*
ec8aa669b8393b6 Felix Fietkau 2010-05-13  811   * Update rate statistics and select new primary rates
ec8aa669b8393b6 Felix Fietkau 2010-05-13  812   *
ec8aa669b8393b6 Felix Fietkau 2010-05-13  813   * Rules for rate selection:
ec8aa669b8393b6 Felix Fietkau 2010-05-13  814   *  - max_prob_rate must use only one stream, as a tradeoff between delivery
ec8aa669b8393b6 Felix Fietkau 2010-05-13  815   *    probability and throughput during strong fluctuations
5935839ad735837 Thomas Huehn  2014-09-09  816   *  - as long as the max prob rate has a probability of more than 75%, pick
ec8aa669b8393b6 Felix Fietkau 2010-05-13  817   *    higher throughput rates, even if the probablity is a bit lower
ec8aa669b8393b6 Felix Fietkau 2010-05-13  818   */
ec8aa669b8393b6 Felix Fietkau 2010-05-13  819  static void
48cb39522a9d4d4 Felix Fietkau 2019-08-20  820  minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
48cb39522a9d4d4 Felix Fietkau 2019-08-20  821  			 bool sample)
ec8aa669b8393b6 Felix Fietkau 2010-05-13  822  {
ec8aa669b8393b6 Felix Fietkau 2010-05-13  823  	struct minstrel_mcs_group_data *mg;
9134073bc693633 Thomas Huehn  2015-03-24  824  	struct minstrel_rate_stats *mrs;
50e55a8ea76fb59 Thomas Huehn  2015-03-24  825  	int group, i, j, cur_prob;
d4d141cae804a43 Karl Beldan   2014-10-20  826  	u16 tmp_mcs_tp_rate[MAX_THR_RATES], tmp_group_tp_rate[MAX_THR_RATES];
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  827  	u16 tmp_legacy_tp_rate[MAX_THR_RATES], tmp_max_prob_rate;
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  828  	u16 index;
a7844a53846017c Felix Fietkau 2021-01-15  829  	bool ht_supported = mi->sta->ht_cap.ht_supported;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  830  
48cb39522a9d4d4 Felix Fietkau 2019-08-20  831  	mi->sample_mode = MINSTREL_SAMPLE_IDLE;
48cb39522a9d4d4 Felix Fietkau 2019-08-20  832  
48cb39522a9d4d4 Felix Fietkau 2019-08-20  833  	if (sample) {
48cb39522a9d4d4 Felix Fietkau 2019-08-20  834  		mi->total_packets_cur = mi->total_packets -
48cb39522a9d4d4 Felix Fietkau 2019-08-20  835  					mi->total_packets_last;
48cb39522a9d4d4 Felix Fietkau 2019-08-20  836  		mi->total_packets_last = mi->total_packets;
48cb39522a9d4d4 Felix Fietkau 2019-08-20  837  	}
48cb39522a9d4d4 Felix Fietkau 2019-08-20  838  	if (!mp->sample_switch)
48cb39522a9d4d4 Felix Fietkau 2019-08-20  839  		sample = false;
48cb39522a9d4d4 Felix Fietkau 2019-08-20  840  	if (mi->total_packets_cur < SAMPLE_SWITCH_THR && mp->sample_switch != 1)
48cb39522a9d4d4 Felix Fietkau 2019-08-20  841  	    sample = false;
48cb39522a9d4d4 Felix Fietkau 2019-08-20  842  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  843  	if (mi->ampdu_packets > 0) {
77f7ffdc335de85 Felix Fietkau 2019-01-16  844  		if (!ieee80211_hw_check(mp->hw, TX_STATUS_NO_AMPDU_LEN))
ec8aa669b8393b6 Felix Fietkau 2010-05-13  845  			mi->avg_ampdu_len = minstrel_ewma(mi->avg_ampdu_len,
77f7ffdc335de85 Felix Fietkau 2019-01-16  846  				MINSTREL_FRAC(mi->ampdu_len, mi->ampdu_packets),
77f7ffdc335de85 Felix Fietkau 2019-01-16  847  					      EWMA_LEVEL);
77f7ffdc335de85 Felix Fietkau 2019-01-16  848  		else
77f7ffdc335de85 Felix Fietkau 2019-01-16  849  			mi->avg_ampdu_len = 0;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  850  		mi->ampdu_len = 0;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  851  		mi->ampdu_packets = 0;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  852  	}
ec8aa669b8393b6 Felix Fietkau 2010-05-13  853  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  854  	mi->sample_slow = 0;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  855  	mi->sample_count = 0;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  856  
21f7981b4bd9048 Felix Fietkau 2019-08-20  857  	memset(tmp_mcs_tp_rate, 0, sizeof(tmp_mcs_tp_rate));
a7844a53846017c Felix Fietkau 2021-01-15  858  	memset(tmp_legacy_tp_rate, 0, sizeof(tmp_legacy_tp_rate));
8a2d01887013f71 Felix Fietkau 2021-01-24  859  
21f7981b4bd9048 Felix Fietkau 2019-08-20  860  	if (mi->supported[MINSTREL_CCK_GROUP])
8a2d01887013f71 Felix Fietkau 2021-01-24  861  		group = MINSTREL_CCK_GROUP;
a7844a53846017c Felix Fietkau 2021-01-15 @862  	else if (mi->supported[MINSTREL_OFDM_GROUP])
8a2d01887013f71 Felix Fietkau 2021-01-24  863  		group = MINSTREL_OFDM_GROUP;
8a2d01887013f71 Felix Fietkau 2021-01-24  864  
8a2d01887013f71 Felix Fietkau 2021-01-24  865  	index = MI_RATE(group, 0);
a7844a53846017c Felix Fietkau 2021-01-15  866  	for (j = 0; j < ARRAY_SIZE(tmp_legacy_tp_rate); j++)
8a2d01887013f71 Felix Fietkau 2021-01-24  867  		tmp_legacy_tp_rate[j] = index;
21f7981b4bd9048 Felix Fietkau 2019-08-20  868  
21f7981b4bd9048 Felix Fietkau 2019-08-20  869  	if (mi->supported[MINSTREL_VHT_GROUP_0])
8a2d01887013f71 Felix Fietkau 2021-01-24  870  		group = MINSTREL_VHT_GROUP_0;
a7844a53846017c Felix Fietkau 2021-01-15  871  	else if (ht_supported)
8a2d01887013f71 Felix Fietkau 2021-01-24  872  		group = MINSTREL_HT_GROUP_0;
a7844a53846017c Felix Fietkau 2021-01-15  873  	else if (mi->supported[MINSTREL_CCK_GROUP])
8a2d01887013f71 Felix Fietkau 2021-01-24  874  		group = MINSTREL_CCK_GROUP;
a7844a53846017c Felix Fietkau 2021-01-15  875  	else
8a2d01887013f71 Felix Fietkau 2021-01-24  876  		group = MINSTREL_OFDM_GROUP;
21f7981b4bd9048 Felix Fietkau 2019-08-20  877  
8a2d01887013f71 Felix Fietkau 2021-01-24  878  	index = MI_RATE(group, 0);
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  879  	tmp_max_prob_rate = index;
21f7981b4bd9048 Felix Fietkau 2019-08-20  880  	for (j = 0; j < ARRAY_SIZE(tmp_mcs_tp_rate); j++)
21f7981b4bd9048 Felix Fietkau 2019-08-20  881  		tmp_mcs_tp_rate[j] = index;
c2eb5b0f342c9b1 Karl Beldan   2013-04-18  882  
5935839ad735837 Thomas Huehn  2014-09-09  883  	/* Find best rate sets within all MCS groups*/
5935839ad735837 Thomas Huehn  2014-09-09  884  	for (group = 0; group < ARRAY_SIZE(minstrel_mcs_groups); group++) {
a7844a53846017c Felix Fietkau 2021-01-15  885  		u16 *tp_rate = tmp_mcs_tp_rate;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  886  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  887  		mg = &mi->groups[group];
41d085835d3d225 Felix Fietkau 2016-12-14  888  		if (!mi->supported[group])
ec8aa669b8393b6 Felix Fietkau 2010-05-13  889  			continue;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  890  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  891  		mi->sample_count++;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  892  
5935839ad735837 Thomas Huehn  2014-09-09  893  		/* (re)Initialize group rate indexes */
5935839ad735837 Thomas Huehn  2014-09-09  894  		for(j = 0; j < MAX_THR_RATES; j++)
8a2d01887013f71 Felix Fietkau 2021-01-24  895  			tmp_group_tp_rate[j] = MI_RATE(group, 0);
5935839ad735837 Thomas Huehn  2014-09-09  896  
a7844a53846017c Felix Fietkau 2021-01-15  897  		if (group == MINSTREL_CCK_GROUP && ht_supported)
a7844a53846017c Felix Fietkau 2021-01-15  898  			tp_rate = tmp_legacy_tp_rate;
a7844a53846017c Felix Fietkau 2021-01-15  899  
ec8aa669b8393b6 Felix Fietkau 2010-05-13  900  		for (i = 0; i < MCS_GROUP_RATES; i++) {
41d085835d3d225 Felix Fietkau 2016-12-14  901  			if (!(mi->supported[group] & BIT(i)))
ec8aa669b8393b6 Felix Fietkau 2010-05-13  902  				continue;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  903  
8a2d01887013f71 Felix Fietkau 2021-01-24  904  			index = MI_RATE(group, i);
351df099721e02e Karl Beldan   2013-11-13  905  
9134073bc693633 Thomas Huehn  2015-03-24  906  			mrs = &mg->rates[i];
9134073bc693633 Thomas Huehn  2015-03-24  907  			mrs->retry_updated = false;
cbda98c710d273b Felix Fietkau 2021-01-15  908  			minstrel_ht_calc_rate_stats(mp, mrs);
5f63afe0288d955 Felix Fietkau 2019-10-08  909  			cur_prob = mrs->prob_avg;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  910  
50e55a8ea76fb59 Thomas Huehn  2015-03-24  911  			if (minstrel_ht_get_tp_avg(mi, group, i, cur_prob) == 0)
ec8aa669b8393b6 Felix Fietkau 2010-05-13  912  				continue;
ec8aa669b8393b6 Felix Fietkau 2010-05-13  913  
5935839ad735837 Thomas Huehn  2014-09-09  914  			/* Find max throughput rate set */
a7844a53846017c Felix Fietkau 2021-01-15  915  			minstrel_ht_sort_best_tp_rates(mi, index, tp_rate);
ec8aa669b8393b6 Felix Fietkau 2010-05-13  916  
5935839ad735837 Thomas Huehn  2014-09-09  917  			/* Find max throughput rate set within a group */
5935839ad735837 Thomas Huehn  2014-09-09  918  			minstrel_ht_sort_best_tp_rates(mi, index,
5935839ad735837 Thomas Huehn  2014-09-09  919  						       tmp_group_tp_rate);
ec8aa669b8393b6 Felix Fietkau 2010-05-13  920  		}
ec8aa669b8393b6 Felix Fietkau 2010-05-13  921  
5935839ad735837 Thomas Huehn  2014-09-09  922  		memcpy(mg->max_group_tp_rate, tmp_group_tp_rate,
5935839ad735837 Thomas Huehn  2014-09-09  923  		       sizeof(mg->max_group_tp_rate));
ec8aa669b8393b6 Felix Fietkau 2010-05-13  924  	}
ec8aa669b8393b6 Felix Fietkau 2010-05-13  925  
5935839ad735837 Thomas Huehn  2014-09-09  926  	/* Assign new rate set per sta */
a7844a53846017c Felix Fietkau 2021-01-15  927  	minstrel_ht_assign_best_tp_rates(mi, tmp_mcs_tp_rate,
a7844a53846017c Felix Fietkau 2021-01-15  928  					 tmp_legacy_tp_rate);
5935839ad735837 Thomas Huehn  2014-09-09  929  	memcpy(mi->max_tp_rate, tmp_mcs_tp_rate, sizeof(mi->max_tp_rate));
ec8aa669b8393b6 Felix Fietkau 2010-05-13  930  
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  931  	for (group = 0; group < ARRAY_SIZE(minstrel_mcs_groups); group++) {
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  932  		if (!mi->supported[group])
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  933  			continue;
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  934  
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  935  		mg = &mi->groups[group];
8a2d01887013f71 Felix Fietkau 2021-01-24  936  		mg->max_group_prob_rate = MI_RATE(group, 0);
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  937  
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  938  		for (i = 0; i < MCS_GROUP_RATES; i++) {
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  939  			if (!(mi->supported[group] & BIT(i)))
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  940  				continue;
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  941  
8a2d01887013f71 Felix Fietkau 2021-01-24  942  			index = MI_RATE(group, i);
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  943  
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  944  			/* Find max probability rate per group and global */
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  945  			minstrel_ht_set_best_prob_rate(mi, &tmp_max_prob_rate,
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  946  						       index);
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  947  		}
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  948  	}
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  949  
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  950  	mi->max_prob_rate = tmp_max_prob_rate;
a7fca4e4037f7e3 Felix Fietkau 2021-01-15  951  
5935839ad735837 Thomas Huehn  2014-09-09  952  	/* Try to increase robustness of max_prob_rate*/
5935839ad735837 Thomas Huehn  2014-09-09  953  	minstrel_ht_prob_rate_reduce_streams(mi);
a299c6d591f8f6a Felix Fietkau 2013-03-02  954  
eeafcb0c80c81d6 Felix Fietkau 2021-01-15  955  	/* try to sample half of all available rates during each interval */
eeafcb0c80c81d6 Felix Fietkau 2021-01-15  956  	mi->sample_count *= 4;
a299c6d591f8f6a Felix Fietkau 2013-03-02  957  
48cb39522a9d4d4 Felix Fietkau 2019-08-20  958  	if (sample)
48cb39522a9d4d4 Felix Fietkau 2019-08-20  959  		minstrel_ht_rate_sample_switch(mp, mi);
48cb39522a9d4d4 Felix Fietkau 2019-08-20  960  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Felix Fietkau Jan. 25, 2021, 8:46 p.m. UTC | #2
On 2021-01-25 12:56, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:

> 

>> The biggest flaw in current minstrel_ht is the fact that it needs way too

>> many probing packets to be able to quickly find the best rate.

>> Depending on the wifi hardware and operating mode, this can significantly

>> reduce throughput when not operating at the highest available data rate.

>>

>> In order to be able to significantly reduce the amount of rate sampling,

>> we need a much smarter selection of probing rates.

>>

>> The new approach introduced by this patch maintains a limited set of

>> available rates to be tested during a statistics window.

>>

>> They are split into distinct categories:

>> - MINSTREL_SAMPLE_TYPE_INC - incremental rate upgrade:

>>   Pick the next rate group and find the first rate that is faster than

>>   the current max. throughput rate

>> - MINSTREL_SAMPLE_TYPE_JUMP - random testing of higher rates:

>>   Pick a random rate from the next group that is faster than the current

>>   max throughput rate. This allows faster adaptation when the link changes

>>   significantly

>> - MINSTREL_SAMPLE_TYPE_SLOW - test a rate between max_prob, max_tp2 and

>>   max_tp in order to reduce the gap between them

>>

>> In order to prioritize sampling, every 6 attempts are split into 3x INC,

>> 2x JUMP, 1x SLOW.

>>

>> Available rates are checked and refilled on every stats window update.

> 

> Very cool!

> 

>> With this approach, we finally get a very small delta in throughput when

>> comparing setting the optimal data rate as a fixed rate vs normal rate

>> control operation.

> 

> Can you quantify this "very small delta"? Would love to see some

> benchmark data :)

Based on a quick test it seems to be around 5% (sometimes less) at VHT80
MCS6 nss=4 with ~350 mbit/s TCP throughput. I guess I might be able to
bring that down even further, once I optimize it some more.

- Felix
Toke Høiland-Jørgensen Jan. 25, 2021, 9:21 p.m. UTC | #3
Felix Fietkau <nbd@nbd.name> writes:

> On 2021-01-25 12:56, Toke Høiland-Jørgensen wrote:

>> Felix Fietkau <nbd@nbd.name> writes:

>> 

>>> The biggest flaw in current minstrel_ht is the fact that it needs way too

>>> many probing packets to be able to quickly find the best rate.

>>> Depending on the wifi hardware and operating mode, this can significantly

>>> reduce throughput when not operating at the highest available data rate.

>>>

>>> In order to be able to significantly reduce the amount of rate sampling,

>>> we need a much smarter selection of probing rates.

>>>

>>> The new approach introduced by this patch maintains a limited set of

>>> available rates to be tested during a statistics window.

>>>

>>> They are split into distinct categories:

>>> - MINSTREL_SAMPLE_TYPE_INC - incremental rate upgrade:

>>>   Pick the next rate group and find the first rate that is faster than

>>>   the current max. throughput rate

>>> - MINSTREL_SAMPLE_TYPE_JUMP - random testing of higher rates:

>>>   Pick a random rate from the next group that is faster than the current

>>>   max throughput rate. This allows faster adaptation when the link changes

>>>   significantly

>>> - MINSTREL_SAMPLE_TYPE_SLOW - test a rate between max_prob, max_tp2 and

>>>   max_tp in order to reduce the gap between them

>>>

>>> In order to prioritize sampling, every 6 attempts are split into 3x INC,

>>> 2x JUMP, 1x SLOW.

>>>

>>> Available rates are checked and refilled on every stats window update.

>> 

>> Very cool!

>> 

>>> With this approach, we finally get a very small delta in throughput when

>>> comparing setting the optimal data rate as a fixed rate vs normal rate

>>> control operation.

>> 

>> Can you quantify this "very small delta"? Would love to see some

>> benchmark data :)

> Based on a quick test it seems to be around 5% (sometimes less) at VHT80

> MCS6 nss=4 with ~350 mbit/s TCP throughput. I guess I might be able to

> bring that down even further, once I optimize it some more.


Cool! And pre-patch?

-Toke
Felix Fietkau Jan. 25, 2021, 9:27 p.m. UTC | #4
On 2021-01-25 22:21, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:

> 

>> On 2021-01-25 12:56, Toke Høiland-Jørgensen wrote:

>>> Felix Fietkau <nbd@nbd.name> writes:

>>>> With this approach, we finally get a very small delta in throughput when

>>>> comparing setting the optimal data rate as a fixed rate vs normal rate

>>>> control operation.

>>> 

>>> Can you quantify this "very small delta"? Would love to see some

>>> benchmark data :)

>> Based on a quick test it seems to be around 5% (sometimes less) at VHT80

>> MCS6 nss=4 with ~350 mbit/s TCP throughput. I guess I might be able to

>> bring that down even further, once I optimize it some more.

> 

> Cool! And pre-patch?

Some users reported more than 30%

- Felix
diff mbox series

Patch

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index e35948f4e1bf..a3b86fd300f0 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -379,13 +379,13 @@  minstrel_ht_get_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 static inline struct minstrel_rate_stats *
 minstrel_get_ratestats(struct minstrel_ht_sta *mi, int index)
 {
-	return &mi->groups[index / MCS_GROUP_RATES].rates[index % MCS_GROUP_RATES];
+	return &mi->groups[MI_RATE_GROUP(index)].rates[MI_RATE_IDX(index)];
 }
 
 static inline int minstrel_get_duration(int index)
 {
-	const struct mcs_group *group = &minstrel_mcs_groups[index / MCS_GROUP_RATES];
-	unsigned int duration = group->duration[index % MCS_GROUP_RATES];
+	const struct mcs_group *group = &minstrel_mcs_groups[MI_RATE_GROUP(index)];
+	unsigned int duration = group->duration[MI_RATE_IDX(index)];
 
 	return duration << group->shift;
 }
@@ -398,7 +398,7 @@  minstrel_ht_avg_ampdu_len(struct minstrel_ht_sta *mi)
 	if (mi->avg_ampdu_len)
 		return MINSTREL_TRUNC(mi->avg_ampdu_len);
 
-	if (minstrel_ht_is_legacy_group(mi->max_tp_rate[0] / MCS_GROUP_RATES))
+	if (minstrel_ht_is_legacy_group(MI_RATE_GROUP(mi->max_tp_rate[0])))
 		return 1;
 
 	duration = minstrel_get_duration(mi->max_tp_rate[0]);
@@ -465,14 +465,14 @@  minstrel_ht_sort_best_tp_rates(struct minstrel_ht_sta *mi, u16 index,
 	int tmp_group, tmp_idx, tmp_tp_avg, tmp_prob;
 	int j = MAX_THR_RATES;
 
-	cur_group = index / MCS_GROUP_RATES;
-	cur_idx = index  % MCS_GROUP_RATES;
+	cur_group = MI_RATE_GROUP(index);
+	cur_idx = MI_RATE_IDX(index);
 	cur_prob = mi->groups[cur_group].rates[cur_idx].prob_avg;
 	cur_tp_avg = minstrel_ht_get_tp_avg(mi, cur_group, cur_idx, cur_prob);
 
 	do {
-		tmp_group = tp_list[j - 1] / MCS_GROUP_RATES;
-		tmp_idx = tp_list[j - 1] % MCS_GROUP_RATES;
+		tmp_group = MI_RATE_GROUP(tp_list[j - 1]);
+		tmp_idx = MI_RATE_IDX(tp_list[j - 1]);
 		tmp_prob = mi->groups[tmp_group].rates[tmp_idx].prob_avg;
 		tmp_tp_avg = minstrel_ht_get_tp_avg(mi, tmp_group, tmp_idx,
 						    tmp_prob);
@@ -504,23 +504,23 @@  minstrel_ht_set_best_prob_rate(struct minstrel_ht_sta *mi, u16 *dest, u16 index)
 	int max_gpr_group, max_gpr_idx;
 	int max_gpr_tp_avg, max_gpr_prob;
 
-	cur_group = index / MCS_GROUP_RATES;
-	cur_idx = index % MCS_GROUP_RATES;
-	mg = &mi->groups[index / MCS_GROUP_RATES];
-	mrs = &mg->rates[index % MCS_GROUP_RATES];
+	cur_group = MI_RATE_GROUP(index);
+	cur_idx = MI_RATE_IDX(index);
+	mg = &mi->groups[cur_group];
+	mrs = &mg->rates[cur_idx];
 
-	tmp_group = *dest / MCS_GROUP_RATES;
-	tmp_idx = *dest % MCS_GROUP_RATES;
+	tmp_group = MI_RATE_GROUP(*dest);
+	tmp_idx = MI_RATE_IDX(*dest);
 	tmp_prob = mi->groups[tmp_group].rates[tmp_idx].prob_avg;
 	tmp_tp_avg = minstrel_ht_get_tp_avg(mi, tmp_group, tmp_idx, tmp_prob);
 
 	/* if max_tp_rate[0] is from MCS_GROUP max_prob_rate get selected from
 	 * MCS_GROUP as well as CCK_GROUP rates do not allow aggregation */
-	max_tp_group = mi->max_tp_rate[0] / MCS_GROUP_RATES;
-	max_tp_idx = mi->max_tp_rate[0] % MCS_GROUP_RATES;
+	max_tp_group = MI_RATE_GROUP(mi->max_tp_rate[0]);
+	max_tp_idx = MI_RATE_IDX(mi->max_tp_rate[0]);
 	max_tp_prob = mi->groups[max_tp_group].rates[max_tp_idx].prob_avg;
 
-	if (minstrel_ht_is_legacy_group(index / MCS_GROUP_RATES) &&
+	if (minstrel_ht_is_legacy_group(MI_RATE_GROUP(index)) &&
 	    !minstrel_ht_is_legacy_group(max_tp_group))
 		return;
 
@@ -529,8 +529,8 @@  minstrel_ht_set_best_prob_rate(struct minstrel_ht_sta *mi, u16 *dest, u16 index)
 	    mrs->prob_avg < max_tp_prob)
 		return;
 
-	max_gpr_group = mg->max_group_prob_rate / MCS_GROUP_RATES;
-	max_gpr_idx = mg->max_group_prob_rate % MCS_GROUP_RATES;
+	max_gpr_group = MI_RATE_GROUP(mg->max_group_prob_rate);
+	max_gpr_idx = MI_RATE_IDX(mg->max_group_prob_rate);
 	max_gpr_prob = mi->groups[max_gpr_group].rates[max_gpr_idx].prob_avg;
 
 	if (mrs->prob_avg > MINSTREL_FRAC(75, 100)) {
@@ -567,13 +567,13 @@  minstrel_ht_assign_best_tp_rates(struct minstrel_ht_sta *mi,
 	unsigned int tmp_group, tmp_idx, tmp_cck_tp, tmp_mcs_tp, tmp_prob;
 	int i;
 
-	tmp_group = tmp_legacy_tp_rate[0] / MCS_GROUP_RATES;
-	tmp_idx = tmp_legacy_tp_rate[0] % MCS_GROUP_RATES;
+	tmp_group = MI_RATE_GROUP(tmp_legacy_tp_rate[0]);
+	tmp_idx = MI_RATE_IDX(tmp_legacy_tp_rate[0]);
 	tmp_prob = mi->groups[tmp_group].rates[tmp_idx].prob_avg;
 	tmp_cck_tp = minstrel_ht_get_tp_avg(mi, tmp_group, tmp_idx, tmp_prob);
 
-	tmp_group = tmp_mcs_tp_rate[0] / MCS_GROUP_RATES;
-	tmp_idx = tmp_mcs_tp_rate[0] % MCS_GROUP_RATES;
+	tmp_group = MI_RATE_GROUP(tmp_mcs_tp_rate[0]);
+	tmp_idx = MI_RATE_IDX(tmp_mcs_tp_rate[0]);
 	tmp_prob = mi->groups[tmp_group].rates[tmp_idx].prob_avg;
 	tmp_mcs_tp = minstrel_ht_get_tp_avg(mi, tmp_group, tmp_idx, tmp_prob);
 
@@ -600,14 +600,14 @@  minstrel_ht_prob_rate_reduce_streams(struct minstrel_ht_sta *mi)
 	if (!mi->sta->ht_cap.ht_supported)
 		return;
 
-	tmp_max_streams = minstrel_mcs_groups[mi->max_tp_rate[0] /
-			  MCS_GROUP_RATES].streams;
+	group = MI_RATE_GROUP(mi->max_tp_rate[0]);
+	tmp_max_streams = minstrel_mcs_groups[group].streams;
 	for (group = 0; group < ARRAY_SIZE(minstrel_mcs_groups); group++) {
 		mg = &mi->groups[group];
 		if (!mi->supported[group] || group == MINSTREL_CCK_GROUP)
 			continue;
 
-		tmp_idx = mg->max_group_prob_rate % MCS_GROUP_RATES;
+		tmp_idx = MI_RATE_IDX(mg->max_group_prob_rate);
 		tmp_prob = mi->groups[group].rates[tmp_idx].prob_avg;
 
 		if (tmp_tp < minstrel_ht_get_tp_avg(mi, group, tmp_idx, tmp_prob) &&
@@ -644,8 +644,8 @@  minstrel_ht_find_probe_rates(struct minstrel_ht_sta *mi, u16 *rates, int *n_rate
 	int i, g, max_dur;
 	int tp_idx;
 
-	tp_group = &minstrel_mcs_groups[mi->max_tp_rate[0] / MCS_GROUP_RATES];
-	tp_idx = mi->max_tp_rate[0] % MCS_GROUP_RATES;
+	tp_group = &minstrel_mcs_groups[MI_RATE_GROUP(mi->max_tp_rate[0])];
+	tp_idx = MI_RATE_IDX(mi->max_tp_rate[0]);
 
 	max_dur = minstrel_get_duration(mi->max_tp_rate[0]);
 	if (faster_rate)
@@ -670,7 +670,7 @@  minstrel_ht_find_probe_rates(struct minstrel_ht_sta *mi, u16 *rates, int *n_rate
 			if ((group->duration[i] << group->shift) > max_dur)
 				continue;
 
-			idx = g * MCS_GROUP_RATES + i;
+			idx = MI_RATE(g, i);
 			if (idx == mi->max_tp_rate[0])
 				continue;
 
@@ -712,10 +712,10 @@  minstrel_ht_rate_sample_switch(struct minstrel_priv *mp,
 
 	/* If no suitable rate was found, try to pick the next one in the group */
 	if (!n_rates) {
-		int g_idx = mi->max_tp_rate[0] / MCS_GROUP_RATES;
+		int g_idx = MI_RATE_GROUP(mi->max_tp_rate[0]);
 		u16 supported = mi->supported[g_idx];
 
-		supported >>= mi->max_tp_rate[0] % MCS_GROUP_RATES;
+		supported >>= MI_RATE_IDX(mi->max_tp_rate[0]);
 		for (i = 0; supported; supported >>= 1, i++) {
 			if (!(supported & 1))
 				continue;
@@ -856,22 +856,26 @@  minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 
 	memset(tmp_mcs_tp_rate, 0, sizeof(tmp_mcs_tp_rate));
 	memset(tmp_legacy_tp_rate, 0, sizeof(tmp_legacy_tp_rate));
+
 	if (mi->supported[MINSTREL_CCK_GROUP])
-		for (j = 0; j < ARRAY_SIZE(tmp_legacy_tp_rate); j++)
-			tmp_legacy_tp_rate[j] = MINSTREL_CCK_GROUP * MCS_GROUP_RATES;
+		group = MINSTREL_CCK_GROUP;
 	else if (mi->supported[MINSTREL_OFDM_GROUP])
-		for (j = 0; j < ARRAY_SIZE(tmp_legacy_tp_rate); j++)
-			tmp_legacy_tp_rate[j] = MINSTREL_OFDM_GROUP * MCS_GROUP_RATES;
+		group = MINSTREL_OFDM_GROUP;
+
+	index = MI_RATE(group, 0);
+	for (j = 0; j < ARRAY_SIZE(tmp_legacy_tp_rate); j++)
+		tmp_legacy_tp_rate[j] = index;
 
 	if (mi->supported[MINSTREL_VHT_GROUP_0])
-		index = MINSTREL_VHT_GROUP_0 * MCS_GROUP_RATES;
+		group = MINSTREL_VHT_GROUP_0;
 	else if (ht_supported)
-		index = MINSTREL_HT_GROUP_0 * MCS_GROUP_RATES;
+		group = MINSTREL_HT_GROUP_0;
 	else if (mi->supported[MINSTREL_CCK_GROUP])
-		index = MINSTREL_CCK_GROUP * MCS_GROUP_RATES;
+		group = MINSTREL_CCK_GROUP;
 	else
-		index = MINSTREL_OFDM_GROUP * MCS_GROUP_RATES;
+		group = MINSTREL_OFDM_GROUP;
 
+	index = MI_RATE(group, 0);
 	tmp_max_prob_rate = index;
 	for (j = 0; j < ARRAY_SIZE(tmp_mcs_tp_rate); j++)
 		tmp_mcs_tp_rate[j] = index;
@@ -888,7 +892,7 @@  minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 
 		/* (re)Initialize group rate indexes */
 		for(j = 0; j < MAX_THR_RATES; j++)
-			tmp_group_tp_rate[j] = MCS_GROUP_RATES * group;
+			tmp_group_tp_rate[j] = MI_RATE(group, 0);
 
 		if (group == MINSTREL_CCK_GROUP && ht_supported)
 			tp_rate = tmp_legacy_tp_rate;
@@ -897,7 +901,7 @@  minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 			if (!(mi->supported[group] & BIT(i)))
 				continue;
 
-			index = MCS_GROUP_RATES * group + i;
+			index = MI_RATE(group, i);
 
 			mrs = &mg->rates[i];
 			mrs->retry_updated = false;
@@ -929,13 +933,13 @@  minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 			continue;
 
 		mg = &mi->groups[group];
-		mg->max_group_prob_rate = MCS_GROUP_RATES * group;
+		mg->max_group_prob_rate = MI_RATE(group, 0);
 
 		for (i = 0; i < MCS_GROUP_RATES; i++) {
 			if (!(mi->supported[group] & BIT(i)))
 				continue;
 
-			index = MCS_GROUP_RATES * group + i;
+			index = MI_RATE(group, i);
 
 			/* Find max probability rate per group and global */
 			minstrel_ht_set_best_prob_rate(mi, &tmp_max_prob_rate,
@@ -1022,7 +1026,7 @@  minstrel_downgrade_rate(struct minstrel_ht_sta *mi, u16 *idx, bool primary)
 {
 	int group, orig_group;
 
-	orig_group = group = *idx / MCS_GROUP_RATES;
+	orig_group = group = MI_RATE_GROUP(*idx);
 	while (group > 0) {
 		group--;
 
@@ -1206,7 +1210,7 @@  minstrel_calc_retransmit(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	ctime += (t_slot * cw) >> 1;
 	cw = min((cw << 1) | 1, mp->cw_max);
 
-	if (minstrel_ht_is_legacy_group(index / MCS_GROUP_RATES)) {
+	if (minstrel_ht_is_legacy_group(MI_RATE_GROUP(index))) {
 		overhead = mi->overhead_legacy;
 		overhead_rtscts = mi->overhead_legacy_rtscts;
 	} else {
@@ -1239,7 +1243,7 @@  static void
 minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
                      struct ieee80211_sta_rates *ratetbl, int offset, int index)
 {
-	int group_idx = index / MCS_GROUP_RATES;
+	int group_idx = MI_RATE_GROUP(index);
 	const struct mcs_group *group = &minstrel_mcs_groups[group_idx];
 	struct minstrel_rate_stats *mrs;
 	u8 idx;
@@ -1259,7 +1263,7 @@  minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 		ratetbl->rate[offset].count_rts = mrs->retry_count_rtscts;
 	}
 
-	index %= MCS_GROUP_RATES;
+	index = MI_RATE_IDX(index);
 	if (group_idx == MINSTREL_CCK_GROUP)
 		idx = mp->cck_rates[index % ARRAY_SIZE(mp->cck_rates)];
 	else if (group_idx == MINSTREL_OFDM_GROUP)
@@ -1289,17 +1293,17 @@  minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 static inline int
 minstrel_ht_get_prob_avg(struct minstrel_ht_sta *mi, int rate)
 {
-	int group = rate / MCS_GROUP_RATES;
-	rate %= MCS_GROUP_RATES;
+	int group = MI_RATE_GROUP(rate);
+	rate = MI_RATE_IDX(rate);
 	return mi->groups[group].rates[rate].prob_avg;
 }
 
 static int
 minstrel_ht_get_max_amsdu_len(struct minstrel_ht_sta *mi)
 {
-	int group = mi->max_prob_rate / MCS_GROUP_RATES;
+	int group = MI_RATE_GROUP(mi->max_prob_rate);
 	const struct mcs_group *g = &minstrel_mcs_groups[group];
-	int rate = mi->max_prob_rate % MCS_GROUP_RATES;
+	int rate = MI_RATE_IDX(mi->max_prob_rate);
 	unsigned int duration;
 
 	/* Disable A-MSDU if max_prob_rate is bad */
@@ -1405,7 +1409,7 @@  minstrel_get_sample_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 		return -1;
 
 	mrs = &mg->rates[sample_idx];
-	sample_idx += sample_group * MCS_GROUP_RATES;
+	sample_idx += MI_RATE(sample_group, 0);
 
 	tp_rate1 = mi->max_tp_rate[0];
 
@@ -1455,8 +1459,7 @@  minstrel_get_sample_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 	 * if the link is working perfectly.
 	 */
 
-	cur_max_tp_streams = minstrel_mcs_groups[tp_rate1 /
-		MCS_GROUP_RATES].streams;
+	cur_max_tp_streams = minstrel_mcs_groups[MI_RATE_GROUP(tp_rate1)].streams;
 	if (sample_dur >= minstrel_get_duration(tp_rate2) &&
 	    (cur_max_tp_streams - 1 <
 	     minstrel_mcs_groups[sample_group].streams ||
@@ -1484,7 +1487,7 @@  minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	int sample_idx;
 
 	if (!(info->flags & IEEE80211_TX_CTL_AMPDU) &&
-	    !minstrel_ht_is_legacy_group(mi->max_prob_rate / MCS_GROUP_RATES))
+	    !minstrel_ht_is_legacy_group(MI_RATE_GROUP(mi->max_prob_rate)))
 		minstrel_aggr_check(sta, txrc->skb);
 
 	info->flags |= mi->tx_flags;
@@ -1512,8 +1515,8 @@  minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 	if (sample_idx < 0)
 		return;
 
-	sample_group = &minstrel_mcs_groups[sample_idx / MCS_GROUP_RATES];
-	sample_idx %= MCS_GROUP_RATES;
+	sample_group = &minstrel_mcs_groups[MI_RATE_GROUP(sample_idx)];
+	sample_idx = MI_RATE_IDX(sample_idx);
 
 	if (sample_group == &minstrel_mcs_groups[MINSTREL_CCK_GROUP] &&
 	    (sample_idx >= 4) != txrc->short_preamble)
@@ -1529,7 +1532,7 @@  minstrel_ht_get_rate(void *priv, struct ieee80211_sta *sta, void *priv_sta,
 		int idx = sample_idx % ARRAY_SIZE(mp->ofdm_rates[0]);
 		rate->idx = mp->ofdm_rates[mi->band][idx];
 	} else if (sample_group->flags & IEEE80211_TX_RC_VHT_MCS) {
-		ieee80211_rate_set_vht(rate, sample_idx % MCS_GROUP_RATES,
+		ieee80211_rate_set_vht(rate, MI_RATE_IDX(sample_idx),
 				       sample_group->streams);
 	} else {
 		rate->idx = sample_idx + (sample_group->streams - 1) * 8;
@@ -1898,8 +1901,8 @@  static u32 minstrel_ht_get_expected_throughput(void *priv_sta)
 	struct minstrel_ht_sta *mi = priv_sta;
 	int i, j, prob, tp_avg;
 
-	i = mi->max_tp_rate[0] / MCS_GROUP_RATES;
-	j = mi->max_tp_rate[0] % MCS_GROUP_RATES;
+	i = MI_RATE_GROUP(mi->max_tp_rate[0]);
+	j = MI_RATE_IDX(mi->max_tp_rate[0]);
 	prob = mi->groups[i].rates[j].prob_avg;
 
 	/* convert tp_avg from pkt per second in kbps */
diff --git a/net/mac80211/rc80211_minstrel_ht.h b/net/mac80211/rc80211_minstrel_ht.h
index 7d6d0b720f6d..5912f7dc5267 100644
--- a/net/mac80211/rc80211_minstrel_ht.h
+++ b/net/mac80211/rc80211_minstrel_ht.h
@@ -6,6 +6,8 @@ 
 #ifndef __RC_MINSTREL_HT_H
 #define __RC_MINSTREL_HT_H
 
+#include <linux/bitfield.h>
+
 /* number of highest throughput rates to consider*/
 #define MAX_THR_RATES 4
 #define SAMPLE_COLUMNS	10	/* number of columns in sample table */
@@ -57,6 +59,17 @@ 
 
 #define MCS_GROUP_RATES		10
 
+#define MI_RATE_IDX_MASK	GENMASK(3, 0)
+#define MI_RATE_GROUP_MASK	GENMASK(15, 4)
+
+#define MI_RATE(_group, _idx)				\
+	(FIELD_PREP(MI_RATE_GROUP_MASK, _group) |	\
+	 FIELD_PREP(MI_RATE_IDX_MASK, _idx))
+
+#define MI_RATE_IDX(_rate) FIELD_GET(MI_RATE_IDX_MASK, _rate)
+#define MI_RATE_GROUP(_rate) FIELD_GET(MI_RATE_GROUP_MASK, _rate)
+
+
 struct minstrel_priv {
 	struct ieee80211_hw *hw;
 	bool has_mrr;
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index 3b7af242cde6..067d10383fa7 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -56,7 +56,7 @@  minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
 
 	for (j = 0; j < MCS_GROUP_RATES; j++) {
 		struct minstrel_rate_stats *mrs = &mi->groups[i].rates[j];
-		int idx = i * MCS_GROUP_RATES + j;
+		int idx = MI_RATE(i, j);
 		unsigned int duration;
 
 		if (!(mi->supported[i] & BIT(j)))
@@ -201,7 +201,7 @@  minstrel_ht_stats_csv_dump(struct minstrel_ht_sta *mi, int i, char *p)
 
 	for (j = 0; j < MCS_GROUP_RATES; j++) {
 		struct minstrel_rate_stats *mrs = &mi->groups[i].rates[j];
-		int idx = i * MCS_GROUP_RATES + j;
+		int idx = MI_RATE(i, j);
 		unsigned int duration;
 
 		if (!(mi->supported[i] & BIT(j)))