2022-03-04 15:19:42

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v4 0/6] selftests/resctrl: Add resctrl_tests into kselftest set

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 v3:
- I reodered all patches of this patch series.
- I updated the print message of ksft_exit_skip() to give more information. [PATCH v4 3/6]
- I simplified tools/testing/selftests/resctrl/Makefile to use kselftest's lib.mk. [PATCH v4 4/6]
- I improved README of resctrl_tests. [PATCH v4 5/6]
- I moved license patch to this patch series. [PATCH v4 6/6]
https://lore.kernel.org/lkml/[email protected]/ [PATCH V3]

This patch series is based on v5.16.

Thanks,

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 | 31 ++++++++++++++++++-
.../testing/selftests/resctrl/resctrl_tests.c | 4 +--
tools/testing/selftests/resctrl/resctrl_val.c | 1 +
tools/testing/selftests/resctrl/settings | 1 +
6 files changed, 39 insertions(+), 17 deletions(-)
create mode 100644 tools/testing/selftests/resctrl/settings

--
2.27.0


2022-03-04 17:05:09

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v4 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework

In kselftest framework, if a sub test can not run by some reasons,
the test result should be marked as SKIP rather than FAIL.
Return KSFT_SKIP(4) instead of KSFT_FAIL(1) if resctrl_tests is not run
as root or it is run on a test environment which does not support resctrl.

- ksft_exit_fail_msg(): returns KSFT_FAIL(1)
- ksft_exit_skip(): returns KSFT_SKIP(4)

Reviewed-by: Shuah Khan <[email protected]>
Signed-off-by: Shaopeng Tan <[email protected]>
---
tools/testing/selftests/resctrl/resctrl_tests.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c
index 973f09a66e1e..a44afb05b848 100644
--- a/tools/testing/selftests/resctrl/resctrl_tests.c
+++ b/tools/testing/selftests/resctrl/resctrl_tests.c
@@ -205,7 +205,7 @@ int main(int argc, char **argv)
* 2. We execute perf commands
*/
if (geteuid() != 0)
- return ksft_exit_fail_msg("Not running as root, abort testing.\n");
+ return ksft_exit_skip("Not running as root. Skipping...\n");

/* Detect AMD vendor */
detect_amd();
@@ -235,7 +235,7 @@ int main(int argc, char **argv)
sprintf(bm_type, "fill_buf");

if (!check_resctrlfs_support())
- return ksft_exit_fail_msg("resctrl FS does not exist\n");
+ return ksft_exit_skip("resctrl FS does not exist. Enable X86_CPU_RESCTRL and PROC_CPU_RESCTRL config options.\n");

filter_dmesg();

--
2.27.0

2022-03-04 17:32:31

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v4 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests

In this patch series, I make restrl_tests build/run using kselftest
framework, but some users do not known how to build/run resctrl_tests
using kseltest framework.

Add manual of how to make resctrl_tests build/run
using kselftest framework into README.

Signed-off-by: Shaopeng Tan <[email protected]>
---
tools/testing/selftests/resctrl/README | 31 +++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README
index 3d2bbd4fa3aa..268cf3f95bd5 100644
--- a/tools/testing/selftests/resctrl/README
+++ b/tools/testing/selftests/resctrl/README
@@ -12,9 +12,37 @@ 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.
+
+USE KSELFTEST FRAMEWORK
+-----------------------
+
+BUILD
+-----
+
+Execute the following command in top level directory of the kernel source.
+
+Build resctrl:
+ $ make -C tools/testing/selftests TARGETS=resctrl
+
+RUN
+---
+
+Run resctrl:
+ $ make -C tools/testing/selftests TARGETS=resctrl run_tests
+
+Using kselftest framework, the ./resctrl_tests will be run without any parameters.
+
+More details about kselftest framework as follow.
+Documentation/dev-tools/kselftest.rst
+
+NOT USE KSELFTEST FRAMEWORK
+---------------------------
+
BUILD
-----

+Execute the following command in this directory(tools/testing/selftests/resctrl/).
Run "make" to build executable file "resctrl_tests".

RUN
@@ -24,7 +52,8 @@ 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:
+Executing the test without any parameter will run all supported tests.
+It takes about 68 seconds on a Intel(R) Xeon(R) Gold 6254 CPU @ 3.10GHz.

