[PATCH v4 0/5] Rework KUnit test execution in modules

3 views
Skip to first unread message

David Gow

unread,
Jul 8, 2022, 11:20:07 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, David Gow, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston
This patch series makes two changes to how KUnit test suites are stored
and executed:
- The .kunit_test_suites section is now used for tests in modules (in
lieu of a module_init funciton), as well as for built-in tests. The
module loader will now trigger test execution. This frees up the
module_init function for other uses.
- Instead of storing an array of arrays of suites, have the
kunit_test_suite() and kunit_test_suites() macros append to one global
(or per-module) list of test suites. This removes a needless layer of
indirection, and removes the need to NULL-terminate suite_sets.

The upshot of this is that it should now be possible to use the
kunit_test_suite() and kunit_test_suites() macros to register test
suites even from within modules which otherwise had module_init
functions. This was proving to be quite a common issue, resulting in
several modules calling into KUnit's private suite execution functions
to run their tests (often introducing incompatibilities with the KUnit
tooling).

This series also fixes the thunderbolt, nitro_enclaves, and
sdhci-of-aspeed tests to use kunit_test_suite() now that it works. This
is required, as otherwise the first two patches may break these tests
entirely.

Huge thanks to Jeremy Kerr, who designed and implemented the module
loader changes, and to Daniel Latypov for pushing the simplification of
the nested arrays in .kunit_test_suites.

I've tested this series both with builtin tests on a number of
architectures, and with modules on x86_64, and it seems good-to-go to
me. More testing (particularly of modules) with more interesting setups
never hurts, though!

Cheers,
-- David

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- Add Brendan's Reviewed/Acked-by tags.

Daniel Latypov (1):
kunit: flatten kunit_suite*** to kunit_suite** in .kunit_test_suites

David Gow (3):
thunderbolt: test: Use kunit_test_suite() macro
nitro_enclaves: test: Use kunit_test_suite() macro
mmc: sdhci-of-aspeed: test: Use kunit_test_suite() macro

Jeremy Kerr (1):
kunit: unify module and builtin suite definitions

drivers/mmc/host/Kconfig | 5 +-
drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +-
drivers/mmc/host/sdhci-of-aspeed.c | 34 +----
drivers/thunderbolt/Kconfig | 6 +-
drivers/thunderbolt/domain.c | 3 -
drivers/thunderbolt/tb.h | 8 -
drivers/thunderbolt/test.c | 12 +-
drivers/virt/nitro_enclaves/Kconfig | 5 +-
drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 ----
.../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +-
include/kunit/test.h | 62 ++------
include/linux/module.h | 5 +
kernel/module/main.c | 6 +
lib/kunit/executor.c | 115 ++++----------
lib/kunit/executor_test.c | 144 +++++-------------
lib/kunit/test.c | 54 ++++++-
16 files changed, 155 insertions(+), 344 deletions(-)

--
2.37.0.rc0.161.g10f37bed90-goog

David Gow

unread,
Jul 8, 2022, 11:20:12 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston, David Gow
From: Jeremy Kerr <j...@codeconstruct.com.au>

Currently, KUnit runs built-in tests and tests loaded from modules
differently. For built-in tests, the kunit_test_suite{,s}() macro adds a
list of suites in the .kunit_test_suites linker section. However, for
kernel modules, a module_init() function is used to run the test suites.

This causes problems if tests are included in a module which already
defines module_init/exit_module functions, as they'll conflict with the
kunit-provided ones.

This change removes the kunit-defined module inits, and instead parses
the kunit tests from their own section in the module. After module init,
we call __kunit_test_suites_init() on the contents of that section,
which prepares and runs the suite.

This essentially unifies the module- and non-module kunit init formats.

Tested-by: Maíra Canal <maira...@usp.br>
Reviewed-by: Brendan Higgins <brendan...@google.com>
Signed-off-by: Jeremy Kerr <j...@codeconstruct.com.au>
Signed-off-by: Daniel Latypov <dlat...@google.com>
Signed-off-by: David Gow <davi...@google.com>
---

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- Add Brendan's Reviewed-by tags.

No changes to this patch since v2:
https://lore.kernel.org/linux-kselftest/20220621085345.6...@google.com/

Changes since v1:
https://lore.kernel.org/linux-kselftest/20220618090310.1...@google.com/
- Fix a compile error with CONFIG_KUNIT=m (Thanks Christophe Leroy,
kernel test robot)
- Add Maíra's Tested-by.

Changes since RFC:
https://lore.kernel.org/linux-kselftest/101d12fc9250b7a445ff50a...@codeconstruct.com.au/
- I've basically just rebased it, tweaked some wording, and it made it
still compile when CONFIG_MODULES is not set.

---
include/kunit/test.h | 49 +++++----------------------------------
include/linux/module.h | 5 ++++
kernel/module/main.c | 6 +++++
lib/kunit/test.c | 52 +++++++++++++++++++++++++++++++++++++++++-
4 files changed, 68 insertions(+), 44 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 7646d1bcf685..cb155d3da284 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -250,42 +250,9 @@ static inline int kunit_run_all_tests(void)
}
#endif /* IS_BUILTIN(CONFIG_KUNIT) */

-#ifdef MODULE
-/**
- * kunit_test_suites_for_module() - used to register one or more
- * &struct kunit_suite with KUnit.
- *
- * @__suites: a statically allocated list of &struct kunit_suite.
- *
- * Registers @__suites with the test framework. See &struct kunit_suite for
- * more information.
- *
- * If a test suite is built-in, module_init() gets translated into
- * an initcall which we don't want as the idea is that for builtins
- * the executor will manage execution. So ensure we do not define
- * module_{init|exit} functions for the builtin case when registering
- * suites via kunit_test_suites() below.
- */
-#define kunit_test_suites_for_module(__suites) \
- static int __init kunit_test_suites_init(void) \
- { \
- return __kunit_test_suites_init(__suites); \
- } \
- module_init(kunit_test_suites_init); \
- \
- static void __exit kunit_test_suites_exit(void) \
- { \
- return __kunit_test_suites_exit(__suites); \
- } \
- module_exit(kunit_test_suites_exit) \
- MODULE_INFO(test, "Y");
-#else
-#define kunit_test_suites_for_module(__suites)
-#endif /* MODULE */
-
#define __kunit_test_suites(unique_array, unique_suites, ...) \
+ MODULE_INFO(test, "Y"); \
static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \
- kunit_test_suites_for_module(unique_array); \
static struct kunit_suite **unique_suites \
__used __section(".kunit_test_suites") = unique_array

