Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- include/asm-generic/vmlinux.lds.h | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index db600ef218d7d..4f9b036fc9616 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -881,6 +881,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .;

+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */ +#define KUNIT_TEST_SUITES \ + . = ALIGN(8); \ + __kunit_suites_start = .; \ + KEEP(*(.kunit_test_suites)) \ + __kunit_suites_end = .; + #ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ @@ -1056,6 +1063,7 @@ INIT_CALLS \ CON_INITCALL \ INIT_RAM_FS \ + KUNIT_TEST_SUITES \ }

#define BSS_SECTION(sbss_align, bss_align, stop_align) \

Add a linker section to microblaze where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Signed-off-by: Brendan Higgins brendanhiggins@google.com --- arch/microblaze/kernel/vmlinux.lds.S | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S index df07b3d06cd6b..4fc32f8979a60 100644 --- a/arch/microblaze/kernel/vmlinux.lds.S +++ b/arch/microblaze/kernel/vmlinux.lds.S @@ -128,6 +128,10 @@ SECTIONS {

__init_end = .;

+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + .bss ALIGN (PAGE_SIZE) : AT(ADDR(.bss) - LOAD_OFFSET) { /* page aligned when MMU used */ __bss_start = . ;

Add a linker section to UML where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- arch/um/include/asm/common.lds.S | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S index eca6c452a41bd..9a9c97f45694c 100644 --- a/arch/um/include/asm/common.lds.S +++ b/arch/um/include/asm/common.lds.S @@ -52,6 +52,10 @@ CON_INITCALL }

+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + .exitcall : { __exitcall_begin = .; *(.exitcall.exit)

From: Alan Maguire alan.maguire@oracle.com

Add a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately. Centralized execution is for built-in tests only; modules will execute tests when loaded.

Signed-off-by: Alan Maguire alan.maguire@oracle.com Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Co-developed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- include/kunit/test.h | 67 +++++++++++++++++++++++++++++--------------- lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 28 ++++++++++++++++++ lib/kunit/test.c | 2 +- 4 files changed, 76 insertions(+), 24 deletions(-) create mode 100644 lib/kunit/executor.c

diff --git a/include/kunit/test.h b/include/kunit/test.h index 47e61e1d53370..f3e86c3953a2b 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -224,7 +224,7 @@ 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 **suites); +int __kunit_test_suites_init(struct kunit_suite * const * const suites);

void __kunit_test_suites_exit(struct kunit_suite **suites);

@@ -237,34 +237,57 @@ void __kunit_test_suites_exit(struct kunit_suite **suites); * Registers @suites_list with the test framework. See &struct kunit_suite for * more information. * - * When builtin, KUnit tests are all run as late_initcalls; this means - * that they cannot test anything where tests must run at a different init - * phase. One significant restriction resulting from this is that KUnit - * cannot reliably test anything that is initialize in the late_init phase; - * another is that KUnit is useless to test things that need to be run in - * an earlier init phase. - * - * An alternative is to build the tests as a module. Because modules - * do not support multiple late_initcall()s, we need to initialize an - * array of suites for a module. - * - * TODO(brendanhiggins@google.com): Don't run all KUnit tests as - * late_initcalls. I have some future work planned to dispatch all KUnit - * tests from the same place, and at the very least to do so after - * everything else is definitely initialized. + * 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(suites_list...) \ - static struct kunit_suite *suites[] = {suites_list, NULL}; \ - static int kunit_test_suites_init(void) \ +#ifdef MODULE +#define kunit_test_suites_for_module(__suites) \ + static int __init kunit_test_suites_init(void) \ { \ - return __kunit_test_suites_init(suites); \ + return __kunit_test_suites_init(__suites); \ } \ - late_initcall(kunit_test_suites_init); \ + module_init(kunit_test_suites_init); \ + \ static void __exit kunit_test_suites_exit(void) \ { \ - return __kunit_test_suites_exit(suites); \ + 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 + +/** + * kunit_test_suites() - 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. + * + * 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. + * + * 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. + * + */ +#define kunit_test_suites(...) \ + __kunit_test_suites(__UNIQUE_ID(array), \ + __UNIQUE_ID(suites), \ + __VA_ARGS__)

#define kunit_test_suite(suite) kunit_test_suites(&suite)

diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index 724b94311ca36..c49f4ffb6273a 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o kunit-objs += test.o \ string-stream.o \ assert.o \ - try-catch.o + try-catch.o \ + executor.o

ifeq ($(CONFIG_KUNIT_DEBUGFS),y) kunit-objs += debugfs.o diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c new file mode 100644 index 0000000000000..7015e7328dce7 --- /dev/null +++ b/lib/kunit/executor.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> + +/* + * 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[]; + +#if IS_BUILTIN(CONFIG_KUNIT) + +static int kunit_run_all_tests(void) +{ + struct kunit_suite * const * const *suites; + + for (suites = __kunit_suites_start; + suites < __kunit_suites_end; + suites++) + __kunit_test_suites_init(*suites); + + return 0; +} + +late_initcall(kunit_run_all_tests); + +#endif /* IS_BUILTIN(CONFIG_KUNIT) */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index ccb2ffad8dcfa..918dff400a9d7 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -380,7 +380,7 @@ static void kunit_init_suite(struct kunit_suite *suite) kunit_debugfs_create_suite(suite); }

