linux-kernel - RE: [PATCH v2 1/6] selftests/resctrl: Extend signal handler coverage to unmount on receiving signal

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] [day] [month] [year] [list]

Message-ID: <TYAPR01MB6330FD62ED8A752ECDA8AF948BCAA@TYAPR01MB6330.jpnprd01.prod.outlook.com>
Date:   Thu, 5 Oct 2023 07:53:29 +0000
From:   "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>
To:     'Reinette Chatre' <reinette.chatre@...el.com>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Shuah Khan <shuah@...nel.org>,
        Shuah Khan <skhan@...uxfoundation.org>,
        "linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
        Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>
CC:     LKML <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2 1/6] selftests/resctrl: Extend signal handler coverage
 to unmount on receiving signal

Hello Reinette,

> On 9/28/2023 1:10 AM, Shaopeng Tan (Fujitsu) wrote:
> >> On 9/15/2023 8:44 AM, Ilpo Järvinen wrote:
> 
> ...
> 
> >>> +static void run_mbm_test(const char * const *benchmark_cmd, int
> >>> +cpu_no) {
> >>> +	int res;
> >>> +
> >>> +	ksft_print_msg("Starting MBM BW change ...\n");
> >>> +
> >>> +	if (test_prepare())
> >>> +		return;
> >>>
> >>
> >> I am not sure about this. With this exit the kselftest machinery is
> >> not aware of the test passing or failing. I wonder if there should
> >> not rather be a "goto" here that triggers ksft_test_result()? This
> >> needs some more thought though. First, with this change
> >> test_prepare() officially gains responsibility to determine if a
> >> failure is transient (just a single test
> >> fails) or permanent (no use trying any other tests if this fails).
> >> For the former it would then be up to the caller to call
> >> ksft_test_result() and for the latter
> >> test_prepare() will call ksft_exit_fail_msg().
> >> Second, that SNC warning may be an inconvenience with a new goto.
> >> Here it may be ok to print that message before the test failure?
> >
> > If a failure may be permanent, it may be best to detect it before running all
> tests, rather than in test_prepare().
> > Now some detections are completed before running all tests. For example:
> > 273         if (geteuid() != 0)
> > 274                 return ksft_exit_skip("Not running as root.
> Skipping...\n");
> > 275
> > 276         if (!check_resctrlfs_support())
> > 277                 return ksft_exit_skip("resctrl FS does not exist. Enable
> X86_CPU_RESCTRL config option.\n");
> > 278
> > 279         if (umount_resctrlfs())
> > 280                 return ksft_exit_skip("resctrl FS unmount failed.\n");
> >
> 
> You are correct that the tests should aim to detect as early as possible if no test
> has a chance of succeeding. This is covered in the checks you mention.
> The purpose of test_prepare()/test_cleanup() pair is to perform actions that
> should be done for every test. For example, resctrl is mounted before each test
> and unmounted after each test. Since these actions are required to be done for
> every test it cannot be a single call before all tests are run.
> 
> It may be possible to add a test_prepare() directly followed by a test_cleanup()
> before any test is run to be more explicit about early detection but that does not
> seem necessary considering the checks would be done anyway when the first
> test is run. Even when doing so it would not eliminate the need for
> test_prepare()/test_cleanup() to form part of every test run and needing to exit
> if, for example, a previous test triggered a fault preventing resctrl from being
> mounted.

Thanks for your explanation. I understand

Best regards,
Shaopeng TAN