@@ -295,16 +262,12 @@ static inline int kunit_run_all_tests(void)
*
* @__suites: a statically allocated list of &struct kunit_suite.
*
- * Registers @suites with the test framework. See &struct kunit_suite for
- * more information.
- *
- * When builtin, KUnit tests are all run via executor; this is done
- * by placing the array of struct kunit_suite * in the .kunit_test_suites
- * ELF section.
+ * Registers @suites with the test framework.
+ * This is done by placing the array of struct kunit_suite * in the
+ * .kunit_test_suites ELF section.
*
- * An alternative is to build the tests as a module. Because modules do not
- * support multiple initcall()s, we need to initialize an array of suites for a
- * module.
+ * When builtin, KUnit tests are all run via the executor at boot, and when
+ * built as a module, they run on module load.
*
*/
#define kunit_test_suites(__suites...) \
diff --git a/include/linux/module.h b/include/linux/module.h
index abd9fa916b7d..2490223c975d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -505,6 +505,11 @@ struct module {
int num_static_call_sites;
struct static_call_site *static_call_sites;
#endif
+#if IS_ENABLED(CONFIG_KUNIT)
+ int num_kunit_suites;
+ struct kunit_suite ***kunit_suites;
+#endif
+

#ifdef CONFIG_LIVEPATCH
bool klp; /* Is this a livepatch module? */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index fed58d30725d..4542db7cdf54 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2087,6 +2087,12 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*mod->static_call_sites),
&mod->num_static_call_sites);
#endif
+#ifdef CONFIG_KUNIT
+ mod->kunit_suites = section_objs(info, ".kunit_test_suites",
+ sizeof(*mod->kunit_suites),
+ &mod->num_kunit_suites);
+#endif
+
mod->extable = section_objs(info, "__ex_table",
sizeof(*mod->extable), &mod->num_exentries);

diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 8b11552dc215..246645eb3cef 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -10,6 +10,7 @@
#include <kunit/test.h>
#include <kunit/test-bug.h>
#include <linux/kernel.h>
+#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/panic.h>
#include <linux/sched/debug.h>
@@ -613,6 +614,49 @@ void __kunit_test_suites_exit(struct kunit_suite **suites)
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);

+#ifdef CONFIG_MODULES
+static void kunit_module_init(struct module *mod)
+{
+ unsigned int i;
+
+ for (i = 0; i < mod->num_kunit_suites; i++)
+ __kunit_test_suites_init(mod->kunit_suites[i]);
+}
+
+static void kunit_module_exit(struct module *mod)
+{
+ unsigned int i;
+
+ for (i = 0; i < mod->num_kunit_suites; i++)
+ __kunit_test_suites_exit(mod->kunit_suites[i]);
+}
+
+static int kunit_module_notify(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct module *mod = data;
+
+ switch (val) {
+ case MODULE_STATE_LIVE:
+ kunit_module_init(mod);
+ break;
+ case MODULE_STATE_GOING:
+ kunit_module_exit(mod);
+ break;
+ case MODULE_STATE_COMING:
+ case MODULE_STATE_UNFORMED:
+ break;
+ }
+
+ return 0;
+}
+
+static struct notifier_block kunit_mod_nb = {
+ .notifier_call = kunit_module_notify,
+ .priority = 0,
+};
+#endif
+
struct kunit_kmalloc_array_params {
size_t n;
size_t size;
@@ -707,13 +751,19 @@ EXPORT_SYMBOL_GPL(kunit_cleanup);
static int __init kunit_init(void)
{
kunit_debugfs_init();
-
+#ifdef CONFIG_MODULES
+ return register_module_notifier(&kunit_mod_nb);
+#else
return 0;
+#endif
}
late_initcall(kunit_init);

static void __exit kunit_exit(void)
{
+#ifdef CONFIG_MODULES
+ unregister_module_notifier(&kunit_mod_nb);
+#endif
kunit_debugfs_cleanup();
}
module_exit(kunit_exit);
--
2.37.0.rc0.161.g10f37bed90-goog

David Gow

unread,
Jul 8, 2022, 11:20:16 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston, David Gow
From: Daniel Latypov <dlat...@google.com>

We currently store kunit suites in the .kunit_test_suites ELF section as
a `struct kunit_suite***` (modulo some `const`s).
For every test file, we store a struct kunit_suite** NULL-terminated array.

This adds quite a bit of complexity to the test filtering code in the
executor.

Instead, let's just make the .kunit_test_suites section contain a single
giant array of struct kunit_suite pointers, which can then be directly
manipulated. This array is not NULL-terminated, and so none of the test
filtering code needs to NULL-terminate anything.

Tested-by: Maíra Canal <maira...@usp.br>
Reviewed-by: Brendan Higgins <brendan...@google.com>
Signed-off-by: Daniel Latypov <dlat...@google.com>
Co-developed-by: David Gow <davi...@google.com>
Signed-off-by: David Gow <davi...@google.com>
---

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- No longer NULL-terminate generated suite_sets
- Add Maíra's Tested-by tag.

Changes since RFC:
https://lore.kernel.org/linux-kselftest/20211013191320.2...@google.com/
- Actually flatten the .kunit_test_suites ELF section,
rather than constructing the flattened version at runtime.
---
include/kunit/test.h | 13 ++--
include/linux/module.h | 2 +-
lib/kunit/executor.c | 115 ++++++++----------------------
lib/kunit/executor_test.c | 144 +++++++++++---------------------------
lib/kunit/test.c | 18 ++---
5 files changed, 82 insertions(+), 210 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index cb155d3da284..c958855681cc 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -237,9 +237,9 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite);
unsigned int kunit_test_case_num(struct kunit_suite *suite,
struct kunit_case *test_case);

