linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set
@ 2022-03-11  7:21 Shaopeng Tan
  2022-03-11  7:21 ` [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received Shaopeng Tan
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng

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

-- 
2.27.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  2022-03-11 23:12   ` Reinette Chatre
  2022-03-11  7:21 ` [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds Shaopeng Tan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng, Shuah Khan

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;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
  2022-03-11  7:21 ` [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  2022-03-11 23:12   ` Reinette Chatre
  2022-03-11  7:21 ` [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework Shaopeng Tan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng

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
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
  2022-03-11  7:21 ` [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received Shaopeng Tan
  2022-03-11  7:21 ` [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  2022-03-11 23:13   ` Reinette Chatre
  2022-03-11  7:21 ` [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework Shaopeng Tan
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng, Shuah Khan

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 <skhan@linuxfoundation.org>
Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
---
 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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
                   ` (2 preceding siblings ...)
  2022-03-11  7:21 ` [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  2022-03-11 23:14   ` Reinette Chatre
  2022-03-13 15:47   ` Muhammad Usama Anjum
  2022-03-11  7:21 ` [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests Shaopeng Tan
  2022-03-11  7:21 ` [PATCH v5 6/6] selftests/resctrl: Add missing SPDX license to Makefile Shaopeng Tan
  5 siblings, 2 replies; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng

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 <tan.shaopeng@jp.fujitsu.com>
---
 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 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..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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
                   ` (3 preceding siblings ...)
  2022-03-11  7:21 ` [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  2022-03-11 23:20   ` Reinette Chatre
  2022-03-11  7:21 ` [PATCH v5 6/6] selftests/resctrl: Add missing SPDX license to Makefile Shaopeng Tan
  5 siblings, 1 reply; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng

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.
+
+USE KSELFTEST FRAMEWORK
+=======================
+
+* BUILD
+-------
+
+Build executable file "resctrl_tests" at top level directory of the kernel source.
+ $ make -C tools/testing/selftests TARGETS=resctrl
+
+* RUN
+-----
+
+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.
+
+ $ sudo make -C tools/testing/selftests TARGETS=resctrl run_tests
+
+More details about kselftest framework as follow.
+Documentation/dev-tools/kselftest.rst
+
+NOT USE KSELFTEST FRAMEWORK
+===========================
+
 BUILD
 -----
 
-Run "make" to build executable file "resctrl_tests".
+Build executable file "resctrl_tests" at this directory(tools/testing/selftests/resctrl/).
+  $ 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.
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 6/6] selftests/resctrl: Add missing SPDX license to Makefile
  2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
                   ` (4 preceding siblings ...)
  2022-03-11  7:21 ` [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests Shaopeng Tan
@ 2022-03-11  7:21 ` Shaopeng Tan
  5 siblings, 0 replies; 13+ messages in thread
From: Shaopeng Tan @ 2022-03-11  7:21 UTC (permalink / raw)
  To: Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: linux-kernel, linux-kselftest, tan.shaopeng

Add the missing SPDX(SPDX-License-Identifier) license header to
tools/testing/selftests/resctrl/Makefile.

Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
---
 tools/testing/selftests/resctrl/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/resctrl/Makefile b/tools/testing/selftests/resctrl/Makefile
index 30af27b07d21..71e80bb19dd6 100644
--- a/tools/testing/selftests/resctrl/Makefile
+++ b/tools/testing/selftests/resctrl/Makefile
@@ -1,3 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
 TEST_GEN_PROGS := resctrl_tests
 
 include ../lib.mk
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received
  2022-03-11  7:21 ` [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received Shaopeng Tan
@ 2022-03-11 23:12   ` Reinette Chatre
  0 siblings, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-03-11 23:12 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Shuah Khan
  Cc: linux-kernel, linux-kselftest, Shuah Khan

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds
  2022-03-11  7:21 ` [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds Shaopeng Tan
@ 2022-03-11 23:12   ` Reinette Chatre
  0 siblings, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-03-11 23:12 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Shuah Khan; +Cc: linux-kernel, linux-kselftest



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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework
  2022-03-11  7:21 ` [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework Shaopeng Tan
@ 2022-03-11 23:13   ` Reinette Chatre
  0 siblings, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-03-11 23:13 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Shuah Khan
  Cc: linux-kernel, linux-kselftest, Shuah Khan

Hi Shaopeng Tan,

On 3/10/2022 11:21 PM, Shaopeng Tan wrote:
> 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 <skhan@linuxfoundation.org>
> Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
> ---
>  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");

The resctrl tests do not depend on or exercise the code enabled when PROC_CPU_RESCTRL
is set so there is no need to require users to enable it. It (PROC_CPU_RESCTRL) is
automatically selected anyway if PROC_FS is enabled.

An explicit check for the existence of /proc/{pid}/cpu_resctrl_groups would be
required if somebody ever does add a resctrl test to exercise the code enabled
by that config. There is no need to require that setting now.

Thus could just be "resctrl FS does not exist. Enable X86_CPU_RESCTRL config option."

>  
>  	filter_dmesg();
>  

Reinette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework
  2022-03-11  7:21 ` [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework Shaopeng Tan
@ 2022-03-11 23:14   ` Reinette Chatre
  2022-03-13 15:47   ` Muhammad Usama Anjum
  1 sibling, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-03-11 23:14 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Shuah Khan; +Cc: linux-kernel, linux-kselftest

Hi Shaopeng Tan,

On 3/10/2022 11:21 PM, Shaopeng Tan wrote:
> 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 <tan.shaopeng@jp.fujitsu.com>
> ---
>  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 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..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

Please do keep these CFLAGS. You can find motivation in
a9d26a302dea ("selftests/resctrl: Enable gcc checks to detect buffer overflows")

> -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)

Reinette

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests
  2022-03-11  7:21 ` [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests Shaopeng Tan
@ 2022-03-11 23:20   ` Reinette Chatre
  0 siblings, 0 replies; 13+ messages in thread
From: Reinette Chatre @ 2022-03-11 23:20 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Shuah Khan; +Cc: linux-kernel, linux-kselftest

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework
  2022-03-11  7:21 ` [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework Shaopeng Tan
  2022-03-11 23:14   ` Reinette Chatre
@ 2022-03-13 15:47   ` Muhammad Usama Anjum
  1 sibling, 0 replies; 13+ messages in thread
From: Muhammad Usama Anjum @ 2022-03-13 15:47 UTC (permalink / raw)
  To: Shaopeng Tan, Fenghua Yu, Reinette Chatre, Shuah Khan
  Cc: usama.anjum, linux-kernel, linux-kselftest

On 3/11/22 12:21 PM, Shaopeng Tan wrote:
> 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 <tan.shaopeng@jp.fujitsu.com>
> ---
>  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 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..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
CFLAGS = $(KHDR_INCLUDES)
Please can you add this line to build this test with kernel's latest
uapi headers always? At this moment, this isn't required. But in future
if some kernel headers are changed and this test is updated to build
with latest kernel headers, the build will start to fail when following
command will be used:
make -C tools/testing/selftests O=build

> -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)

-- 
Muhammad Usama Anjum

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-03-13 15:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-11  7:21 [PATCH v5 0/6] selftests/resctrl: Add resctrl_tests into kselftest set Shaopeng Tan
2022-03-11  7:21 ` [PATCH v5 1/6] selftests/resctrl: Kill child process before parent process terminates if SIGTERM is received Shaopeng Tan
2022-03-11 23:12   ` Reinette Chatre
2022-03-11  7:21 ` [PATCH v5 2/6] selftests/resctrl: Change the default limited time to 120 seconds Shaopeng Tan
2022-03-11 23:12   ` Reinette Chatre
2022-03-11  7:21 ` [PATCH v5 3/6] selftests/resctrl: Fix resctrl_tests' return code to work with selftest framework Shaopeng Tan
2022-03-11 23:13   ` Reinette Chatre
2022-03-11  7:21 ` [PATCH v5 4/6] selftests/resctrl: Make resctrl_tests run using kselftest framework Shaopeng Tan
2022-03-11 23:14   ` Reinette Chatre
2022-03-13 15:47   ` Muhammad Usama Anjum
2022-03-11  7:21 ` [PATCH v5 5/6] selftests/resctrl: Update README about using kselftest framework to build/run resctrl_tests Shaopeng Tan
2022-03-11 23:20   ` Reinette Chatre
2022-03-11  7:21 ` [PATCH v5 6/6] selftests/resctrl: Add missing SPDX license to Makefile Shaopeng Tan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).