-int __kunit_test_suites_init(struct kunit_suite **suites) +int __kunit_test_suites_init(struct kunit_suite * const * const suites) { unsigned int i;

TAP 14 allows an optional test plan to be emitted before the start of the start of testing[1]; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed.

Link[1]: https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-s... Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- lib/kunit/executor.c | 17 ++++ lib/kunit/test.c | 11 --- tools/testing/kunit/kunit_parser.py | 76 +++++++++++++++--- .../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes .../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes .../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes 6 files changed, 80 insertions(+), 24 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 4aab7f70a88c3..a95742a4ece73 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -11,10 +11,27 @@ extern struct kunit_suite * const * const __kunit_suites_end[];

#if IS_BUILTIN(CONFIG_KUNIT)

+static void kunit_print_tap_header(void) +{ + struct kunit_suite * const * const *suites, * const *subsuite; + int num_of_suites = 0; + + for (suites = __kunit_suites_start; + suites < __kunit_suites_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); +} + int kunit_run_all_tests(void) { struct kunit_suite * const * const *suites;

+ kunit_print_tap_header(); + for (suites = __kunit_suites_start; suites < __kunit_suites_end; suites++) diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 918dff400a9d7..b1835ccb3fce2 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -19,16 +19,6 @@ static void kunit_set_failure(struct kunit *test) WRITE_ONCE(test->success, false); }

-static void kunit_print_tap_version(void) -{ - static bool kunit_has_printed_tap_version; - - if (!kunit_has_printed_tap_version) { - pr_info("TAP version 14\n"); - kunit_has_printed_tap_version = true; - } -} - /* * Append formatted message to log, size of which is limited to * KUNIT_LOG_SIZE bytes (including null terminating byte). @@ -68,7 +58,6 @@ EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);

static void kunit_print_subtest_start(struct kunit_suite *suite) { - kunit_print_tap_version(); kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "# Subtest: %s", suite->name); kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "1..%zd", diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 64aac9dcd4314..6d6d94a0ee7db 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -45,10 +45,11 @@ class TestStatus(Enum): FAILURE = auto() TEST_CRASHED = auto() NO_TESTS = auto() + FAILURE_TO_PARSE_TESTS = auto()

kunit_start_re = re.compile(r'TAP version [0-9]+$') kunit_end_re = re.compile('(List of all partitions:|' - 'Kernel panic - not syncing: VFS:|reboot: System halted)') + 'Kernel panic - not syncing: VFS:)')

def isolate_kunit_output(kernel_output): started = False @@ -109,7 +110,7 @@ OkNotOkResult = namedtuple('OkNotOkResult', ['is_ok','description', 'text'])

OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$')

-OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) [0-9]+ - (.*)$') +OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$')

def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: save_non_diagnositic(lines, test_case) @@ -197,7 +198,9 @@ def max_status(left: TestStatus, right: TestStatus) -> TestStatus: else: return TestStatus.SUCCESS

-def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool: +def parse_ok_not_ok_test_suite(lines: List[str], + test_suite: TestSuite, + expected_suite_index: int) -> bool: consume_non_diagnositic(lines) if not lines: test_suite.status = TestStatus.TEST_CRASHED @@ -210,6 +213,12 @@ def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool: test_suite.status = TestStatus.SUCCESS else: test_suite.status = TestStatus.FAILURE + suite_index = int(match.group(2)) + if suite_index != expected_suite_index: + print_with_timestamp( + red('[ERROR] ') + 'expected_suite_index ' + + str(expected_suite_index) + ', but got ' + + str(suite_index)) return True else: return False @@ -222,7 +231,7 @@ def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus: max_test_case_status = bubble_up_errors(lambda x: x.status, test_suite.cases) return max_status(max_test_case_status, test_suite.status)

-def parse_test_suite(lines: List[str]) -> TestSuite: +def parse_test_suite(lines: List[str], expected_suite_index: int) -> TestSuite: if not lines: return None consume_non_diagnositic(lines) @@ -241,7 +250,7 @@ def parse_test_suite(lines: List[str]) -> TestSuite: break test_suite.cases.append(test_case) expected_test_case_num -= 1 - if parse_ok_not_ok_test_suite(lines, test_suite): + if parse_ok_not_ok_test_suite(lines, test_suite, expected_suite_index): test_suite.status = bubble_up_test_case_errors(test_suite) return test_suite elif not lines: @@ -261,6 +270,17 @@ def parse_tap_header(lines: List[str]) -> bool: else: return False

+TEST_PLAN = re.compile(r'[0-9]+..([0-9]+)') + +def parse_test_plan(lines: List[str]) -> int: + consume_non_diagnositic(lines) + match = TEST_PLAN.match(lines[0]) + if match: + lines.pop(0) + return int(match.group(1)) + else: + return None + def bubble_up_suite_errors(test_suite_list: List[TestSuite]) -> TestStatus: return bubble_up_errors(lambda x: x.status, test_suite_list)

@@ -269,19 +289,34 @@ def parse_test_result(lines: List[str]) -> TestResult: return TestResult(TestStatus.NO_TESTS, [], lines) consume_non_diagnositic(lines) if not parse_tap_header(lines): - return None + return TestResult(TestStatus.NO_TESTS, [], lines) + expected_test_suite_num = parse_test_plan(lines) + if not expected_test_suite_num: + return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines) test_suites = [] - test_suite = parse_test_suite(lines) - while test_suite: - test_suites.append(test_suite) - test_suite = parse_test_suite(lines) - return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines) + for i in range(1, expected_test_suite_num + 1): + test_suite = parse_test_suite(lines, i) + if test_suite: + test_suites.append(test_suite) + else: + print_with_timestamp( + red('[ERROR] ') + ' expected ' + + str(expected_test_suite_num) + + ' test suites, but got ' + str(i - 2)) + break + test_suite = parse_test_suite(lines, -1) + if test_suite: + print_with_timestamp(red('[ERROR] ') + + 'got unexpected test suite: ' + test_suite.name) + if test_suites: + return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines) + else: + return TestResult(TestStatus.NO_TESTS, [], lines)

-def parse_run_tests(kernel_output) -> TestResult: +def print_and_count_results(test_result: TestResult) -> None: total_tests = 0 failed_tests = 0 crashed_tests = 0 - test_result = parse_test_result(list(isolate_kunit_output(kernel_output))) for test_suite in test_result.suites: if test_suite.status == TestStatus.SUCCESS: print_suite_divider(green('[PASSED] ') + test_suite.name) @@ -303,6 +338,21 @@ def parse_run_tests(kernel_output) -> TestResult: print_with_timestamp(red('[FAILED] ') + test_case.name) print_log(map(yellow, test_case.log)) print_with_timestamp('') + return total_tests, failed_tests, crashed_tests + +def parse_run_tests(kernel_output) -> TestResult: + total_tests = 0 + failed_tests = 0 + crashed_tests = 0 + test_result = parse_test_result(list(isolate_kunit_output(kernel_output))) + if test_result.status == TestStatus.NO_TESTS: + print(red('[ERROR] ') + yellow('no tests run!')) + elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS: + print(red('[ERROR] ') + yellow('could not parse test results!')) + else: + (total_tests, + failed_tests, + crashed_tests) = print_and_count_results(test_result) print_with_timestamp(DIVIDER) fmt = green if test_result.status == TestStatus.SUCCESS else red print_with_timestamp( diff --git a/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log b/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log index 62ebc0288355c4b122ccc18ae2505f971efa57bc..bc0dc8fe35b760b1feb74ec419818dbfae1adb5c 100644 GIT binary patch delta 28 jcmbQmGoME|#4$jjEVZaOGe1wk(1goSPtRy09}gP<dC~`u

delta 23 ecmbQwGmD2W#4$jjEVZaOGe1wk&}5@94;uhhkp{*9

diff --git a/tools/testing/kunit/test_data/test_is_test_passed-crash.log b/tools/testing/kunit/test_data/test_is_test_passed-crash.log index 0b249870c8be417a5865bd40a24c8597bb7f5ab1..4d97f6708c4a5ad5bb2ac879e12afca6e816d83d 100644 GIT binary patch delta 15 WcmX>hepY;fFN>j`p3z318g2k9Uj*m?

delta 10 RcmX>renNbL@5Z2NZU7lr1S$Xk

diff --git a/tools/testing/kunit/test_data/test_is_test_passed-failure.log b/tools/testing/kunit/test_data/test_is_test_passed-failure.log index 9e89d32d5667a59d137f8adacf3a88fdb7f88baf..7a416497e3bec044eefc1535f7d84ee85703ba97 100644 GIT binary patch delta 28 jcmZ3&yOLKp#4$jjEVZaOGe1wk(1goSPtRy0-!wJ=eKrU$

delta 23 ecmZ3<yM&i7#4$jjEVZaOGe1wk&}5_VG&TTPhX-Z=

Add kunit_shutdown, an option to specify that the kernel shutsdown after running KUnit tests, to the kernel-parameters.txt documentation.

Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index fb95fad81c79a..e7d5eb7249e7f 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2183,6 +2183,14 @@ 0: force disabled 1: force enabled

+ kunit_shutdown=[KERNEL UNIT TESTING FRAMEWORK] Shutdown kernel after + running built-in tests. Tests configured as modules will + not be run. + Default: (flag not present) don't shutdown + poweroff: poweroff the kernel after running tests + halt: halt the kernel after running tests + reboot: reboot the kernel after running tests + kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs. Default is 0 (don't ignore, but inject #GP)

On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:

Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

include/asm-generic/vmlinux.lds.h | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index db600ef218d7d..4f9b036fc9616 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -881,6 +881,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .; +/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */

Nit on naming:

+#define KUNIT_TEST_SUITES \

I would call this KUNIT_TABLE to maintain the same names as other things of this nature.

• . = ALIGN(8);						\
  

• __kunit_suites_start = .;				\
  

• KEEP(*(.kunit_test_suites))				\
  

• __kunit_suites_end = .;
  

#ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ @@ -1056,6 +1063,7 @@ INIT_CALLS \ CON_INITCALL \ INIT_RAM_FS \

• KUNIT_TEST_SUITES					\
  

  }

Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything uses INIT_DATA.