-int __kunit_test_suites_init(struct kunit_suite * const * const suites);
+int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites);

-void __kunit_test_suites_exit(struct kunit_suite **suites);
+void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);

#if IS_BUILTIN(CONFIG_KUNIT)
int kunit_run_all_tests(void);
@@ -250,11 +250,11 @@ static inline int kunit_run_all_tests(void)
}
#endif /* IS_BUILTIN(CONFIG_KUNIT) */

-#define __kunit_test_suites(unique_array, unique_suites, ...) \
+#define __kunit_test_suites(unique_array, ...) \
MODULE_INFO(test, "Y"); \
- static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \
- static struct kunit_suite **unique_suites \
- __used __section(".kunit_test_suites") = unique_array
+ static struct kunit_suite *unique_array[] \
+ __aligned(sizeof(struct kunit_suite *)) \
+ __used __section(".kunit_test_suites") = { __VA_ARGS__ }

/**
* kunit_test_suites() - used to register one or more &struct kunit_suite
@@ -272,7 +272,6 @@ static inline int kunit_run_all_tests(void)
*/
#define kunit_test_suites(__suites...) \
__kunit_test_suites(__UNIQUE_ID(array), \
- __UNIQUE_ID(suites), \
##__suites)

#define kunit_test_suite(suite) kunit_test_suites(&suite)
diff --git a/include/linux/module.h b/include/linux/module.h
index 2490223c975d..518296ea7f73 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -507,7 +507,7 @@ struct module {
#endif
#if IS_ENABLED(CONFIG_KUNIT)
int num_kunit_suites;
- struct kunit_suite ***kunit_suites;
+ struct kunit_suite **kunit_suites;
#endif


diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 572f64e0a41a..6c489d6c5e5d 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -9,8 +9,8 @@
* These symbols point to the .kunit_test_suites section and are defined in
* include/asm-generic/vmlinux.lds.h, and consequently must be extern.
*/
-extern struct kunit_suite * const * const __kunit_suites_start[];
-extern struct kunit_suite * const * const __kunit_suites_end[];
+extern struct kunit_suite * const __kunit_suites_start[];
+extern struct kunit_suite * const __kunit_suites_end[];

#if IS_BUILTIN(CONFIG_KUNIT)

@@ -90,62 +90,18 @@ kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob)
static char *kunit_shutdown;
core_param(kunit_shutdown, kunit_shutdown, charp, 0644);

-static struct kunit_suite * const *
-kunit_filter_subsuite(struct kunit_suite * const * const subsuite,
- struct kunit_test_filter *filter)
-{
- int i, n = 0;
- struct kunit_suite **filtered, *filtered_suite;
-
- n = 0;
- for (i = 0; subsuite[i]; ++i) {
- if (glob_match(filter->suite_glob, subsuite[i]->name))
- ++n;
- }
-
- if (n == 0)
- return NULL;
-
- filtered = kmalloc_array(n + 1, sizeof(*filtered), GFP_KERNEL);
- if (!filtered)
- return ERR_PTR(-ENOMEM);
-
- n = 0;
- for (i = 0; subsuite[i] != NULL; ++i) {
- if (!glob_match(filter->suite_glob, subsuite[i]->name))
- continue;
- filtered_suite = kunit_filter_tests(subsuite[i], filter->test_glob);
- if (IS_ERR(filtered_suite))
- return ERR_CAST(filtered_suite);
- else if (filtered_suite)
- filtered[n++] = filtered_suite;
- }
- filtered[n] = NULL;
-
- return filtered;
-}
-
+/* Stores an array of suites, end points one past the end */
struct suite_set {
- struct kunit_suite * const * const *start;
- struct kunit_suite * const * const *end;
+ struct kunit_suite * const *start;
+ struct kunit_suite * const *end;
};

-static void kunit_free_subsuite(struct kunit_suite * const *subsuite)
-{
- unsigned int i;
-
- for (i = 0; subsuite[i]; i++)
- kfree(subsuite[i]);
-
- kfree(subsuite);
-}
-
static void kunit_free_suite_set(struct suite_set suite_set)
{
- struct kunit_suite * const * const *suites;
+ struct kunit_suite * const *suites;

for (suites = suite_set.start; suites < suite_set.end; suites++)
- kunit_free_subsuite(*suites);
+ kfree(*suites);
kfree(suite_set.start);
}

@@ -154,7 +110,7 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,
int *err)
{
int i;
- struct kunit_suite * const **copy, * const *filtered_subsuite;
+ struct kunit_suite **copy, *filtered_suite;
struct suite_set filtered;
struct kunit_test_filter filter;

@@ -169,14 +125,19 @@ static struct suite_set kunit_filter_suites(const struct suite_set *suite_set,

kunit_parse_filter_glob(&filter, filter_glob);

- for (i = 0; i < max; ++i) {
- filtered_subsuite = kunit_filter_subsuite(suite_set->start[i], &filter);
- if (IS_ERR(filtered_subsuite)) {
- *err = PTR_ERR(filtered_subsuite);
+ for (i = 0; &suite_set->start[i] != suite_set->end; i++) {
+ if (!glob_match(filter.suite_glob, suite_set->start[i]->name))
+ continue;
+
+ filtered_suite = kunit_filter_tests(suite_set->start[i], filter.test_glob);
+ if (IS_ERR(filtered_suite)) {
+ *err = PTR_ERR(filtered_suite);
return filtered;
}
- if (filtered_subsuite)
- *copy++ = filtered_subsuite;
+ if (!filtered_suite)
+ continue;
+
+ *copy++ = filtered_suite;
}
filtered.end = copy;

@@ -199,52 +160,33 @@ static void kunit_handle_shutdown(void)

}

-static void kunit_print_tap_header(struct suite_set *suite_set)
-{
- struct kunit_suite * const * const *suites, * const *subsuite;
- int num_of_suites = 0;
-
- for (suites = suite_set->start; suites < suite_set->end; suites++)
- for (subsuite = *suites; *subsuite != NULL; subsuite++)
- num_of_suites++;
-
- pr_info("TAP version 14\n");
- pr_info("1..%d\n", num_of_suites);
-}
-
static void kunit_exec_run_tests(struct suite_set *suite_set)
{
- struct kunit_suite * const * const *suites;
+ size_t num_suites = suite_set->end - suite_set->start;

- kunit_print_tap_header(suite_set);
+ pr_info("TAP version 14\n");
+ pr_info("1..%zu\n", num_suites);

- for (suites = suite_set->start; suites < suite_set->end; suites++)
- __kunit_test_suites_init(*suites);
+ __kunit_test_suites_init(suite_set->start, num_suites);
}

static void kunit_exec_list_tests(struct suite_set *suite_set)
{
- unsigned int i;
- struct kunit_suite * const * const *suites;
+ struct kunit_suite * const *suites;
struct kunit_case *test_case;

/* Hack: print a tap header so kunit.py can find the start of KUnit output. */
pr_info("TAP version 14\n");

for (suites = suite_set->start; suites < suite_set->end; suites++)
- for (i = 0; (*suites)[i] != NULL; i++) {
- kunit_suite_for_each_test_case((*suites)[i], test_case) {
- pr_info("%s.%s\n", (*suites)[i]->name, test_case->name);
- }
+ kunit_suite_for_each_test_case((*suites), test_case) {
+ pr_info("%s.%s\n", (*suites)->name, test_case->name);
}
}

int kunit_run_all_tests(void)
{
- struct suite_set suite_set = {
- .start = __kunit_suites_start,
- .end = __kunit_suites_end,
- };
+ struct suite_set suite_set = {__kunit_suites_start, __kunit_suites_end};
int err = 0;

if (filter_glob_param) {
@@ -262,11 +204,10 @@ int kunit_run_all_tests(void)
else
pr_err("kunit executor: unknown action '%s'\n", action_param);

- if (filter_glob_param) { /* a copy was made of each array */
+ if (filter_glob_param) { /* a copy was made of each suite */
kunit_free_suite_set(suite_set);
}

-
out:
kunit_handle_shutdown();
return err;
diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
index eac6ff480273..0cea31c27b23 100644
--- a/lib/kunit/executor_test.c
+++ b/lib/kunit/executor_test.c
@@ -9,8 +9,6 @@
#include <kunit/test.h>

static void kfree_at_end(struct kunit *test, const void *to_free);
-static void free_subsuite_at_end(struct kunit *test,
- struct kunit_suite *const *to_free);
static struct kunit_suite *alloc_fake_suite(struct kunit *test,
const char *suite_name,
struct kunit_case *test_cases);
@@ -41,126 +39,80 @@ static void parse_filter_test(struct kunit *test)
kfree(filter.test_glob);
}

-static void filter_subsuite_test(struct kunit *test)
+static void filter_suites_test(struct kunit *test)
{
- struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
- struct kunit_suite * const *filtered;
- struct kunit_test_filter filter = {
- .suite_glob = "suite2",
- .test_glob = NULL,
- };
+ struct kunit_suite *subsuite[3] = {NULL, NULL};
+ struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]};
+ struct suite_set got;
+ int err = 0;

subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases);
subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);

/* Want: suite1, suite2, NULL -> suite2, NULL */
- filtered = kunit_filter_subsuite(subsuite, &filter);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered);
- free_subsuite_at_end(test, filtered);
+ got = kunit_filter_suites(&suite_set, "suite2", &err);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ kfree_at_end(test, got.start);

/* Validate we just have suite2 */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]);
- KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2");
- KUNIT_EXPECT_FALSE(test, filtered[1]);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]);
+ KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->name, "suite2");
+
+ /* Contains one element (end is 1 past end) */
+ KUNIT_ASSERT_EQ(test, got.end - got.start, 1);
}

-static void filter_subsuite_test_glob_test(struct kunit *test)
+static void filter_suites_test_glob_test(struct kunit *test)
{
- struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
- struct kunit_suite * const *filtered;
- struct kunit_test_filter filter = {
- .suite_glob = "suite2",
- .test_glob = "test2",
- };
+ struct kunit_suite *subsuite[3] = {NULL, NULL};
+ struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]};
+ struct suite_set got;
+ int err = 0;

subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases);
subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);

/* Want: suite1, suite2, NULL -> suite2 (just test1), NULL */
- filtered = kunit_filter_subsuite(subsuite, &filter);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered);
- free_subsuite_at_end(test, filtered);
+ got = kunit_filter_suites(&suite_set, "suite2.test2", &err);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ kfree_at_end(test, got.start);

/* Validate we just have suite2 */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]);
- KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->name, "suite2");
- KUNIT_EXPECT_FALSE(test, filtered[1]);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]);
+ KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->name, "suite2");
+ KUNIT_ASSERT_EQ(test, got.end - got.start, 1);

/* Now validate we just have test2 */
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered[0]->test_cases);
- KUNIT_EXPECT_STREQ(test, (const char *)filtered[0]->test_cases[0].name, "test2");
- KUNIT_EXPECT_FALSE(test, filtered[0]->test_cases[1].name);
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, got.start[0]->test_cases);
+ KUNIT_EXPECT_STREQ(test, (const char *)got.start[0]->test_cases[0].name, "test2");
+ KUNIT_EXPECT_FALSE(test, got.start[0]->test_cases[1].name);
}

-static void filter_subsuite_to_empty_test(struct kunit *test)
+static void filter_suites_to_empty_test(struct kunit *test)
{
- struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
- struct kunit_suite * const *filtered;
- struct kunit_test_filter filter = {
- .suite_glob = "not_found",
- .test_glob = NULL,
- };
+ struct kunit_suite *subsuite[3] = {NULL, NULL};
+ struct suite_set suite_set = {.start = subsuite, .end = &subsuite[2]};
+ struct suite_set got;
+ int err = 0;

subsuite[0] = alloc_fake_suite(test, "suite1", dummy_test_cases);
subsuite[1] = alloc_fake_suite(test, "suite2", dummy_test_cases);

- filtered = kunit_filter_subsuite(subsuite, &filter);
- free_subsuite_at_end(test, filtered); /* just in case */
+ got = kunit_filter_suites(&suite_set, "not_found", &err);
+ KUNIT_ASSERT_EQ(test, err, 0);
+ kfree_at_end(test, got.start); /* just in case */

- KUNIT_EXPECT_FALSE_MSG(test, filtered,
- "should be NULL to indicate no match");
-}
-
-static void kfree_subsuites_at_end(struct kunit *test, struct suite_set *suite_set)
-{
- struct kunit_suite * const * const *suites;
-
- kfree_at_end(test, suite_set->start);
- for (suites = suite_set->start; suites < suite_set->end; suites++)
- free_subsuite_at_end(test, *suites);
-}
-
-static void filter_suites_test(struct kunit *test)
-{
- /* Suites per-file are stored as a NULL terminated array */
- struct kunit_suite *subsuites[2][2] = {
- {NULL, NULL},
- {NULL, NULL},
- };
- /* Match the memory layout of suite_set */
- struct kunit_suite * const * const suites[2] = {
- subsuites[0], subsuites[1],
- };
-
- const struct suite_set suite_set = {
- .start = suites,
- .end = suites + 2,
- };
- struct suite_set filtered = {.start = NULL, .end = NULL};
- int err = 0;
-
- /* Emulate two files, each having one suite */
- subsuites[0][0] = alloc_fake_suite(test, "suite0", dummy_test_cases);
- subsuites[1][0] = alloc_fake_suite(test, "suite1", dummy_test_cases);
-
- /* Filter out suite1 */
- filtered = kunit_filter_suites(&suite_set, "suite0", &err);
- kfree_subsuites_at_end(test, &filtered); /* let us use ASSERTs without leaking */
- KUNIT_EXPECT_EQ(test, err, 0);
- KUNIT_ASSERT_EQ(test, filtered.end - filtered.start, (ptrdiff_t)1);
-
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0]);
- KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filtered.start[0][0]);
- KUNIT_EXPECT_STREQ(test, (const char *)filtered.start[0][0]->name, "suite0");
+ KUNIT_EXPECT_PTR_EQ_MSG(test, got.start, got.end,
+ "should be empty to indicate no match");
}

static struct kunit_case executor_test_cases[] = {
KUNIT_CASE(parse_filter_test),
- KUNIT_CASE(filter_subsuite_test),
- KUNIT_CASE(filter_subsuite_test_glob_test),
- KUNIT_CASE(filter_subsuite_to_empty_test),
KUNIT_CASE(filter_suites_test),
+ KUNIT_CASE(filter_suites_test_glob_test),
+ KUNIT_CASE(filter_suites_to_empty_test),
{}
};

@@ -190,20 +142,6 @@ static void kfree_at_end(struct kunit *test, const void *to_free)
(void *)to_free);
}

-static void free_subsuite_res_free(struct kunit_resource *res)
-{
- kunit_free_subsuite(res->data);
-}
-
-static void free_subsuite_at_end(struct kunit *test,
- struct kunit_suite *const *to_free)
-{
- if (IS_ERR_OR_NULL(to_free))
- return;
- kunit_alloc_resource(test, NULL, free_subsuite_res_free,
- GFP_KERNEL, (void *)to_free);
-}
-
static struct kunit_suite *alloc_fake_suite(struct kunit *test,
const char *suite_name,
struct kunit_case *test_cases)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 246645eb3cef..b73d5bb5c473 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -586,11 +586,11 @@ static void kunit_init_suite(struct kunit_suite *suite)
suite->suite_init_err = 0;
}