sudo ./resctrl_tests

--
2.27.0

2022-03-04 18:21:45

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v4 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework

In kselftest framework, all tests can be build/run at a time,
and a sub test also can be build/run individually. As follows:
$ make kselftest-all TARGETS=resctrl
$ make -C tools/testing/selftests run_tests
$ make -C tools/testing/selftests TARGETS=resctrl run_tests

However, resctrl_tests cannot be run using kselftest framework,
users have to change directory to tools/testing/selftests/resctrl/,
run "make" to build executable file "resctrl_tests",
and run "sudo ./resctrl_tests" to execute the test.

To build/run resctrl_tests using kselftest framework.
Modify tools/testing/selftests/Makefile
and tools/testing/selftests/resctrl/Makefile.

Even after this change, users can still build/run resctrl_tests
without using framework as before.

Signed-off-by: Shaopeng Tan <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/resctrl/Makefile | 18 +++---------------
2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index c852eb40c4f7..7df397c6893c 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -51,6 +51,7 @@ TARGETS += proc
TARGETS += pstore
TARGETS += ptrace
TARGETS += openat2
+TARGETS += resctrl
TARGETS += rlimits
TARGETS += rseq
TARGETS += rtc
diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
index 6bcee2ec91a9..30af27b07d21 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -1,17 +1,5 @@
-CC = $(CROSS_COMPILE)gcc
-CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
-SRCS=$(wildcard *.c)
-OBJS=$(SRCS:.c=.o)
+TEST_GEN_PROGS := resctrl_tests

-all: resctrl_tests
+include ../lib.mk

-$(OBJS): $(SRCS)
- $(CC) $(CFLAGS) -c $(SRCS)
-
-resctrl_tests: $(OBJS)
- $(CC) $(CFLAGS) -o $@ $^
-
-.PHONY: clean
-
-clean:
- $(RM) $(OBJS) resctrl_tests
+$(OUTPUT)/resctrl_tests: $(wildcard *.c)
--
2.27.0

2022-03-04 18:46:10

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v4 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received

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 <[email protected]>
Signed-off-by: Shaopeng Tan <[email protected]>
---
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;
--
2.27.0

2022-03-04 20:39:57

by Fenghua Yu

