On Fri, 30 Aug 2024, Reinette Chatre wrote: > On 8/30/24 4:42 AM, Ilpo Järvinen wrote: > > On Thu, 29 Aug 2024, Reinette Chatre wrote: > > > > > The MBA test incrementally throttles memory bandwidth, each time > > > followed by a comparison between the memory bandwidth observed > > > by the performance counters and resctrl respectively. > > > > > > While a comparison between performance counters and resctrl is > > > generally appropriate, they do not have an identical view of > > > memory bandwidth. For example RAS features or memory performance > > > features that generate memory traffic may drive accesses that are > > > counted differently by performance counters and MBM respectively, > > > for instance generating "overhead" traffic which is not counted > > > against any specific RMID. As a ratio, this different view of memory > > > bandwidth becomes more apparent at low memory bandwidths. > > > > Interesting. > > > > I did some time back prototype with a change to MBM test such that instead > > of using once=false I changed fill_buf to be able to run N passes through > > the buffer which allowed me to know how many reads were performed by the > > benchmark. This yielded numerical difference between all those 3 values > > (# of reads, MBM, perf) which also varied from arch to another so it > > didn't end up making an usable test. > > > > I guess I now have an explanation for at least a part of the differences. > > > > > It is not practical to enable/disable the various features that > > > may generate memory bandwidth to give performance counters and > > > resctrl an identical view. Instead, do not compare performance > > > counters and resctrl view of memory bandwidth when the memory > > > bandwidth is low. > > > > > > Bandwidth throttling behaves differently across platforms > > > so it is not appropriate to drop measurement data simply based > > > on the throttling level. Instead, use a threshold of 750MiB > > > that has been observed to support adequate comparison between > > > performance counters and resctrl. > > > > > > Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx> > > > --- > > > tools/testing/selftests/resctrl/mba_test.c | 7 +++++++ > > > tools/testing/selftests/resctrl/resctrl.h | 6 ++++++ > > > 2 files changed, 13 insertions(+) > > > > > > diff --git a/tools/testing/selftests/resctrl/mba_test.c > > > b/tools/testing/selftests/resctrl/mba_test.c > > > index cad473b81a64..204b9ac4b108 100644 > > > --- a/tools/testing/selftests/resctrl/mba_test.c > > > +++ b/tools/testing/selftests/resctrl/mba_test.c > > > @@ -96,6 +96,13 @@ static bool show_mba_info(unsigned long *bw_imc, > > > unsigned long *bw_resc) > > > avg_bw_imc = sum_bw_imc / (NUM_OF_RUNS - 1); > > > avg_bw_resc = sum_bw_resc / (NUM_OF_RUNS - 1); > > > + if (avg_bw_imc < THROTTLE_THRESHOLD || avg_bw_resc < > > > THROTTLE_THRESHOLD) { > > > + ksft_print_msg("Bandwidth below threshold (%d MiB). > > > Dropping results from MBA schemata %u.\n", > > > + THROTTLE_THRESHOLD, > > > + ALLOCATION_MAX - ALLOCATION_STEP * > > > allocation); > > > > The second one too should be %d. > > > > hmmm ... I intended to have it be consistent with the ksft_print_msg() that > follows. Perhaps allocation can be made unsigned instead? If you go that way, then it would be good to make the related defines and allocation in mba_setup() unsigned too, although the latter is a bit scary because it does allocation -= ALLOCATION_STEP which could underflow if the defines are ever changed. -- i.