2022-03-18 15:06:13

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v6 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 v5:
- Changed skip message when resctrlfs is not supported. [PATCH v6 3/6]
- Changed resctrl's Makefile to always build with latest kernel headers and
keep enabling gcc checks to detect buffer overflows. [PATCH v6 4/6]
- Made README easier to understand. [PATCH v6 5/6]
https://lore.kernel.org/lkml/[email protected]/ [PATCH V5]

This patch series is based on v5.17-rc8.

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 | 19 +++------
tools/testing/selftests/resctrl/README | 39 +++++++++++++++----
.../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(+), 22 deletions(-)
create mode 100644 tools/testing/selftests/resctrl/settings

--
2.27.0


2022-03-18 15:06:15

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v6 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]>
Reviewed-by: Reinette Chatre <[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-19 14:22:09

by Reinette Chatre

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

Hi Shaopeng Tan,

On 3/18/2022 12:58 AM, Shaopeng Tan wrote:
> 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 v5:
> - Changed skip message when resctrlfs is not supported. [PATCH v6 3/6]
> - Changed resctrl's Makefile to always build with latest kernel headers and
> keep enabling gcc checks to detect buffer overflows. [PATCH v6 4/6]
> - Made README easier to understand. [PATCH v6 5/6]
> https://lore.kernel.org/lkml/[email protected]/ [PATCH V5]
>
> This patch series is based on v5.17-rc8.
>

I do not think this is accurate because the KHDR_INCLUDES you use in patch 4/6
does not yet exist in v5.17.rc8. It seems to be available on the "next" branch
of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git

Reinette

2022-03-21 12:50:52

by Shaopeng Tan

[permalink] [raw]
Subject: [PATCH v6 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 | 17 ++++-------------
2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index d08fe4cfe811..6138354b3760 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -52,6 +52,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..bee5fa8f1ac9 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -1,17 +1,8 @@
-CC = $(CROSS_COMPILE)gcc
CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
-SRCS=$(wildcard *.c)
-OBJS=$(SRCS:.c=.o)
+CFLAGS += $(KHDR_INCLUDES)

-all: resctrl_tests
+TEST_GEN_PROGS := resctrl_tests

-$(OBJS): $(SRCS)
- $(CC) $(CFLAGS) -c $(SRCS)
+include ../lib.mk

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

2022-03-21 16:10:15

by Fenghua Yu

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

On Fri, Mar 18, 2022 at 04:58:02PM +0900, Shaopeng Tan wrote:
> In kselftest framework, a sub test is run using the timeout utility
s/is run/runs/
> 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

s/kill/Kill the/ add "the" before "parent"

> if SIGTERM signal is received.
>
> Reviewed-by: Shuah Khan <[email protected]>
> Reviewed-by: Reinette Chatre <[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

Please fix the typos.

Reviewed-by: Fenghua Yu <[email protected]>

Thanks.

-Fenghua

2022-03-21 23:20:41

by Fenghua Yu

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

On Fri, Mar 18, 2022 at 04:58:05PM +0900, Shaopeng Tan wrote:
> In kselftest framework, all tests can be build/run at a time,
s/build/built/
> and a sub test also can be build/run individually. As follows:
s/build/built/
> $ 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 | 17 ++++-------------
> 2 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index d08fe4cfe811..6138354b3760 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -52,6 +52,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..bee5fa8f1ac9 100644
> --- a/tools/testing/selftests/resctrl/Makefile
> +++ b/tools/testing/selftests/resctrl/Makefile
> @@ -1,17 +1,8 @@
> -CC = $(CROSS_COMPILE)gcc
> CFLAGS = -g -Wall -O2 -D_FORTIFY_SOURCE=2
> -SRCS=$(wildcard *.c)
> -OBJS=$(SRCS:.c=.o)
> +CFLAGS += $(KHDR_INCLUDES)
>
> -all: resctrl_tests
> +TEST_GEN_PROGS := resctrl_tests
>
> -$(OBJS): $(SRCS)
> - $(CC) $(CFLAGS) -c $(SRCS)
> +include ../lib.mk
>
> -resctrl_tests: $(OBJS)
> - $(CC) $(CFLAGS) -o $@ $^
> -
> -.PHONY: clean
> -
> -clean:
> - $(RM) $(OBJS) resctrl_tests
> +$(OUTPUT)/resctrl_tests: $(wildcard *.c)
> --
> 2.27.0
>

Reviewed-by: Fenghua Yu <[email protected]>

Thanks.

-Fenghua