| Message ID | 20230418114506.46788-8-ilpo.jarvinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Fixes, cleanups, and rewritten CAT test | expand |
Hi Ilpo, On 4/18/2023 4:44 AM, Ilpo Järvinen wrote: > When no benchmark_cmd is given, benchmark_cmd[1] is set to span in > main(). There's no need to do it again in run_mba_test(). > > Remove the duplicated preparation for span argument into > benchmark_cmd[1] from run_mba_test(). It enables also removing has_ben > argument from run_mba_test() as unnecessary. I find the last sentence a bit hard to read. How about "After this, the has_ben argument to run_mba_test() can be removed.". > > Co-developed-by: Fenghua Yu <fenghua.yu@intel.com> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > tools/testing/selftests/resctrl/resctrl_tests.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c > index f1ed2c89f228..3c8ec68eb507 100644 > --- a/tools/testing/selftests/resctrl/resctrl_tests.c > +++ b/tools/testing/selftests/resctrl/resctrl_tests.c > @@ -99,8 +99,8 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, unsigned long span, > umount_resctrlfs(); > } > > -static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, > - int cpu_no, char *bw_report) > +static void run_mba_test(char **benchmark_cmd, unsigned long span, int cpu_no, > + char *bw_report) > { > int res; > > @@ -117,8 +117,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, > goto umount; > } > > - if (!has_ben) > - sprintf(benchmark_cmd[1], "%lu", span); Can "span" also be removed? > res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd); > ksft_test_result(!res, "MBA: schemata change\n"); > > @@ -297,7 +295,7 @@ int main(int argc, char **argv) > run_mbm_test(has_ben, benchmark_cmd, span, cpu_no, bw_report); > > if (mba_test) > - run_mba_test(has_ben, benchmark_cmd, span, cpu_no, bw_report); > + run_mba_test(benchmark_cmd, span, cpu_no, bw_report); > > if (cmt_test) > run_cmt_test(has_ben, benchmark_cmd, cpu_no); Reinette
On Fri, 21 Apr 2023, Reinette Chatre wrote: > On 4/18/2023 4:44 AM, Ilpo Järvinen wrote: > > When no benchmark_cmd is given, benchmark_cmd[1] is set to span in > > main(). There's no need to do it again in run_mba_test(). > > > > Remove the duplicated preparation for span argument into > > benchmark_cmd[1] from run_mba_test(). It enables also removing has_ben > > argument from run_mba_test() as unnecessary. > > I find the last sentence a bit hard to read. How about > "After this, the has_ben argument to run_mba_test() can be removed.". > > > > > Co-developed-by: Fenghua Yu <fenghua.yu@intel.com> > > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > > --- > > tools/testing/selftests/resctrl/resctrl_tests.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c > > index f1ed2c89f228..3c8ec68eb507 100644 > > --- a/tools/testing/selftests/resctrl/resctrl_tests.c > > +++ b/tools/testing/selftests/resctrl/resctrl_tests.c > > @@ -99,8 +99,8 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, unsigned long span, > > umount_resctrlfs(); > > } > > > > -static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, > > - int cpu_no, char *bw_report) > > +static void run_mba_test(char **benchmark_cmd, unsigned long span, int cpu_no, > > + char *bw_report) > > { > > int res; > > > > @@ -117,8 +117,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, > > goto umount; > > } > > > > - if (!has_ben) > > - sprintf(benchmark_cmd[1], "%lu", span); > > Can "span" also be removed? Yes.
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index f1ed2c89f228..3c8ec68eb507 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -99,8 +99,8 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, unsigned long span, umount_resctrlfs(); } -static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, - int cpu_no, char *bw_report) +static void run_mba_test(char **benchmark_cmd, unsigned long span, int cpu_no, + char *bw_report) { int res; @@ -117,8 +117,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, unsigned long span, goto umount; } - if (!has_ben) - sprintf(benchmark_cmd[1], "%lu", span); res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd); ksft_test_result(!res, "MBA: schemata change\n"); @@ -297,7 +295,7 @@ int main(int argc, char **argv) run_mbm_test(has_ben, benchmark_cmd, span, cpu_no, bw_report); if (mba_test) - run_mba_test(has_ben, benchmark_cmd, span, cpu_no, bw_report); + run_mba_test(benchmark_cmd, span, cpu_no, bw_report); if (cmt_test) run_cmt_test(has_ben, benchmark_cmd, cpu_no);