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