Re: [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run
From: David Gow
Date: Tue Oct 06 2026 - 02:48:35 EST
Le 01/09/2026 à 19:02, Malte Wechter a écrit :
Den tirs. 25. aug. 2026 kl. 13.00 skrev David Gow <david@xxxxxxxxxxxx>:
I get your point, the purpose of this special test suite is to assert
Le 24/08/2026 à 21:32, Malte Wechter a écrit :
add pre-defined KUnit test suite and test case that asserts both
`debug_locks` and `TAINT_WARN` prior to running any (user) KUnit tests.
This asserts integrity before tests are run.
Signed-off-by: Malte Wechter <maltewechter@xxxxxxxxx>
---
I'm not quite as convinced by this as I am by the first patch. While
ensuring the state of the system is good before tests are run is useful,
this does seem a bit heavy-handed in some respects.
This could probably use a more detailed description, particularly
describing why such a test is useful, and why it would need to be
implemented in a special way.
And I do think the implementation here is a bit _too_ special-cased. One
other possibility would be to prepend this suite using
kunit_merge_suite_sets(), so we don't need to have any special handling
of (e.g.) the test count. This could also allow this special suite to be
filtered out (which has both advantages and disadvantages).
It might also be nice to have this configurable independently from the
other checks, and maybe at runtime (via a KUnit module / command-line
parameter), particularly if this can't be filtered on. And, as before,
this definitely needs to be documented. People need to know how to
enable it, and where all of these extra results from tests they didn't
enable came from.
Thoughts?
that the kernel is in a "fine" state before any unit tests are run. If
this
check is left out the false positives could occur if the system is in
a bad state before the tests are run.
The reason that the case was handled differently compared to other was
because this assertion _must_ be run before any other tests,
and when kunit_merge_suite_sets() is called from kunit_run_all_tests()
it also filters the test suites, which would not give any guarentee
that this
special suite gets run first.
I do agree that this initial way is maybe a bit coarse, and i will see
if i can find a better fit for this assertion. But i dont want to
leave it out.
Had a chat with Andreas about this yesterday, and we think we've come up with another possible option here: instead of doing one check to verify the state before any KUnit tests are run, have an option to check the taint before _every_ test, and to skip it if the kernel isn't clean.
There are a few consequences of this:
- You'd need to be able to mask out taints you don't care about, especially TAINT_TEST (as without it all tests but the first would fail). Of course, that might still be useful if you really want to ensure isolation between tests (e.g. --run_isolated)
- If the kernel is already tainted, and so all tests are skipped, kunit.py should report 'no tests run' as an error. I think this actually only reports an error if no tests are executed at the moment, so this may need looking at.
- This will trigger if the kernel is tainted by some other part of the kernel either between KUnit tests, or in another kthread. However, in this case, the results will look like a series of successful tests, followed by skipped ones, with no failed tests. (And hence won't trigger a failure exit code from kunit.py). This is likely to be incredibly rare, but we could add a fail-on-skip option or similar if needed.
Thoughts?
Cheers,
-- David
Best regards,
Cheers,
-- David
lib/kunit/executor.c | 8 +++++++-
lib/kunit/test.c | 30 ++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index b0f8a41d61d36..0db67fe7f09f9 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -290,9 +290,15 @@ void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin)
size_t num_suites = suite_set->end - suite_set->start;
bool autorun = kunit_autorun();
+ #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
Nit: Let's not indent the #ifdefs.
+ size_t num_suites_plus_extra = num_suites+1;
+ #else
+ size_t num_suites_plus_extra = num_suites;
+ #endif
+
I'm not particularly happy with this way of adding an extra suite.
if (autorun && (builtin || num_suites)) {
pr_info("KTAP version 1\n");
- pr_info("1..%zu\n", num_suites);
+ pr_info("1..%zu\n", num_suites_plus_extra);
}
__kunit_test_suites_init(suite_set->start, num_suites, autorun);
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 99773e000e1b7..e64c6d1575280 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -835,6 +835,30 @@ bool kunit_enabled(void)
return enable_param;
}
+#ifdef CONFIG_KUNIT_EXTRA_ASSERTS
+#define DEBUG_LOCKS_OK 1
+#define TAINT_WARN_OK 0
Not totally sold on these #defines: I think I'd prefer to just have the
literal 1/0.
+
+static void pre_kunit_assert(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ_MSG(test, debug_locks, DEBUG_LOCKS_OK,
+ "debug_locks are off before any test ran");
+ KUNIT_EXPECT_EQ_MSG(test, test_taint(TAINT_WARN), TAINT_WARN_OK,
+ "kernel already TAINT_WARN tainted before any test ran");
+}
If we are going to generate a special suite, let's have the taint and
lockdep checks as separate tests.
This would also make it easier to have them be configurable separately.
+
+static struct kunit_case pre_kunit_assert_cases[] = {
+ KUNIT_CASE(pre_kunit_assert),
+ {}
+};
+
+static struct kunit_suite pre_kunit_assert_clean_state_suite = {
+ .name = "pre_kunit_extra_asserts",
I think we could probably find a better name for this.
"initial_system_state" or similar might be better?
+ .test_cases = pre_kunit_assert_cases,
+};
+
+#endif /* CONFIG_RUST_KUNIT_EXTRA_ASSERTS */
+
int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites,
bool run_tests)
{
@@ -857,6 +881,12 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
}
static_branch_inc(&kunit_running);
+ #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
+ kunit_init_suite(&pre_kunit_assert_clean_state_suite);
+ if (run_tests)
+ kunit_run_tests(&pre_kunit_assert_clean_state_suite);
+ #endif
+
for (i = 0; i < num_suites; i++) {
kunit_init_suite(suites[i]);
if (run_tests)
Malte :)