Re: [PATCH v4 2/6] kunit: add KUNIT_INIT_TABLE to init linker section

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