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
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;
}
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
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.
I've been trying to do some stuff with KUnit but I can't seem to
find a current tree where KUnit builds. Running on Debian stable
starting from a clean -next tree and running:
./tools/testing/kunit/kunit.py config
./tools/testing/kunit/kunit.py build
based on Documentation/dev-tools/kunit/start.rst. However I get:
[00:42:59] Configuring KUnit Kernel ...
[00:42:59] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make ARCH=um O=.kunit --jobs=8
ERROR:root:In file included from /usr/include/stdlib.h:1013,
from ../arch/x86/um/os-Linux/registers.c:8:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
26 | {
| ^
make[4]: *** [../scripts/Makefile.build:252: arch/x86/um/os-Linux/registers.o] Error 1
make[3]: *** [../scripts/Makefile.build:494: arch/x86/um/os-Linux] Error 2
make[3]: *** Waiting for unfinished jobs....
In file included from /usr/include/stdlib.h:1013,
from ../arch/um/drivers/fd.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
26 | {
| ^
make[3]: *** [../scripts/Makefile.build:252: arch/um/drivers/fd.o] Error 1
make[3]: *** Waiting for unfinished jobs....
In file included from /usr/include/stdlib.h:1013,
from ../arch/um/os-Linux/skas/process.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
26 | {
| ^
make[4]: *** [../scripts/Makefile.build:252: arch/um/os-Linux/skas/process.o] Error 1
make[3]: *** [../scripts/Makefile.build:494: arch/um/os-Linux/skas] Error 2
make[2]: *** [../scripts/Makefile.build:494: arch/um/os-Linux] Error 2
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:494: arch/x86/um] Error 2
make[2]: *** [../scripts/Makefile.build:494: arch/um/drivers] Error 2
In file included from /usr/include/stdlib.h:1013,
from arch/um/kernel/config.c:7:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h: In function ‘atof’:
/usr/include/x86_64-linux-gnu/bits/stdlib-float.h:26:1: error: SSE register return with SSE disabled
26 | {
| ^
make[3]: *** [../scripts/Makefile.build:252: arch/um/kernel/config.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [../scripts/Makefile.build:494: arch/um/kernel] Error 2
make[1]: *** [/home/broonie/git/bisect/Makefile:2028: .] Error 2
make: *** [Makefile:226: __sub-make] Error 2
[00:43:20] Elapsed time: 20.233s
which isn't ideal. v6.2 is also broken, albeit differently:
ERROR:root:`.exit.text' referenced in section `.uml.exitcall.exit' of arch/um/drivers/virtio_uml.o: defined in discarded section `.exit.text' of arch/um/drivers/virtio_uml.o
collect2: error: ld returned 1 exit status
make[2]: *** [../scripts/Makefile.vmlinux:35: vmlinux] Error 1
make[1]: *** [/home/broonie/git/linux/Makefile:1264: vmlinux] Error 2
make: *** [Makefile:242: __sub-make] Error 2
which makes bisecting a bit of an issue. The kunit-fixes, kunit
and kunit-next trees in -next have the former error. Can anyone
point me at a tree/config/commands that's suitable for working on
KUnit at the minute?
There is a 'malloc' call in vcpu_save_state function, which can
be unsuccessful. This patch will add the malloc failure checking
to avoid possible null dereference and give more information
about test fail reasons.
Signed-off-by: Ivan Orlov <ivan.orlov0322(a)gmail.com>
---
tools/testing/selftests/kvm/lib/x86_64/processor.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index c39a4353ba19..827647ff3d41 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -954,6 +954,7 @@ struct kvm_x86_state *vcpu_save_state(struct kvm_vcpu *vcpu)
vcpu_run_complete_io(vcpu);
state = malloc(sizeof(*state) + msr_list->nmsrs * sizeof(state->msrs.entries[0]));
+ TEST_ASSERT(state, "-ENOMEM when allocating kvm state");
vcpu_events_get(vcpu, &state->events);
vcpu_mp_state_get(vcpu, &state->mp_state);
--
2.34.1
In this version, I have integrated Aaron's changes to the amx_test. In
addition, we also integrated one fix patch for a kernel warning due to
xsave address issue.
Patch 1:
Fix a host FPU kernel warning due to missing XTILEDATA in xinit.
Patch 2-8:
Overhaul amx_test. These patches were basically from v2.
Patch 9-13:
Overhaul amx_test from Aaron. I modified the changelog a little bit.
v2 -> v3:
- integrate Aaron's 5 commits with minor changes on commit message.
- Add one fix patch for a kernel warning.
v2:
https://lore.kernel.org/all/20230214184606.510551-1-mizhang@google.com/
Aaron Lewis (5):
KVM: selftests: x86: Assert that XTILE is XSAVE-enabled
KVM: selftests: x86: Assert that both XTILE{CFG,DATA} are
XSAVE-enabled
KVM: selftests: x86: Remove redundant check that XSAVE is supported
KVM: selftests: x86: Check that the palette table exists before using
it
KVM: selftests: x86: Check that XTILEDATA supports XFD
Mingwei Zhang (8):
x86/fpu/xstate: Avoid getting xstate address of init_fpstate if
fpstate contains the component
KVM: selftests: x86: Add a working xstate data structure
KVM: selftests: x86: Fix an error in comment of amx_test
KVM: selftests: x86: Add check of CR0.TS in the #NM handler in
amx_test
KVM: selftests: x86: Add the XFD check to IA32_XFD in #NM handler
KVM: selftests: x86: Fix the checks to XFD_ERR using and operation
KVM: selftests: x86: Enable checking on xcomp_bv in amx_test
KVM: selftests: x86: Repeat the checking of xheader when
IA32_XFD[XTILEDATA] is set in amx_test
arch/x86/kernel/fpu/xstate.c | 10 ++-
.../selftests/kvm/include/x86_64/processor.h | 14 ++++
tools/testing/selftests/kvm/x86_64/amx_test.c | 80 +++++++++----------
3 files changed, 59 insertions(+), 45 deletions(-)
--
2.39.2.637.g21b0678d19-goog
Align the guest stack to match calling sequence requirements in
section "The Stack Frame" of the System V ABI AMD64 Architecture
Processor Supplement, which requires the value (%rsp + 8), NOT %rsp,
to be a multiple of 16 when control is transferred to the function
entry point. I.e. in a normal function call, %rsp needs to be 16-byte
aligned _before_ CALL, not after.
This fixes unexpected #GPs in guest code when the compiler uses SSE
instructions, e.g. to initialize memory, as many SSE instructions
require memory operands (including those on the stack) to be
16-byte-aligned.
Signed-off-by: Ackerley Tng <ackerleytng(a)google.com>
---
This patch is a follow-up from discussions at
https://lore.kernel.org/lkml/20230121001542.2472357-9-ackerleytng@google.co…
v1 -> v2: Cleaned the patch up after getting comments from Sean in
v1: https://lore.kernel.org/lkml/Y%2FfHLdvKHlK6D%2F1v@google.com/
Please also see
https://lore.kernel.org/lkml/20230227174654.94641-1-ackerleytng@google.com/
regarding providing alignment macros for selftests.
---
.../selftests/kvm/lib/x86_64/processor.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index ae1e573d94ce..a0669d31bb85 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -5,6 +5,7 @@
* Copyright (C) 2018, Google LLC.
*/
+#include "linux/bitmap.h"
#include "test_util.h"
#include "kvm_util.h"
#include "processor.h"
@@ -573,6 +574,21 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
DEFAULT_GUEST_STACK_VADDR_MIN,
MEM_REGION_DATA);
+ stack_vaddr += DEFAULT_STACK_PGS * getpagesize();
+
+ /*
+ * Align stack to match calling sequence requirements in section "The
+ * Stack Frame" of the System V ABI AMD64 Architecture Processor
+ * Supplement, which requires the value (%rsp + 8) to be a multiple of
+ * 16 when control is transferred to the function entry point.
+ *
+ * If this code is ever used to launch a vCPU with 32-bit entry point it
+ * may need to subtract 4 bytes instead of 8 bytes.
+ */
+ TEST_ASSERT(IS_ALIGNED(stack_vaddr, PAGE_SIZE),
+ "__vm_vaddr_alloc() did not provide a page-aligned address");
+ stack_vaddr -= 8;
+
vcpu = __vm_vcpu_add(vm, vcpu_id);
vcpu_init_cpuid(vcpu, kvm_get_supported_cpuid());
vcpu_setup(vm, vcpu);
@@ -580,7 +596,7 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
/* Setup guest general purpose registers */
vcpu_regs_get(vcpu, ®s);
regs.rflags = regs.rflags | 0x2;
- regs.rsp = stack_vaddr + (DEFAULT_STACK_PGS * getpagesize());
+ regs.rsp = stack_vaddr;
regs.rip = (unsigned long) guest_code;
vcpu_regs_set(vcpu, ®s);
--
2.39.2.722.g9855ee24e9-goog