On Mon, 2 Feb 2026 at 17:49, Arnd Bergmann <[email protected]> wrote: > > From: Arnd Bergmann <[email protected]> > > Some of the recent changes to the kunit framework caused the stack usage > for kunit_run_tests() to grow higher than most other kernel functions, > which triggers a warning when CONFIG_FRAME_WARN is set to a relatively > low value: > > lib/kunit/test.c: In function 'kunit_run_tests': > lib/kunit/test.c:801:1: error: the frame size of 1312 bytes is larger than > 1280 bytes [-Werror=frame-larger-than=] > > Split out the inner loop into a separate function to ensure that each > function remains under the limit, and pass the kunit_result_stats > structures by reference to avoid excessive copies. > > Cc: Carlos Llamas <[email protected]> > Signed-off-by: Arnd Bergmann <[email protected]> > --- > v2: > - plit out two functions instead of one, as there were still > stack size warnings with clang on the earlier version > - use void return type suggested by Carlos > > Carlos has successfully tested version 1, but after I rebased on > current linux-next, the build warnings came back due to some other > changes, and I had to rework the patch. > ---
Thanks very much. I couldn't reproduce it here (kunit_run_tests uses only 1104 bytes on my setup), but making it lower still and relying less on the compiler "just happening" to optimise it enough is a good thing. There's one small checkpatch warning below, otherwise this looks good. Reviewed-by: David Gow <[email protected]> Cheers, -- David > lib/kunit/test.c | 230 +++++++++++++++++++++++++---------------------- > 1 file changed, 124 insertions(+), 106 deletions(-) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 62eb529824c6..8668ed8d4553 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -94,7 +94,7 @@ struct kunit_result_stats { > unsigned long total; > }; > > -static bool kunit_should_print_stats(struct kunit_result_stats stats) > +static bool kunit_should_print_stats(struct kunit_result_stats *stats) > { > if (kunit_stats_enabled == 0) > return false; > @@ -102,11 +102,11 @@ static bool kunit_should_print_stats(struct > kunit_result_stats stats) > if (kunit_stats_enabled == 2) > return true; > > - return (stats.total > 1); > + return (stats->total > 1); > } > > static void kunit_print_test_stats(struct kunit *test, > - struct kunit_result_stats stats) > + struct kunit_result_stats *stats) > { > if (!kunit_should_print_stats(stats)) > return; > @@ -115,10 +115,10 @@ static void kunit_print_test_stats(struct kunit *test, > KUNIT_SUBTEST_INDENT > "# %s: pass:%lu fail:%lu skip:%lu total:%lu", > test->name, > - stats.passed, > - stats.failed, > - stats.skipped, > - stats.total); > + stats->passed, > + stats->failed, > + stats->skipped, > + stats->total); > } > > /* Append formatted message to log. */ > @@ -600,26 +600,26 @@ static void kunit_run_case_catch_errors(struct > kunit_suite *suite, > } > > static void kunit_print_suite_stats(struct kunit_suite *suite, > - struct kunit_result_stats suite_stats, > - struct kunit_result_stats param_stats) > + struct kunit_result_stats *suite_stats, > + struct kunit_result_stats *param_stats) > { > if (kunit_should_print_stats(suite_stats)) { > kunit_log(KERN_INFO, suite, > "# %s: pass:%lu fail:%lu skip:%lu total:%lu", > suite->name, > - suite_stats.passed, > - suite_stats.failed, > - suite_stats.skipped, > - suite_stats.total); > + suite_stats->passed, > + suite_stats->failed, > + suite_stats->skipped, > + suite_stats->total); > } > > if (kunit_should_print_stats(param_stats)) { > kunit_log(KERN_INFO, suite, > "# Totals: pass:%lu fail:%lu skip:%lu total:%lu", > - param_stats.passed, > - param_stats.failed, > - param_stats.skipped, > - param_stats.total); > + param_stats->passed, > + param_stats->failed, > + param_stats->skipped, > + param_stats->total); > } > } > > @@ -681,13 +681,115 @@ static void kunit_init_parent_param_test(struct > kunit_case *test_case, struct ku > } > } > > -int kunit_run_tests(struct kunit_suite *suite) > +static noinline_for_stack void > +kunit_run_param_test(struct kunit_suite *suite, struct kunit_case *test_case, > + struct kunit *test, > + struct kunit_result_stats *suite_stats, > + struct kunit_result_stats *total_stats, > + struct kunit_result_stats *param_stats) > { > char param_desc[KUNIT_PARAM_DESC_SIZE]; > + const void *curr_param; checkpatch complains about a missing newline here: WARNING: Missing a blank line after declarations #130: FILE: lib/kunit/test.c:693: + const void *curr_param; + kunit_init_parent_param_test(test_case, test); > + kunit_init_parent_param_test(test_case, test); > + if (test_case->status == KUNIT_FAILURE) { > + kunit_update_stats(param_stats, test->status); > + return; > + } > + /* Get initial param. */ > + param_desc[0] = '\0'; > + /* TODO: Make generate_params try-catch */ > + curr_param = test_case->generate_params(test, NULL, param_desc); > + test_case->status = KUNIT_SKIPPED; > + kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT > + "KTAP version 1\n"); > + kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT > + "# Subtest: %s", test_case->name); > + if (test->params_array.params && > + test_case->generate_params == kunit_array_gen_params) { > + kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT > + KUNIT_SUBTEST_INDENT "1..%zd\n", > + test->params_array.num_params); > + } > + > + while (curr_param) { > + struct kunit param_test = { > + .param_value = curr_param, > + .param_index = ++test->param_index, > + .parent = test, > + }; > + kunit_init_test(¶m_test, test_case->name, NULL); > + param_test.log = test_case->log; > + kunit_run_case_catch_errors(suite, test_case, ¶m_test); > + > + if (param_desc[0] == '\0') { > + snprintf(param_desc, sizeof(param_desc), > + "param-%d", param_test.param_index); > + } > + > + kunit_print_ok_not_ok(¶m_test, KUNIT_LEVEL_CASE_PARAM, > + param_test.status, > + param_test.param_index, > + param_desc, > + param_test.status_comment); > + > + kunit_update_stats(param_stats, param_test.status); > + > + /* Get next param. */ > + param_desc[0] = '\0'; > + curr_param = test_case->generate_params(test, curr_param, > + param_desc); > + } > + /* > + * TODO: Put into a try catch. Since we don't need suite->exit > + * for it we can't reuse kunit_try_run_cleanup for this yet. > + */ > + if (test_case->param_exit) > + test_case->param_exit(test); > + /* TODO: Put this kunit_cleanup into a try-catch. */ > + kunit_cleanup(test); > +} > + > +static noinline_for_stack void > +kunit_run_one_test(struct kunit_suite *suite, struct kunit_case *test_case, > + struct kunit_result_stats *suite_stats, > + struct kunit_result_stats *total_stats) > +{ > + struct kunit test = { .param_value = NULL, .param_index = 0 }; > + struct kunit_result_stats param_stats = { 0 }; > + > + kunit_init_test(&test, test_case->name, test_case->log); > + if (test_case->status == KUNIT_SKIPPED) { > + /* Test marked as skip */ > + test.status = KUNIT_SKIPPED; > + kunit_update_stats(¶m_stats, test.status); > + } else if (!test_case->generate_params) { > + /* Non-parameterised test. */ > + test_case->status = KUNIT_SKIPPED; > + kunit_run_case_catch_errors(suite, test_case, &test); > + kunit_update_stats(¶m_stats, test.status); > + } else { > + kunit_run_param_test(suite, test_case, &test, suite_stats, > + total_stats, ¶m_stats); > + } > + kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE); > + > + kunit_print_test_stats(&test, ¶m_stats); > + > + kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status, > + kunit_test_case_num(suite, test_case), > + test_case->name, > + test.status_comment); > + > + kunit_update_stats(suite_stats, test_case->status); > + kunit_accumulate_stats(total_stats, param_stats); > +} > + > + > +int kunit_run_tests(struct kunit_suite *suite) > +{ > struct kunit_case *test_case; > struct kunit_result_stats suite_stats = { 0 }; > struct kunit_result_stats total_stats = { 0 }; > - const void *curr_param; > > /* Taint the kernel so we know we've run tests. */ > add_taint(TAINT_TEST, LOCKDEP_STILL_OK); > @@ -703,97 +805,13 @@ int kunit_run_tests(struct kunit_suite *suite) > > kunit_print_suite_start(suite); > > - kunit_suite_for_each_test_case(suite, test_case) { > - struct kunit test = { .param_value = NULL, .param_index = 0 }; > - struct kunit_result_stats param_stats = { 0 }; > - > - kunit_init_test(&test, test_case->name, test_case->log); > - if (test_case->status == KUNIT_SKIPPED) { > - /* Test marked as skip */ > - test.status = KUNIT_SKIPPED; > - kunit_update_stats(¶m_stats, test.status); > - } else if (!test_case->generate_params) { > - /* Non-parameterised test. */ > - test_case->status = KUNIT_SKIPPED; > - kunit_run_case_catch_errors(suite, test_case, &test); > - kunit_update_stats(¶m_stats, test.status); > - } else { > - kunit_init_parent_param_test(test_case, &test); > - if (test_case->status == KUNIT_FAILURE) { > - kunit_update_stats(¶m_stats, test.status); > - goto test_case_end; > - } > - /* Get initial param. */ > - param_desc[0] = '\0'; > - /* TODO: Make generate_params try-catch */ > - curr_param = test_case->generate_params(&test, NULL, > param_desc); > - test_case->status = KUNIT_SKIPPED; > - kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT > KUNIT_SUBTEST_INDENT > - "KTAP version 1\n"); > - kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT > KUNIT_SUBTEST_INDENT > - "# Subtest: %s", test_case->name); > - if (test.params_array.params && > - test_case->generate_params == > kunit_array_gen_params) { > - kunit_log(KERN_INFO, &test, > KUNIT_SUBTEST_INDENT > - KUNIT_SUBTEST_INDENT "1..%zd\n", > - test.params_array.num_params); > - } > - > - while (curr_param) { > - struct kunit param_test = { > - .param_value = curr_param, > - .param_index = ++test.param_index, > - .parent = &test, > - }; > - kunit_init_test(¶m_test, test_case->name, > NULL); > - param_test.log = test_case->log; > - kunit_run_case_catch_errors(suite, test_case, > ¶m_test); > - > - if (param_desc[0] == '\0') { > - snprintf(param_desc, > sizeof(param_desc), > - "param-%d", > param_test.param_index); > - } > - > - kunit_print_ok_not_ok(¶m_test, > KUNIT_LEVEL_CASE_PARAM, > - param_test.status, > - param_test.param_index, > - param_desc, > - > param_test.status_comment); > - > - kunit_update_stats(¶m_stats, > param_test.status); > - > - /* Get next param. */ > - param_desc[0] = '\0'; > - curr_param = > test_case->generate_params(&test, curr_param, > - > param_desc); > - } > - /* > - * TODO: Put into a try catch. Since we don't need > suite->exit > - * for it we can't reuse kunit_try_run_cleanup for > this yet. > - */ > - if (test_case->param_exit) > - test_case->param_exit(&test); > - /* TODO: Put this kunit_cleanup into a try-catch. */ > - kunit_cleanup(&test); > - } > -test_case_end: > - kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE); > - > - kunit_print_test_stats(&test, param_stats); > - > - kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, > test_case->status, > - kunit_test_case_num(suite, test_case), > - test_case->name, > - test.status_comment); > - > - kunit_update_stats(&suite_stats, test_case->status); > - kunit_accumulate_stats(&total_stats, param_stats); > - } > + kunit_suite_for_each_test_case(suite, test_case) > + kunit_run_one_test(suite, test_case, &suite_stats, > &total_stats); > > if (suite->suite_exit) > suite->suite_exit(suite); > > - kunit_print_suite_stats(suite, suite_stats, total_stats); > + kunit_print_suite_stats(suite, &suite_stats, &total_stats); > suite_end: > kunit_print_suite_end(suite); > > -- > 2.39.5 >
smime.p7s
Description: S/MIME Cryptographic Signature
