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.
Tested-by: Maíra Canal <[email protected]>
Signed-off-by: Jeremy Kerr <[email protected]>
Signed-off-by: Daniel Latypov <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
No changes to this patch since v2:
https://lore.kernel.org/linux-kselftest/[email protected]/
Changes since v1:
https://lore.kernel.org/linux-kselftest/[email protected]/
- Fix a compile error with CONFIG_KUNIT=m (Thanks Christophe Leroy,
kernel test robot)
- Add Maíra's Tested-by.
Changes since RFC:
https://lore.kernel.org/linux-kselftest/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..2490223c975d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -505,6 +505,11 @@ struct module {
int num_static_call_sites;
struct static_call_site *static_call_sites;
#endif
+#if IS_ENABLED(CONFIG_KUNIT)
+ int num_kunit_suites;
+ struct kunit_suite ***kunit_suites;
+#endif
+
#ifdef CONFIG_LIVEPATCH
bool klp; /* Is this a livepatch module? */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index fed58d30725d..4542db7cdf54 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2087,6 +2087,12 @@ static int find_module_sections(struct module *mod, struct load_info *info)
sizeof(*mod->static_call_sites),
&mod->num_static_call_sites);
#endif
+#ifdef CONFIG_KUNIT
+ mod->kunit_suites = section_objs(info, ".kunit_test_suites",
+ sizeof(*mod->kunit_suites),
+ &mod->num_kunit_suites);
+#endif
+
mod->extable = section_objs(info, "__ex_table",
sizeof(*mod->extable), &mod->num_exentries);
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 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.37.0.rc0.161.g10f37bed90-goog
On Sat, Jun 25, 2022 at 1:10 AM David Gow <[email protected]> wrote:
>
> 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.
>
> Tested-by: Maíra Canal <[email protected]>
> Signed-off-by: Jeremy Kerr <[email protected]>
> Signed-off-by: Daniel Latypov <[email protected]>
> Signed-off-by: David Gow <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>
On Fri, Jun 24, 2022 at 10:10 PM David Gow <[email protected]> wrote:
> 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) \
Deleting this bit now causes merge conflicts with Shuah's kunit
branch, due to https://patchwork.kernel.org/project/linux-kselftest/patch/[email protected]/
I.e. We added in a MODULE_INFO(test, "Y") in this to-be-deleted section.
Perhaps something like this would be a fix?
#ifdef MODULE
#define _kunit_mark_test_module MODULE_INFO(test, "Y")
#else
#define _kunit_mark_test_module
#endif /* MODULE */
#define __kunit_test_suites(unique_array, unique_suites, ...)
\
_kunit_mark_test_module;
\
static struct kunit_suite *unique_array[] = { __VA_ARGS__,
NULL }; \
static struct kunit_suite **unique_suites
\
__used __section(".kunit_test_suites") = unique_array
> - 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
>
On Sat, Jul 9, 2022 at 2:23 AM Daniel Latypov <[email protected]> wrote:
>
> On Fri, Jun 24, 2022 at 10:10 PM David Gow <[email protected]> wrote:
> > 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) \
>
> Deleting this bit now causes merge conflicts with Shuah's kunit
> branch, due to https://patchwork.kernel.org/project/linux-kselftest/patch/[email protected]/
> I.e. We added in a MODULE_INFO(test, "Y") in this to-be-deleted section.
Nice catch. I've rebased this series on top of the taint stuff:
https://lore.kernel.org/linux-kselftest/[email protected]/T/#u
> Perhaps something like this would be a fix?
Thanks.The rebased version is basically this, but without the #ifdef
MODULE indirection, as MODULE_INFO() will decay to nothing if MODULE
is not defined, anyway.
> #ifdef MODULE
> #define _kunit_mark_test_module MODULE_INFO(test, "Y")
> #else
> #define _kunit_mark_test_module
> #endif /* MODULE */
>
> #define __kunit_test_suites(unique_array, unique_suites, ...)
> \
> _kunit_mark_test_module;
> \
> static struct kunit_suite *unique_array[] = { __VA_ARGS__,
> NULL }; \
> static struct kunit_suite **unique_suites
> \
> __used __section(".kunit_test_suites") = unique_array
>
>
> > - 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
> >