From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f180.google.com (mail-pl1-f180.google.com [209.85.214.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 549F51E9B3D for ; Sun, 25 Jan 2026 18:54:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.180 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769367263; cv=none; b=UbYg/8a5Dcb6atpn+gazJKeBZnWQEePzHgbMXW87n47sChlATNbBJJc0hQ+Hhk4bF4aWCcNr2no++kRAY126x2DmdvA6gDtfct/iWaq9lrrq6smBY3dTeHRtmiDmt+BOu/eOdWqHXXZDQ5O9bbrtDIzvAsCiJ45/M9FaIYbnNMM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769367263; c=relaxed/simple; bh=MNx7CpVPfK4SIp6L5qZvc4IDpYVFgEwS7rEMUXt6xuE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=VIUcBQ/pZq988xnv2Gx2Bg1KNcN7vnxxqAyynCJXoRWpkNog+73rgCWfhss0YmoJcJNeteeEvX4pnRHC7fRSpn1Z5cCoN7miaAFrao1Ydedz+wjK22tKQEBCWouc1Y3TaHAvkldzjdjuDlwUZhb/Ut/B2wvGC4L2BqVLgRhFMfk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=qS9l5kyW; arc=none smtp.client-ip=209.85.214.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="qS9l5kyW" Received: by mail-pl1-f180.google.com with SMTP id d9443c01a7336-2a35ae38bdfso87045ad.1 for ; Sun, 25 Jan 2026 10:54:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1769367262; x=1769972062; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bJwaR/zGofB89ztFQf0PisW/NqMpT7fZXv1USedVEw8=; b=qS9l5kyWJyOguTlDDoSHkXs/uHwTFPunwt4K9gcQYSTxNZANAcQsZaifrlQOtTIl8m TjnS+FFi7fpRYTrCZDVQMKdMoTIJy0S26GJg+L3cvpqOedVdtazS2xWNWgpP3hccGAnU eTNI80X7z0krS/6ih2gk2mJhtZkyDqBckXfvcZXjjJr6d5LjYQdV8ZGUFCsVrwjP1pXW KIWq988JC1qmucGSvnWhykpbu7IkWsWqekpMOxQ3JUP4WUtLwipkzmbn5EoF2Sxd/VJG PGbOQILXrRWTEqVlG8h5HHwRaeITTo9GopC3DiJe412R+lYCeXbjIwZm9CayKZRQWJo9 n4Gw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1769367262; x=1769972062; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bJwaR/zGofB89ztFQf0PisW/NqMpT7fZXv1USedVEw8=; b=TmO3udKjY5vuu6dd1Zts2EzpA8b94Teezi8sPdYejnAeNAKK0yw1665DE5FjerzWbn stV8EYBFxqBYPACZC8MvNba1S5IP4jmg9DBb/mWpmP5a88Vvm4w9ZeSEDJ8ftVd9lu9Q gVTZpfgdAeMknBP4NtcXEnrfIIiRRRyJ888r0PSJQFu9Li8W2SmIH3njR1DnU2RJekjV xNycG1S3seh/FeMqMDPSjPUdUpOYgiQ1F9thoyQiCJT0k2a5r/dqyGCAHkGxsJ5nvVIK t6wniIiCVfHFQevS4prFeve/jVKWoAm7BCYXeViPYHOBRdEbB63+/BiU5QuW5jUKEfV7 Br7A== X-Forwarded-Encrypted: i=1; AJvYcCV9l/Y43TrpUhzaEl1RdSZwKPK7Da+MyYtxItctKBu7GM4xeroOFCxEXZCBEbzlwJRnscUlaQTxlEd2JzU=@vger.kernel.org X-Gm-Message-State: AOJu0YytQ5DTUxR83dxmaCepkKu5o+fG+9eRjvSSsy35g9yrEFcpbjax 2xXfO1SdDuPLG55sNqEo/it9oHgEK3vYp93iEAdcAehjj5xrCa/MWhskhkA+Fe6+jg== X-Gm-Gg: AZuq6aIU/c9JGtvYATiKXC0a9lCiOU4ruOXOH425008KieBu2oPvJrwHN3M2lK1ElQl u1QAMV///JXdcqae1ET5ic5Nt0qCVSKQJvBD2jB1wc3UcAo13qud7UsKMcZs3JdhJAxr65U/4pT Fyew11tyYEDSfS15ZRVO5OpYO+cmNIggzwQk8dUTdom7uZGmxRJEVcP26MqnLv5AIZ8nkDGpout RGLNKopecTXVECcWQ/5Gz9bY6C+YLP0YEpHrbA/3G/4EEYsE1hI+Jmt8Kcq4FO1E8kCU+1b7mCY zXPJSrEjv6ZoMTQxMBOYnHlH0RVlyJt8rRzsKf3m5+R5NMWYrgdMi6S4P/aPl7wtIdSRpK3WCVI UblXXESPYV+oqZH9VpZLRPZUtATKKuwhMWP/Z7iiVJl05y03oJaqwxWkTitJhseMNLneRaZlBei jFoS21gFV33OCilcVqya7wwpYROTRb/GK03qCCUF7Xp5AszHBBlKY= X-Received: by 2002:a17:902:f788:b0:290:8ecf:e9f9 with SMTP id d9443c01a7336-2a844b04233mr923075ad.7.1769367261004; Sun, 25 Jan 2026 10:54:21 -0800 (PST) Received: from google.com (28.117.125.34.bc.googleusercontent.com. [34.125.117.28]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82318644885sm7489927b3a.4.2026.01.25.10.54.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 25 Jan 2026 10:54:20 -0800 (PST) Date: Sun, 25 Jan 2026 18:54:15 +0000 From: Carlos Llamas To: Arnd Bergmann Cc: Brendan Higgins , David Gow , Arnd Bergmann , Rae Moar , Shuah Khan , Marie Zhussupova , Daniel Gomez , Stanislav Kinsburskii , Thomas =?iso-8859-1?Q?Wei=DFschuh?= , Ujwal Jain , linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] kunit: reduce stack usage in kunit_run_tests() Message-ID: References: <20251204101050.1035801-1-arnd@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251204101050.1035801-1-arnd@kernel.org> On Thu, Dec 04, 2025 at 11:10:46AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > 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. > > Signed-off-by: Arnd Bergmann > --- > Sending it now as I ran into the build failure while testing 6.18. > I only build-tested this, so please test it properly before applying. I just ran this through our CI with no problems. Tested-by: Carlos Llamas > --- > lib/kunit/test.c | 221 ++++++++++++++++++++++++----------------------- > 1 file changed, 115 insertions(+), 106 deletions(-) > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 62eb529824c6..c5fce199d880 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,106 @@ static void kunit_init_parent_param_test(struct kunit_case *test_case, struct ku > } > } > > -int kunit_run_tests(struct kunit_suite *suite) > +static int 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 }; > char param_desc[KUNIT_PARAM_DESC_SIZE]; > + const void *curr_param; > + > + 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, ¶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); > + > + return 0; nit: the return value is always zero and it's not being used either, so maybe switch to void? > +} > + > + > +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 +796,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); > > -- I don't see any logical changes, everything seems to have been extracted cleanly into kunit_run_one_test(). So this LGTM, just a minor nit. -- Carlos LLamas