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