[permalink] [raw]
Subject: Re: [PATCH v4 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests

On Fri, Mar 04, 2022 at 07:38:33PM +0900, Shaopeng Tan wrote:
> In this patch series, I make restrl_tests build/run using kselftest
> framework, but some users do not known how to build/run resctrl_tests
> using kseltest framework.

Please don't use "I" or "we" in commit messages. Also the grammar seems
not right here.

>
> Add manual of how to make resctrl_tests build/run
> using kselftest framework into README.

Maybe change the commit message to this:

resctrl_tests can be built or run using kselftests framework. Add
description on how to do so in README.

>
> Signed-off-by: Shaopeng Tan <[email protected]>
> ---
> tools/testing/selftests/resctrl/README | 31 +++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/resctrl/README b/tools/testing/selftests/resctrl/README
> index 3d2bbd4fa3aa..268cf3f95bd5 100644
> --- a/tools/testing/selftests/resctrl/README
> +++ b/tools/testing/selftests/resctrl/README
> @@ -12,9 +12,37 @@ 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.
> +
> +USE KSELFTEST FRAMEWORK
> +-----------------------
> +
> +BUILD
> +-----

The "---" under titles are all same. This cannot tell readers clearly
topic hierarchies.

How about this?
+USE KSELFTEST FRAMEWORK
+-----------------------
+
+* BUILD
+

> +
> +Execute the following command in top level directory of the kernel source.
> +
> +Build resctrl:
> + $ make -C tools/testing/selftests TARGETS=resctrl
> +
> +RUN
> +---

How about this?
+* RUN
+
> +
> +Run resctrl:
> + $ make -C tools/testing/selftests TARGETS=resctrl run_tests

Run this as sudo or root.
+ $ sudo make -C tools/testing/selftests TARGETS=resctrl run_tests

> +
> +Using kselftest framework, the ./resctrl_tests will be run without any parameters.
> +
> +More details about kselftest framework as follow.
> +Documentation/dev-tools/kselftest.rst
> +
> +NOT USE KSELFTEST FRAMEWORK
> +---------------------------
> +
> BUILD
> -----
>
> +Execute the following command in this directory(tools/testing/selftests/resctrl/).
> Run "make" to build executable file "resctrl_tests".
>
> RUN
> @@ -24,7 +52,8 @@ 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:
> +Executing the test without any parameter will run all supported tests.
> +It takes about 68 seconds on a Intel(R) Xeon(R) Gold 6254 CPU @ 3.10GHz.

resctrl will add more tests in the future. Running time may be longer in
the future. I would suggest to remove the "It takes about 68 seconds..." line.

>
> sudo ./resctrl_tests
>
> --
> 2.27.0
>

Thanks.

-Fenghua

2022-03-07 09:32:14

by Shaopeng Tan (Fujitsu)

[permalink] [raw]
Subject: RE: [PATCH v4 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests

Hi Fenghua,

> On Fri, Mar 04, 2022 at 07:38:33PM +0900, Shaopeng Tan wrote:
> > In this patch series, I make restrl_tests build/run using kselftest
> > framework, but some users do not known how to build/run resctrl_tests
> > using kseltest framework.
>
> Please don't use "I" or "we" in commit messages. Also the grammar seems not
> right here.
>
> >
> > Add manual of how to make resctrl_tests build/run using kselftest
> > framework into README.
>
> Maybe change the commit message to this:
>
> resctrl_tests can be built or run using kselftests framework. Add description on
> how to do so in README.

Thanks for your advice. I will use it for commit log in next version.

> >
> > Signed-off-by: Shaopeng Tan <[email protected]>
> > ---
> > tools/testing/selftests/resctrl/README | 31
> > +++++++++++++++++++++++++-
> > 1 file changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/resctrl/README
> > b/tools/testing/selftests/resctrl/README
> > index 3d2bbd4fa3aa..268cf3f95bd5 100644
> > --- a/tools/testing/selftests/resctrl/README
> > +++ b/tools/testing/selftests/resctrl/README
> > @@ -12,9 +12,37 @@ 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.
> > +
> > +USE KSELFTEST FRAMEWORK
> > +-----------------------
> > +
> > +BUILD
> > +-----
>
> The "---" under titles are all same. This cannot tell readers clearly topic
> hierarchies.
>
> How about this?
> +USE KSELFTEST FRAMEWORK
> +-----------------------
> +
> +* BUILD
> +
>
> > +
> > +Execute the following command in top level directory of the kernel source.
> > +
> > +Build resctrl:
> > + $ make -C tools/testing/selftests TARGETS=resctrl
> > +
> > +RUN
> > +---
>
> How about this?
> +* RUN
> +
> > +
> > +Run resctrl:
> > + $ make -C tools/testing/selftests TARGETS=resctrl run_tests
>
> Run this as sudo or root.
> + $ sudo make -C tools/testing/selftests TARGETS=resctrl run_tests

Thanks, I will take your above advice in next version.

> > +
> > +Using kselftest framework, the ./resctrl_tests will be run without any
> parameters.
> > +
> > +More details about kselftest framework as follow.
> > +Documentation/dev-tools/kselftest.rst
> > +
> > +NOT USE KSELFTEST FRAMEWORK
> > +---------------------------
> > +
> > BUILD
> > -----
> >
> > +Execute the following command in this
> directory(tools/testing/selftests/resctrl/).
> > Run "make" to build executable file "resctrl_tests".
> >
> > RUN
> > @@ -24,7 +52,8 @@ 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:
> > +Executing the test without any parameter will run all supported tests.
> > +It takes about 68 seconds on a Intel(R) Xeon(R) Gold 6254 CPU @ 3.10GHz.
>
> resctrl will add more tests in the future. Running time may be longer in the
> future. I would suggest to remove the "It takes about 68 seconds..." line.

I added this sentence based on Shuah Khan's feedback, but I didn't consider extending this test.
I will delete this sentence in next version.

> > sudo ./resctrl_tests
> >
> > --
> > 2.27.0
> >

Best regards,
Tan Shaopeng