On Fri, Jun 26, 2020 at 2:20 PM Kees Cook keescook@chromium.org wrote:

On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:

...

Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

include/asm-generic/vmlinux.lds.h | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index db600ef218d7d..4f9b036fc9616 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -881,6 +881,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .;

+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */

Nit on naming:

...

+#define KUNIT_TEST_SUITES \

I would call this KUNIT_TABLE to maintain the same names as other things of this nature.

...

•         . = ALIGN(8);                                           \
  

•         __kunit_suites_start = .;                               \
  

•         KEEP(*(.kunit_test_suites))                             \
  

•         __kunit_suites_end = .;
  

#ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ @@ -1056,6 +1063,7 @@ INIT_CALLS \ CON_INITCALL \ INIT_RAM_FS \

•         KUNIT_TEST_SUITES                                       \
  }
  

Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything uses INIT_DATA.

Oh, maybe that would eliminate the need for the other linkerscript patches? That would be nice.

Alright, will fix.

On Fri, Jun 26, 2020 at 02:09:12PM -0700, Brendan Higgins wrote:

From: Alan Maguire alan.maguire@oracle.com

Add a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately. Centralized execution is for built-in tests only; modules will execute tests when loaded.

Signed-off-by: Alan Maguire alan.maguire@oracle.com Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Co-developed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

