- Linux-kselftest-mirror - lists.linaro.org

[PATCH] selftests/ftrace: Improve integration with kselftest runner

by Mark Brown

The ftrace selftests do not currently produce KTAP output, they produce a custom format much nicer for human consumption. This means that when run in automated test systems we just get a single result for the suite as a whole rather than recording results for individual test cases, making it harder to look at the test data and masking things like inappropriate skips. Address this by adding support for KTAP output to the ftracetest script and providing a trivial wrapper which will be invoked by the kselftest runner to generate output in this format by default, users using ftracetest directly will continue to get the existing output. This is not the most elegant solution but it is simple and effective. I did consider implementing this by post processing the existing output format but that felt more complex and likely to result in all output being lost if something goes seriously wrong during the run which would not be helpful. I did also consider just writing a separate runner script but there's enough going on with things like the signal handling for that to seem like it would be duplicating too much. Signed-off-by: Mark Brown <broonie(a)kernel.org> --- tools/testing/selftests/ftrace/Makefile | 3 +- tools/testing/selftests/ftrace/ftracetest | 63 ++++++++++++++++++++++++-- tools/testing/selftests/ftrace/ftracetest-ktap | 8 ++++ 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/ftrace/Makefile b/tools/testing/selftests/ftrace/Makefile index d6e106fbce11..a1e955d2de4c 100644 --- a/tools/testing/selftests/ftrace/Makefile +++ b/tools/testing/selftests/ftrace/Makefile @@ -1,7 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 all: -TEST_PROGS := ftracetest +TEST_PROGS_EXTENDED := ftracetest +TEST_PROGS := ftracetest-ktap TEST_FILES := test.d settings EXTRA_CLEAN := $(OUTPUT)/logs/* diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index c3311c8c4089..539c8d6d5d71 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -13,6 +13,7 @@ echo "Usage: ftracetest [options] [testcase(s)] [testcase-directory(s)]" echo " Options:" echo " -h|--help Show help message" echo " -k|--keep Keep passed test logs" +echo " -K|--KTAP Output in KTAP format" echo " -v|--verbose Increase verbosity of test messages" echo " -vv Alias of -v -v (Show all results in stdout)" echo " -vvv Alias of -v -v -v (Show all commands immediately)" @@ -85,6 +86,10 @@ parse_opts() { # opts KEEP_LOG=1 shift 1 ;; + --ktap|-K) + KTAP=1 + shift 1 + ;; --verbose|-v|-vv|-vvv) if [ $VERBOSE -eq -1 ]; then usage "--console can not use with --verbose" @@ -178,6 +183,7 @@ TEST_DIR=$TOP_DIR/test.d TEST_CASES=`find_testcases $TEST_DIR` LOG_DIR=$TOP_DIR/logs/`date +%Y%m%d-%H%M%S`/ KEEP_LOG=0 +KTAP=0 DEBUG=0 VERBOSE=0 UNSUPPORTED_RESULT=0 @@ -229,7 +235,7 @@ prlog() { # messages newline= shift fi - printf "$*$newline" + [ "$KTAP" != "1" ] && printf "$*$newline" [ "$LOG_FILE" ] && printf "$*$newline" | strip_esc >> $LOG_FILE } catlog() { #file @@ -260,11 +266,11 @@ TOTAL_RESULT=0 INSTANCE= CASENO=0 +CASENAME= testcase() { # testfile CASENO=$((CASENO+1)) - desc=`grep "^#[ \t]*description:" $1 | cut -f2- -d:` - prlog -n "[$CASENO]$INSTANCE$desc" + CASENAME=`grep "^#[ \t]*description:" $1 | cut -f2- -d:` } checkreq() { # testfile @@ -277,40 +283,68 @@ test_on_instance() { # testfile grep -q "^#[ \t]*flags:.*instance" $1 } +ktaptest() { # result comment + if [ "$KTAP" != "1" ]; then + return + fi + + local result= + if [ "$1" = "1" ]; then + result="ok" + else + result="not ok" + fi + shift + + local comment=$* + if [ "$comment" != "" ]; then + comment="# $comment" + fi + + echo $CASENO $result $INSTANCE$CASENAME $comment +} + eval_result() { # sigval case $1 in $PASS) prlog " [${color_green}PASS${color_reset}]" + ktaptest 1 PASSED_CASES="$PASSED_CASES $CASENO" return 0 ;; $FAIL) prlog " [${color_red}FAIL${color_reset}]" + ktaptest 0 FAILED_CASES="$FAILED_CASES $CASENO" return 1 # this is a bug. ;; $UNRESOLVED) prlog " [${color_blue}UNRESOLVED${color_reset}]" + ktaptest 0 UNRESOLVED UNRESOLVED_CASES="$UNRESOLVED_CASES $CASENO" return $UNRESOLVED_RESULT # depends on use case ;; $UNTESTED) prlog " [${color_blue}UNTESTED${color_reset}]" + ktaptest 1 SKIP UNTESTED_CASES="$UNTESTED_CASES $CASENO" return 0 ;; $UNSUPPORTED) prlog " [${color_blue}UNSUPPORTED${color_reset}]" + ktaptest 1 SKIP UNSUPPORTED_CASES="$UNSUPPORTED_CASES $CASENO" return $UNSUPPORTED_RESULT # depends on use case ;; $XFAIL) prlog " [${color_green}XFAIL${color_reset}]" + ktaptest 1 XFAIL XFAILED_CASES="$XFAILED_CASES $CASENO" return 0 ;; *) prlog " [${color_blue}UNDEFINED${color_reset}]" + ktaptest 0 error UNDEFINED_CASES="$UNDEFINED_CASES $CASENO" return 1 # this must be a test bug ;; @@ -371,6 +405,7 @@ __run_test() { # testfile run_test() { # testfile local testname=`basename $1` testcase $1 + prlog -n "[$CASENO]$INSTANCE$CASENAME" if [ ! -z "$LOG_FILE" ] ; then local testlog=`mktemp $LOG_DIR/${CASENO}-${testname}-log.XXXXXX` else @@ -405,6 +440,17 @@ run_test() { # testfile # load in the helper functions . $TEST_DIR/functions +if [ "$KTAP" = "1" ]; then + echo "TAP version 13" + + casecount=`echo $TEST_CASES | wc -w` + for t in $TEST_CASES; do + test_on_instance $t || continue + casecount=$((casecount+1)) + done + echo "1..${casecount}" +fi + # Main loop for t in $TEST_CASES; do run_test $t @@ -439,6 +485,17 @@ prlog "# of unsupported: " `echo $UNSUPPORTED_CASES | wc -w` prlog "# of xfailed: " `echo $XFAILED_CASES | wc -w` prlog "# of undefined(test bug): " `echo $UNDEFINED_CASES | wc -w` +if [ "$KTAP" = "1" ]; then + echo -n "# Totals:" + echo -n " pass:"`echo $PASSED_CASES | wc -w` + echo -n " faii:"`echo $FAILED_CASES | wc -w` + echo -n " xfail:"`echo $XFAILED_CASES | wc -w` + echo -n " xpass:0" + echo -n " skip:"`echo $UNTESTED_CASES $UNSUPPORTED_CASES | wc -w` + echo -n " error:"`echo $UNRESOLVED_CASES $UNDEFINED_CASES | wc -w` + echo +fi + cleanup # if no error, return 0 diff --git a/tools/testing/selftests/ftrace/ftracetest-ktap b/tools/testing/selftests/ftrace/ftracetest-ktap new file mode 100755 index 000000000000..b3284679ef3a --- /dev/null +++ b/tools/testing/selftests/ftrace/ftracetest-ktap @@ -0,0 +1,8 @@ +#!/bin/sh -e +# SPDX-License-Identifier: GPL-2.0-only +# +# ftracetest-ktap: Wrapper to integrate ftracetest with the kselftest runner +# +# Copyright (C) Arm Ltd., 2023 + +./ftracetest -K --- base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6 change-id: 20230302-ftrace-kselftest-ktap-9d7878691557 Best regards, -- Mark Brown <broonie(a)kernel.org>