-int __kunit_test_suites_init(struct kunit_suite * const * const suites)
+int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites)
{
unsigned int i;

- for (i = 0; suites[i] != NULL; i++) {
+ for (i = 0; i < num_suites; i++) {
kunit_init_suite(suites[i]);
kunit_run_tests(suites[i]);
}
@@ -603,11 +603,11 @@ static void kunit_exit_suite(struct kunit_suite *suite)
kunit_debugfs_destroy_suite(suite);
}

-void __kunit_test_suites_exit(struct kunit_suite **suites)
+void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites)
{
unsigned int i;

- for (i = 0; suites[i] != NULL; i++)
+ for (i = 0; i < num_suites; i++)
kunit_exit_suite(suites[i]);

kunit_suite_counter = 1;
@@ -617,18 +617,12 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
#ifdef CONFIG_MODULES
static void kunit_module_init(struct module *mod)
{
- unsigned int i;
-
- for (i = 0; i < mod->num_kunit_suites; i++)
- __kunit_test_suites_init(mod->kunit_suites[i]);
+ __kunit_test_suites_init(mod->kunit_suites, mod->num_kunit_suites);
}

static void kunit_module_exit(struct module *mod)
{
- unsigned int i;
-
- for (i = 0; i < mod->num_kunit_suites; i++)
- __kunit_test_suites_exit(mod->kunit_suites[i]);
+ __kunit_test_suites_exit(mod->kunit_suites, mod->num_kunit_suites);
}

static int kunit_module_notify(struct notifier_block *nb, unsigned long val,
--
2.37.0.rc0.161.g10f37bed90-goog

David Gow

unread,
Jul 8, 2022, 11:20:21 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, David Gow, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston
The new implementation of kunit_test_suite() for modules no longer
conflicts with module_init, so can now be used by the thunderbolt tests.

Also update the Kconfig entry to enable the test when KUNIT_ALL_TESTS is
enabled.

This means that kunit_tool can now successfully run and parse the test
results with, for example:
./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add CONFIG_PCI=y --kconfig_add CONFIG_USB4=y \
'thunderbolt'

Acked-by: Mika Westerberg <mika.we...@linux.intel.com>
Acked-by: Daniel Latypov <dlat...@google.com>
Acked-by: Brendan Higgins <brendan...@google.com>
Signed-off-by: David Gow <davi...@google.com>
---

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- Add Brendan's Acked-by tag.

Changes since v2:
https://lore.kernel.org/linux-kselftest/20220621085345.6...@google.com/
- Don't permit USB4_KUNIT_TESTS to be enabled when USB4=y and KUNIT=m
i.e., add a dependency on (USB4=m || KUNIT=y)
This would result in undefined kunit symbols being used, otherwise.
- Add Daniel's Acked-by
- Actually include the Kconfig changes, which were mistakenly added to
the next patch in the series in v1.
- Add Acked-by tag from Mika Westerberg

---
drivers/thunderbolt/Kconfig | 6 ++++--
drivers/thunderbolt/domain.c | 3 ---
drivers/thunderbolt/tb.h | 8 --------
drivers/thunderbolt/test.c | 12 +-----------
4 files changed, 5 insertions(+), 24 deletions(-)

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index 4bfec8a28064..e76a6c173637 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -28,8 +28,10 @@ config USB4_DEBUGFS_WRITE
this for production systems or distro kernels.

config USB4_KUNIT_TEST
- bool "KUnit tests"
- depends on KUNIT=y
+ bool "KUnit tests" if !KUNIT_ALL_TESTS
+ depends on (USB4=m || KUNIT=y)
+ depends on KUNIT
+ default KUNIT_ALL_TESTS

config USB4_DMA_TEST
tristate "DMA traffic test driver"
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 2889a214dadc..99211f35a5cd 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -872,7 +872,6 @@ int tb_domain_init(void)
{
int ret;

- tb_test_init();
tb_debugfs_init();
tb_acpi_init();

@@ -890,7 +889,6 @@ int tb_domain_init(void)
err_acpi:
tb_acpi_exit();
tb_debugfs_exit();
- tb_test_exit();

return ret;
}
@@ -903,5 +901,4 @@ void tb_domain_exit(void)
tb_xdomain_exit();
tb_acpi_exit();
tb_debugfs_exit();
- tb_test_exit();
}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 4602c69913fa..a831faa50f65 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1271,12 +1271,4 @@ static inline void tb_service_debugfs_init(struct tb_service *svc) { }
static inline void tb_service_debugfs_remove(struct tb_service *svc) { }
#endif

-#ifdef CONFIG_USB4_KUNIT_TEST
-int tb_test_init(void);
-void tb_test_exit(void);
-#else
-static inline int tb_test_init(void) { return 0; }
-static inline void tb_test_exit(void) { }
-#endif
-
#endif
diff --git a/drivers/thunderbolt/test.c b/drivers/thunderbolt/test.c
index ee37f8b58f50..24c06e7354cd 100644
--- a/drivers/thunderbolt/test.c
+++ b/drivers/thunderbolt/test.c
@@ -2817,14 +2817,4 @@ static struct kunit_suite tb_test_suite = {
.test_cases = tb_test_cases,
};

-static struct kunit_suite *tb_test_suites[] = { &tb_test_suite, NULL };
-
-int tb_test_init(void)
-{
- return __kunit_test_suites_init(tb_test_suites);
-}
-
-void tb_test_exit(void)
-{
- return __kunit_test_suites_exit(tb_test_suites);
-}
+kunit_test_suite(tb_test_suite);
--
2.37.0.rc0.161.g10f37bed90-goog

David Gow

unread,
Jul 8, 2022, 11:20:25 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, David Gow, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston
The kunit_test_suite() macro previously conflicted with module_init,
making it unsuitable for use in the nitro_enclaves test. Now that it's
fixed, we can use it instead of a custom call into internal KUnit
functions to run the test.

As a side-effect, this means that the test results are properly included
with other suites when built-in. To celebrate, enable the test by
default when KUNIT_ALL_TESTS is set (and NITRO_ENCLAVES enabled).

The nitro_enclave tests can now be run via kunit_tool with:
./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add CONFIG_PCI=y --kconfig_add CONFIG_SMP=y \
--kconfig_add CONFIG_HOTPLUG_CPU=y \
--kconfig_add CONFIG_VIRT_DRIVERS=y \
--kconfig_add CONFIG_NITRO_ENCLAVES=y \
'ne_misc_dev_test'

(This is a pretty long command, so it may be worth adding a .kunitconfig
file at some point, instead.)

Reviewed-by: Andra Paraschiv <andr...@amazon.com>
Acked-by: Brendan Higgins <brendan...@google.com>
Signed-off-by: David Gow <davi...@google.com>
---

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- Add Brendan's Acked-by tag.

Changes since v2:
https://lore.kernel.org/linux-kselftest/20220621085345.6...@google.com/
- Add Andra's Reviewed-by tag.
- Move the mistakenly-added thunderbolt Kconfig to the previous patch
(Thanks Andra)
- Add Andra's Acked-by tag.

---
drivers/virt/nitro_enclaves/Kconfig | 5 ++--
drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 -------------------
.../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +---
3 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/drivers/virt/nitro_enclaves/Kconfig b/drivers/virt/nitro_enclaves/Kconfig
index 2d3d98158121..ce91add81401 100644
--- a/drivers/virt/nitro_enclaves/Kconfig
+++ b/drivers/virt/nitro_enclaves/Kconfig
@@ -16,8 +16,9 @@ config NITRO_ENCLAVES
The module will be called nitro_enclaves.

config NITRO_ENCLAVES_MISC_DEV_TEST
- bool "Tests for the misc device functionality of the Nitro Enclaves"
- depends on NITRO_ENCLAVES && KUNIT=y
+ bool "Tests for the misc device functionality of the Nitro Enclaves" if !KUNIT_ALL_TESTS
+ depends on NITRO_ENCLAVES && KUNIT
+ default KUNIT_ALL_TESTS
help
Enable KUnit tests for the misc device functionality of the Nitro
Enclaves. Select this option only if you will boot the kernel for
diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
index 20c881b6a4b6..241b94f62e56 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
@@ -1759,35 +1759,10 @@ static long ne_ioctl(struct file *file, unsigned int cmd, unsigned long arg)

#if defined(CONFIG_NITRO_ENCLAVES_MISC_DEV_TEST)
#include "ne_misc_dev_test.c"
-
-static inline int ne_misc_dev_test_init(void)
-{
- return __kunit_test_suites_init(ne_misc_dev_test_suites);
-}
-
-static inline void ne_misc_dev_test_exit(void)
-{
- __kunit_test_suites_exit(ne_misc_dev_test_suites);
-}
-#else
-static inline int ne_misc_dev_test_init(void)
-{
- return 0;
-}
-
-static inline void ne_misc_dev_test_exit(void)
-{
-}
#endif

static int __init ne_init(void)
{
- int rc = 0;
-
- rc = ne_misc_dev_test_init();
- if (rc < 0)
- return rc;
-
mutex_init(&ne_cpu_pool.mutex);

return pci_register_driver(&ne_pci_driver);
@@ -1798,8 +1773,6 @@ static void __exit ne_exit(void)
pci_unregister_driver(&ne_pci_driver);

ne_teardown_cpu_pool();
-
- ne_misc_dev_test_exit();
}

module_init(ne_init);
diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
index 265797bed0ea..74df43b925be 100644
--- a/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
+++ b/drivers/virt/nitro_enclaves/ne_misc_dev_test.c
@@ -151,7 +151,4 @@ static struct kunit_suite ne_misc_dev_test_suite = {
.test_cases = ne_misc_dev_test_cases,
};

-static struct kunit_suite *ne_misc_dev_test_suites[] = {
- &ne_misc_dev_test_suite,
- NULL
-};
+kunit_test_suite(ne_misc_dev_test_suite);
--
2.37.0.rc0.161.g10f37bed90-goog

David Gow

unread,
Jul 8, 2022, 11:20:29 PMJul 8
to Brendan Higgins, Luis Chamberlain, Jeremy Kerr, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, David Gow, kuni...@googlegroups.com, linux-k...@vger.kernel.org, linux-...@vger.kernel.org, Maíra Canal, linu...@vger.kernel.org, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, linu...@vger.kernel.org, linux-...@vger.kernel.org, Matt Johnston, Ulf Hansson
The kunit_test_suite() macro is no-longer incompatible with module_add,
so its use can be reinstated.

Since this fixes parsing with builtins and kunit_tool, also enable the
test by default when KUNIT_ALL_TESTS is enabled.

The test can now be run via kunit_tool with:
./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add CONFIG_OF=y --kconfig_add CONFIG_OF_ADDRESS=y \
--kconfig_add CONFIG_MMC=y --kconfig_add CONFIG_MMC_SDHCI=y \
--kconfig_add CONFIG_MMC_SDHCI_PLTFM=y \
--kconfig_add CONFIG_MMC_SDHCI_OF_ASPEED=y \
'sdhci-of-aspeed'

(It may be worth adding a .kunitconfig at some point, as there are
enough dependencies to make that command scarily long.)

Acked-by: Daniel Latypov <dlat...@google.com>
Acked-by: Ulf Hansson <ulf.h...@linaro.org>
Acked-by: Brendan Higgins <brendan...@google.com>
Signed-off-by: David Gow <davi...@google.com>
---

Changes since v3:
https://lore.kernel.org/linux-kselftest/20220625050838.1...@google.com/
- Rebase on top of the TAINT_TEST patch series. This should now apply
cleanly on top of the kunit branch:
https://lore.kernel.org/linux-kselftest/20220708044847.5...@google.com/T/#u
- Add Brendan's Acked-by tag.

Changes since v2:
https://lore.kernel.org/linux-kselftest/20220621085345.6...@google.com/
- Clean up the aspeed_sdc_init() function to get rid of an obsolete goto
(Thanks Daniel)
- Add Daniel and Ulf's Acked-by tags.

No changes since v1:
https://lore.kernel.org/linux-kselftest/20220618090310.1...@google.com/

---
drivers/mmc/host/Kconfig | 5 ++--
drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +-----
drivers/mmc/host/sdhci-of-aspeed.c | 34 +------------------------
3 files changed, 5 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index d6144978e32d..10c563999d3d 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -169,8 +169,9 @@ config MMC_SDHCI_OF_ASPEED
If unsure, say N.

config MMC_SDHCI_OF_ASPEED_TEST
- bool "Tests for the ASPEED SDHCI driver"
- depends on MMC_SDHCI_OF_ASPEED && KUNIT=y
+ bool "Tests for the ASPEED SDHCI driver" if !KUNIT_ALL_TESTS
+ depends on MMC_SDHCI_OF_ASPEED && KUNIT
+ default KUNIT_ALL_TESTS
help
Enable KUnit tests for the ASPEED SDHCI driver. Select this
option only if you will boot the kernel for the purpose of running
diff --git a/drivers/mmc/host/sdhci-of-aspeed-test.c b/drivers/mmc/host/sdhci-of-aspeed-test.c
index 1ed4f86291f2..ecb502606c53 100644
--- a/drivers/mmc/host/sdhci-of-aspeed-test.c
+++ b/drivers/mmc/host/sdhci-of-aspeed-test.c
@@ -96,10 +96,4 @@ static struct kunit_suite aspeed_sdhci_test_suite = {
.test_cases = aspeed_sdhci_test_cases,
};

-static struct kunit_suite *aspeed_sdc_test_suite_array[] = {
- &aspeed_sdhci_test_suite,
- NULL,
-};
-
-static struct kunit_suite **aspeed_sdc_test_suites
- __used __section(".kunit_test_suites") = aspeed_sdc_test_suite_array;
+kunit_test_suite(aspeed_sdhci_test_suite);
diff --git a/drivers/mmc/host/sdhci-of-aspeed.c b/drivers/mmc/host/sdhci-of-aspeed.c
index 6e4e132903a6..ba6677bf7372 100644
--- a/drivers/mmc/host/sdhci-of-aspeed.c
+++ b/drivers/mmc/host/sdhci-of-aspeed.c
@@ -606,25 +606,6 @@ static struct platform_driver aspeed_sdc_driver = {

#if defined(CONFIG_MMC_SDHCI_OF_ASPEED_TEST)
#include "sdhci-of-aspeed-test.c"
-
-static inline int aspeed_sdc_tests_init(void)
-{
- return __kunit_test_suites_init(aspeed_sdc_test_suites);
-}
-
-static inline void aspeed_sdc_tests_exit(void)
-{
- __kunit_test_suites_exit(aspeed_sdc_test_suites);
-}
-#else
-static inline int aspeed_sdc_tests_init(void)
-{
- return 0;
-}
-
-static inline void aspeed_sdc_tests_exit(void)
-{
-}
#endif

static int __init aspeed_sdc_init(void)
@@ -637,18 +618,7 @@ static int __init aspeed_sdc_init(void)

rc = platform_driver_register(&aspeed_sdc_driver);
if (rc < 0)
- goto cleanup_sdhci;
-
- rc = aspeed_sdc_tests_init();
- if (rc < 0) {
- platform_driver_unregister(&aspeed_sdc_driver);
- goto cleanup_sdhci;
- }
-
- return 0;
-
-cleanup_sdhci:
- platform_driver_unregister(&aspeed_sdhci_driver);
+ platform_driver_unregister(&aspeed_sdhci_driver);

return rc;
}
@@ -656,8 +626,6 @@ module_init(aspeed_sdc_init);

static void __exit aspeed_sdc_exit(void)
{
- aspeed_sdc_tests_exit();
-
platform_driver_unregister(&aspeed_sdc_driver);
platform_driver_unregister(&aspeed_sdhci_driver);
}
--
2.37.0.rc0.161.g10f37bed90-goog

Geert Uytterhoeven

unread,
Aug 11, 2022, 9:49:41 AMAug 11
to David Gow, Jeremy Kerr, Brendan Higgins, Luis Chamberlain, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List, Maíra Canal, Linux MMC List, linux-...@lists.ozlabs.org, ope...@lists.ozlabs.org, USB list, linux-...@vger.kernel.org, Matt Johnston
Hi David, Jeremy,

On Sat, Jul 9, 2022 at 5:21 AM David Gow <davi...@google.com> wrote:
> From: Jeremy Kerr <j...@codeconstruct.com.au>
>
> Currently, KUnit runs built-in tests and tests loaded from modules
> differently. For built-in tests, the kunit_test_suite{,s}() macro adds a
> list of suites in the .kunit_test_suites linker section. However, for
> kernel modules, a module_init() function is used to run the test suites.
>
> This causes problems if tests are included in a module which already
> defines module_init/exit_module functions, as they'll conflict with the
> kunit-provided ones.
>
> This change removes the kunit-defined module inits, and instead parses
> the kunit tests from their own section in the module. After module init,
> we call __kunit_test_suites_init() on the contents of that section,
> which prepares and runs the suite.
>
> This essentially unifies the module- and non-module kunit init formats.
>
> Tested-by: Maíra Canal <maira...@usp.br>
> Reviewed-by: Brendan Higgins <brendan...@google.com>
> Signed-off-by: Jeremy Kerr <j...@codeconstruct.com.au>
> Signed-off-by: Daniel Latypov <dlat...@google.com>
> Signed-off-by: David Gow <davi...@google.com>

Thanks for your patch, which is now commit 3d6e44623841c8b8 ("kunit:
unify module and builtin suite definitions") upstream.

Since this commit, modular kunit tests are no longer run at all.

Before:

# insmod lib/kunit/kunit.ko
# insmod lib/test_hash.ko
test_hash: loading test module taints kernel.
# Subtest: hash
1..2
ok 1 - test_string_or
ok 2 - test_hash_or
# hash: pass:2 fail:0 skip:0 total:2
# Totals: pass:2 fail:0 skip:0 total:2
ok 1 - hash

After:

# insmod lib/kunit/kunit.ko
# insmod lib/test_hash.ko
test_hash: loading test module taints kernel.

The actual test code (and test init code, if it exists) is not run.

Reverting commits e5857d396f35e59e ("kunit: flatten kunit_suite***
to kunit_suite** in .kunit_test_suites") and 3d6e44623841c8b8 ("kunit:
unify module and builtin suite definitions") fixes the issue.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

David Gow

unread,
Aug 11, 2022, 12:56:10 PMAug 11
to Geert Uytterhoeven, Jeremy Kerr, Brendan Higgins, Luis Chamberlain, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List, Maíra Canal, Linux MMC List, linux-aspeed, OpenBMC Maillist, USB list, linux-...@vger.kernel.org, Matt Johnston
Thanks Geert,

This is a known issue. There's a patch to fix it here, which just
missed the pull request:
https://patchwork.kernel.org/project/linux-kselftest/patch/20220713005221.1...@google.com/

We'll try to get it merged as soon as possible.

Cheers,
-- David

Geert Uytterhoeven

unread,
Aug 12, 2022, 4:08:35 AMAug 12
to David Gow, Jeremy Kerr, Brendan Higgins, Luis Chamberlain, Daniel Latypov, Shuah Khan, Andrew Jeffery, Mika Westerberg, Andra Paraschiv, Longpeng, Greg KH, KUnit Development, open list:KERNEL SELFTEST FRAMEWORK, Linux Kernel Mailing List, Maíra Canal, Linux MMC List, linux-aspeed, OpenBMC Maillist, USB list, linux-...@vger.kernel.org, Matt Johnston
Hi David,
Thanks for the pointer. I can confirm it fixes the issue, so I provided
my Tb tag.
Reply all
Reply to author
Forward
0 new messages