include/kunit/test.h | 67 +++++++++++++++++++++++++++++--------------- lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 28 ++++++++++++++++++ lib/kunit/test.c | 2 +- 4 files changed, 76 insertions(+), 24 deletions(-) create mode 100644 lib/kunit/executor.c

diff --git a/include/kunit/test.h b/include/kunit/test.h index 47e61e1d53370..f3e86c3953a2b 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -224,7 +224,7 @@ 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 **suites); +int __kunit_test_suites_init(struct kunit_suite * const * const suites); void __kunit_test_suites_exit(struct kunit_suite **suites); @@ -237,34 +237,57 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);

• Registers @suites_list with the test framework. See &struct kunit_suite for
• more information.

• • When builtin, KUnit tests are all run as late_initcalls; this means
• • that they cannot test anything where tests must run at a different init
• • phase. One significant restriction resulting from this is that KUnit
• • cannot reliably test anything that is initialize in the late_init phase;
• • another is that KUnit is useless to test things that need to be run in
• • an earlier init phase.
• • An alternative is to build the tests as a module. Because modules
• • do not support multiple late_initcall()s, we need to initialize an
• • array of suites for a module.
• • TODO(brendanhiggins@google.com): Don't run all KUnit tests as
• • late_initcalls. I have some future work planned to dispatch all KUnit
• • tests from the same place, and at the very least to do so after
• • everything else is definitely initialized.

