Skip to content

Commit b11b9b6

Browse files
arndbshuahkh
arndb
authored and
shuahkh
committed
kunit: reduce stack usage in kunit_run_tests()
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. Fixed checkpatch warnings at commit time: Shuah Khan <[email protected]> Cc: Carlos Llamas <[email protected]> Signed-off-by: Arnd Bergmann <[email protected]> Reviewed-by: David Gow <[email protected]> Signed-off-by: Shuah Khan <[email protected]>
1 parent 40804c4 commit b11b9b6

1 file changed

Lines changed: 125 additions & 106 deletions

File tree

‎lib/kunit/test.c‎

Lines changed: 125 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,19 @@ struct kunit_result_stats {
9494
unsigned long total;
9595
};
9696

97-
static bool kunit_should_print_stats(struct kunit_result_stats stats)
97+
static bool kunit_should_print_stats(struct kunit_result_stats *stats)
9898
{
9999
if (kunit_stats_enabled == 0)
100100
return false;
101101

102102
if (kunit_stats_enabled == 2)
103103
return true;
104104

105-
return (stats.total > 1);
105+
return (stats->total > 1);
106106
}
107107

108108
static void kunit_print_test_stats(struct kunit *test,
109-
struct kunit_result_stats stats)
109+
struct kunit_result_stats *stats)
110110
{
111111
if (!kunit_should_print_stats(stats))
112112
return;
@@ -115,10 +115,10 @@ static void kunit_print_test_stats(struct kunit *test,
115115
KUNIT_SUBTEST_INDENT
116116
"# %s: pass:%lu fail:%lu skip:%lu total:%lu",
117117
test->name,
118-
stats.passed,
119-
stats.failed,
120-
stats.skipped,
121-
stats.total);
118+
stats->passed,
119+
stats->failed,
120+
stats->skipped,
121+
stats->total);
122122
}
123123

124124
/* Append formatted message to log. */
@@ -600,26 +600,26 @@ static void kunit_run_case_catch_errors(struct kunit_suite *suite,
600600
}
601601

602602
static void kunit_print_suite_stats(struct kunit_suite *suite,
603-
struct kunit_result_stats suite_stats,
604-
struct kunit_result_stats param_stats)
603+
struct kunit_result_stats *suite_stats,
604+
struct kunit_result_stats *param_stats)
605605
{
606606
if (kunit_should_print_stats(suite_stats)) {
607607
kunit_log(KERN_INFO, suite,
608608
"# %s: pass:%lu fail:%lu skip:%lu total:%lu",
609609
suite->name,
610-
suite_stats.passed,
611-
suite_stats.failed,
612-
suite_stats.skipped,
613-
suite_stats.total);
610+
suite_stats->passed,
611+
suite_stats->failed,
612+
suite_stats->skipped,
613+
suite_stats->total);
614614
}
615615

616616
if (kunit_should_print_stats(param_stats)) {
617617
kunit_log(KERN_INFO, suite,
618618
"# Totals: pass:%lu fail:%lu skip:%lu total:%lu",
619-
param_stats.passed,
620-
param_stats.failed,
621-
param_stats.skipped,
622-
param_stats.total);
619+
param_stats->passed,
620+
param_stats->failed,
621+
param_stats->skipped,
622+
param_stats->total);
623623
}
624624
}
625625

@@ -681,13 +681,116 @@ static void kunit_init_parent_param_test(struct kunit_case *test_case, struct ku
681681
}
682682
}
683683

684-
int kunit_run_tests(struct kunit_suite *suite)
684+
static noinline_for_stack void
685+
kunit_run_param_test(struct kunit_suite *suite, struct kunit_case *test_case,
686+
struct kunit *test,
687+
struct kunit_result_stats *suite_stats,
688+
struct kunit_result_stats *total_stats,
689+
struct kunit_result_stats *param_stats)
685690
{
686691
char param_desc[KUNIT_PARAM_DESC_SIZE];
692+
const void *curr_param;
693+
694+
kunit_init_parent_param_test(test_case, test);
695+
if (test_case->status == KUNIT_FAILURE) {
696+
kunit_update_stats(param_stats, test->status);
697+
return;
698+
}
699+
/* Get initial param. */
700+
param_desc[0] = '\0';
701+
/* TODO: Make generate_params try-catch */
702+
curr_param = test_case->generate_params(test, NULL, param_desc);
703+
test_case->status = KUNIT_SKIPPED;
704+
kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
705+
"KTAP version 1\n");
706+
kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
707+
"# Subtest: %s", test_case->name);
708+
if (test->params_array.params &&
709+
test_case->generate_params == kunit_array_gen_params) {
710+
kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT
711+
KUNIT_SUBTEST_INDENT "1..%zd\n",
712+
test->params_array.num_params);
713+
}
714+
715+
while (curr_param) {
716+
struct kunit param_test = {
717+
.param_value = curr_param,
718+
.param_index = ++test->param_index,
719+
.parent = test,
720+
};
721+
kunit_init_test(&param_test, test_case->name, NULL);
722+
param_test.log = test_case->log;
723+
kunit_run_case_catch_errors(suite, test_case, &param_test);
724+
725+
if (param_desc[0] == '\0') {
726+
snprintf(param_desc, sizeof(param_desc),
727+
"param-%d", param_test.param_index);
728+
}
729+
730+
kunit_print_ok_not_ok(&param_test, KUNIT_LEVEL_CASE_PARAM,
731+
param_test.status,
732+
param_test.param_index,
733+
param_desc,
734+
param_test.status_comment);
735+
736+
kunit_update_stats(param_stats, param_test.status);
737+
738+
/* Get next param. */
739+
param_desc[0] = '\0';
740+
curr_param = test_case->generate_params(test, curr_param,
741+
param_desc);
742+
}
743+
/*
744+
* TODO: Put into a try catch. Since we don't need suite->exit
745+
* for it we can't reuse kunit_try_run_cleanup for this yet.
746+
*/
747+
if (test_case->param_exit)
748+
test_case->param_exit(test);
749+
/* TODO: Put this kunit_cleanup into a try-catch. */
750+
kunit_cleanup(test);
751+
}
752+
753+
static noinline_for_stack void
754+
kunit_run_one_test(struct kunit_suite *suite, struct kunit_case *test_case,
755+
struct kunit_result_stats *suite_stats,
756+
struct kunit_result_stats *total_stats)
757+
{
758+
struct kunit test = { .param_value = NULL, .param_index = 0 };
759+
struct kunit_result_stats param_stats = { 0 };
760+
761+
kunit_init_test(&test, test_case->name, test_case->log);
762+
if (test_case->status == KUNIT_SKIPPED) {
763+
/* Test marked as skip */
764+
test.status = KUNIT_SKIPPED;
765+
kunit_update_stats(&param_stats, test.status);
766+
} else if (!test_case->generate_params) {
767+
/* Non-parameterised test. */
768+
test_case->status = KUNIT_SKIPPED;
769+
kunit_run_case_catch_errors(suite, test_case, &test);
770+
kunit_update_stats(&param_stats, test.status);
771+
} else {
772+
kunit_run_param_test(suite, test_case, &test, suite_stats,
773+
total_stats, &param_stats);
774+
}
775+
kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE);
776+
777+
kunit_print_test_stats(&test, &param_stats);
778+
779+
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status,
780+
kunit_test_case_num(suite, test_case),
781+
test_case->name,
782+
test.status_comment);
783+
784+
kunit_update_stats(suite_stats, test_case->status);
785+
kunit_accumulate_stats(total_stats, param_stats);
786+
}
787+
788+
789+
int kunit_run_tests(struct kunit_suite *suite)
790+
{
687791
struct kunit_case *test_case;
688792
struct kunit_result_stats suite_stats = { 0 };
689793
struct kunit_result_stats total_stats = { 0 };
690-
const void *curr_param;
691794

692795
/* Taint the kernel so we know we've run tests. */
693796
add_taint(TAINT_TEST, LOCKDEP_STILL_OK);
@@ -703,97 +806,13 @@ int kunit_run_tests(struct kunit_suite *suite)
703806

704807
kunit_print_suite_start(suite);
705808

706-
kunit_suite_for_each_test_case(suite, test_case) {
707-
struct kunit test = { .param_value = NULL, .param_index = 0 };
708-
struct kunit_result_stats param_stats = { 0 };
709-
710-
kunit_init_test(&test, test_case->name, test_case->log);
711-
if (test_case->status == KUNIT_SKIPPED) {
712-
/* Test marked as skip */
713-
test.status = KUNIT_SKIPPED;
714-
kunit_update_stats(&param_stats, test.status);
715-
} else if (!test_case->generate_params) {
716-
/* Non-parameterised test. */
717-
test_case->status = KUNIT_SKIPPED;
718-
kunit_run_case_catch_errors(suite, test_case, &test);
719-
kunit_update_stats(&param_stats, test.status);
720-
} else {
721-
kunit_init_parent_param_test(test_case, &test);
722-
if (test_case->status == KUNIT_FAILURE) {
723-
kunit_update_stats(&param_stats, test.status);
724-
goto test_case_end;
725-
}
726-
/* Get initial param. */
727-
param_desc[0] = '\0';
728-
/* TODO: Make generate_params try-catch */
729-
curr_param = test_case->generate_params(&test, NULL, param_desc);
730-
test_case->status = KUNIT_SKIPPED;
731-
kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
732-
"KTAP version 1\n");
733-
kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT
734-
"# Subtest: %s", test_case->name);
735-
if (test.params_array.params &&
736-
test_case->generate_params == kunit_array_gen_params) {
737-
kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT
738-
KUNIT_SUBTEST_INDENT "1..%zd\n",
739-
test.params_array.num_params);
740-
}
741-
742-
while (curr_param) {
743-
struct kunit param_test = {
744-
.param_value = curr_param,
745-
.param_index = ++test.param_index,
746-
.parent = &test,
747-
};
748-
kunit_init_test(&param_test, test_case->name, NULL);
749-
param_test.log = test_case->log;
750-
kunit_run_case_catch_errors(suite, test_case, &param_test);
751-
752-
if (param_desc[0] == '\0') {
753-
snprintf(param_desc, sizeof(param_desc),
754-
"param-%d", param_test.param_index);
755-
}
756-
757-
kunit_print_ok_not_ok(&param_test, KUNIT_LEVEL_CASE_PARAM,
758-
param_test.status,
759-
param_test.param_index,
760-
param_desc,
761-
param_test.status_comment);
762-
763-
kunit_update_stats(&param_stats, param_test.status);
764-
765-
/* Get next param. */
766-
param_desc[0] = '\0';
767-
curr_param = test_case->generate_params(&test, curr_param,
768-
param_desc);
769-
}
770-
/*
771-
* TODO: Put into a try catch. Since we don't need suite->exit
772-
* for it we can't reuse kunit_try_run_cleanup for this yet.
773-
*/
774-
if (test_case->param_exit)
775-
test_case->param_exit(&test);
776-
/* TODO: Put this kunit_cleanup into a try-catch. */
777-
kunit_cleanup(&test);
778-
}
779-
test_case_end:
780-
kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE);
781-
782-
kunit_print_test_stats(&test, param_stats);
783-
784-
kunit_print_ok_not_ok(&test, KUNIT_LEVEL_CASE, test_case->status,
785-
kunit_test_case_num(suite, test_case),
786-
test_case->name,
787-
test.status_comment);
788-
789-
kunit_update_stats(&suite_stats, test_case->status);
790-
kunit_accumulate_stats(&total_stats, param_stats);
791-
}
809+
kunit_suite_for_each_test_case(suite, test_case)
810+
kunit_run_one_test(suite, test_case, &suite_stats, &total_stats);
792811

793812
if (suite->suite_exit)
794813
suite->suite_exit(suite);
795814

796-
kunit_print_suite_stats(suite, suite_stats, total_stats);
815+
kunit_print_suite_stats(suite, &suite_stats, &total_stats);
797816
suite_end:
798817
kunit_print_suite_end(suite);
799818

0 commit comments

Comments
 (0)