| Message ID | 20220311072147.3301525-1-tan.shaopeng@jp.fujitsu.com |
|---|---|
| Headers | show |
| Series | selftests/resctrl: Add resctrl_tests into kselftest set | expand |
Hi Shaopeng Tan, On 3/10/2022 11:21 PM, Shaopeng Tan wrote: > In kselftest framework, a sub test is run using the timeout utility > and it will send SIGTERM to the test upon timeout. > > In resctrl_tests, a child process is created by fork() to > run benchmark but SIGTERM is not set in sigaction(). > If SIGTERM signal is received, the parent process will be killed, > but the child process still exists. > > kill child process before parent process terminates > if SIGTERM signal is received. > > Reviewed-by: Shuah Khan <skhan@linuxfoundation.org> > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> > --- > tools/testing/selftests/resctrl/resctrl_val.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c > index 95224345c78e..b32b96356ec7 100644 > --- a/tools/testing/selftests/resctrl/resctrl_val.c > +++ b/tools/testing/selftests/resctrl/resctrl_val.c > @@ -678,6 +678,7 @@ int resctrl_val(char **benchmark_cmd, struct resctrl_val_param *param) > sigemptyset(&sigact.sa_mask); > sigact.sa_flags = SA_SIGINFO; > if (sigaction(SIGINT, &sigact, NULL) || > + sigaction(SIGTERM, &sigact, NULL) || > sigaction(SIGHUP, &sigact, NULL)) { > perror("# sigaction"); > ret = errno; Thank you. Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
On 3/10/2022 11:21 PM, Shaopeng Tan wrote: > When testing on a Intel(R) Xeon(R) Gold 6254 CPU @ 3.10GHz the resctrl > selftests fail due to timeout after exceeding the default time limit of > 45 seconds. On this system the test takes about 68 seconds. > Since the failing test by default accesses a fixed size of memory, the > execution time should not vary significantly between different environment. > A new default of 120 seconds should be sufficient yet easy to customize > with the introduction of the "settings" file for reference. > > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> > --- > tools/testing/selftests/resctrl/settings | 3 +++ > 1 file changed, 3 insertions(+) > create mode 100644 tools/testing/selftests/resctrl/settings > > diff --git a/tools/testing/selftests/resctrl/settings b/tools/testing/selftests/resctrl/settings > new file mode 100644 > index 000000000000..a383f3d4565b > --- /dev/null > +++ b/tools/testing/selftests/resctrl/settings > @@ -0,0 +1,3 @@ > +# If running time is longer than 120 seconds when new tests are added in > +# the future, increase timeout here. > +timeout=120 Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
Hi Shaopeng Tan, On 3/10/2022 11:21 PM, Shaopeng Tan wrote: > resctrl_tests can be built or run using kselftests framework. > Add description on how to do so in README. > > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> > --- > tools/testing/selftests/resctrl/README | 43 ++++++++++++++++++++------ > 1 file changed, 34 insertions(+), 9 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README > index 3d2bbd4fa3aa..c74b8246e6c9 100644 > --- a/tools/testing/selftests/resctrl/README > +++ b/tools/testing/selftests/resctrl/README > @@ -12,24 +12,49 @@ Allocation test on Intel RDT hardware. More tests will be added in the future. > And the test suit can be extended to cover AMD QoS and ARM MPAM hardware > as well. > > +resctrl_tests can be run with or without kselftest framework. Saying "with or without kselftest framework" sounds good. Could you please keep using that instead of the "use" and "not use" in the new headers? > + > +USE KSELFTEST FRAMEWORK > +======================= So, here it can be "WITH KSELFTEST FRAMEWORK" > + > +* BUILD > +------- Please remove the "*" to make it consistent with the "BUILD" later in the file. > + > +Build executable file "resctrl_tests" at top level directory of the kernel source. > + $ make -C tools/testing/selftests TARGETS=resctrl Please use ":" after the sentence that describes a command that follows. For example, above would read: Build executable file "resctrl_tests" from top level directory of the kernel source: $ make -C tools/testing/selftests TARGETS=resctrl (also note me changing "at" to "from") > + > +* RUN Please remove the "*" to make it consistent with the "RUN" later in the file. > +----- > + > +Run resctrl_tests as sudo or root since the test needs to mount resctrl file > +system and change contents in the file system. > +Using kselftest framework will run all supported tests of resctrl_tests. "tests of resctrl_tests." -> "tests within resctrl_tests:"? > + > + $ sudo make -C tools/testing/selftests TARGETS=resctrl run_tests > + > +More details about kselftest framework as follow. > +Documentation/dev-tools/kselftest.rst "More details about kselftest framework can be found in Documentation/dev-tools/kselftest.rst." > + > +NOT USE KSELFTEST FRAMEWORK > +=========================== ("WITHOUT KSELFTEST FRAMEWORK") > + > BUILD > ----- > > -Run "make" to build executable file "resctrl_tests". > +Build executable file "resctrl_tests" at this directory(tools/testing/selftests/resctrl/). "at this directory" -> "from this directory" "." -> ":" > + $ make > > RUN > --- > > -To use resctrl_tests, root or sudoer privileges are required. This is because > -the test needs to mount resctrl file system and change contents in the file > -system. > - > -Executing the test without any parameter will run all supported tests: > +Run resctrl_tests as sudo or root since the test needs to mount resctrl file > +system and change contents in the file system. > +Executing the test without any parameter will run all supported tests. "." -> ":" > > - sudo ./resctrl_tests > + $ sudo ./resctrl_tests > > OVERVIEW OF EXECUTION > ---------------------- > +===================== > > A test case has four stages: > > @@ -41,7 +66,7 @@ A test case has four stages: > - teardown: umount resctrl and clear temporary files. > > ARGUMENTS > ---------- > +========= > > Parameter '-h' shows usage information. > Reinette
Hello, The aim of this series is to make resctrl_tests run by using kselftest framework. - I modify resctrl_test Makefile and kselftest Makefile, to enable build/run resctrl_tests by using kselftest framework. Of course, users can also build/run resctrl_tests without using framework as before. - I change the default limited time for resctrl_tests to 120 seconds, to ensure the resctrl_tests finish in limited time on different environments. - When resctrl file system is not supported by environment or resctrl_tests is not run as root, return skip code of kselftest framework. - If resctrl_tests does not finish in limited time, terminate it as same as executing ctrl+c that kills parent process and child process. Difference from v4: - Add comment for settings file. [PATCH v5 2/6] - Improved README of resctrl_tests and rewirte changelog. [PATCH v5 5/6] https://lore.kernel.org/lkml/20220304103834.486892-1-tan.shaopeng@jp.fujitsu.com/ [PATCH V4] This patch series is based on v5.17-rc7. Shaopeng Tan (6): selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received selftests/resctrl: Change the default limited time to 120 seconds selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework selftests/resctrl: Make resctrl_tests run using kselftest framework selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests selftests/resctrl: Add missing SPDX license to Makefile tools/testing/selftests/Makefile | 1 + tools/testing/selftests/resctrl/Makefile | 18 ++------ tools/testing/selftests/resctrl/README | 43 +++++++++++++++---- .../testing/selftests/resctrl/resctrl_tests.c | 4 +- tools/testing/selftests/resctrl/resctrl_val.c | 1 + tools/testing/selftests/resctrl/settings | 3 ++ 6 files changed, 45 insertions(+), 25 deletions(-) create mode 100644 tools/testing/selftests/resctrl/settings