* [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
@ 2023-12-13 1:01 ` Rae Moar
2023-12-13 7:13 ` David Gow
2023-12-13 1:01 ` [PATCH v4 3/6] kunit: add example suite to test init suites Rae Moar
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rae Moar @ 2023-12-13 1:01 UTC (permalink / raw)
To: shuah, davidgow, dlatypov, brendan.higgins, sadiyakazi
Cc: keescook, arnd, linux-kselftest, linux-arch, kunit-dev,
linux-kernel, Rae Moar
Add KUNIT_INIT_TABLE to the INIT_DATA linker section.
Alter the KUnit macros to create init tests:
kunit_test_init_section_suites
Update lib/kunit/executor.c to run both the suites in KUNIT_TABLE and
KUNIT_INIT_TABLE.
Signed-off-by: Rae Moar <rmoar@google.com>
---
Changes since v3:
- Add to comments in test.h for kunit_test_init_section_suites macro to
note init tests cannot be run after boot and the structs cannot be
marked with __initdata
include/asm-generic/vmlinux.lds.h | 9 ++++-
include/kunit/test.h | 30 +++++++++------
include/linux/module.h | 2 +
kernel/module/main.c | 3 ++
lib/kunit/executor.c | 64 ++++++++++++++++++++++++++++---
lib/kunit/test.c | 26 +++++++++----
6 files changed, 109 insertions(+), 25 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 1107905d37fc..5dd3a61d673d 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -700,7 +700,8 @@
THERMAL_TABLE(governor) \
EARLYCON_TABLE() \
LSM_TABLE() \
- EARLY_LSM_TABLE()
+ EARLY_LSM_TABLE() \
+ KUNIT_INIT_TABLE()
#define INIT_TEXT \
*(.init.text .init.text.*) \
@@ -926,6 +927,12 @@
. = ALIGN(8); \
BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)
+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
+#define KUNIT_INIT_TABLE() \
+ . = ALIGN(8); \
+ BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \
+ __kunit_init_suites, _start, _end)
+
#ifdef CONFIG_BLK_DEV_INITRD
#define INIT_RAM_FS \
. = ALIGN(4); \
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 20ed9f9275c9..fe79cd736e94 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -337,6 +337,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);
void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin);
void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr);
+struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
+ struct kunit_suite_set suite_set);
+
#if IS_BUILTIN(CONFIG_KUNIT)
int kunit_run_all_tests(void);
#else
@@ -371,6 +374,11 @@ static inline int kunit_run_all_tests(void)
#define kunit_test_suite(suite) kunit_test_suites(&suite)
+#define __kunit_init_test_suites(unique_array, ...) \
+ static struct kunit_suite *unique_array[] \
+ __aligned(sizeof(struct kunit_suite *)) \
+ __used __section(".kunit_init_test_suites") = { __VA_ARGS__ }
+
/**
* kunit_test_init_section_suites() - used to register one or more &struct
* kunit_suite containing init functions or
@@ -378,21 +386,21 @@ static inline int kunit_run_all_tests(void)
*
* @__suites: a statically allocated list of &struct kunit_suite.
*
- * This functions identically as kunit_test_suites() except that it suppresses
- * modpost warnings for referencing functions marked __init or data marked
- * __initdata; this is OK because currently KUnit only runs tests upon boot
- * during the init phase or upon loading a module during the init phase.
+ * This functions similar to kunit_test_suites() except that it compiles the
+ * list of suites during init phase.
+ *
+ * This macro also suffixes the array and suite declarations it makes with
+ * _probe; so that modpost suppresses warnings about referencing init data
+ * for symbols named in this manner.
*
- * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these
- * tests must be excluded.
+ * Note: these init tests are not able to be run after boot so there is no
+ * "run" debugfs file generated for these tests.
*
- * The only thing this macro does that's different from kunit_test_suites is
- * that it suffixes the array and suite declarations it makes with _probe;
- * modpost suppresses warnings about referencing init data for symbols named in
- * this manner.
+ * Also, do not mark the suite or test case structs with __initdata because
+ * they will be used after the init phase with debugfs.
*/
#define kunit_test_init_section_suites(__suites...) \
- __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
+ __kunit_init_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
##__suites)
#define kunit_test_init_section_suite(suite) \
diff --git a/include/linux/module.h b/include/linux/module.h
index a98e188cf37b..9cd0009bd050 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -540,6 +540,8 @@ struct module {
struct static_call_site *static_call_sites;
#endif
#if IS_ENABLED(CONFIG_KUNIT)
+ int num_kunit_init_suites;
+ struct kunit_suite **kunit_init_suites;
int num_kunit_suites;
struct kunit_suite **kunit_suites;
#endif
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 98fedfdb8db5..36681911c05a 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2199,6 +2199,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->kunit_suites = section_objs(info, ".kunit_test_suites",
sizeof(*mod->kunit_suites),
&mod->num_kunit_suites);
+ mod->kunit_init_suites = section_objs(info, ".kunit_init_test_suites",
+ sizeof(*mod->kunit_init_suites),
+ &mod->num_kunit_init_suites);
#endif
mod->extable = section_objs(info, "__ex_table",
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 1236b3cd2fbb..847329c51e91 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -12,6 +12,8 @@
*/
extern struct kunit_suite * const __kunit_suites_start[];
extern struct kunit_suite * const __kunit_suites_end[];
+extern struct kunit_suite * const __kunit_init_suites_start[];
+extern struct kunit_suite * const __kunit_init_suites_end[];
static char *action_param;
@@ -292,6 +294,33 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr)
}
}
+struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
+ struct kunit_suite_set suite_set)
+{
+ struct kunit_suite_set total_suite_set = {NULL, NULL};
+ struct kunit_suite **total_suite_start = NULL;
+ size_t init_num_suites, num_suites, suite_size;
+
+ init_num_suites = init_suite_set.end - init_suite_set.start;
+ num_suites = suite_set.end - suite_set.start;
+ suite_size = sizeof(suite_set.start);
+
+ /* Allocate memory for array of all kunit suites */
+ total_suite_start = kmalloc_array(init_num_suites + num_suites, suite_size, GFP_KERNEL);
+ if (!total_suite_start)
+ return total_suite_set;
+
+ /* Append init suites and then all other kunit suites */
+ memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
+ memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
+
+ /* Set kunit suite set start and end */
+ total_suite_set.start = total_suite_start;
+ total_suite_set.end = total_suite_start + (init_num_suites + num_suites);
+
+ return total_suite_set;
+}
+
#if IS_BUILTIN(CONFIG_KUNIT)
static char *kunit_shutdown;
@@ -313,21 +342,41 @@ static void kunit_handle_shutdown(void)
int kunit_run_all_tests(void)
{
- struct kunit_suite_set suite_set = {
+ struct kunit_suite_set suite_set = {NULL, NULL};
+ struct kunit_suite_set filtered_suite_set = {NULL, NULL};
+ struct kunit_suite_set init_suite_set = {
+ __kunit_init_suites_start, __kunit_init_suites_end,
+ };
+ struct kunit_suite_set normal_suite_set = {
__kunit_suites_start, __kunit_suites_end,
};
+ size_t init_num_suites = init_suite_set.end - init_suite_set.start;
int err = 0;
+
+ if (init_num_suites > 0) {
+ suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
+ if (!suite_set.start)
+ goto out;
+ } else
+ suite_set = normal_suite_set;
+
if (!kunit_enabled()) {
pr_info("kunit: disabled\n");
- goto out;
+ goto free_out;
}
if (filter_glob_param || filter_param) {
- suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
+ filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
filter_param, filter_action_param, &err);
+
+ /* Free original suite set before using filtered suite set */
+ if (init_num_suites > 0)
+ kfree(suite_set.start);
+ suite_set = filtered_suite_set;
+
if (err) {
pr_err("kunit executor: error filtering suites: %d\n", err);
- goto out;
+ goto free_out;
}
}
@@ -340,9 +389,12 @@ int kunit_run_all_tests(void)
else
pr_err("kunit executor: unknown action '%s'\n", action_param);
- if (filter_glob_param || filter_param) { /* a copy was made of each suite */
+free_out:
+ if (filter_glob_param || filter_param)
kunit_free_suite_set(suite_set);
- }
+ else if (init_num_suites > 0)
+ /* Don't use kunit_free_suite_set because suites aren't individually allocated */
+ kfree(suite_set.start);
out:
kunit_handle_shutdown();
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 0308865194bb..6c082911a85f 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -704,28 +704,40 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
#ifdef CONFIG_MODULES
static void kunit_module_init(struct module *mod)
{
- struct kunit_suite_set suite_set = {
+ struct kunit_suite_set suite_set, filtered_set;
+ struct kunit_suite_set normal_suite_set = {
mod->kunit_suites, mod->kunit_suites + mod->num_kunit_suites,
};
+ struct kunit_suite_set init_suite_set = {
+ mod->kunit_init_suites, mod->kunit_init_suites + mod->num_kunit_init_suites,
+ };
const char *action = kunit_action();
int err = 0;
- suite_set = kunit_filter_suites(&suite_set,
+ if (mod->num_kunit_init_suites > 0)
+ suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
+ else
+ suite_set = normal_suite_set;
+
+ filtered_set = kunit_filter_suites(&suite_set,
kunit_filter_glob() ?: "*.*",
kunit_filter(), kunit_filter_action(),
&err);
if (err)
pr_err("kunit module: error filtering suites: %d\n", err);
- mod->kunit_suites = (struct kunit_suite **)suite_set.start;
- mod->num_kunit_suites = suite_set.end - suite_set.start;
+ mod->kunit_suites = (struct kunit_suite **)filtered_set.start;
+ mod->num_kunit_suites = filtered_set.end - filtered_set.start;
+
+ if (mod->num_kunit_init_suites > 0)
+ kfree(suite_set.start);
if (!action)
- kunit_exec_run_tests(&suite_set, false);
+ kunit_exec_run_tests(&filtered_set, false);
else if (!strcmp(action, "list"))
- kunit_exec_list_tests(&suite_set, false);
+ kunit_exec_list_tests(&filtered_set, false);
else if (!strcmp(action, "list_attr"))
- kunit_exec_list_tests(&suite_set, true);
+ kunit_exec_list_tests(&filtered_set, true);
else
pr_err("kunit: unknown action '%s'\n", action);
}
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section
2023-12-13 1:01 ` [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section Rae Moar
@ 2023-12-13 7:13 ` David Gow
0 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 13533 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Add KUNIT_INIT_TABLE to the INIT_DATA linker section.
>
> Alter the KUnit macros to create init tests:
> kunit_test_init_section_suites
>
> Update lib/kunit/executor.c to run both the suites in KUNIT_TABLE and
> KUNIT_INIT_TABLE.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
> Changes since v3:
> - Add to comments in test.h for kunit_test_init_section_suites macro to
> note init tests cannot be run after boot and the structs cannot be
> marked with __initdata
>
Thanks -- this is looking good.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> include/asm-generic/vmlinux.lds.h | 9 ++++-
> include/kunit/test.h | 30 +++++++++------
> include/linux/module.h | 2 +
> kernel/module/main.c | 3 ++
> lib/kunit/executor.c | 64 ++++++++++++++++++++++++++++---
> lib/kunit/test.c | 26 +++++++++----
> 6 files changed, 109 insertions(+), 25 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 1107905d37fc..5dd3a61d673d 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -700,7 +700,8 @@
> THERMAL_TABLE(governor) \
> EARLYCON_TABLE() \
> LSM_TABLE() \
> - EARLY_LSM_TABLE()
> + EARLY_LSM_TABLE() \
> + KUNIT_INIT_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
> @@ -926,6 +927,12 @@
> . = ALIGN(8); \
> BOUNDED_SECTION_POST_LABEL(.kunit_test_suites, __kunit_suites, _start, _end)
>
> +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */
> +#define KUNIT_INIT_TABLE() \
> + . = ALIGN(8); \
> + BOUNDED_SECTION_POST_LABEL(.kunit_init_test_suites, \
> + __kunit_init_suites, _start, _end)
> +
> #ifdef CONFIG_BLK_DEV_INITRD
> #define INIT_RAM_FS \
> . = ALIGN(4); \
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 20ed9f9275c9..fe79cd736e94 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -337,6 +337,9 @@ void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);
> void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin);
> void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr);
>
> +struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
> + struct kunit_suite_set suite_set);
> +
> #if IS_BUILTIN(CONFIG_KUNIT)
> int kunit_run_all_tests(void);
> #else
> @@ -371,6 +374,11 @@ static inline int kunit_run_all_tests(void)
>
> #define kunit_test_suite(suite) kunit_test_suites(&suite)
>
> +#define __kunit_init_test_suites(unique_array, ...) \
> + static struct kunit_suite *unique_array[] \
> + __aligned(sizeof(struct kunit_suite *)) \
> + __used __section(".kunit_init_test_suites") = { __VA_ARGS__ }
> +
> /**
> * kunit_test_init_section_suites() - used to register one or more &struct
> * kunit_suite containing init functions or
> @@ -378,21 +386,21 @@ static inline int kunit_run_all_tests(void)
> *
> * @__suites: a statically allocated list of &struct kunit_suite.
> *
> - * This functions identically as kunit_test_suites() except that it suppresses
> - * modpost warnings for referencing functions marked __init or data marked
> - * __initdata; this is OK because currently KUnit only runs tests upon boot
> - * during the init phase or upon loading a module during the init phase.
> + * This functions similar to kunit_test_suites() except that it compiles the
> + * list of suites during init phase.
> + *
> + * This macro also suffixes the array and suite declarations it makes with
> + * _probe; so that modpost suppresses warnings about referencing init data
> + * for symbols named in this manner.
> *
> - * NOTE TO KUNIT DEVS: If we ever allow KUnit tests to be run after boot, these
> - * tests must be excluded.
> + * Note: these init tests are not able to be run after boot so there is no
> + * "run" debugfs file generated for these tests.
> *
> - * The only thing this macro does that's different from kunit_test_suites is
> - * that it suffixes the array and suite declarations it makes with _probe;
> - * modpost suppresses warnings about referencing init data for symbols named in
> - * this manner.
> + * Also, do not mark the suite or test case structs with __initdata because
> + * they will be used after the init phase with debugfs.
> */
> #define kunit_test_init_section_suites(__suites...) \
> - __kunit_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
> + __kunit_init_test_suites(CONCATENATE(__UNIQUE_ID(array), _probe), \
> ##__suites)
>
> #define kunit_test_init_section_suite(suite) \
> diff --git a/include/linux/module.h b/include/linux/module.h
> index a98e188cf37b..9cd0009bd050 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -540,6 +540,8 @@ struct module {
> struct static_call_site *static_call_sites;
> #endif
> #if IS_ENABLED(CONFIG_KUNIT)
> + int num_kunit_init_suites;
> + struct kunit_suite **kunit_init_suites;
> int num_kunit_suites;
> struct kunit_suite **kunit_suites;
> #endif
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 98fedfdb8db5..36681911c05a 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2199,6 +2199,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
> mod->kunit_suites = section_objs(info, ".kunit_test_suites",
> sizeof(*mod->kunit_suites),
> &mod->num_kunit_suites);
> + mod->kunit_init_suites = section_objs(info, ".kunit_init_test_suites",
> + sizeof(*mod->kunit_init_suites),
> + &mod->num_kunit_init_suites);
> #endif
>
> mod->extable = section_objs(info, "__ex_table",
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 1236b3cd2fbb..847329c51e91 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -12,6 +12,8 @@
> */
> extern struct kunit_suite * const __kunit_suites_start[];
> extern struct kunit_suite * const __kunit_suites_end[];
> +extern struct kunit_suite * const __kunit_init_suites_start[];
> +extern struct kunit_suite * const __kunit_init_suites_end[];
>
> static char *action_param;
>
> @@ -292,6 +294,33 @@ void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr)
> }
> }
>
> +struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_set,
> + struct kunit_suite_set suite_set)
> +{
> + struct kunit_suite_set total_suite_set = {NULL, NULL};
> + struct kunit_suite **total_suite_start = NULL;
> + size_t init_num_suites, num_suites, suite_size;
> +
> + init_num_suites = init_suite_set.end - init_suite_set.start;
> + num_suites = suite_set.end - suite_set.start;
> + suite_size = sizeof(suite_set.start);
> +
> + /* Allocate memory for array of all kunit suites */
> + total_suite_start = kmalloc_array(init_num_suites + num_suites, suite_size, GFP_KERNEL);
> + if (!total_suite_start)
> + return total_suite_set;
> +
> + /* Append init suites and then all other kunit suites */
> + memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
> + memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
> +
> + /* Set kunit suite set start and end */
> + total_suite_set.start = total_suite_start;
> + total_suite_set.end = total_suite_start + (init_num_suites + num_suites);
> +
> + return total_suite_set;
> +}
> +
> #if IS_BUILTIN(CONFIG_KUNIT)
>
> static char *kunit_shutdown;
> @@ -313,21 +342,41 @@ static void kunit_handle_shutdown(void)
>
> int kunit_run_all_tests(void)
> {
> - struct kunit_suite_set suite_set = {
> + struct kunit_suite_set suite_set = {NULL, NULL};
> + struct kunit_suite_set filtered_suite_set = {NULL, NULL};
> + struct kunit_suite_set init_suite_set = {
> + __kunit_init_suites_start, __kunit_init_suites_end,
> + };
> + struct kunit_suite_set normal_suite_set = {
> __kunit_suites_start, __kunit_suites_end,
> };
> + size_t init_num_suites = init_suite_set.end - init_suite_set.start;
> int err = 0;
> +
> + if (init_num_suites > 0) {
> + suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
> + if (!suite_set.start)
> + goto out;
> + } else
> + suite_set = normal_suite_set;
> +
> if (!kunit_enabled()) {
> pr_info("kunit: disabled\n");
> - goto out;
> + goto free_out;
> }
>
> if (filter_glob_param || filter_param) {
> - suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> + filtered_suite_set = kunit_filter_suites(&suite_set, filter_glob_param,
> filter_param, filter_action_param, &err);
> +
> + /* Free original suite set before using filtered suite set */
> + if (init_num_suites > 0)
> + kfree(suite_set.start);
> + suite_set = filtered_suite_set;
> +
> if (err) {
> pr_err("kunit executor: error filtering suites: %d\n", err);
> - goto out;
> + goto free_out;
> }
> }
>
> @@ -340,9 +389,12 @@ int kunit_run_all_tests(void)
> else
> pr_err("kunit executor: unknown action '%s'\n", action_param);
>
> - if (filter_glob_param || filter_param) { /* a copy was made of each suite */
> +free_out:
> + if (filter_glob_param || filter_param)
> kunit_free_suite_set(suite_set);
> - }
> + else if (init_num_suites > 0)
> + /* Don't use kunit_free_suite_set because suites aren't individually allocated */
> + kfree(suite_set.start);
>
> out:
> kunit_handle_shutdown();
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 0308865194bb..6c082911a85f 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -704,28 +704,40 @@ EXPORT_SYMBOL_GPL(__kunit_test_suites_exit);
> #ifdef CONFIG_MODULES
> static void kunit_module_init(struct module *mod)
> {
> - struct kunit_suite_set suite_set = {
> + struct kunit_suite_set suite_set, filtered_set;
> + struct kunit_suite_set normal_suite_set = {
> mod->kunit_suites, mod->kunit_suites + mod->num_kunit_suites,
> };
> + struct kunit_suite_set init_suite_set = {
> + mod->kunit_init_suites, mod->kunit_init_suites + mod->num_kunit_init_suites,
> + };
> const char *action = kunit_action();
> int err = 0;
>
> - suite_set = kunit_filter_suites(&suite_set,
> + if (mod->num_kunit_init_suites > 0)
> + suite_set = kunit_merge_suite_sets(init_suite_set, normal_suite_set);
> + else
> + suite_set = normal_suite_set;
> +
> + filtered_set = kunit_filter_suites(&suite_set,
> kunit_filter_glob() ?: "*.*",
> kunit_filter(), kunit_filter_action(),
> &err);
> if (err)
> pr_err("kunit module: error filtering suites: %d\n", err);
>
> - mod->kunit_suites = (struct kunit_suite **)suite_set.start;
> - mod->num_kunit_suites = suite_set.end - suite_set.start;
> + mod->kunit_suites = (struct kunit_suite **)filtered_set.start;
> + mod->num_kunit_suites = filtered_set.end - filtered_set.start;
> +
> + if (mod->num_kunit_init_suites > 0)
> + kfree(suite_set.start);
>
> if (!action)
> - kunit_exec_run_tests(&suite_set, false);
> + kunit_exec_run_tests(&filtered_set, false);
> else if (!strcmp(action, "list"))
> - kunit_exec_list_tests(&suite_set, false);
> + kunit_exec_list_tests(&filtered_set, false);
> else if (!strcmp(action, "list_attr"))
> - kunit_exec_list_tests(&suite_set, true);
> + kunit_exec_list_tests(&filtered_set, true);
> else
> pr_err("kunit: unknown action '%s'\n", action);
> }
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/6] kunit: add example suite to test init suites
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
2023-12-13 1:01 ` [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section Rae Moar
@ 2023-12-13 1:01 ` Rae Moar
2023-12-13 7:13 ` David Gow
2023-12-13 1:01 ` [PATCH v4 4/6] kunit: add is_init test attribute Rae Moar
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rae Moar @ 2023-12-13 1:01 UTC (permalink / raw)
To: shuah, davidgow, dlatypov, brendan.higgins, sadiyakazi
Cc: keescook, arnd, linux-kselftest, linux-arch, kunit-dev,
linux-kernel, Rae Moar
Add example_init_test_suite to allow for testing the feature of running
test suites marked as init to indicate they use init data and/or
functions.
This suite should always pass and uses a simple init function.
This suite can also be used to test the is_init attribute introduced in
the next patch.
Signed-off-by: Rae Moar <rmoar@google.com>
---
Changes since v3:
- I ended up not changing anything as adding __init to the test gave
a build warning. It did still work so I could add it back if wanted.
lib/kunit/kunit-example-test.c | 37 ++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index 6bb5c2ef6696..18495778de7c 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -287,4 +287,41 @@ static struct kunit_suite example_test_suite = {
*/
kunit_test_suites(&example_test_suite);
+static int __init init_add(int x, int y)
+{
+ return (x + y);
+}
+
+/*
+ * This test should always pass. Can be used to test init suites.
+ */
+static void example_init_test(struct kunit *test)
+{
+ KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
+}
+
+/*
+ * The kunit_case struct cannot be marked as __initdata as this will be
+ * used in debugfs to retrieve results after test has run
+ */
+static struct kunit_case example_init_test_cases[] = {
+ KUNIT_CASE(example_init_test),
+ {}
+};
+
+/*
+ * The kunit_suite struct cannot be marked as __initdata as this will be
+ * used in debugfs to retrieve results after test has run
+ */
+static struct kunit_suite example_init_test_suite = {
+ .name = "example_init",
+ .test_cases = example_init_test_cases,
+};
+
+/*
+ * This registers the test suite and marks the suite as using init data
+ * and/or functions.
+ */
+kunit_test_init_section_suites(&example_init_test_suite);
+
MODULE_LICENSE("GPL v2");
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 3/6] kunit: add example suite to test init suites
2023-12-13 1:01 ` [PATCH v4 3/6] kunit: add example suite to test init suites Rae Moar
@ 2023-12-13 7:13 ` David Gow
0 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2558 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Add example_init_test_suite to allow for testing the feature of running
> test suites marked as init to indicate they use init data and/or
> functions.
>
> This suite should always pass and uses a simple init function.
>
> This suite can also be used to test the is_init attribute introduced in
> the next patch.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
> Changes since v3:
> - I ended up not changing anything as adding __init to the test gave
> a build warning. It did still work so I could add it back if wanted.
I had another look at this, and I think the most correct solution here
is to make the test __init, and the array of tests __refdata.
(Ideally this would be something we could wrap in a macro, but I think
it's fine to just have it written here for now, so it's explicit in
the example._
How does that sound?
-- David
>
> lib/kunit/kunit-example-test.c | 37 ++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index 6bb5c2ef6696..18495778de7c 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -287,4 +287,41 @@ static struct kunit_suite example_test_suite = {
> */
> kunit_test_suites(&example_test_suite);
>
> +static int __init init_add(int x, int y)
> +{
> + return (x + y);
> +}
> +
> +/*
> + * This test should always pass. Can be used to test init suites.
> + */
> +static void example_init_test(struct kunit *test)
Add __init here.
> +{
> + KUNIT_EXPECT_EQ(test, init_add(1, 1), 2);
> +}
> +
> +/*
> + * The kunit_case struct cannot be marked as __initdata as this will be
> + * used in debugfs to retrieve results after test has run
> + */
> +static struct kunit_case example_init_test_cases[] = {
Make this 'static struct kunit_case __refdata example_init_test_cases[] = {'...
> + KUNIT_CASE(example_init_test),
> + {}
> +};
> +
> +/*
> + * The kunit_suite struct cannot be marked as __initdata as this will be
> + * used in debugfs to retrieve results after test has run
> + */
> +static struct kunit_suite example_init_test_suite = {
> + .name = "example_init",
> + .test_cases = example_init_test_cases,
> +};
> +
> +/*
> + * This registers the test suite and marks the suite as using init data
> + * and/or functions.
> + */
> +kunit_test_init_section_suites(&example_init_test_suite);
> +
> MODULE_LICENSE("GPL v2");
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 4/6] kunit: add is_init test attribute
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
2023-12-13 1:01 ` [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section Rae Moar
2023-12-13 1:01 ` [PATCH v4 3/6] kunit: add example suite to test init suites Rae Moar
@ 2023-12-13 1:01 ` Rae Moar
2023-12-13 7:13 ` David Gow
2023-12-13 1:02 ` [PATCH v4 5/6] kunit: add ability to run tests after boot using debugfs Rae Moar
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Rae Moar @ 2023-12-13 1:01 UTC (permalink / raw)
To: shuah, davidgow, dlatypov, brendan.higgins, sadiyakazi
Cc: keescook, arnd, linux-kselftest, linux-arch, kunit-dev,
linux-kernel, Rae Moar
Add is_init test attribute of type bool. Add to_string, get, and filter
methods to lib/kunit/attributes.c.
Mark each of the tests in the init section with the is_init=true attribute.
Add is_init to the attributes documentation.
Signed-off-by: Rae Moar <rmoar@google.com>
---
Changes since v3:
- Move the attribute from kunit_attributes to a suite field.
.../dev-tools/kunit/running_tips.rst | 7 +++
include/kunit/test.h | 1 +
lib/kunit/attributes.c | 60 +++++++++++++++++++
lib/kunit/executor.c | 6 +-
4 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kunit/running_tips.rst b/Documentation/dev-tools/kunit/running_tips.rst
index 766f9cdea0fa..024e9ad1d1e9 100644
--- a/Documentation/dev-tools/kunit/running_tips.rst
+++ b/Documentation/dev-tools/kunit/running_tips.rst
@@ -428,3 +428,10 @@ This attribute indicates the name of the module associated with the test.
This attribute is automatically saved as a string and is printed for each suite.
Tests can also be filtered using this attribute.
+
+``is_init``
+
+This attribute indicates whether the test uses init data or functions.
+
+This attribute is automatically saved as a boolean and tests can also be
+filtered using this attribute.
diff --git a/include/kunit/test.h b/include/kunit/test.h
index fe79cd736e94..b163b9984b33 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -253,6 +253,7 @@ struct kunit_suite {
struct dentry *debugfs;
struct string_stream *log;
int suite_init_err;
+ bool is_init;
};
/* Stores an array of suites, end points one past the end */
diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c
index 1b512f7e1838..2cf04cc09372 100644
--- a/lib/kunit/attributes.c
+++ b/lib/kunit/attributes.c
@@ -58,6 +58,16 @@ static const char *attr_enum_to_string(void *attr, const char * const str_list[]
return str_list[val];
}
+static const char *attr_bool_to_string(void *attr, bool *to_free)
+{
+ bool val = (bool)attr;
+
+ *to_free = false;
+ if (val)
+ return "true";
+ return "false";
+}
+
static const char *attr_speed_to_string(void *attr, bool *to_free)
{
return attr_enum_to_string(attr, speed_str_list, to_free);
@@ -166,6 +176,37 @@ static int attr_string_filter(void *attr, const char *input, int *err)
return false;
}
+static int attr_bool_filter(void *attr, const char *input, int *err)
+{
+ int i, input_int = -1;
+ long val = (long)attr;
+ const char *input_str = NULL;
+
+ for (i = 0; input[i]; i++) {
+ if (!strchr(op_list, input[i])) {
+ input_str = input + i;
+ break;
+ }
+ }
+
+ if (!input_str) {
+ *err = -EINVAL;
+ pr_err("kunit executor: filter value not found: %s\n", input);
+ return false;
+ }
+
+ if (!strcmp(input_str, "true"))
+ input_int = (int)true;
+ else if (!strcmp(input_str, "false"))
+ input_int = (int)false;
+ else {
+ *err = -EINVAL;
+ pr_err("kunit executor: invalid filter input: %s\n", input);
+ return false;
+ }
+
+ return int_filter(val, input, input_int, err);
+}
/* Get Attribute Methods */
@@ -194,6 +235,17 @@ static void *attr_module_get(void *test_or_suite, bool is_test)
return (void *) "";
}
+static void *attr_is_init_get(void *test_or_suite, bool is_test)
+{
+ struct kunit_suite *suite = is_test ? NULL : test_or_suite;
+ struct kunit_case *test = is_test ? test_or_suite : NULL;
+
+ if (test)
+ return ((void *) NULL);
+ else
+ return ((void *) suite->is_init);
+}
+
/* List of all Test Attributes */
static struct kunit_attr kunit_attr_list[] = {
@@ -212,6 +264,14 @@ static struct kunit_attr kunit_attr_list[] = {
.filter = attr_string_filter,
.attr_default = (void *)"",
.print = PRINT_SUITE,
+ },
+ {
+ .name = "is_init",
+ .get_attr = attr_is_init_get,
+ .to_string = attr_bool_to_string,
+ .filter = attr_bool_filter,
+ .attr_default = (void *)false,
+ .print = PRINT_SUITE,
}
};
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 847329c51e91..717b9599036b 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -300,6 +300,7 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
struct kunit_suite_set total_suite_set = {NULL, NULL};
struct kunit_suite **total_suite_start = NULL;
size_t init_num_suites, num_suites, suite_size;
+ int i = 0;
init_num_suites = init_suite_set.end - init_suite_set.start;
num_suites = suite_set.end - suite_set.start;
@@ -310,8 +311,11 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
if (!total_suite_start)
return total_suite_set;
- /* Append init suites and then all other kunit suites */
+ /* Append and mark init suites and then append all other kunit suites */
memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
+ for (i = 0; i < init_num_suites; i++)
+ total_suite_start[i]->is_init = true;
+
memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
/* Set kunit suite set start and end */
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 4/6] kunit: add is_init test attribute
2023-12-13 1:01 ` [PATCH v4 4/6] kunit: add is_init test attribute Rae Moar
@ 2023-12-13 7:13 ` David Gow
0 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6376 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Add is_init test attribute of type bool. Add to_string, get, and filter
> methods to lib/kunit/attributes.c.
>
> Mark each of the tests in the init section with the is_init=true attribute.
>
> Add is_init to the attributes documentation.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
This looks much better, thanks.
Maybe in the future, we'll want to collate several suite-level
attributes (in a suite_attributes struct or a bitfield), but I think
that should wait until after we have more examples.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> Changes since v3:
> - Move the attribute from kunit_attributes to a suite field.
>
> .../dev-tools/kunit/running_tips.rst | 7 +++
> include/kunit/test.h | 1 +
> lib/kunit/attributes.c | 60 +++++++++++++++++++
> lib/kunit/executor.c | 6 +-
> 4 files changed, 73 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/running_tips.rst b/Documentation/dev-tools/kunit/running_tips.rst
> index 766f9cdea0fa..024e9ad1d1e9 100644
> --- a/Documentation/dev-tools/kunit/running_tips.rst
> +++ b/Documentation/dev-tools/kunit/running_tips.rst
> @@ -428,3 +428,10 @@ This attribute indicates the name of the module associated with the test.
>
> This attribute is automatically saved as a string and is printed for each suite.
> Tests can also be filtered using this attribute.
> +
> +``is_init``
> +
> +This attribute indicates whether the test uses init data or functions.
> +
> +This attribute is automatically saved as a boolean and tests can also be
> +filtered using this attribute.
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index fe79cd736e94..b163b9984b33 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -253,6 +253,7 @@ struct kunit_suite {
> struct dentry *debugfs;
> struct string_stream *log;
> int suite_init_err;
> + bool is_init;
> };
>
> /* Stores an array of suites, end points one past the end */
> diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c
> index 1b512f7e1838..2cf04cc09372 100644
> --- a/lib/kunit/attributes.c
> +++ b/lib/kunit/attributes.c
> @@ -58,6 +58,16 @@ static const char *attr_enum_to_string(void *attr, const char * const str_list[]
> return str_list[val];
> }
>
> +static const char *attr_bool_to_string(void *attr, bool *to_free)
> +{
> + bool val = (bool)attr;
> +
> + *to_free = false;
> + if (val)
> + return "true";
> + return "false";
> +}
> +
> static const char *attr_speed_to_string(void *attr, bool *to_free)
> {
> return attr_enum_to_string(attr, speed_str_list, to_free);
> @@ -166,6 +176,37 @@ static int attr_string_filter(void *attr, const char *input, int *err)
> return false;
> }
>
> +static int attr_bool_filter(void *attr, const char *input, int *err)
> +{
> + int i, input_int = -1;
> + long val = (long)attr;
> + const char *input_str = NULL;
> +
> + for (i = 0; input[i]; i++) {
> + if (!strchr(op_list, input[i])) {
> + input_str = input + i;
> + break;
> + }
> + }
> +
> + if (!input_str) {
> + *err = -EINVAL;
> + pr_err("kunit executor: filter value not found: %s\n", input);
> + return false;
> + }
> +
> + if (!strcmp(input_str, "true"))
> + input_int = (int)true;
> + else if (!strcmp(input_str, "false"))
> + input_int = (int)false;
> + else {
> + *err = -EINVAL;
> + pr_err("kunit executor: invalid filter input: %s\n", input);
> + return false;
> + }
> +
> + return int_filter(val, input, input_int, err);
> +}
>
> /* Get Attribute Methods */
>
> @@ -194,6 +235,17 @@ static void *attr_module_get(void *test_or_suite, bool is_test)
> return (void *) "";
> }
>
> +static void *attr_is_init_get(void *test_or_suite, bool is_test)
> +{
> + struct kunit_suite *suite = is_test ? NULL : test_or_suite;
> + struct kunit_case *test = is_test ? test_or_suite : NULL;
> +
> + if (test)
> + return ((void *) NULL);
> + else
> + return ((void *) suite->is_init);
> +}
> +
> /* List of all Test Attributes */
>
> static struct kunit_attr kunit_attr_list[] = {
> @@ -212,6 +264,14 @@ static struct kunit_attr kunit_attr_list[] = {
> .filter = attr_string_filter,
> .attr_default = (void *)"",
> .print = PRINT_SUITE,
> + },
> + {
> + .name = "is_init",
> + .get_attr = attr_is_init_get,
> + .to_string = attr_bool_to_string,
> + .filter = attr_bool_filter,
> + .attr_default = (void *)false,
> + .print = PRINT_SUITE,
> }
> };
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 847329c51e91..717b9599036b 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -300,6 +300,7 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
> struct kunit_suite_set total_suite_set = {NULL, NULL};
> struct kunit_suite **total_suite_start = NULL;
> size_t init_num_suites, num_suites, suite_size;
> + int i = 0;
>
> init_num_suites = init_suite_set.end - init_suite_set.start;
> num_suites = suite_set.end - suite_set.start;
> @@ -310,8 +311,11 @@ struct kunit_suite_set kunit_merge_suite_sets(struct kunit_suite_set init_suite_
> if (!total_suite_start)
> return total_suite_set;
>
> - /* Append init suites and then all other kunit suites */
> + /* Append and mark init suites and then append all other kunit suites */
> memcpy(total_suite_start, init_suite_set.start, init_num_suites * suite_size);
> + for (i = 0; i < init_num_suites; i++)
> + total_suite_start[i]->is_init = true;
> +
> memcpy(total_suite_start + init_num_suites, suite_set.start, num_suites * suite_size);
>
> /* Set kunit suite set start and end */
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 5/6] kunit: add ability to run tests after boot using debugfs
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
` (2 preceding siblings ...)
2023-12-13 1:01 ` [PATCH v4 4/6] kunit: add is_init test attribute Rae Moar
@ 2023-12-13 1:02 ` Rae Moar
2023-12-13 7:13 ` David Gow
2023-12-13 1:02 ` [PATCH v4 6/6] Documentation: Add debugfs docs with run after boot Rae Moar
2023-12-13 7:13 ` [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA David Gow
5 siblings, 1 reply; 12+ messages in thread
From: Rae Moar @ 2023-12-13 1:02 UTC (permalink / raw)
To: shuah, davidgow, dlatypov, brendan.higgins, sadiyakazi
Cc: keescook, arnd, linux-kselftest, linux-arch, kunit-dev,
linux-kernel, Rae Moar
Add functionality to run built-in tests after boot by writing to a
debugfs file.
Add a new debugfs file labeled "run" for each test suite to use for
this purpose.
As an example, write to the file using the following:
echo "any string" > /sys/kernel/debugfs/kunit/<testsuite>/run
This will trigger the test suite to run and will print results to the
kernel log.
To guard against running tests concurrently with this feature, add a
mutex lock around running kunit. This supports the current practice of
not allowing tests to be run concurrently on the same kernel.
This new functionality could be used to design a parameter
injection feature in the future.
Signed-off-by: Rae Moar <rmoar@google.com>
---
Changes since v4:
- Rebased series causing a few small changes in debugfs.c in this patch
lib/kunit/debugfs.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
lib/kunit/test.c | 10 +++++++
2 files changed, 78 insertions(+)
diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 382706dfb47d..d548750a325a 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -8,12 +8,14 @@
#include <linux/module.h>
#include <kunit/test.h>
+#include <kunit/test-bug.h>
#include "string-stream.h"
#include "debugfs.h"
#define KUNIT_DEBUGFS_ROOT "kunit"
#define KUNIT_DEBUGFS_RESULTS "results"
+#define KUNIT_DEBUGFS_RUN "run"
/*
* Create a debugfs representation of test suites:
@@ -21,6 +23,8 @@
* Path Semantics
* /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
* testsuite
+ * /sys/kernel/debug/kunit/<testsuite>/run Write to this file to trigger
+ * testsuite to run
*
*/
@@ -101,6 +105,51 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
return single_open(file, debugfs_print_results, suite);
}
+/*
+ * Print a usage message to the debugfs "run" file
+ * (/sys/kernel/debug/kunit/<testsuite>/run) if opened.
+ */
+static int debugfs_print_run(struct seq_file *seq, void *v)
+{
+ struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+
+ seq_puts(seq, "Write to this file to trigger the test suite to run.\n");
+ seq_printf(seq, "usage: echo \"any string\" > /sys/kernel/debugfs/kunit/%s/run\n",
+ suite->name);
+ return 0;
+}
+
+/*
+ * The debugfs "run" file (/sys/kernel/debug/kunit/<testsuite>/run)
+ * contains no information. Write to the file to trigger the test suite
+ * to run.
+ */
+static int debugfs_run_open(struct inode *inode, struct file *file)
+{
+ struct kunit_suite *suite;
+
+ suite = (struct kunit_suite *)inode->i_private;
+
+ return single_open(file, debugfs_print_run, suite);
+}
+
+/*
+ * Trigger a test suite to run by writing to the suite's "run" debugfs
+ * file found at: /sys/kernel/debug/kunit/<testsuite>/run
+ *
+ * Note: what is written to this file will not be saved.
+ */
+static ssize_t debugfs_run(struct file *file,
+ const char __user *buf, size_t count, loff_t *ppos)
+{
+ struct inode *f_inode = file->f_inode;
+ struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
+
+ __kunit_test_suites_init(&suite, 1);
+
+ return count;
+}
+
static const struct file_operations debugfs_results_fops = {
.open = debugfs_results_open,
.read = seq_read,
@@ -108,11 +157,23 @@ static const struct file_operations debugfs_results_fops = {
.release = debugfs_release,
};
+static const struct file_operations debugfs_run_fops = {
+ .open = debugfs_run_open,
+ .read = seq_read,
+ .write = debugfs_run,
+ .llseek = seq_lseek,
+ .release = debugfs_release,
+};
+
void kunit_debugfs_create_suite(struct kunit_suite *suite)
{
struct kunit_case *test_case;
struct string_stream *stream;
+ /* If suite log already allocated, do not create new debugfs files. */
+ if (suite->log)
+ return;
+
/*
* Allocate logs before creating debugfs representation.
* The suite->log and test_case->log pointer are expected to be NULL
@@ -140,6 +201,13 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
suite->debugfs,
suite, &debugfs_results_fops);
+
+ /* Do not create file to re-run test if test runs on init */
+ if (!suite->is_init) {
+ debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0644,
+ suite->debugfs,
+ suite, &debugfs_run_fops);
+ }
return;
err:
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index 6c082911a85f..a52fcb9a4457 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/mutex.h>
#include <linux/panic.h>
#include <linux/sched/debug.h>
#include <linux/sched.h>
@@ -22,6 +23,8 @@
#include "string-stream.h"
#include "try-catch-impl.h"
+static DEFINE_MUTEX(kunit_run_lock);
+
/*
* Hook to fail the current test and print an error message to the log.
*/
@@ -654,6 +657,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
kunit_debugfs_create_suite(suite);
suite->status_comment[0] = '\0';
suite->suite_init_err = 0;
+ string_stream_clear(suite->log);
}
bool kunit_enabled(void)
@@ -670,6 +674,11 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
return 0;
}
+ /* Use mutex lock to guard against running tests concurrently. */
+ if (mutex_lock_interruptible(&kunit_run_lock)) {
+ pr_err("kunit: test interrupted\n");
+ return -EINTR;
+ }
static_branch_inc(&kunit_running);
for (i = 0; i < num_suites; i++) {
@@ -678,6 +687,7 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
}
static_branch_dec(&kunit_running);
+ mutex_unlock(&kunit_run_lock);
return 0;
}
EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 5/6] kunit: add ability to run tests after boot using debugfs
2023-12-13 1:02 ` [PATCH v4 5/6] kunit: add ability to run tests after boot using debugfs Rae Moar
@ 2023-12-13 7:13 ` David Gow
0 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6988 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Add functionality to run built-in tests after boot by writing to a
> debugfs file.
>
> Add a new debugfs file labeled "run" for each test suite to use for
> this purpose.
>
> As an example, write to the file using the following:
>
> echo "any string" > /sys/kernel/debugfs/kunit/<testsuite>/run
>
> This will trigger the test suite to run and will print results to the
> kernel log.
>
> To guard against running tests concurrently with this feature, add a
> mutex lock around running kunit. This supports the current practice of
> not allowing tests to be run concurrently on the same kernel.
>
> This new functionality could be used to design a parameter
> injection feature in the future.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
Excellent! Thanks for rebasing it!
This still works fine here, and looks good.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> Changes since v4:
> - Rebased series causing a few small changes in debugfs.c in this patch
>
> lib/kunit/debugfs.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
> lib/kunit/test.c | 10 +++++++
> 2 files changed, 78 insertions(+)
>
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> index 382706dfb47d..d548750a325a 100644
> --- a/lib/kunit/debugfs.c
> +++ b/lib/kunit/debugfs.c
> @@ -8,12 +8,14 @@
> #include <linux/module.h>
>
> #include <kunit/test.h>
> +#include <kunit/test-bug.h>
>
> #include "string-stream.h"
> #include "debugfs.h"
>
> #define KUNIT_DEBUGFS_ROOT "kunit"
> #define KUNIT_DEBUGFS_RESULTS "results"
> +#define KUNIT_DEBUGFS_RUN "run"
>
> /*
> * Create a debugfs representation of test suites:
> @@ -21,6 +23,8 @@
> * Path Semantics
> * /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
> * testsuite
> + * /sys/kernel/debug/kunit/<testsuite>/run Write to this file to trigger
> + * testsuite to run
> *
> */
>
> @@ -101,6 +105,51 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
> return single_open(file, debugfs_print_results, suite);
> }
>
> +/*
> + * Print a usage message to the debugfs "run" file
> + * (/sys/kernel/debug/kunit/<testsuite>/run) if opened.
> + */
> +static int debugfs_print_run(struct seq_file *seq, void *v)
> +{
> + struct kunit_suite *suite = (struct kunit_suite *)seq->private;
> +
> + seq_puts(seq, "Write to this file to trigger the test suite to run.\n");
> + seq_printf(seq, "usage: echo \"any string\" > /sys/kernel/debugfs/kunit/%s/run\n",
> + suite->name);
> + return 0;
> +}
> +
> +/*
> + * The debugfs "run" file (/sys/kernel/debug/kunit/<testsuite>/run)
> + * contains no information. Write to the file to trigger the test suite
> + * to run.
> + */
> +static int debugfs_run_open(struct inode *inode, struct file *file)
> +{
> + struct kunit_suite *suite;
> +
> + suite = (struct kunit_suite *)inode->i_private;
> +
> + return single_open(file, debugfs_print_run, suite);
> +}
> +
> +/*
> + * Trigger a test suite to run by writing to the suite's "run" debugfs
> + * file found at: /sys/kernel/debug/kunit/<testsuite>/run
> + *
> + * Note: what is written to this file will not be saved.
> + */
> +static ssize_t debugfs_run(struct file *file,
> + const char __user *buf, size_t count, loff_t *ppos)
> +{
> + struct inode *f_inode = file->f_inode;
> + struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
> +
> + __kunit_test_suites_init(&suite, 1);
> +
> + return count;
> +}
> +
> static const struct file_operations debugfs_results_fops = {
> .open = debugfs_results_open,
> .read = seq_read,
> @@ -108,11 +157,23 @@ static const struct file_operations debugfs_results_fops = {
> .release = debugfs_release,
> };
>
> +static const struct file_operations debugfs_run_fops = {
> + .open = debugfs_run_open,
> + .read = seq_read,
> + .write = debugfs_run,
> + .llseek = seq_lseek,
> + .release = debugfs_release,
> +};
> +
> void kunit_debugfs_create_suite(struct kunit_suite *suite)
> {
> struct kunit_case *test_case;
> struct string_stream *stream;
>
> + /* If suite log already allocated, do not create new debugfs files. */
> + if (suite->log)
> + return;
> +
> /*
> * Allocate logs before creating debugfs representation.
> * The suite->log and test_case->log pointer are expected to be NULL
> @@ -140,6 +201,13 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
> debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
> suite->debugfs,
> suite, &debugfs_results_fops);
> +
> + /* Do not create file to re-run test if test runs on init */
> + if (!suite->is_init) {
> + debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0644,
> + suite->debugfs,
> + suite, &debugfs_run_fops);
> + }
> return;
>
> err:
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 6c082911a85f..a52fcb9a4457 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -13,6 +13,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/mutex.h>
> #include <linux/panic.h>
> #include <linux/sched/debug.h>
> #include <linux/sched.h>
> @@ -22,6 +23,8 @@
> #include "string-stream.h"
> #include "try-catch-impl.h"
>
> +static DEFINE_MUTEX(kunit_run_lock);
> +
> /*
> * Hook to fail the current test and print an error message to the log.
> */
> @@ -654,6 +657,7 @@ static void kunit_init_suite(struct kunit_suite *suite)
> kunit_debugfs_create_suite(suite);
> suite->status_comment[0] = '\0';
> suite->suite_init_err = 0;
> + string_stream_clear(suite->log);
> }
>
> bool kunit_enabled(void)
> @@ -670,6 +674,11 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
> return 0;
> }
>
> + /* Use mutex lock to guard against running tests concurrently. */
> + if (mutex_lock_interruptible(&kunit_run_lock)) {
> + pr_err("kunit: test interrupted\n");
> + return -EINTR;
> + }
> static_branch_inc(&kunit_running);
>
> for (i = 0; i < num_suites; i++) {
> @@ -678,6 +687,7 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
> }
>
> static_branch_dec(&kunit_running);
> + mutex_unlock(&kunit_run_lock);
> return 0;
> }
> EXPORT_SYMBOL_GPL(__kunit_test_suites_init);
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 6/6] Documentation: Add debugfs docs with run after boot
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
` (3 preceding siblings ...)
2023-12-13 1:02 ` [PATCH v4 5/6] kunit: add ability to run tests after boot using debugfs Rae Moar
@ 2023-12-13 1:02 ` Rae Moar
2023-12-13 7:13 ` David Gow
2023-12-13 7:13 ` [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA David Gow
5 siblings, 1 reply; 12+ messages in thread
From: Rae Moar @ 2023-12-13 1:02 UTC (permalink / raw)
To: shuah, davidgow, dlatypov, brendan.higgins, sadiyakazi
Cc: keescook, arnd, linux-kselftest, linux-arch, kunit-dev,
linux-kernel, Rae Moar
Expand the documentation on the KUnit debugfs filesystem on the
run_manual.rst page.
Add section describing how to access results using debugfs.
Add section describing how to run tests after boot using debugfs.
Signed-off-by: Rae Moar <rmoar@google.com>
---
Changes since v3:
- Change the introduction of the debugfs section
- Add detail to not being able to run concurrently or run with init data
Documentation/dev-tools/kunit/run_manual.rst | 51 ++++++++++++++++++--
1 file changed, 47 insertions(+), 4 deletions(-)
diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
index e7b46421f247..699d92885075 100644
--- a/Documentation/dev-tools/kunit/run_manual.rst
+++ b/Documentation/dev-tools/kunit/run_manual.rst
@@ -49,9 +49,52 @@ loaded.
The results will appear in TAP format in ``dmesg``.
+debugfs
+=======
+
+KUnit can be accessed from userspace via the debugfs filesystem (See more
+information about debugfs at Documentation/filesystems/debugfs.rst).
+
+If ``CONFIG_KUNIT_DEBUGFS`` is enabled, the KUnit debugfs filesystem is
+mounted at /sys/kernel/debug/kunit. You can use this filesystem to perform
+the following actions.
+
+Retrieve Test Results
+=====================
+
+You can use debugfs to retrieve KUnit test results. The test results are
+accessible from the debugfs filesystem in the following read-only file:
+
+.. code-block :: bash
+
+ /sys/kernel/debug/kunit/<test_suite>/results
+
+The test results are printed in a KTAP document. Note this document is separate
+to the kernel log and thus, may have different test suite numbering.
+
+Run Tests After Kernel Has Booted
+=================================
+
+You can use the debugfs filesystem to trigger built-in tests to run after
+boot. To run the test suite, you can use the following command to write to
+the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
+
+.. code-block :: bash
+
+ echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
+
+As a result, the test suite runs and the results are printed to the kernel
+log.
+
+However, this feature is not available with KUnit suites that use init data,
+because init data may have been discarded after the kernel boots. KUnit
+suites that use init data should be defined using the
+kunit_test_init_section_suites() macro.
+
+Also, you cannot use this feature to run tests concurrently. Instead a test
+will wait to run until other tests have completed or failed.
+
.. note ::
- If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
- be accessible from the ``debugfs`` filesystem (if mounted).
- They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
- TAP format.
+ For test authors, to use this feature, tests will need to correctly initialise
+ and/or clean up any data, so the test runs correctly a second time.
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 6/6] Documentation: Add debugfs docs with run after boot
2023-12-13 1:02 ` [PATCH v4 6/6] Documentation: Add debugfs docs with run after boot Rae Moar
@ 2023-12-13 7:13 ` David Gow
0 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Expand the documentation on the KUnit debugfs filesystem on the
> run_manual.rst page.
>
> Add section describing how to access results using debugfs.
>
> Add section describing how to run tests after boot using debugfs.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
Looks good!
Reviewed-by: David Gow <davidgow@google.com>
Thanks,
-- David
> Changes since v3:
> - Change the introduction of the debugfs section
> - Add detail to not being able to run concurrently or run with init data
>
> Documentation/dev-tools/kunit/run_manual.rst | 51 ++++++++++++++++++--
> 1 file changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/run_manual.rst b/Documentation/dev-tools/kunit/run_manual.rst
> index e7b46421f247..699d92885075 100644
> --- a/Documentation/dev-tools/kunit/run_manual.rst
> +++ b/Documentation/dev-tools/kunit/run_manual.rst
> @@ -49,9 +49,52 @@ loaded.
>
> The results will appear in TAP format in ``dmesg``.
>
> +debugfs
> +=======
> +
> +KUnit can be accessed from userspace via the debugfs filesystem (See more
> +information about debugfs at Documentation/filesystems/debugfs.rst).
> +
> +If ``CONFIG_KUNIT_DEBUGFS`` is enabled, the KUnit debugfs filesystem is
> +mounted at /sys/kernel/debug/kunit. You can use this filesystem to perform
> +the following actions.
> +
> +Retrieve Test Results
> +=====================
> +
> +You can use debugfs to retrieve KUnit test results. The test results are
> +accessible from the debugfs filesystem in the following read-only file:
> +
> +.. code-block :: bash
> +
> + /sys/kernel/debug/kunit/<test_suite>/results
> +
> +The test results are printed in a KTAP document. Note this document is separate
> +to the kernel log and thus, may have different test suite numbering.
> +
> +Run Tests After Kernel Has Booted
> +=================================
> +
> +You can use the debugfs filesystem to trigger built-in tests to run after
> +boot. To run the test suite, you can use the following command to write to
> +the ``/sys/kernel/debug/kunit/<test_suite>/run`` file:
> +
> +.. code-block :: bash
> +
> + echo "any string" > /sys/kernel/debugfs/kunit/<test_suite>/run
> +
> +As a result, the test suite runs and the results are printed to the kernel
> +log.
> +
> +However, this feature is not available with KUnit suites that use init data,
> +because init data may have been discarded after the kernel boots. KUnit
> +suites that use init data should be defined using the
> +kunit_test_init_section_suites() macro.
> +
> +Also, you cannot use this feature to run tests concurrently. Instead a test
> +will wait to run until other tests have completed or failed.
> +
> .. note ::
>
> - If ``CONFIG_KUNIT_DEBUGFS`` is enabled, KUnit test results will
> - be accessible from the ``debugfs`` filesystem (if mounted).
> - They will be in ``/sys/kernel/debug/kunit/<test_suite>/results``, in
> - TAP format.
> + For test authors, to use this feature, tests will need to correctly initialise
> + and/or clean up any data, so the test runs correctly a second time.
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA
2023-12-13 1:01 [PATCH v4 1/6] kunit: move KUNIT_TABLE out of INIT_DATA Rae Moar
` (4 preceding siblings ...)
2023-12-13 1:02 ` [PATCH v4 6/6] Documentation: Add debugfs docs with run after boot Rae Moar
@ 2023-12-13 7:13 ` David Gow
5 siblings, 0 replies; 12+ messages in thread
From: David Gow @ 2023-12-13 7:13 UTC (permalink / raw)
To: Rae Moar
Cc: shuah, dlatypov, brendan.higgins, sadiyakazi, keescook, arnd,
linux-kselftest, linux-arch, kunit-dev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]
On Wed, 13 Dec 2023 at 09:02, Rae Moar <rmoar@google.com> wrote:
>
> Alter the linker section of KUNIT_TABLE to move it out of INIT_DATA and
> into DATA_DATA.
>
> Data for KUnit tests does not need to be in the init section.
>
> In order to run tests again after boot the KUnit data cannot be labeled as
> init data as the kernel could write over it.
>
> Add a KUNIT_INIT_TABLE in the next patch for KUnit tests that test init
> data/functions.
>
> Signed-off-by: Rae Moar <rmoar@google.com>
> ---
Looks good to me.
This shouldn't be strictly necessary for the rest of the series, but
will be useful for future features which depend on having the
suite_set around after boot.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
> Changes since v3:
> - No changes
>
> include/asm-generic/vmlinux.lds.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index bae0fe4d499b..1107905d37fc 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -370,7 +370,8 @@
> BRANCH_PROFILE() \
> TRACE_PRINTKS() \
> BPF_RAW_TP() \
> - TRACEPOINT_STR()
> + TRACEPOINT_STR() \
> + KUNIT_TABLE()
>
> /*
> * Data section helpers
> @@ -699,8 +700,7 @@
> THERMAL_TABLE(governor) \
> EARLYCON_TABLE() \
> LSM_TABLE() \
> - EARLY_LSM_TABLE() \
> - KUNIT_TABLE()
> + EARLY_LSM_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
>
> base-commit: b285ba6f8cc1b2bfece0b4350fdb92c8780bc698
> --
> 2.43.0.472.g3155946c3a-goog
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread