Per recently added KUnit style recommendations at
Documentation/dev-tools/kunit/style.rst, make the following changes to
the KCSAN test:
1. Rename 'kcsan-test.c' to 'kcsan_test.c'.
2. Rename suite name 'kcsan-test' to 'kcsan'.
3. Rename CONFIG_KCSAN_TEST to CONFIG_KCSAN_KUNIT_TEST and
default to KUNIT_ALL_TESTS.
Cc: David Gow <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
kernel/kcsan/Makefile | 4 ++--
kernel/kcsan/{kcsan-test.c => kcsan_test.c} | 2 +-
lib/Kconfig.kcsan | 5 +++--
3 files changed, 6 insertions(+), 5 deletions(-)
rename kernel/kcsan/{kcsan-test.c => kcsan_test.c} (99%)
diff --git a/kernel/kcsan/Makefile b/kernel/kcsan/Makefile
index 65ca5539c470..c2bb07f5bcc7 100644
--- a/kernel/kcsan/Makefile
+++ b/kernel/kcsan/Makefile
@@ -13,5 +13,5 @@ CFLAGS_core.o := $(call cc-option,-fno-conserve-stack) \
obj-y := core.o debugfs.o report.o
obj-$(CONFIG_KCSAN_SELFTEST) += selftest.o
-CFLAGS_kcsan-test.o := $(CFLAGS_KCSAN) -g -fno-omit-frame-pointer
-obj-$(CONFIG_KCSAN_TEST) += kcsan-test.o
+CFLAGS_kcsan_test.o := $(CFLAGS_KCSAN) -g -fno-omit-frame-pointer
+obj-$(CONFIG_KCSAN_KUNIT_TEST) += kcsan_test.o
diff --git a/kernel/kcsan/kcsan-test.c b/kernel/kcsan/kcsan_test.c
similarity index 99%
rename from kernel/kcsan/kcsan-test.c
rename to kernel/kcsan/kcsan_test.c
index ebe7fd245104..f16f632eb416 100644
--- a/kernel/kcsan/kcsan-test.c
+++ b/kernel/kcsan/kcsan_test.c
@@ -1156,7 +1156,7 @@ static void test_exit(struct kunit *test)
}
static struct kunit_suite kcsan_test_suite = {
- .name = "kcsan-test",
+ .name = "kcsan",
.test_cases = kcsan_test_cases,
.init = test_init,
.exit = test_exit,
diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index f271ff5fbb5a..0440f373248e 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -69,8 +69,9 @@ config KCSAN_SELFTEST
panic. Recommended to be enabled, ensuring critical functionality
works as intended.
-config KCSAN_TEST
- tristate "KCSAN test for integrated runtime behaviour"
+config KCSAN_KUNIT_TEST
+ tristate "KCSAN test for integrated runtime behaviour" if !KUNIT_ALL_TESTS
+ default KUNIT_ALL_TESTS
depends on TRACEPOINTS && KUNIT
select TORTURE_TEST
help
--
2.30.0.284.gd98b1dd5eaa7-goog
Since KUnit now support parameterized tests via KUNIT_CASE_PARAM, update
KCSAN's test to switch to it for parameterized tests. This simplifies
parameterized tests and gets rid of the "parameters in case name"
workaround (hack).
At the same time, we can increase the maximum number of threads used,
because on systems with too few CPUs, KUnit allows us to now stop at the
maximum useful threads and not unnecessarily execute redundant test
cases with (the same) limited threads as had been the case before.
Cc: David Gow <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
---
kernel/kcsan/kcsan_test.c | 116 ++++++++++++++++++--------------------
1 file changed, 54 insertions(+), 62 deletions(-)
diff --git a/kernel/kcsan/kcsan_test.c b/kernel/kcsan/kcsan_test.c
index f16f632eb416..b71751fc9f4f 100644
--- a/kernel/kcsan/kcsan_test.c
+++ b/kernel/kcsan/kcsan_test.c
@@ -13,6 +13,8 @@
* Author: Marco Elver <[email protected]>
*/
+#define pr_fmt(fmt) "kcsan_test: " fmt
+
#include <kunit/test.h>
#include <linux/jiffies.h>
#include <linux/kcsan-checks.h>
@@ -951,22 +953,53 @@ static void test_atomic_builtins(struct kunit *test)
}
/*
- * Each test case is run with different numbers of threads. Until KUnit supports
- * passing arguments for each test case, we encode #threads in the test case
- * name (read by get_num_threads()). [The '-' was chosen as a stylistic
- * preference to separate test name and #threads.]
+ * Generate thread counts for all test cases. Values generated are in interval
+ * [2, 5] followed by exponentially increasing thread counts from 8 to 32.
*
* The thread counts are chosen to cover potentially interesting boundaries and
- * corner cases (range 2-5), and then stress the system with larger counts.
+ * corner cases (2 to 5), and then stress the system with larger counts.
*/
-#define KCSAN_KUNIT_CASE(test_name) \
- { .run_case = test_name, .name = #test_name "-02" }, \
- { .run_case = test_name, .name = #test_name "-03" }, \
- { .run_case = test_name, .name = #test_name "-04" }, \
- { .run_case = test_name, .name = #test_name "-05" }, \
- { .run_case = test_name, .name = #test_name "-08" }, \
- { .run_case = test_name, .name = #test_name "-16" }
+static const void *nthreads_gen_params(const void *prev, char *desc)
+{
+ long nthreads = (long)prev;
+
+ if (nthreads < 0 || nthreads >= 32)
+ nthreads = 0; /* stop */
+ else if (!nthreads)
+ nthreads = 2; /* initial value */
+ else if (nthreads < 5)
+ nthreads++;
+ else if (nthreads == 5)
+ nthreads = 8;
+ else
+ nthreads *= 2;
+ if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
+ /*
+ * Without any preemption, keep 2 CPUs free for other tasks, one
+ * of which is the main test case function checking for
+ * completion or failure.
+ */
+ const long min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0;
+ const long min_required_cpus = 2 + min_unused_cpus;
+
+ if (num_online_cpus() < min_required_cpus) {
+ pr_err_once("Too few online CPUs (%u < %d) for test\n",
+ num_online_cpus(), min_required_cpus);
+ nthreads = 0;
+ } else if (nthreads >= num_online_cpus() - min_unused_cpus) {
+ /* Use negative value to indicate last param. */
+ nthreads = -(num_online_cpus() - min_unused_cpus);
+ pr_warn_once("Limiting number of threads to %ld (only %d online CPUs)\n",
+ -nthreads, num_online_cpus());
+ }
+ }
+
+ snprintf(desc, KUNIT_PARAM_DESC_SIZE, "threads=%ld", abs(nthreads));
+ return (void *)nthreads;
+}
+
+#define KCSAN_KUNIT_CASE(test_name) KUNIT_CASE_PARAM(test_name, nthreads_gen_params)
static struct kunit_case kcsan_test_cases[] = {
KCSAN_KUNIT_CASE(test_basic),
KCSAN_KUNIT_CASE(test_concurrent_races),
@@ -996,24 +1029,6 @@ static struct kunit_case kcsan_test_cases[] = {
/* ===== End test cases ===== */
-/* Get number of threads encoded in test name. */
-static bool __no_kcsan
-get_num_threads(const char *test, int *nthreads)
-{
- int len = strlen(test);
-
- if (WARN_ON(len < 3))
- return false;
-
- *nthreads = test[len - 1] - '0';
- *nthreads += (test[len - 2] - '0') * 10;
-
- if (WARN_ON(*nthreads < 0))
- return false;
-
- return true;
-}
-
/* Concurrent accesses from interrupts. */
__no_kcsan
static void access_thread_timer(struct timer_list *timer)
@@ -1076,9 +1091,6 @@ static int test_init(struct kunit *test)
if (!torture_init_begin((char *)test->name, 1))
return -EBUSY;
- if (!get_num_threads(test->name, &nthreads))
- goto err;
-
if (WARN_ON(threads))
goto err;
@@ -1087,38 +1099,18 @@ static int test_init(struct kunit *test)
goto err;
}
- if (!IS_ENABLED(CONFIG_PREEMPT) || !IS_ENABLED(CONFIG_KCSAN_INTERRUPT_WATCHER)) {
- /*
- * Without any preemption, keep 2 CPUs free for other tasks, one
- * of which is the main test case function checking for
- * completion or failure.
- */
- const int min_unused_cpus = IS_ENABLED(CONFIG_PREEMPT_NONE) ? 2 : 0;
- const int min_required_cpus = 2 + min_unused_cpus;
+ nthreads = abs((long)test->param_value);
+ if (WARN_ON(!nthreads))
+ goto err;
- if (num_online_cpus() < min_required_cpus) {
- pr_err("%s: too few online CPUs (%u < %d) for test",
- test->name, num_online_cpus(), min_required_cpus);
- goto err;
- } else if (nthreads > num_online_cpus() - min_unused_cpus) {
- nthreads = num_online_cpus() - min_unused_cpus;
- pr_warn("%s: limiting number of threads to %d\n",
- test->name, nthreads);
- }
- }
+ threads = kcalloc(nthreads + 1, sizeof(struct task_struct *), GFP_KERNEL);
+ if (WARN_ON(!threads))
+ goto err;
- if (nthreads) {
- threads = kcalloc(nthreads + 1, sizeof(struct task_struct *),
- GFP_KERNEL);
- if (WARN_ON(!threads))
+ threads[nthreads] = NULL;
+ for (i = 0; i < nthreads; ++i) {
+ if (torture_create_kthread(access_thread, NULL, threads[i]))
goto err;
-
- threads[nthreads] = NULL;
- for (i = 0; i < nthreads; ++i) {
- if (torture_create_kthread(access_thread, NULL,
- threads[i]))
- goto err;
- }
}
torture_init_end();
--
2.30.0.284.gd98b1dd5eaa7-goog
On Thu, Jan 14, 2021 at 12:06 AM Marco Elver <[email protected]> wrote:
>
> Per recently added KUnit style recommendations at
> Documentation/dev-tools/kunit/style.rst, make the following changes to
> the KCSAN test:
>
> 1. Rename 'kcsan-test.c' to 'kcsan_test.c'.
>
> 2. Rename suite name 'kcsan-test' to 'kcsan'.
>
> 3. Rename CONFIG_KCSAN_TEST to CONFIG_KCSAN_KUNIT_TEST and
> default to KUNIT_ALL_TESTS.
>
> Cc: David Gow <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
Thanks very much -- it's great to see the naming guidelines starting
to be picked up. I also tested the KUNIT_ALL_TESTS config option w/
KCSAN enabled, and it worked a treat.
My only note is that we've had some problems[1] with mm-related
changes which rename files getting corrupted at some point before
reaching Linus, so it's probably worth keeping a close eye on this
change to make sure nothing goes wrong.
Reviewed-by: David Gow <[email protected]>
-- David
[1]: https://www.spinics.net/lists/linux-mm/msg239149.html
On Tue, 26 Jan 2021 at 05:35, David Gow <[email protected]> wrote:
>
> On Thu, Jan 14, 2021 at 12:06 AM Marco Elver <[email protected]> wrote:
> >
> > Per recently added KUnit style recommendations at
> > Documentation/dev-tools/kunit/style.rst, make the following changes to
> > the KCSAN test:
> >
> > 1. Rename 'kcsan-test.c' to 'kcsan_test.c'.
> >
> > 2. Rename suite name 'kcsan-test' to 'kcsan'.
> >
> > 3. Rename CONFIG_KCSAN_TEST to CONFIG_KCSAN_KUNIT_TEST and
> > default to KUNIT_ALL_TESTS.
> >
> > Cc: David Gow <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
>
> Thanks very much -- it's great to see the naming guidelines starting
> to be picked up. I also tested the KUNIT_ALL_TESTS config option w/
> KCSAN enabled, and it worked a treat.
>
> My only note is that we've had some problems[1] with mm-related
> changes which rename files getting corrupted at some point before
> reaching Linus, so it's probably worth keeping a close eye on this
> change to make sure nothing goes wrong.
KCSAN changes go through Paul's -rcu tree, and once there's a stable
commit (latest when it reaches -tip) I would expect Git won't mess
things up.
> Reviewed-by: David Gow <[email protected]>
Thanks for taking a look!
-- Marco
> -- David
>
> [1]: https://www.spinics.net/lists/linux-mm/msg239149.html
On Tue, Jan 26, 2021 at 12:35:47PM +0800, David Gow wrote:
> On Thu, Jan 14, 2021 at 12:06 AM Marco Elver <[email protected]> wrote:
> >
> > Since KUnit now support parameterized tests via KUNIT_CASE_PARAM, update
> > KCSAN's test to switch to it for parameterized tests. This simplifies
> > parameterized tests and gets rid of the "parameters in case name"
> > workaround (hack).
> >
> > At the same time, we can increase the maximum number of threads used,
> > because on systems with too few CPUs, KUnit allows us to now stop at the
> > maximum useful threads and not unnecessarily execute redundant test
> > cases with (the same) limited threads as had been the case before.
> >
> > Cc: David Gow <[email protected]>
> > Signed-off-by: Marco Elver <[email protected]>
> > ---
>
> Thanks! This looks great from the KUnit point of view: I'm
> particularly excited to see a use of the parameterised test generator
> that's not just reading from an array.
>
> I tested this as well, and it all seemed to work fine for me.
>
> Reviewed-by: David Gow <[email protected]>
I applied both Reviewed-by tags, thank you!
Thanx, Paul
On Thu, Jan 14, 2021 at 12:06 AM Marco Elver <[email protected]> wrote:
>
> Since KUnit now support parameterized tests via KUNIT_CASE_PARAM, update
> KCSAN's test to switch to it for parameterized tests. This simplifies
> parameterized tests and gets rid of the "parameters in case name"
> workaround (hack).
>
> At the same time, we can increase the maximum number of threads used,
> because on systems with too few CPUs, KUnit allows us to now stop at the
> maximum useful threads and not unnecessarily execute redundant test
> cases with (the same) limited threads as had been the case before.
>
> Cc: David Gow <[email protected]>
> Signed-off-by: Marco Elver <[email protected]>
> ---
Thanks! This looks great from the KUnit point of view: I'm
particularly excited to see a use of the parameterised test generator
that's not just reading from an array.
I tested this as well, and it all seemed to work fine for me.
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David