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(-)
--
2.36.1.476.g0c4daa206d-goog
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 <[email protected]>
---
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);
}
--
2.36.1.476.g0c4daa206d-goog
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 <[email protected]>
---
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);
--
2.36.1.476.g0c4daa206d-goog
From: Jeremy Kerr <[email protected]>
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 <[email protected]>
Signed-off-by: Daniel Latypov <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
This is essentially the patch at:
https://lore.kernel.org/linux-kselftest/101d12fc9250b7a445ff50a9e7a25cd74d0e16eb.camel@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 | 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);
--
2.36.1.476.g0c4daa206d-goog
From: Daniel Latypov <[email protected]>
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 <[email protected]>
Co-developed-by: David Gow <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
Original RFC version of part of this patch:
https://lore.kernel.org/linux-kselftest/[email protected]/
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,
--
2.36.1.476.g0c4daa206d-goog
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 <[email protected]>
---
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);
--
2.36.1.476.g0c4daa206d-goog
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-execution-in-modules/20220618-170653
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/[email protected]/config)
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/14ff6ae01a41e301f1409874dd5aa38f73bc96f5
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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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-execution-in-modules/20220618-170653
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/[email protected]/config)
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/14ff6ae01a41e301f1409874dd5aa38f73bc96f5
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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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-execution-in-modules/20220618-170653
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/[email protected]/config)
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/c2386c54cc9fd471e5353f375ff71734214ed3c6
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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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-execution-in-modules/20220618-170653
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/[email protected]/config)
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/c2386c54cc9fd471e5353f375ff71734214ed3c6
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 <[email protected]>
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);
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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 <[email protected]>
[1] https://lore.kernel.org/dri-devel/20220608010709.272962-1
[email protected]/
Best Regards,
- Maíra Canal
Le 18/06/2022 à 11:03, David Gow a écrit :
> [Vous ne recevez pas souvent de courriers de [email protected]. 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
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 <[email protected]>
Acked-by: Mika Westerberg <[email protected]>
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 <[email protected]>
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_TESTS
> + depends on KUNIT
> + default KUNIT_ALL_TESTS
>
> config 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_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.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.