| Message ID | 20240319104857.70783-8-mic@digikod.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Handle faults in KUnit tests | expand |
Hi, On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün wrote: > Add a test case to check NULL pointer dereference and make sure it would > result as a failed test. > > The full kunit_fault test suite is marked as skipped when run on UML > because it would result to a kernel panic. > > Tested with: > ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault > ./tools/testing/kunit/kunit.py run --arch arm64 \ > --cross_compile=aarch64-linux-gnu- kunit_fault > What is the rationale for adding those tests unconditionally whenever CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system because it concludes that it is pointless to continue testing after the "Unable to handle kernel NULL pointer dereference" backtrace. At the same time, it is all or nothing, meaning I can not disable it but still run other kunit tests. Guenter
On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote: > Hi, > > On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün wrote: > > Add a test case to check NULL pointer dereference and make sure it would > > result as a failed test. > > > > The full kunit_fault test suite is marked as skipped when run on UML > > because it would result to a kernel panic. > > > > Tested with: > > ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault > > ./tools/testing/kunit/kunit.py run --arch arm64 \ > > --cross_compile=aarch64-linux-gnu- kunit_fault > > > > What is the rationale for adding those tests unconditionally whenever > CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system > because it concludes that it is pointless to continue testing > after the "Unable to handle kernel NULL pointer dereference" backtrace. > At the same time, it is all or nothing, meaning I can not disable > it but still run other kunit tests. > Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed to "solve" the problem. I'll take that as one of those "unintended consequences" items: Instead of more tests, there are fewer. Guenter
On Fri, Apr 19, 2024 at 04:38:01PM -0700, Guenter Roeck wrote: > On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote: > > Hi, > > > > On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün wrote: > > > Add a test case to check NULL pointer dereference and make sure it would > > > result as a failed test. > > > > > > The full kunit_fault test suite is marked as skipped when run on UML > > > because it would result to a kernel panic. > > > > > > Tested with: > > > ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault > > > ./tools/testing/kunit/kunit.py run --arch arm64 \ > > > --cross_compile=aarch64-linux-gnu- kunit_fault > > > > > > > What is the rationale for adding those tests unconditionally whenever > > CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system > > because it concludes that it is pointless to continue testing > > after the "Unable to handle kernel NULL pointer dereference" backtrace. > > At the same time, it is all or nothing, meaning I can not disable > > it but still run other kunit tests. > > CONFIG_KUNIT_TEST is to test KUnit itself. Why does this messes up your test system, and what is your test system? Is it related to the kernel warning and then the message you previously sent? https://lore.kernel.org/r/fd604ae0-5630-4745-acf2-1e51c69cf0c0@roeck-us.net It seems David has a solution to suppress such warning. > > Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed > to "solve" the problem. I'll take that as one of those "unintended > consequences" items: Instead of more tests, there are fewer. > > Guenter >
On 4/22/24 06:08, Mickaël Salaün wrote: > On Fri, Apr 19, 2024 at 04:38:01PM -0700, Guenter Roeck wrote: >> On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote: >>> Hi, >>> >>> On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün wrote: >>>> Add a test case to check NULL pointer dereference and make sure it would >>>> result as a failed test. >>>> >>>> The full kunit_fault test suite is marked as skipped when run on UML >>>> because it would result to a kernel panic. >>>> >>>> Tested with: >>>> ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault >>>> ./tools/testing/kunit/kunit.py run --arch arm64 \ >>>> --cross_compile=aarch64-linux-gnu- kunit_fault >>>> >>> >>> What is the rationale for adding those tests unconditionally whenever >>> CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system >>> because it concludes that it is pointless to continue testing >>> after the "Unable to handle kernel NULL pointer dereference" backtrace. >>> At the same time, it is all or nothing, meaning I can not disable >>> it but still run other kunit tests. >>> > > CONFIG_KUNIT_TEST is to test KUnit itself. Why does this messes up your > test system, and what is your test system? Is it related to the kernel > warning and then the message you previously sent? It is not a warning, it is a BUG which terminates the affected kernel thread. NULL pointer dereferences are normally fatal, which is why I abort tests if one is encountered. I am not going to start introducing code into my scripts to ignore such warnings (or BUG messages) on a case by case basis; this would be unmaintainable. > https://lore.kernel.org/r/fd604ae0-5630-4745-acf2-1e51c69cf0c0@roeck-us.net > It seems David has a solution to suppress such warning. > I don't think so. My series tried to suppress warning backtraces, not BUG messages. BUG messages can not easily be suppressed since the reaction is architecture specific and typically fatal. As I said below, never mind, I just disabled CONFIG_KUNIT_TEST in my testing. Guenter >> >> Oh, never mind. I just disabled CONFIG_KUNIT_TEST in my test bed >> to "solve" the problem. I'll take that as one of those "unintended >> consequences" items: Instead of more tests, there are fewer. >> >> Guenter >>
On Mon, 22 Apr 2024 at 21:36, Guenter Roeck <linux@roeck-us.net> wrote: > > On 4/22/24 06:08, Mickaël Salaün wrote: > > On Fri, Apr 19, 2024 at 04:38:01PM -0700, Guenter Roeck wrote: > >> On Fri, Apr 19, 2024 at 03:33:49PM -0700, Guenter Roeck wrote: > >>> Hi, > >>> > >>> On Tue, Mar 19, 2024 at 11:48:57AM +0100, Mickaël Salaün wrote: > >>>> Add a test case to check NULL pointer dereference and make sure it would > >>>> result as a failed test. > >>>> > >>>> The full kunit_fault test suite is marked as skipped when run on UML > >>>> because it would result to a kernel panic. > >>>> > >>>> Tested with: > >>>> ./tools/testing/kunit/kunit.py run --arch x86_64 kunit_fault > >>>> ./tools/testing/kunit/kunit.py run --arch arm64 \ > >>>> --cross_compile=aarch64-linux-gnu- kunit_fault > >>>> > >>> > >>> What is the rationale for adding those tests unconditionally whenever > >>> CONFIG_KUNIT_TEST is enabled ? This completely messes up my test system > >>> because it concludes that it is pointless to continue testing > >>> after the "Unable to handle kernel NULL pointer dereference" backtrace. > >>> At the same time, it is all or nothing, meaning I can not disable > >>> it but still run other kunit tests. > >>> > > > > CONFIG_KUNIT_TEST is to test KUnit itself. Why does this messes up your > > test system, and what is your test system? Is it related to the kernel > > warning and then the message you previously sent? > > It is not a warning, it is a BUG which terminates the affected kernel thread. > NULL pointer dereferences are normally fatal, which is why I abort tests > if one is encountered. I am not going to start introducing code into my > scripts to ignore such warnings (or BUG messages) on a case by case basis; > this would be unmaintainable. > > > https://lore.kernel.org/r/fd604ae0-5630-4745-acf2-1e51c69cf0c0@roeck-us.net > > It seems David has a solution to suppress such warning. > > > > I don't think so. My series tried to suppress warning backtraces, not BUG > messages. BUG messages can not easily be suppressed since the reaction is > architecture specific and typically fatal. > > As I said below, never mind, I just disabled CONFIG_KUNIT_TEST in my testing. > > Guenter > I think it probably makes sense to permit disabling the fault tests independently, at least until we have a way of suppressing the warnings. I've sent out a patch to add a CONFIG_KUNIT_FAULT_TEST option to disable these tests. Would that help? https://lore.kernel.org/linux-kselftest/20240423090808.242389-1-davidgow@google.com/ (The other option is to split the tests out into a totally separate file / module. I think that's an option (and would make the config option more consistent with other test options) but since they're otherwise part of the KUnit tests, I think I prefer to keep them together.) Cheers, -- David
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index f7980ef236a3..0fdca5fffaec 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -109,6 +109,48 @@ static struct kunit_suite kunit_try_catch_test_suite = { .test_cases = kunit_try_catch_test_cases, }; +#ifndef CONFIG_UML + +static void kunit_test_null_dereference(void *data) +{ + struct kunit *test = data; + int *null = NULL; + + *null = 0; + + KUNIT_FAIL(test, "This line should never be reached\n"); +} + +static void kunit_test_fault_null_dereference(struct kunit *test) +{ + struct kunit_try_catch_test_context *ctx = test->priv; + struct kunit_try_catch *try_catch = ctx->try_catch; + + kunit_try_catch_init(try_catch, + test, + kunit_test_null_dereference, + kunit_test_catch); + kunit_try_catch_run(try_catch, test); + + KUNIT_EXPECT_EQ(test, try_catch->try_result, -EINTR); + KUNIT_EXPECT_TRUE(test, ctx->function_called); +} + +#endif /* !CONFIG_UML */ + +static struct kunit_case kunit_fault_test_cases[] = { +#ifndef CONFIG_UML + KUNIT_CASE(kunit_test_fault_null_dereference), +#endif /* !CONFIG_UML */ + {} +}; + +static struct kunit_suite kunit_fault_test_suite = { + .name = "kunit_fault", + .init = kunit_try_catch_test_init, + .test_cases = kunit_fault_test_cases, +}; + /* * Context for testing test managed resources * is_resource_initialized is used to test arbitrary resources @@ -826,6 +868,7 @@ static struct kunit_suite kunit_current_test_suite = { kunit_test_suites(&kunit_try_catch_test_suite, &kunit_resource_test_suite, &kunit_log_test_suite, &kunit_status_test_suite, - &kunit_current_test_suite, &kunit_device_test_suite); + &kunit_current_test_suite, &kunit_device_test_suite, + &kunit_fault_test_suite); MODULE_LICENSE("GPL v2");