Commit b11b9b67 authored by Arnd Bergmann's avatar Arnd Bergmann Committed by Shuah Khan
Browse files

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 <skhan@linuxfoundation.org>

Cc: Carlos Llamas <cmllamas@google.com>
Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
Reviewed-by: default avatarDavid Gow <davidgow@google.com>
Signed-off-by: default avatarShuah Khan <skhan@linuxfoundation.org>
parent 40804c49

lib/kunit/test.c

+125 −106
Original line number Diff line number Diff line
@@ -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,69 +681,42 @@ 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];

	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);



	if (suite->suite_init) {

		suite->suite_init_err = suite->suite_init(suite);

		if (suite->suite_init_err) {

			kunit_err(suite, KUNIT_SUBTEST_INDENT

				  "# failed to initialize (%d)", suite->suite_init_err);

			goto suite_end;

		}

	}



	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(&param_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(&param_stats, test.status);

		} else {

			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);

				goto test_case_end;

		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);

	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

	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

	kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT KUNIT_SUBTEST_INDENT

		  "# Subtest: %s", test_case->name);

			if (test.params_array.params &&

	if (test->params_array.params &&

	    test_case->generate_params == kunit_array_gen_params) {

				kunit_log(KERN_INFO, &test, KUNIT_SUBTEST_INDENT

		kunit_log(KERN_INFO, test, KUNIT_SUBTEST_INDENT

			  KUNIT_SUBTEST_INDENT "1..%zd\n",

					  test.params_array.num_params);

			  test->params_array.num_params);

	}



	while (curr_param) {

		struct kunit param_test = {

			.param_value = curr_param,

					.param_index = ++test.param_index,

					.parent = &test,

			.param_index = ++test->param_index,

			.parent = test,

		};

		kunit_init_test(&param_test, test_case->name, NULL);

		param_test.log = test_case->log;
@@ -760,11 +733,11 @@ int kunit_run_tests(struct kunit_suite *suite) 
				      param_desc,

				      param_test.status_comment);



				kunit_update_stats(&param_stats, param_test.status);

		kunit_update_stats(param_stats, param_test.status);



		/* Get next param. */

		param_desc[0] = '\0';

				curr_param = test_case->generate_params(&test, curr_param,

		curr_param = test_case->generate_params(test, curr_param,

							param_desc);

	}

	/*
@@ -772,28 +745,74 @@ int kunit_run_tests(struct kunit_suite *suite) 
	 * for it we can't reuse kunit_try_run_cleanup for this yet.

	 */

	if (test_case->param_exit)

				test_case->param_exit(&test);

		test_case->param_exit(test);

	/* TODO: Put this kunit_cleanup into a try-catch. */

			kunit_cleanup(&test);

	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(&param_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(&param_stats, test.status);

	} else {

		kunit_run_param_test(suite, test_case, &test, suite_stats,

				     total_stats, &param_stats);

	}

test_case_end:

	kunit_print_attr((void *)test_case, true, KUNIT_LEVEL_CASE);



		kunit_print_test_stats(&test, param_stats);

	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_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 };



	/* Taint the kernel so we know we've run tests. */

	add_taint(TAINT_TEST, LOCKDEP_STILL_OK);



	if (suite->suite_init) {

		suite->suite_init_err = suite->suite_init(suite);

		if (suite->suite_init_err) {

			kunit_err(suite, KUNIT_SUBTEST_INDENT

				  "# failed to initialize (%d)", suite->suite_init_err);

			goto suite_end;

		}

	}



	kunit_print_suite_start(suite);



	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);