• • 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(suites_list...) \

• static struct kunit_suite *suites[] = {suites_list, NULL}; \
• static int kunit_test_suites_init(void) \

+#ifdef MODULE +#define kunit_test_suites_for_module(__suites) \

• static int __init kunit_test_suites_init(void) \ { \

• return __kunit_test_suites_init(suites);		\
  

• return __kunit_test_suites_init(__suites);		\
  

  } \

• late_initcall(kunit_test_suites_init); \

• module_init(kunit_test_suites_init); \

• 							\
  

  static void __exit kunit_test_suites_exit(void) \ { \

• return __kunit_test_suites_exit(suites);		\
  

• 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

+/**

• • kunit_test_suites() - 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.
• • 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.
• • 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.
• */

+#define kunit_test_suites(...) \

• __kunit_test_suites(__UNIQUE_ID(array), \

• 	    __UNIQUE_ID(suites),			\
  

• 	    __VA_ARGS__)
  

#define kunit_test_suite(suite) kunit_test_suites(&suite) diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index 724b94311ca36..c49f4ffb6273a 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o kunit-objs += test.o \ string-stream.o \ assert.o \

• 			try-catch.o
  

• 			try-catch.o \
  

• 			executor.o
  

ifeq ($(CONFIG_KUNIT_DEBUGFS),y) kunit-objs += debugfs.o diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c new file mode 100644 index 0000000000000..7015e7328dce7 --- /dev/null +++ b/lib/kunit/executor.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0

+#include <kunit/test.h>

+/*

• • 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[];

I would expect these to be in include/asm-generic/sections.h but I guess it's not required.

Reviewed-by: Kees Cook keescook@chromium.org

On Fri, Jun 26, 2020 at 02:09:15PM -0700, Brendan Higgins wrote:

From: David Gow davidgow@google.com

Add a new kernel command-line option, 'kunit_shutdown', which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs.

Signed-off-by: David Gow davidgow@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

lib/kunit/executor.c | 20 ++++++++++++++++++++ tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index a95742a4ece73..38061d456afb2 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/reboot.h> #include <kunit/test.h> /* @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[]; #if IS_BUILTIN(CONFIG_KUNIT) +static char *kunit_shutdown; +core_param(kunit_shutdown, kunit_shutdown, charp, 0644);

+static void kunit_handle_shutdown(void) +{

• if (!kunit_shutdown)

• return;
  

• if (!strcmp(kunit_shutdown, "poweroff"))

• kernel_power_off();
  

• else if (!strcmp(kunit_shutdown, "halt"))

• kernel_halt();
  

• else if (!strcmp(kunit_shutdown, "reboot"))

• kernel_restart(NULL);
  

+}

If you have patches that do something just before the initrd, and then you add more patches to shut down immediately after an initrd, people may ask you to just use an initrd instead of filling the kernel with these changes...

I mean, I get it, but it's not hard to make an initrd that poke a sysctl to start the tests...

In fact, you don't even need a initrd to poke sysctls these days.

On Fri, Jun 26, 2020 at 02:22:11PM -0700, Brendan Higgins wrote:

On Fri, Jun 26, 2020 at 2:20 PM Kees Cook keescook@chromium.org wrote:

...

On Fri, Jun 26, 2020 at 02:09:06PM -0700, Brendan Higgins wrote:

...

Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.

Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

include/asm-generic/vmlinux.lds.h | 8 ++++++++ 1 file changed, 8 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index db600ef218d7d..4f9b036fc9616 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -881,6 +881,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .;

+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */

Nit on naming:

...

+#define KUNIT_TEST_SUITES \

I would call this KUNIT_TABLE to maintain the same names as other things of this nature.

...

•         . = ALIGN(8);                                           \
  

•         __kunit_suites_start = .;                               \
  

•         KEEP(*(.kunit_test_suites))                             \
  

•         __kunit_suites_end = .;
  

#ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ @@ -1056,6 +1063,7 @@ INIT_CALLS \ CON_INITCALL \ INIT_RAM_FS \

•         KUNIT_TEST_SUITES                                       \
  }
  