2 years, 3 months

4
4
0 0

[PATCH] selftests: sched: Add more core schedule prctl calls

by Ivan Orlov

The core sched kselftest makes prctl calls only with correct parameters. This patch will extend this test with more core schedule prctl calls with wrong parameters to increase code coverage. Signed-off-by: Ivan Orlov <ivan.orlov0322(a)gmail.com> --- tools/testing/selftests/sched/cs_prctl_test.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/testing/selftests/sched/cs_prctl_test.c b/tools/testing/selftests/sched/cs_prctl_test.c index 25e0d95d3713..df8e4629b3b5 100644 --- a/tools/testing/selftests/sched/cs_prctl_test.c +++ b/tools/testing/selftests/sched/cs_prctl_test.c @@ -334,6 +334,14 @@ int main(int argc, char *argv[]) validate(get_cs_cookie(pid) != 0); validate(get_cs_cookie(pid) == get_cs_cookie(procs[pidx].thr_tids[0])); + printf("\n## Try to pass wrong subcommand PR_SCHED_CORE_MAX = %d to prctl (shouldn't succeed)\n", PR_SCHED_CORE_MAX); + validate(_prctl(PR_SCHED_CORE, PR_SCHED_CORE_MAX, 0, PIDTYPE_PGID, 0) < 0 + && errno == EINVAL); + + printf("\n## Try to pass not-null uaddr with forbidden command (shouldn't succeed)\n"); + validate(_prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, 0, PIDTYPE_PGID, 1) < 0 + && errno == EINVAL); + if (errors) { printf("TESTS FAILED. errors: %d\n", errors); res = 10; -- 2.34.1

2 years, 3 months

2
1
0 0

[PATCH rcu v2 0/7] RCU-related lockdep changes for v6.4

by Boqun Feng

Hi, Another week, another version ;-) Previous versions: v1: https://lore.kernel.org/rcu/20230317031339.10277-1-boqun.feng@gmail.com/ Changes since v1: * Minor typo fixes * Apply Acked-by tags * Add license and copyright headers for srcu_lockdep.sh Thank you all for helping improve this! You can also find this series at https://github/fbq/linux rcu/lockdep.2023.03.22a top commit is: 450ef54e199f List of changes: Boqun Feng (4): locking/lockdep: Introduce lock_sync() rcu: Annotate SRCU's update-side lockdep dependencies locking: Reduce the number of locks in ww_mutex stress tests locking/lockdep: Improve the deadlock scenario print for sync and read lock Paul E. McKenney (3): rcutorture: Add SRCU deadlock scenarios rcutorture: Add RCU Tasks Trace and SRCU deadlock scenarios rcutorture: Add srcu_lockdep.sh include/linux/lockdep.h | 8 +- include/linux/srcu.h | 34 +++- kernel/locking/lockdep.c | 64 +++++- kernel/locking/test-ww_mutex.c | 2 +- kernel/rcu/rcutorture.c | 185 ++++++++++++++++++ kernel/rcu/srcutiny.c | 2 + kernel/rcu/srcutree.c | 2 + .../selftests/rcutorture/bin/srcu_lockdep.sh | 78 ++++++++ 8 files changed, 364 insertions(+), 11 deletions(-) create mode 100755 tools/testing/selftests/rcutorture/bin/srcu_lockdep.sh -- 2.38.1

