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.
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.
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, and with modules on x86_64, but there's always the possibility that there's something subtle and nasty on another architecture, so please test!
Cheers, -- David
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 | 27 ---- drivers/thunderbolt/Kconfig | 5 +- 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 | 60 ++------ include/linux/module.h | 5 + kernel/module/main.c | 6 + lib/kunit/executor.c | 117 ++++----------- lib/kunit/executor_test.c | 139 +++++------------- lib/kunit/test.c | 54 ++++++- 16 files changed, 152 insertions(+), 334 deletions(-)
From: Jeremy Kerr jk@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.
Signed-off-by: Jeremy Kerr jk@codeconstruct.com.au Signed-off-by: Daniel Latypov dlatypov@google.com Signed-off-by: David Gow davidgow@google.com ---
This is essentially the patch at: https://lore.kernel.org/linux-kselftest/101d12fc9250b7a445ff50a9e7a25cd74d0e... 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 | 47 ++++---------------------------------- include/linux/module.h | 5 ++++ kernel/module/main.c | 6 +++++ lib/kunit/test.c | 52 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 67 insertions(+), 43 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 8ffcd7de9607..54306271cfbf 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -250,41 +250,8 @@ 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) -#else -#define kunit_test_suites_for_module(__suites) -#endif /* MODULE */ - #define __kunit_test_suites(unique_array, unique_suites, ...) \ 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
@@ -294,16 +261,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..e3cd6c325794 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 +#ifdef 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 a5053a07409f..3052526b9b89 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/sched/debug.h> #include <linux/sched.h> @@ -609,6 +610,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; @@ -703,13 +747,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);
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master] [cannot apply to mcgrof/modules-next joel-aspeed/for-next ulf-hansson-mmc-mirror/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/Rework-KUnit-test-e... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4b35035bcf80ddb47c0112c4fbd84a63a2836a18 config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20220618/202206181950.qNG3jcE8-lkp@i...) compiler: s390-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/14ff6ae01a41e301f1409874dd5aa3... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 git checkout 14ff6ae01a41e301f1409874dd5aa38f73bc96f5 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash lib/kunit/
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
lib/kunit/test.c: In function 'kunit_module_init':
lib/kunit/test.c:618:28: error: 'struct module' has no member named 'num_kunit_suites'
618 | for (i = 0; i < mod->num_kunit_suites; i++) | ^~
lib/kunit/test.c:619:45: error: 'struct module' has no member named 'kunit_suites'
619 | __kunit_test_suites_init(mod->kunit_suites[i]); | ^~ lib/kunit/test.c: In function 'kunit_module_exit': lib/kunit/test.c:626:28: error: 'struct module' has no member named 'num_kunit_suites' 626 | for (i = 0; i < mod->num_kunit_suites; i++) | ^~ lib/kunit/test.c:627:45: error: 'struct module' has no member named 'kunit_suites' 627 | __kunit_test_suites_exit(mod->kunit_suites[i]); | ^~
vim +618 lib/kunit/test.c
612 613 #ifdef CONFIG_MODULES 614 static void kunit_module_init(struct module *mod) 615 { 616 unsigned int i; 617
618 for (i = 0; i < mod->num_kunit_suites; i++) 619 __kunit_test_suites_init(mod->kunit_suites[i]);
620 } 621
Hi David,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v5.19-rc2 next-20220617] [cannot apply to mcgrof/modules-next joel-aspeed/for-next ulf-hansson-mmc-mirror/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/Rework-KUnit-test-e... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4b35035bcf80ddb47c0112c4fbd84a63a2836a18 config: riscv-randconfig-r034-20220617 (https://download.01.org/0day-ci/archive/20220618/202206182025.UNVY0coI-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 91688716ba49942051dccdf7b9c4f81a7ec8feaf) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install riscv cross compiling tool for clang build # apt-get install binutils-riscv-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/14ff6ae01a41e301f1409874dd5aa3... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 git checkout 14ff6ae01a41e301f1409874dd5aa38f73bc96f5 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash lib/kunit/
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
lib/kunit/test.c:618:23: error: no member named 'num_kunit_suites' in 'struct module'
for (i = 0; i < mod->num_kunit_suites; i++) ~~~ ^
lib/kunit/test.c:619:33: error: no member named 'kunit_suites' in 'struct module'
__kunit_test_suites_init(mod->kunit_suites[i]); ~~~ ^ lib/kunit/test.c:626:23: error: no member named 'num_kunit_suites' in 'struct module' for (i = 0; i < mod->num_kunit_suites; i++) ~~~ ^ lib/kunit/test.c:627:33: error: no member named 'kunit_suites' in 'struct module' __kunit_test_suites_exit(mod->kunit_suites[i]); ~~~ ^ 4 errors generated.
vim +618 lib/kunit/test.c
612 613 #ifdef CONFIG_MODULES 614 static void kunit_module_init(struct module *mod) 615 { 616 unsigned int i; 617
618 for (i = 0; i < mod->num_kunit_suites; i++) 619 __kunit_test_suites_init(mod->kunit_suites[i]);
620 } 621
From: Daniel Latypov dlatypov@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.
Signed-off-by: Daniel Latypov dlatypov@google.com Co-developed-by: David Gow davidgow@google.com Signed-off-by: David Gow davidgow@google.com ---
Original RFC version of part of this patch: https://lore.kernel.org/linux-kselftest/20211013191320.2490913-1-dlatypov@go...
This version updates the way the .kunit_test_suites section is generated, rather than constructing the flattened version at runtime.
--- include/kunit/test.h | 13 ++-- include/linux/module.h | 2 +- lib/kunit/executor.c | 117 ++++++++------------------------ lib/kunit/executor_test.c | 139 +++++++++++--------------------------- lib/kunit/test.c | 18 ++--- 5 files changed, 82 insertions(+), 207 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 54306271cfbf..bd8d772979a6 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,10 +250,10 @@ static inline int kunit_run_all_tests(void) } #endif /* IS_BUILTIN(CONFIG_KUNIT) */
-#define __kunit_test_suites(unique_array, unique_suites, ...) \ - static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \ - static struct kunit_suite **unique_suites \ - __used __section(".kunit_test_suites") = unique_array +#define __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 @@ -271,7 +271,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 e3cd6c325794..a9e9180ba3c3 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -507,7 +507,7 @@ struct module { #endif #ifdef 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 96f96e42ce06..53fab8d76a00 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)
@@ -92,62 +92,18 @@ kunit_filter_tests(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 a NULL-terminated array of suites. */ 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); }
@@ -156,10 +112,11 @@ 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;
+ /* Note: this includes space for the terminating NULL. */ const size_t max = suite_set->end - suite_set->start;
copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL); @@ -171,15 +128,21 @@ 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] != NULL; 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; } + *copy = NULL; filtered.end = copy;
kfree(filter.suite_glob); @@ -201,52 +164,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) { @@ -264,11 +208,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..acbc44af49ed 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,81 @@ 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 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"); + // DO NOT SUBMIT: null-terminated for now. + KUNIT_ASSERT_EQ(test, got.end - got.start, 1); + KUNIT_EXPECT_FALSE(test, *got.end); }
-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 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); + KUNIT_EXPECT_FALSE(test, *got.end);
/* 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 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 */ - - 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); + got = kunit_filter_suites(&suite_set, "not_found", &err); + KUNIT_ASSERT_EQ(test, err, 0); + kfree_at_end(test, got.start); /* just in case */
- /* 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 +143,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 3052526b9b89..b6495c7f9a7e 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -582,11 +582,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]); } @@ -599,11 +599,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; @@ -613,18 +613,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,
Hi David,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v5.19-rc2 next-20220617] [cannot apply to mcgrof/modules-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/Rework-KUnit-test-e... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4b35035bcf80ddb47c0112c4fbd84a63a2836a18 config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20220618/202206182117.58z5vWxq-lkp@i...) compiler: hppa-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/c2386c54cc9fd471e5353f375ff717... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 git checkout c2386c54cc9fd471e5353f375ff71734214ed3c6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
Note: the linux-review/David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 HEAD fddb3ea0ed5627098eabc542fdba5a8b4b769066 builds fine. It only hurts bisectability.
All errors (new ones prefixed by >>):
drivers/mmc/host/sdhci-of-aspeed.c: In function 'aspeed_sdc_tests_init':
drivers/mmc/host/sdhci-of-aspeed.c:612:16: error: too few arguments to function '__kunit_test_suites_init'
612 | return __kunit_test_suites_init(aspeed_sdc_test_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/mmc/host/sdhci-of-aspeed-test.c:4, from drivers/mmc/host/sdhci-of-aspeed.c:608: include/kunit/test.h:240:5: note: declared here 240 | int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/mmc/host/sdhci-of-aspeed.c: In function 'aspeed_sdc_tests_exit':
drivers/mmc/host/sdhci-of-aspeed.c:617:9: error: too few arguments to function '__kunit_test_suites_exit'
617 | __kunit_test_suites_exit(aspeed_sdc_test_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/mmc/host/sdhci-of-aspeed-test.c:4, from drivers/mmc/host/sdhci-of-aspeed.c:608: include/kunit/test.h:242:6: note: declared here 242 | void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/mmc/host/sdhci-of-aspeed.c: In function 'aspeed_sdc_tests_init': drivers/mmc/host/sdhci-of-aspeed.c:613:1: error: control reaches end of non-void function [-Werror=return-type] 613 | } | ^ cc1: some warnings being treated as errors
vim +/__kunit_test_suites_init +612 drivers/mmc/host/sdhci-of-aspeed.c
4af307f574260c Andrew Jeffery 2021-01-22 609 4af307f574260c Andrew Jeffery 2021-01-22 610 static inline int aspeed_sdc_tests_init(void) 4af307f574260c Andrew Jeffery 2021-01-22 611 { 4af307f574260c Andrew Jeffery 2021-01-22 @612 return __kunit_test_suites_init(aspeed_sdc_test_suites); 4af307f574260c Andrew Jeffery 2021-01-22 613 } 4af307f574260c Andrew Jeffery 2021-01-22 614 4af307f574260c Andrew Jeffery 2021-01-22 615 static inline void aspeed_sdc_tests_exit(void) 4af307f574260c Andrew Jeffery 2021-01-22 616 { 4af307f574260c Andrew Jeffery 2021-01-22 @617 __kunit_test_suites_exit(aspeed_sdc_test_suites); 4af307f574260c Andrew Jeffery 2021-01-22 618 } 4af307f574260c Andrew Jeffery 2021-01-22 619 #else 4af307f574260c Andrew Jeffery 2021-01-22 620 static inline int aspeed_sdc_tests_init(void) 4af307f574260c Andrew Jeffery 2021-01-22 621 { 4af307f574260c Andrew Jeffery 2021-01-22 622 return 0; 4af307f574260c Andrew Jeffery 2021-01-22 623 } 4af307f574260c Andrew Jeffery 2021-01-22 624
Hi David,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v5.19-rc2 next-20220617] [cannot apply to mcgrof/modules-next joel-aspeed/for-next ulf-hansson-mmc-mirror/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/David-Gow/Rework-KUnit-test-e... base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 4b35035bcf80ddb47c0112c4fbd84a63a2836a18 config: xtensa-allyesconfig (https://download.01.org/0day-ci/archive/20220618/202206182258.EahbTrAv-lkp@i...) compiler: xtensa-linux-gcc (GCC) 11.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/c2386c54cc9fd471e5353f375ff717... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 git checkout c2386c54cc9fd471e5353f375ff71734214ed3c6 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=xtensa SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot lkp@intel.com
Note: the linux-review/David-Gow/Rework-KUnit-test-execution-in-modules/20220618-170653 HEAD fddb3ea0ed5627098eabc542fdba5a8b4b769066 builds fine. It only hurts bisectability.
All errors (new ones prefixed by >>):
drivers/thunderbolt/test.c: In function 'tb_test_init':
drivers/thunderbolt/test.c:2824:16: error: too few arguments to function '__kunit_test_suites_init'
2824 | return __kunit_test_suites_init(tb_test_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/thunderbolt/test.c:9: include/kunit/test.h:240:5: note: declared here 240 | int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/thunderbolt/test.c: In function 'tb_test_exit':
drivers/thunderbolt/test.c:2829:16: error: too few arguments to function '__kunit_test_suites_exit'
2829 | return __kunit_test_suites_exit(tb_test_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/thunderbolt/test.c:9: include/kunit/test.h:242:6: note: declared here 242 | void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/thunderbolt/test.c:2829:16: error: 'return' with a value, in function returning void [-Werror=return-type] 2829 | return __kunit_test_suites_exit(tb_test_suites); | ^~~~~~~~~~~~~~~~~~~~~~~~ drivers/thunderbolt/test.c:2827:6: note: declared here 2827 | void tb_test_exit(void) | ^~~~~~~~~~~~ drivers/thunderbolt/test.c: In function 'tb_test_init': drivers/thunderbolt/test.c:2825:1: error: control reaches end of non-void function [-Werror=return-type] 2825 | } | ^ cc1: some warnings being treated as errors
vim +/__kunit_test_suites_init +2824 drivers/thunderbolt/test.c
2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2821 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2822 int tb_test_init(void) 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2823 { 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 @2824 return __kunit_test_suites_init(tb_test_suites); 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2825 } 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2826 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2827 void tb_test_exit(void) 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 2828 { 2c6ea4e2cefe2e Mika Westerberg 2020-08-24 @2829 return __kunit_test_suites_exit(tb_test_suites);
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'
Signed-off-by: David Gow davidgow@google.com --- drivers/thunderbolt/domain.c | 3 --- drivers/thunderbolt/tb.h | 8 -------- drivers/thunderbolt/test.c | 12 +----------- 3 files changed, 1 insertion(+), 22 deletions(-)
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);
On Sat, Jun 18, 2022 at 05:03:08PM +0800, David Gow wrote:
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'
Signed-off-by: David Gow davidgow@google.com
Acked-by: Mika Westerberg mika.westerberg@linux.intel.com
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.)
Signed-off-by: David Gow davidgow@google.com --- drivers/thunderbolt/Kconfig | 5 ++-- drivers/virt/nitro_enclaves/Kconfig | 5 ++-- drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 ------------------- .../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +--- 4 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig index 4bfec8a28064..2a063d344b94 100644 --- a/drivers/thunderbolt/Kconfig +++ b/drivers/thunderbolt/Kconfig @@ -28,8 +28,9 @@ 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 KUNIT + default KUNIT_ALL_TESTS
config USB4_DMA_TEST tristate "DMA traffic test driver" 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);
On 18.06.2022 12:03, David Gow wrote:
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.)
Signed-off-by: David Gow davidgow@google.com
Thank you, David, for the KUnit update and for this patch.
drivers/thunderbolt/Kconfig | 5 ++-- drivers/virt/nitro_enclaves/Kconfig | 5 ++-- drivers/virt/nitro_enclaves/ne_misc_dev.c | 27 ------------------- .../virt/nitro_enclaves/ne_misc_dev_test.c | 5 +--- 4 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig index 4bfec8a28064..2a063d344b94 100644 --- a/drivers/thunderbolt/Kconfig +++ b/drivers/thunderbolt/Kconfig @@ -28,8 +28,9 @@ 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_TESTSdepends on KUNITdefault KUNIT_ALL_TESTSconfig USB4_DMA_TEST tristate "DMA traffic test driver"
This needs to be included in the previous patch instead (patch 3/5, for thunderbolt).
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_TESTSdepends on NITRO_ENCLAVES && KUNITdefault 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 fordiff --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.36.1.476.g0c4daa206d-goog
Other than that, looks good to me.
Thank you.
Andra
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
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.)
Signed-off-by: David Gow davidgow@google.com --- drivers/mmc/host/Kconfig | 5 +++-- drivers/mmc/host/sdhci-of-aspeed-test.c | 8 +------- drivers/mmc/host/sdhci-of-aspeed.c | 27 ------------------------- 3 files changed, 4 insertions(+), 36 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..c10367946bc7 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) @@ -639,12 +620,6 @@ static int __init aspeed_sdc_init(void) 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: @@ -656,8 +631,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); }
On 6/18/22 06:03, David Gow wrote:
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.
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.
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, and with modules on x86_64, but there's always the possibility that there's something subtle and nasty on another architecture, so please test!
Cheers, -- David
I've tested the modules on x86_64 machines, and everything looks fine. Also, I applied the AMDGPU KUnit tests [1] on top of these patches, tried out compiling as a module, and it runs pretty well!
Great to see this feature on KUnit!
Tested-by: Maíra Canal maira.canal@usp.br
[1] https://lore.kernel.org/dri-devel/20220608010709.272962-1 maira.canal@usp.br/
Best Regards, - Maíra Canal
Le 18/06/2022 à 11:03, David Gow a écrit :
[Vous ne recevez pas souvent de courriers de davidgow@google.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
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.
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.
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, and with modules on x86_64, but there's always the possibility that there's something subtle and nasty on another architecture, so please test!
Build failure on powerpc architecture with ppc64_defconfig + CONFIG_KUNIT_TEST=m
CC [M] lib/kunit/test.o lib/kunit/test.c: In function 'kunit_module_init': lib/kunit/test.c:616:37: error: 'struct module' has no member named 'kunit_suites' 616 | __kunit_test_suites_init(mod->kunit_suites, mod->num_kunit_suites); | ^~ lib/kunit/test.c:616:56: error: 'struct module' has no member named 'num_kunit_suites' 616 | __kunit_test_suites_init(mod->kunit_suites, mod->num_kunit_suites); | ^~ lib/kunit/test.c: In function 'kunit_module_exit': lib/kunit/test.c:621:37: error: 'struct module' has no member named 'kunit_suites' 621 | __kunit_test_suites_exit(mod->kunit_suites, mod->num_kunit_suites); | ^~ lib/kunit/test.c:621:56: error: 'struct module' has no member named 'num_kunit_suites' 621 | __kunit_test_suites_exit(mod->kunit_suites, mod->num_kunit_suites); | ^~
Christophe
linux-kselftest-mirror@lists.linaro.org
-
Christophe Leroy -
David Gow -
kernel test robot -
Maíra Canal -
Mika Westerberg -
Paraschiv, Andra-Irina