Hi Shaopeng,
On 11/8/2022 12:32 AM, Shaopeng Tan (Fujitsu) wrote:
Hi Shuah and Reinette,
On 11/1/2022 2:43 AM, Shaopeng Tan wrote:
Before exiting each test function(run_cmt/cat/mbm/mba_test()), test results("ok","not ok") are printed by ksft_test_result() and then temporary result files are cleaned by function cmt/cat/mbm/mba_test_cleanup(). However, before running ksft_test_result(), function 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.
This isn't making much sense to me. Please include test report before and after this change in the change log.
With or without this patch, there is no effect on the result message. These functions were executed twice, in brief, it runs as follows:
- cmt/cat/mbm/mba_test_cleanup()
- ksft_test_result()
- cmt/cat/mbm/mba_test_cleanup()
So, I deleted once.
From what I can tell this still seem to suffer from the problem where the test files may not be cleaned. With the removal of mbm_test_cleanup() the cleanup is now expected to be done in mbm_bw_change().
Note that:
mbm_bw_change() { ...
ret = resctrl_val(benchmark_cmd, ¶m); if (ret) return ret;
/* Test results stored in file */
ret = check_results(span); if (ret) return ret; <== Return without cleaning test result file
mbm_test_cleanup(); <== Test result file cleaned only when test passed.
return 0; }
I intend to avoid this problem through the following codes.
mbm_bw_change() { ret = resctrl_val(benchmark_cmd, ¶m); if (ret)
return ret;
goto out; ret = check_results(span); if (ret)
return ret;
goto out;+out: mbm_test_cleanup();
return 0;
return ret;}
Yes, even though file removal may now encounter ENOENT this does seem the most robust route and the possible error is ok since mbm_test_cleanup() does not check the return code. Could you please replicate this pattern to the other functions (mba_schemata_change() and cmt_resctrl_val()) also?
Reinette