Hi Shaopeng, On 9/13/2022 6:51 PM, Shaopeng Tan wrote: > Before exiting each test function(run_cmt/cat/mbm/mba_test()), > test results are printed by ksft_print_msg() and then temporary result > files are cleaned by function cmt/cat/mbm/mba_test_cleanup(). > However, before running ksft_print_msg(), function before -> after? > cmt/cat/mbm/mba_test_cleanup() > has been run in each test function as follows: > cmt_resctrl_val() > cat_perf_miss_val() > mba_schemata_change() > mbm_bw_change() > > Remove duplicate codes that clear each test result file. > Another good catch, thank you. > Signed-off-by: Shaopeng Tan <tan.shaopeng@xxxxxxxxxxxxxx> > --- > tools/testing/selftests/resctrl/cat_test.c | 1 - > tools/testing/selftests/resctrl/cmt_test.c | 2 -- > tools/testing/selftests/resctrl/mba_test.c | 2 -- > tools/testing/selftests/resctrl/mbm_test.c | 2 -- > 4 files changed, 7 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/cat_test.c b/tools/testing/selftests/resctrl/cat_test.c > index 1c5e90c63254..d1134f66469f 100644 > --- a/tools/testing/selftests/resctrl/cat_test.c > +++ b/tools/testing/selftests/resctrl/cat_test.c > @@ -221,7 +221,6 @@ int cat_perf_miss_val(int cpu_no, int n, char *cache_type) > kill(bm_pid, SIGKILL); > } > > - cat_test_cleanup(); > if (bm_pid) > umount_resctrlfs(); > It makes it much easier to understand code if it is symmetrical. Since the files are created within cat_perf_miss_val() I think it would be better to perform the cleanup in the same function. So, keep this cleanup but remove the call from run_cat_test() instead. Similar for the cleanups below ... could you please keep them and instead remove the duplicate cleanup from run_cmt/mbm/mba_test() instead? When you do so, please be careful since it seems that there is (another!) bug where the cleanup is not done if the test fails. > diff --git a/tools/testing/selftests/resctrl/cmt_test.c b/tools/testing/selftests/resctrl/cmt_test.c > index 8968e36db99d..b3b17fb876f4 100644 > --- a/tools/testing/selftests/resctrl/cmt_test.c > +++ b/tools/testing/selftests/resctrl/cmt_test.c > @@ -139,7 +139,5 @@ int cmt_resctrl_val(int cpu_no, int n, char **benchmark_cmd) > if (ret) > return ret; > > - cmt_test_cleanup(); > - > return 0; > } > diff --git a/tools/testing/selftests/resctrl/mba_test.c b/tools/testing/selftests/resctrl/mba_test.c > index 1a1bdb6180cf..93ffacb416df 100644 > --- a/tools/testing/selftests/resctrl/mba_test.c > +++ b/tools/testing/selftests/resctrl/mba_test.c > @@ -166,7 +166,5 @@ int mba_schemata_change(int cpu_no, char *bw_report, char **benchmark_cmd) > if (ret) > return ret; > > - mba_test_cleanup(); > - > return 0; > } > diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c > index 38a3b3ad1c76..a453db4c221f 100644 > --- a/tools/testing/selftests/resctrl/mbm_test.c > +++ b/tools/testing/selftests/resctrl/mbm_test.c > @@ -134,7 +134,5 @@ int mbm_bw_change(int span, int cpu_no, char *bw_report, char **benchmark_cmd) > if (ret) > return ret; > > - mbm_test_cleanup(); > - > return 0; > } Thank you Reinette