Nack: this must be in INIT_DATA, not in INIT_DATA_SECTION. Not all architectures use the INIT_DATA_SECTION macro (e.g. arm64), but everything uses INIT_DATA.

Oh, maybe that would eliminate the need for the other linkerscript patches? That would be nice.

Curious, did changing it as Kees suggest fix it for m68k?

Luis

On Fri, Jun 26, 2020 at 2:29 PM Kees Cook keescook@chromium.org wrote:

On Fri, Jun 26, 2020 at 02:09:12PM -0700, Brendan Higgins wrote:

...

From: Alan Maguire alan.maguire@oracle.com

Add a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately. Centralized execution is for built-in tests only; modules will execute tests when loaded.

Signed-off-by: Alan Maguire alan.maguire@oracle.com Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Co-developed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

include/kunit/test.h | 67 +++++++++++++++++++++++++++++--------------- lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 28 ++++++++++++++++++ lib/kunit/test.c | 2 +- 4 files changed, 76 insertions(+), 24 deletions(-) create mode 100644 lib/kunit/executor.c

diff --git a/include/kunit/test.h b/include/kunit/test.h index 47e61e1d53370..f3e86c3953a2b 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -224,7 +224,7 @@ 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 **suites); +int __kunit_test_suites_init(struct kunit_suite * const * const suites);

void __kunit_test_suites_exit(struct kunit_suite **suites);

@@ -237,34 +237,57 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);

• Registers @suites_list with the test framework. See &struct kunit_suite for
• more information.

• • When builtin, KUnit tests are all run as late_initcalls; this means
• • that they cannot test anything where tests must run at a different init
• • phase. One significant restriction resulting from this is that KUnit
• • cannot reliably test anything that is initialize in the late_init phase;
• • another is that KUnit is useless to test things that need to be run in
• • an earlier init phase.
• • An alternative is to build the tests as a module. Because modules
• • do not support multiple late_initcall()s, we need to initialize an
• • array of suites for a module.
• • TODO(brendanhiggins@google.com): Don't run all KUnit tests as
• • late_initcalls. I have some future work planned to dispatch all KUnit
• • tests from the same place, and at the very least to do so after
• • everything else is definitely initialized.

• • 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(suites_list...) \

• static struct kunit_suite *suites[] = {suites_list, NULL};      \
  

• static int kunit_test_suites_init(void)                         \
  

+#ifdef MODULE +#define kunit_test_suites_for_module(__suites) \

• static int __init kunit_test_suites_init(void)                  \
  {                                                               \
  

•         return __kunit_test_suites_init(suites);                \
  

•         return __kunit_test_suites_init(__suites);              \
  }                                                               \
  

• late_initcall(kunit_test_suites_init);                          \
  

• module_init(kunit_test_suites_init);                            \
  

•                                                                 \
  static void __exit kunit_test_suites_exit(void)                 \
  {                                                               \
  

•         return __kunit_test_suites_exit(suites);                \
  

•         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
  

+/**

• • kunit_test_suites() - 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.
• • 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.
• • 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.
• */

+#define kunit_test_suites(...) \

• __kunit_test_suites(__UNIQUE_ID(array),                         \
  

•                     __UNIQUE_ID(suites),                        \
  

•                     __VA_ARGS__)
  
  

#define kunit_test_suite(suite) kunit_test_suites(&suite)

diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index 724b94311ca36..c49f4ffb6273a 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o kunit-objs += test.o \ string-stream.o \ assert.o \

•                                 try-catch.o
  

•                                 try-catch.o \
  

•                                 executor.o
  
  

ifeq ($(CONFIG_KUNIT_DEBUGFS),y) kunit-objs += debugfs.o diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c new file mode 100644 index 0000000000000..7015e7328dce7 --- /dev/null +++ b/lib/kunit/executor.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0

+#include <kunit/test.h>

+/*

• • 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[];

I would expect these to be in include/asm-generic/sections.h but I guess it's not required.

I don't have strong opinions either way, but I think this is less clutter since KUnit is the only one that uses it.

Reviewed-by: Kees Cook keescook@chromium.org

Thanks!

On Fri, Jun 26, 2020 at 2:40 PM Kees Cook keescook@chromium.org wrote:

On Fri, Jun 26, 2020 at 02:09:15PM -0700, Brendan Higgins wrote:

...

From: David Gow davidgow@google.com

Add a new kernel command-line option, 'kunit_shutdown', which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs.

Signed-off-by: David Gow davidgow@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org

---

lib/kunit/executor.c | 20 ++++++++++++++++++++ tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 2 +- 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index a95742a4ece73..38061d456afb2 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0

+#include <linux/reboot.h> #include <kunit/test.h>

/* @@ -11,6 +12,23 @@ extern struct kunit_suite * const * const __kunit_suites_end[];

#if IS_BUILTIN(CONFIG_KUNIT)

+static char *kunit_shutdown; +core_param(kunit_shutdown, kunit_shutdown, charp, 0644);

+static void kunit_handle_shutdown(void) +{

• if (!kunit_shutdown)
  

•         return;
  

• if (!strcmp(kunit_shutdown, "poweroff"))
  

•         kernel_power_off();
  

• else if (!strcmp(kunit_shutdown, "halt"))
  

•         kernel_halt();
  

• else if (!strcmp(kunit_shutdown, "reboot"))
  

•         kernel_restart(NULL);
  

+}

If you have patches that do something just before the initrd, and then you add more patches to shut down immediately after an initrd, people may ask you to just use an initrd instead of filling the kernel with these changes...

I mean, I get it, but it's not hard to make an initrd that poke a sysctl to start the tests...

In fact, you don't even need a initrd to poke sysctls these days.

True, but it is nice to not have to maintain an initramfs for each architecture that you want to test. Still, I see your point. If we can find a convenient way to distribute the needed dependencies for running KUnit on each non-UML architecture then I think I can abandon this change.

So how about this: I will drop this patch from this patchset and move it up to the follow up patchset that adds multiarchitecture support to kunit_tool. There we can address the problem of how to best track the necessary dependencies including possibly intitramfss.