2 years, 3 months

2
8
0 0

[PATCH 0/5] usermode_driver: Add management library and API

by Roberto Sassu

From: Roberto Sassu <roberto.sassu(a)huawei.com> A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH), which runs a user space process from a binary blob, and creates a bidirectional pipe, so that the kernel can make a request to that process, and the latter provides its response. It is currently used by bpfilter, although it does not seem to do any useful work. The problem is, if other users would like to implement a UMD similar to bpfilter, they would have to duplicate the code. Instead, make an UMD management library and API from the existing bpfilter and sockopt code, and move it to common kernel code. Also, define the software architecture and the main components of the library: the UMD Manager, running in the kernel, acting as the frontend interface to any user or kernel-originated request; the UMD Loader, also running in the kernel, responsible to load the UMD Handler; the UMD Handler, running in user space, responsible to handle requests from the UMD Manager and to send to it the response. I have two use cases, but for sake of brevity I will propose one. I would like to add support for PGP keys and signatures in the kernel, so that I can extend secure boot to applications, and allow/deny code execution based on the signed file digests included in RPM headers. While I proposed a patch set a while ago (based on a previous work of David Howells), the main objection was that the PGP packet parser should not run in the kernel. That makes a perfect example for using a UMD. If the PGP parser is moved to user space (UMD Handler), and the kernel (UMD Manager) just instantiates the key and verifies the signature on already parsed data, this would address the concern. Patch 1 moves the function bpfilter_send_req() to usermode_driver.c and makes the pipe between the kernel and the user space process suitable for larger quantity of data (> 64K). Patch 2 introduces the management library and API. Patch 3 replaces the existing bpfilter and sockopt code with calls to the management API. To use the new mechanism, sockopt itself (acts as UMD Manager) now sends/receives messages to/from bpfilter_umh (acts as UMD Handler), instead of bpfilter (acts as UMD Loader). Patch 4 introduces a sample UMD, useful for other implementors, and uses it for testing. Patch 5 introduces the documentation of the new management library and API. Roberto Sassu (5): usermode_driver: Introduce umd_send_recv() from bpfilter usermode_driver_mgmt: Introduce management of user mode drivers bpfilter: Port to user mode driver management API selftests/umd_mgmt: Add selftests for UMD management library doc: Add documentation for the User Mode Driver management library Documentation/driver-api/index.rst | 1 + Documentation/driver-api/umd_mgmt.rst | 99 +++++++++++++ MAINTAINERS | 9 ++ include/linux/bpfilter.h | 12 +- include/linux/usermode_driver.h | 2 + include/linux/usermode_driver_mgmt.h | 35 +++++ kernel/Makefile | 2 +- kernel/usermode_driver.c | 47 +++++- kernel/usermode_driver_mgmt.c | 137 ++++++++++++++++++ net/bpfilter/bpfilter_kern.c | 120 +-------------- net/ipv4/bpfilter/sockopt.c | 67 +++++---- tools/testing/selftests/Makefile | 1 + tools/testing/selftests/umd_mgmt/.gitignore | 1 + tools/testing/selftests/umd_mgmt/Makefile | 14 ++ tools/testing/selftests/umd_mgmt/config | 1 + .../selftests/umd_mgmt/sample_umd/Makefile | 22 +++ .../selftests/umd_mgmt/sample_umd/msgfmt.h | 13 ++ .../umd_mgmt/sample_umd/sample_binary_blob.S | 7 + .../umd_mgmt/sample_umd/sample_handler.c | 81 +++++++++++ .../umd_mgmt/sample_umd/sample_loader.c | 28 ++++ .../umd_mgmt/sample_umd/sample_mgr.c | 124 ++++++++++++++++ tools/testing/selftests/umd_mgmt/umd_mgmt.sh | 40 +++++ 22 files changed, 707 insertions(+), 156 deletions(-) create mode 100644 Documentation/driver-api/umd_mgmt.rst create mode 100644 include/linux/usermode_driver_mgmt.h create mode 100644 kernel/usermode_driver_mgmt.c create mode 100644 tools/testing/selftests/umd_mgmt/.gitignore create mode 100644 tools/testing/selftests/umd_mgmt/Makefile create mode 100644 tools/testing/selftests/umd_mgmt/config create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/Makefile create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c create mode 100755 tools/testing/selftests/umd_mgmt/umd_mgmt.sh -- 2.25.1

2 years, 3 months

3
12
0 0

Re: [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()

by David Vernet

On Mon, Mar 27, 2023 at 07:56:03AM +0200, Markus Elfring wrote: > >> 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed? > > > > Perhaps next time you can answer your own question by spending 30 > > seconds actually reading the code you're "fixing": > > > > int cg_destroy(const char *cgroup) > > { > … > > ret = rmdir(cgroup); > … > > if (ret && errno == ENOENT) <<< that case is explicitly handled here > > ret = 0; > > > > return ret; > > } > > Is it interesting somehow that a non-existing directory (which would occasionally > not be found) is tolerated so far? > https://elixir.bootlin.com/linux/v6.3-rc3/source/tools/testing/selftests/cg… > > Should such a function call be avoided because of a failed cg_create() call? The point is that (a) you were wrong that this is fixing anything, and (b) this patch is functionally useless. Sure, we could move some goto's around and subjectively improve "something". Why? What's the point? It's highly debatable that what you're doing is even an improvement, and I'm not interested in wasting time pontificating about the merits of a trivial "fix" for a test cleanup function that isn't even broken. Several people have already either advised or directly asked you to stop sending these patches. I'm not sure why you're choosing to ignore them, but I'll throw my hat in the ring regardless and do the same. Please stop sending these fake cleanup patches.

2 years, 3 months

1
0
0 0

[PATCH net-next] selftests: tls: add a test for queuing data before setting the ULP

by Jakub Kicinski

Other tests set up the connection fully on both ends before communicating any data. Add a test which will queue up TLS records to TCP before the TLS ULP is installed. Signed-off-by: Jakub Kicinski <kuba(a)kernel.org> --- CC: shuah(a)kernel.org CC: linux-kselftest(a)vger.kernel.org --- tools/testing/selftests/net/tls.c | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c index 2cbb12736596..e699548d4247 100644 --- a/tools/testing/selftests/net/tls.c +++ b/tools/testing/selftests/net/tls.c @@ -1820,4 +1820,49 @@ TEST(tls_v6ops) { close(sfd); } +TEST(prequeue) { + struct tls_crypto_info_keys tls12; + char buf[20000], buf2[20000]; + struct sockaddr_in addr; + int sfd, cfd, ret, fd; + socklen_t len; + + len = sizeof(addr); + memrnd(buf, sizeof(buf)); + + tls_crypto_info_init(TLS_1_2_VERSION, TLS_CIPHER_AES_GCM_256, &tls12); + + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_ANY); + addr.sin_port = 0; + + fd = socket(AF_INET, SOCK_STREAM, 0); + sfd = socket(AF_INET, SOCK_STREAM, 0); + + ASSERT_EQ(bind(sfd, &addr, sizeof(addr)), 0); + ASSERT_EQ(listen(sfd, 10), 0); + ASSERT_EQ(getsockname(sfd, &addr, &len), 0); + ASSERT_EQ(connect(fd, &addr, sizeof(addr)), 0); + ASSERT_GE(cfd = accept(sfd, &addr, &len), 0); + close(sfd); + + ret = setsockopt(fd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")); + if (ret) { + ASSERT_EQ(errno, ENOENT); + SKIP(return, "no TLS support"); + } + + ASSERT_EQ(setsockopt(fd, SOL_TLS, TLS_TX, &tls12, tls12.len), 0); + EXPECT_EQ(send(fd, buf, sizeof(buf), MSG_DONTWAIT), sizeof(buf)); + + ASSERT_EQ(setsockopt(cfd, IPPROTO_TCP, TCP_ULP, "tls", sizeof("tls")), 0); + ASSERT_EQ(setsockopt(cfd, SOL_TLS, TLS_RX, &tls12, tls12.len), 0); + EXPECT_EQ(recv(cfd, buf2, sizeof(buf2), MSG_WAITALL), sizeof(buf2)); + + EXPECT_EQ(memcmp(buf, buf2, sizeof(buf)), 0); + + close(fd); + close(cfd); +} + TEST_HARNESS_MAIN -- 2.39.2

2 years, 3 months

2
1
0 0

[KTAP V2 PATCH] ktap_v2: add skip test result

by Rae Moar

Add the test result "skip" to KTAP version 2 as an alternative way to indicate a test was skipped. The current spec uses the "#SKIP" directive to indicate that a test was skipped. However, the "#SKIP" directive is not always evident when quickly skimming through KTAP results. The "skip" result would provide an alternative that could make it clearer that a test has not successfully passed because it was skipped. Before: KTAP version 1 1..1 KTAP version 1 1..2 ok 1 case_1 ok 2 case_2 #SKIP ok 1 suite After: KTAP version 2 1..1 KTAP version 2 1..2 ok 1 case_1 skip 2 case_2 ok 1 suite Here is a link to a version of the KUnit parser that is able to parse the skip test result for KTAP version 2. Note this parser is still able to parse the "#SKIP" directive. Link: https://kunit-review.googlesource.com/c/linux/+/5689 Signed-off-by: Rae Moar <rmoar(a)google.com> --- Note: this patch is based on Frank's ktap_spec_version_2 branch. Documentation/dev-tools/ktap.rst | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/Documentation/dev-tools/ktap.rst b/Documentation/dev-tools/ktap.rst index ff77f4aaa6ef..f48aa00db8f0 100644 --- a/Documentation/dev-tools/ktap.rst +++ b/Documentation/dev-tools/ktap.rst @@ -74,7 +74,8 @@ They are required and must have the format: <result> <number> [<description>][ # [<directive>] [<diagnostic data>]] The result can be either "ok", which indicates the test case passed, -or "not ok", which indicates that the test case failed. +"not ok", which indicates that the test case failed, or "skip", which indicates +the test case did not run. <number> represents the number of the test being performed. The first test must have the number 1 and the number then must increase by 1 for each additional @@ -91,12 +92,13 @@ A directive is a keyword that indicates a different outcome for a test other than passed and failed. The directive is optional, and consists of a single keyword preceding the diagnostic data. In the event that a parser encounters a directive it doesn't support, it should fall back to the "ok" / "not ok" -result. +/ "skip" result. Currently accepted directives are: -- "SKIP", which indicates a test was skipped (note the result of the test case - result line can be either "ok" or "not ok" if the SKIP directive is used) +- "SKIP", which indicates a test was skipped (note this is an alternative to + the "skip" result type and if the SKIP directive is used, the + result can be any type - "ok", "not ok", or "skip") - "TODO", which indicates that a test is not expected to pass at the moment, e.g. because the feature it is testing is known to be broken. While this directive is inherited from TAP, its use in the kernel is discouraged. @@ -110,7 +112,7 @@ Currently accepted directives are: The diagnostic data is a plain-text field which contains any additional details about why this result was produced. This is typically an error message for ERROR -or failed tests, or a description of missing dependencies for a SKIP result. +or failed tests, or a description of missing dependencies for a skipped test. The diagnostic data field is optional, and results which have neither a directive nor any diagnostic data do not need to include the "#" field @@ -130,11 +132,18 @@ The test "test_case_name" failed. :: - ok 1 test # SKIP necessary dependency unavailable + skip 1 test # necessary dependency unavailable -The test "test" was SKIPPED with the diagnostic message "necessary dependency +The test "test" was skipped with the diagnostic message "necessary dependency unavailable". +:: + + ok 1 test_2 # SKIP this test should not run + +The test "test_2" was skipped with the diagnostic message "this test +should not run". + :: not ok 1 test # TIMEOUT 30 seconds @@ -225,7 +234,7 @@ An example format with multiple levels of nested testing: not ok 1 test_1 ok 2 test_2 not ok 1 test_3 - ok 2 test_4 # SKIP + skip 2 test_4 not ok 1 example_test_1 ok 2 example_test_2 @@ -262,7 +271,7 @@ Example KTAP output ok 1 example_test_1 KTAP version 2 1..2 - ok 1 test_1 # SKIP test_1 skipped + skip 1 test_1 # test_1 skipped ok 2 test_2 ok 2 example_test_2 KTAP version 2 base-commit: 906f02e42adfbd5ae70d328ee71656ecb602aaf5 -- 2.40.0.rc1.284.g88254d51c5-goog

2 years, 3 months

5
14
0 0

Re: [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()

by David Vernet

On Sun, Mar 26, 2023 at 10:15:31AM +0200, Markus Elfring wrote: [...] > >> > >> Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group") > > > > Fixes what in the what now? > > 1. Check repetition (which can be undesirable) > > 2. Can a cg_destroy() call ever work as expected if a cg_create() call failed? Perhaps next time you can answer your own question by spending 30 seconds actually reading the code you're "fixing": int cg_destroy(const char *cgroup) { int ret; retry: ret = rmdir(cgroup); if (ret && errno == EBUSY) { cg_killall(cgroup); usleep(100); goto retry; } if (ret && errno == ENOENT) <<< that case is explicitly handled here ret = 0; return ret; }

2 years, 3 months

1
0
0 0

[PATCH 0/5] cgroup/cpuset: Miscellaneous updates

by Waiman Long

This patch series includes miscellaneous update to the cpuset and its testing code. Patch 2 is actually a follow-up of commit 3fb906e7fabb ("cgroup/cpuset: Don't filter offline CPUs in cpuset_cpus_allowed() for top cpuset tasks"). Patches 3-4 are for handling corner cases when dealing with task_cpu_possible_mask(). Waiman Long (5): cgroup/cpuset: Skip task update if hotplug doesn't affect current cpuset cgroup/cpuset: Include offline CPUs when tasks' cpumasks in top_cpuset are updated cgroup/cpuset: Find another usable CPU if none found in current cpuset cgroup/cpuset: Add CONFIG_DEBUG_CPUSETS config for cpuset testing cgroup/cpuset: Minor updates to test_cpuset_prs.sh init/Kconfig | 5 + kernel/cgroup/cpuset.c | 155 +++++++++++++++++- .../selftests/cgroup/test_cpuset_prs.sh | 25 +-- 3 files changed, 165 insertions(+), 20 deletions(-) -- 2.31.1

2 years, 3 months

4
22
0 0

Re: [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events()

by Lorenzo Stoakes

On Sat, Mar 25, 2023 at 07:30:21PM +0100, Markus Elfring wrote: > Date: Sat, 25 Mar 2023 19:11:13 +0100 > > The label “cleanup” was used to jump to another pointer check despite of > the detail in the implementation of the function > “test_memcg_oom_group_score_events” that it was determined already > that a corresponding variable contained a null pointer. This is poorly writte and confusing. Something like 'avoid unnecessary null check/cg_destroy() invocation' would be far clearer. > > 1. Thus return directly after a call of the function “cg_name” failed. > This feels superfluious. > 2. Use an additional label. This also feels superfluious. > > 3. Delete a questionable check. This seems superfluois and frankly, rude. It's not questionable, it's readable, you should try to avoid language like 'questionable' when the purpose of the check is obvious. > > > This issue was detected by using the Coccinelle software. > > Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group") Fixes what in the what now? This is not a bug fix, it's a 'questionable' refactoring. > Signed-off-by: Markus Elfring <elfring(a)users.sourceforge.net> > --- > tools/testing/selftests/cgroup/test_memcontrol.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > index f4f7c0aef702..afcd1752413e 100644 > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > @@ -1242,12 +1242,11 @@ static int test_memcg_oom_group_score_events(const char *root) > int safe_pid; > > memcg = cg_name(root, "memcg_test_0"); > - > if (!memcg) > - goto cleanup; > + return ret; > > if (cg_create(memcg)) > - goto cleanup; > + goto free_cg; > > if (cg_write(memcg, "memory.max", "50M")) > goto cleanup; > @@ -1275,8 +1274,8 @@ static int test_memcg_oom_group_score_events(const char *root) > ret = KSFT_PASS; > > cleanup: > - if (memcg) > - cg_destroy(memcg); > + cg_destroy(memcg); > +free_cg: > free(memcg); > > return ret; > -- > 2.40.0 > > I dislike this patch, it adds complexity for no discernible purpose and actively makes the code _less_ readable and in a self-test of all places (!) Not all pedantic Coccinelle results are actionable. Remember that it's humans who are reading the code. Your email client/scripting is still somehow broken, I couldn't get b4 to pull it correctly and it seems to have a duplicate message ID. You really need to take a look at that (git send-email should do this fine for example). Please try to filter the output of Coccinelle and instead of spamming thousands of pointless patches that add no value, try to choose those that do add value. My advice overall would be to just stop.

2 years, 3 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror