linux-kernel - RE: [PATCH v3 5/5] selftests/resctrl: Remove duplicate codes that clear each test result file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Nov 2022 08:32:24 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'Reinette Chatre' <reinette.chatre@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Shuah Khan <shuah@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH v3 5/5] selftests/resctrl: Remove duplicate codes that
 clear each test result file

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, &param);
> 	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, &param);
        if (ret)
-               return ret;
+               goto out;

        ret = check_results(span);
        if (ret)
-               return ret;
+               goto out;

+out:
        mbm_test_cleanup();

-       return 0;
+       return ret;
}


Best regards,
Shaopeng Tan