* [PATCH v2 0/2] kunit: add optional assertions to catch taint and lockdep warnings
@ 2026-08-24 13:32 Malte Wechter
2026-08-24 13:32 ` [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases Malte Wechter
2026-08-24 13:32 ` [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run Malte Wechter
0 siblings, 2 replies; 7+ messages in thread
From: Malte Wechter @ 2026-08-24 13:32 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan
Cc: linux-kselftest, kunit-dev, linux-kernel, rust-for-linux,
Malte Wechter
this patch tries to expand KUnit by using existing kernel diagnostics to
catch certain locking related bugs. Specifically this adds assertions for
`debug_locks` from `lockdep` and the `TAINT_WARN` flag from `panic`.
This tries to prevent bugs as: circular locking dependency and sleeping
function called from invalid context.
Add config CONFIG_KUNIT_EXTRA_ASSERTS that enables extra assertions
to be emitted: First, before _any_ KUnit test suite is ran, this is to ensure
that the extra assertions are triggered by a KUnit test and not prior.
Secondly, assertions a made for each test case, failing them if the
assertions fail, even if the KUnit test is marked as passed.
Signed-off-by: Malte Wechter <maltewechter@gmail.com>
---
Changes in v2:
- Implement changes directly in C KUnit instead of Rust KUnit wrapper
- Remove rust specific configs and add general CONFIG_KUNIT_EXTRA_ASSERTS config
- Link to v1: https://patch.msgid.link/20260818-lockdep-kunit-v1-0-66ceb1a272e3@gmail.com
To: Brendan Higgins <brendan.higgins@linux.dev>
To: David Gow <david@davidgow.net>
To: Rae Moar <raemoar63@gmail.com>
To: Miguel Ojeda <ojeda@kernel.org>
To: Boqun Feng <boqun@kernel.org>
To: Gary Guo <gary@garyguo.net>
To: Björn Roy Baron <bjorn3_gh@protonmail.com>
To: Benno Lossin <lossin@kernel.org>
To: Andreas Hindborg <a.hindborg@kernel.org>
To: Alice Ryhl <aliceryhl@google.com>
To: Trevor Gross <tmgross@umich.edu>
To: Danilo Krummrich <dakr@kernel.org>
To: Daniel Almeida <daniel.almeida@collabora.com>
To: Tamir Duberstein <tamird@kernel.org>
To: Alexandre Courbot <acourbot@nvidia.com>
To: Onur Özkan <work@onurozkan.dev>
Cc: linux-kselftest@vger.kernel.org
Cc: kunit-dev@googlegroups.com
Cc: linux-kernel@vger.kernel.org
Cc: rust-for-linux@vger.kernel.org
---
Malte Wechter (2):
kunit: add extra assertions to KUnit test cases
kunit: add KUnit test to assert kernel state before KUnit suites are run
lib/kunit/Kconfig | 12 ++++++++++++
lib/kunit/executor.c | 8 +++++++-
lib/kunit/test.c | 30 ++++++++++++++++++++++++++++++
lib/kunit/try-catch.c | 23 ++++++++++++++++++++++-
4 files changed, 71 insertions(+), 2 deletions(-)
---
base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
change-id: 20260812-lockdep-kunit-9628f4bcad90
Best regards,
--
Malte Wechter <maltewechter@gmail.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases
2026-08-24 13:32 [PATCH v2 0/2] kunit: add optional assertions to catch taint and lockdep warnings Malte Wechter
@ 2026-08-24 13:32 ` Malte Wechter
2026-08-25 11:00 ` David Gow
2026-08-24 13:32 ` [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run Malte Wechter
1 sibling, 1 reply; 7+ messages in thread
From: Malte Wechter @ 2026-08-24 13:32 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan
Cc: linux-kselftest, kunit-dev, linux-kernel, rust-for-linux,
Malte Wechter
add extra assertions for each individual test case, that checks that
both `debug_locks` and `TAINT_WARN` are intact after the test is run.
The assertions are optional behind CONFIG_KUNIT_EXTRA_ASSERTS.
Signed-off-by: Malte Wechter <maltewechter@gmail.com>
---
lib/kunit/Kconfig | 12 ++++++++++++
lib/kunit/try-catch.c | 23 ++++++++++++++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
index 94ff8e4089bfb..38801f7493669 100644
--- a/lib/kunit/Kconfig
+++ b/lib/kunit/Kconfig
@@ -142,4 +142,16 @@ config KUNIT_UML_PCI
If unsure, say N.
+config KUNIT_EXTRA_ASSERTS
+ bool "Enable extra assertions in KUnit tests"
+ depends on LOCKDEP
+ default n
+ help
+ Enables all extra assertions for KUnit which includes asserting `TAINT_WARN` and
+ `debug_locks` from lockdep. A KUnit test suite (and test case) is inserted
+ at the start of all KUnit test suites. This makes assertions prior to running any
+ tests, as a pre-test integrity check. Assertions are made after each test case which
+ asserts that each test case did not trigger either `TAINT_WARN` or `debug_locks`.
+
+ If unsure, say N.
endif # KUNIT
diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
index d84a879f0a789..7eea3af4c9671 100644
--- a/lib/kunit/try-catch.c
+++ b/lib/kunit/try-catch.c
@@ -41,6 +41,11 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
struct completion *task_done;
int exit_code, time_remaining;
+ #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
+ int debug_locks_snapshot = debug_locks;
+ int tainted_warn_snapshot = test_taint(TAINT_WARN);
+ #endif
+
try_catch->context = context;
try_catch->try_result = 0;
task_struct = kthread_create(kunit_generic_run_threadfn_adapter,
@@ -70,7 +75,23 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
put_task_struct(task_struct);
exit_code = try_catch->try_result;
- if (!exit_code)
+ #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
+ bool extra_assert = false;
+
+ if (debug_locks_snapshot != debug_locks && !exit_code) {
+ extra_assert = true;
+ try_catch->try_result = -EDEADLK;
+ kunit_err(test, "Test triggered lockdep\n");
+ } else if (tainted_warn_snapshot != test_taint(TAINT_WARN) && !exit_code) {
+ extra_assert = true;
+ try_catch->try_result = -EDEADLK;
+ kunit_err(test, "Test tainted kernel with TAINT_WARN\n");
+ }
+ #else
+ bool extra_assert = false;
+ #endif
+
+ if (!exit_code && !extra_assert)
return;
if (exit_code == -EFAULT)
--
2.51.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run
2026-08-24 13:32 [PATCH v2 0/2] kunit: add optional assertions to catch taint and lockdep warnings Malte Wechter
2026-08-24 13:32 ` [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases Malte Wechter
@ 2026-08-24 13:32 ` Malte Wechter
2026-08-25 11:00 ` David Gow
1 sibling, 1 reply; 7+ messages in thread
From: Malte Wechter @ 2026-08-24 13:32 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan
Cc: linux-kselftest, kunit-dev, linux-kernel, rust-for-linux,
Malte Wechter
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@gmail.com>
---
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
+ size_t num_suites_plus_extra = num_suites+1;
+ #else
+ size_t num_suites_plus_extra = num_suites;
+ #endif
+
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
+
+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");
+}
+
+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",
+ .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)
--
2.51.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases
2026-08-24 13:32 ` [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases Malte Wechter
@ 2026-08-25 11:00 ` David Gow
2026-09-01 9:19 ` Malte Wechter
0 siblings, 1 reply; 7+ messages in thread
From: David Gow @ 2026-08-25 11:00 UTC (permalink / raw)
To: Malte Wechter, Brendan Higgins, Rae Moar, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan
Cc: linux-kselftest, kunit-dev, linux-kernel, rust-for-linux
Le 24/08/2026 à 21:32, Malte Wechter a écrit :
> add extra assertions for each individual test case, that checks that
> both `debug_locks` and `TAINT_WARN` are intact after the test is run.
> The assertions are optional behind CONFIG_KUNIT_EXTRA_ASSERTS.
>
> Signed-off-by: Malte Wechter <maltewechter@gmail.com>
> ---
Thanks for this.
I'd prefer to split this up into two separate changes, one to check for
warnings, and one for lockdep issues.
There's already a patch series to handle lockdep failures here:
https://lore.kernel.org/all/20200814205527.1833459-1-urielguajardojr@gmail.com/
It's pretty similar to this, but may have some good ideas in it.
As for WARN, we have a new feature which allows individual WARN calls to
be expected and ignored. This works by having the WARN() macro directly
check if a test is failing. The difference there is that the code only
runs if the current thread is part of the test (so it wouldn't be
affected by a WARN() in another, non-test thread). There are advantages
and disadvantages to this, but it's probably sensible to be consistent here.
Take a look at this for more details on how it works:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=85347718ab0dd7ede9c3e1dcff2d604c7073df05
(That being said, the current implementation does seem to work pretty
well even with the backtrace suppression, so continuing to use this
implementation is not necessarily a dealbreaker.)
Finally, I'd appreciate there being some more documentation of this, if
possible. In particular, it'd be nice to extend the KUnit documentation
in Documentation/dev-tools/kunit to mention this and provide some
examples of how to enable it. In particular, it'd be good to have an
example kunit.py invocation which enables lockdep, e.g.
./tools/testing/kunit/kunit.py run --kconfig_add
CONFIG_KUNIT_EXTRA_ASSERTS=y --kconfig_add CONFIG_PROVE_LOCKING=y
--kconfig_add CONFIG_DEBUG_KERNEL=y
Thanks again for looking into this!
Cheers,
-- David
> lib/kunit/Kconfig | 12 ++++++++++++
> lib/kunit/try-catch.c | 23 ++++++++++++++++++++++-
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 94ff8e4089bfb..38801f7493669 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -142,4 +142,16 @@ config KUNIT_UML_PCI
>
> If unsure, say N.
>
> +config KUNIT_EXTRA_ASSERTS
> + bool "Enable extra assertions in KUnit tests"
> + depends on LOCKDEP
> + default n
> + help
> + Enables all extra assertions for KUnit which includes asserting `TAINT_WARN` and
> + `debug_locks` from lockdep. A KUnit test suite (and test case) is inserted
> + at the start of all KUnit test suites. This makes assertions prior to running any
> + tests, as a pre-test integrity check. Assertions are made after each test case which
> + asserts that each test case did not trigger either `TAINT_WARN` or `debug_locks`.
> +
> + If unsure, say N.
A few notes here:
- I'd rather have the taint/warn and lockdep assertions separate here.
- A more descriptive name than "EXTRA_ASSERTS" would be nice. Perhaps
something like KUNIT_FAIL_TEST_ON_WARN / KUNIT_FAIL_ON_LOCKDEP or similar?
> endif # KUNIT
> diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
> index d84a879f0a789..7eea3af4c9671 100644
> --- a/lib/kunit/try-catch.c
> +++ b/lib/kunit/try-catch.c
> @@ -41,6 +41,11 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
> struct completion *task_done;
> int exit_code, time_remaining;
>
> + #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
> + int debug_locks_snapshot = debug_locks;
> + int tainted_warn_snapshot = test_taint(TAINT_WARN);
> + #endif
Nit: Let's keep the preprocessor #ifdef/#endif lines un-indented here.
> +
> try_catch->context = context;
> try_catch->try_result = 0;
> task_struct = kthread_create(kunit_generic_run_threadfn_adapter,
> @@ -70,7 +75,23 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
> put_task_struct(task_struct);
> exit_code = try_catch->try_result;
>
> - if (!exit_code)
> + #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
> + bool extra_assert = false;
Please define all variables at the top of the function / scope.
That being said, we don't need this extra_assert variable at all if you
move the setting of try_result above moving it to exit_code above?
> +
> + if (debug_locks_snapshot != debug_locks && !exit_code) {
> + extra_assert = true;
> + try_catch->try_result = -EDEADLK;
Why are we setting this, and then never handling it?
> + kunit_err(test, "Test triggered lockdep\n");
> + } else if (tainted_warn_snapshot != test_taint(TAINT_WARN) && !exit_code) {
> + extra_assert = true;
> + try_catch->try_result = -EDEADLK;
-EDEADLK made sense for the lockdep implementation, but makes less sense
for WARN(). If you really want to keep it, please document it.
That being said, it should be possible to do this outside of the
try/catch scope, as both lockdep and taints are global. And if you fail
the test using the hook mechanism (like the warning suppression does),
you won't need a separate check afterwards.
> + kunit_err(test, "Test tainted kernel with TAINT_WARN\n");
> + }
> + #else
> + bool extra_assert = false;
If we declare this unconditionally at the top of the function, there's
no need to have it in an #ifdef.
> + #endif
> +
> + if (!exit_code && !extra_assert)
> return;
>
> if (exit_code == -EFAULT)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run
2026-08-24 13:32 ` [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run Malte Wechter
@ 2026-08-25 11:00 ` David Gow
2026-09-01 11:02 ` Malte Wechter
0 siblings, 1 reply; 7+ messages in thread
From: David Gow @ 2026-08-25 11:00 UTC (permalink / raw)
To: Malte Wechter, Brendan Higgins, Rae Moar, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan
Cc: linux-kselftest, kunit-dev, linux-kernel, rust-for-linux
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@gmail.com>
> ---
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?
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)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases
2026-08-25 11:00 ` David Gow
@ 2026-09-01 9:19 ` Malte Wechter
0 siblings, 0 replies; 7+ messages in thread
From: Malte Wechter @ 2026-09-01 9:19 UTC (permalink / raw)
To: David Gow
Cc: Brendan Higgins, Rae Moar, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, linux-kselftest, kunit-dev,
linux-kernel, rust-for-linux
Den tirs. 25. aug. 2026 kl. 13.00 skrev David Gow <david@davidgow.net>:
>
> Le 24/08/2026 à 21:32, Malte Wechter a écrit :
> > add extra assertions for each individual test case, that checks that
> > both `debug_locks` and `TAINT_WARN` are intact after the test is run.
> > The assertions are optional behind CONFIG_KUNIT_EXTRA_ASSERTS.
> >
> > Signed-off-by: Malte Wechter <maltewechter@gmail.com>
> > ---
>
> Thanks for this.
>
> I'd prefer to split this up into two separate changes, one to check for
> warnings, and one for lockdep issues.
I agree that this would be better.
>
> There's already a patch series to handle lockdep failures here:
> https://lore.kernel.org/all/20200814205527.1833459-1-urielguajardojr@gmail.com/
>
> It's pretty similar to this, but may have some good ideas in it.
>
> As for WARN, we have a new feature which allows individual WARN calls to
> be expected and ignored. This works by having the WARN() macro directly
> check if a test is failing. The difference there is that the code only
> runs if the current thread is part of the test (so it wouldn't be
> affected by a WARN() in another, non-test thread). There are advantages
> and disadvantages to this, but it's probably sensible to be consistent here.
>
> Take a look at this for more details on how it works:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=85347718ab0dd7ede9c3e1dcff2d604c7073df05
>
> (That being said, the current implementation does seem to work pretty
> well even with the backtrace suppression, so continuing to use this
> implementation is not necessarily a dealbreaker.)
>
> Finally, I'd appreciate there being some more documentation of this, if
> possible. In particular, it'd be nice to extend the KUnit documentation
> in Documentation/dev-tools/kunit to mention this and provide some
> examples of how to enable it. In particular, it'd be good to have an
> example kunit.py invocation which enables lockdep, e.g.
> ./tools/testing/kunit/kunit.py run --kconfig_add
> CONFIG_KUNIT_EXTRA_ASSERTS=y --kconfig_add CONFIG_PROVE_LOCKING=y
> --kconfig_add CONFIG_DEBUG_KERNEL=y
Will do!
>
> Thanks again for looking into this!
>
> Cheers,
> -- David
>
>
> > lib/kunit/Kconfig | 12 ++++++++++++
> > lib/kunit/try-catch.c | 23 ++++++++++++++++++++++-
> > 2 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> > index 94ff8e4089bfb..38801f7493669 100644
> > --- a/lib/kunit/Kconfig
> > +++ b/lib/kunit/Kconfig
> > @@ -142,4 +142,16 @@ config KUNIT_UML_PCI
> >
> > If unsure, say N.
> >
> > +config KUNIT_EXTRA_ASSERTS
> > + bool "Enable extra assertions in KUnit tests"
> > + depends on LOCKDEP
> > + default n
> > + help
> > + Enables all extra assertions for KUnit which includes asserting `TAINT_WARN` and
> > + `debug_locks` from lockdep. A KUnit test suite (and test case) is inserted
> > + at the start of all KUnit test suites. This makes assertions prior to running any
> > + tests, as a pre-test integrity check. Assertions are made after each test case which
> > + asserts that each test case did not trigger either `TAINT_WARN` or `debug_locks`.
> > +
> > + If unsure, say N.
>
> A few notes here:
> - I'd rather have the taint/warn and lockdep assertions separate here.
> - A more descriptive name than "EXTRA_ASSERTS" would be nice. Perhaps
> something like KUNIT_FAIL_TEST_ON_WARN / KUNIT_FAIL_ON_LOCKDEP or similar?
>
> > endif # KUNIT
> > diff --git a/lib/kunit/try-catch.c b/lib/kunit/try-catch.c
> > index d84a879f0a789..7eea3af4c9671 100644
> > --- a/lib/kunit/try-catch.c
> > +++ b/lib/kunit/try-catch.c
> > @@ -41,6 +41,11 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
> > struct completion *task_done;
> > int exit_code, time_remaining;
> >
> > + #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
> > + int debug_locks_snapshot = debug_locks;
> > + int tainted_warn_snapshot = test_taint(TAINT_WARN);
> > + #endif
>
> Nit: Let's keep the preprocessor #ifdef/#endif lines un-indented here.
>
> > +
> > try_catch->context = context;
> > try_catch->try_result = 0;
> > task_struct = kthread_create(kunit_generic_run_threadfn_adapter,
> > @@ -70,7 +75,23 @@ void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context)
> > put_task_struct(task_struct);
> > exit_code = try_catch->try_result;
> >
> > - if (!exit_code)
> > + #ifdef CONFIG_KUNIT_EXTRA_ASSERTS
> > + bool extra_assert = false;
>
> Please define all variables at the top of the function / scope.
>
> That being said, we don't need this extra_assert variable at all if you
> move the setting of try_result above moving it to exit_code above?
>
> > +
> > + if (debug_locks_snapshot != debug_locks && !exit_code) {
> > + extra_assert = true;
> > + try_catch->try_result = -EDEADLK;
>
> Why are we setting this, and then never handling it?
>
> > + kunit_err(test, "Test triggered lockdep\n");
> > + } else if (tainted_warn_snapshot != test_taint(TAINT_WARN) && !exit_code) {
> > + extra_assert = true;
> > + try_catch->try_result = -EDEADLK;
>
> -EDEADLK made sense for the lockdep implementation, but makes less sense
> for WARN(). If you really want to keep it, please document it.
>
> That being said, it should be possible to do this outside of the
> try/catch scope, as both lockdep and taints are global. And if you fail
> the test using the hook mechanism (like the warning suppression does),
> you won't need a separate check afterwards.
I believe that installing two additional hooks (one for lockdep and
one for warn, possibly a third for the initial suite check) is a
cleaner approach. I
will move the assertion logic into KUnit hooks instead of having it
inside of the try_catch_run.
>
> > + kunit_err(test, "Test tainted kernel with TAINT_WARN\n");
> > + }
> > + #else
> > + bool extra_assert = false;
>
> If we declare this unconditionally at the top of the function, there's
> no need to have it in an #ifdef.
>
> > + #endif
> > +
> > + if (!exit_code && !extra_assert)
> > return;
> >
> > if (exit_code == -EFAULT)
> >
>
Best regards,
Malte :)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run
2026-08-25 11:00 ` David Gow
@ 2026-09-01 11:02 ` Malte Wechter
0 siblings, 0 replies; 7+ messages in thread
From: Malte Wechter @ 2026-09-01 11:02 UTC (permalink / raw)
To: David Gow
Cc: Brendan Higgins, Rae Moar, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, linux-kselftest, kunit-dev,
linux-kernel, rust-for-linux
Den tirs. 25. aug. 2026 kl. 13.00 skrev David Gow <david@davidgow.net>:
>
> 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@gmail.com>
> > ---
>
> 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?
I get your point, the purpose of this special test suite is to assert
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.
>
> 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)
> >
>
Best regards,
Malte :)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-01 11:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 13:32 [PATCH v2 0/2] kunit: add optional assertions to catch taint and lockdep warnings Malte Wechter
2026-08-24 13:32 ` [PATCH v2 1/2] kunit: add extra assertions to KUnit test cases Malte Wechter
2026-08-25 11:00 ` David Gow
2026-09-01 9:19 ` Malte Wechter
2026-08-24 13:32 ` [PATCH v2 2/2] kunit: add KUnit test to assert kernel state before KUnit suites are run Malte Wechter
2026-08-25 11:00 ` David Gow
2026-09-01 11:02 ` Malte Wechter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox