[v3,5/5] selftests/resctrl: Remove duplicate codes that clear each test result file
Message ID | 20221101094341.3383073-6-tan.shaopeng@jp.fujitsu.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2857600wru; Tue, 1 Nov 2022 03:03:31 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5p+jxpvgl+p7q47XE33+x/imEqaZpAlNsNlIH3zlwS8/kdmD8+7tJdYCbxoR4TYaqCSRJ9 X-Received: by 2002:a63:5b48:0:b0:458:1e98:c862 with SMTP id l8-20020a635b48000000b004581e98c862mr2094727pgm.568.1667297010720; Tue, 01 Nov 2022 03:03:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667297010; cv=none; d=google.com; s=arc-20160816; b=vzKhf5u/IbjyZkX8oe+Y7/nxLVjL/a6Lmh7y5TLwgpR00uVXprqGD7+6wIwChSCRWV 2b8zo9U9CB88wM9ex8PJ1fIiPWIGSOgcgHDca2nVtHSyHERprxJcch7oBYK37VmmGHXf CrU/0sFjJ0Vu2wKyr2WNBkNfOGYrWw1kLJY5DFA0FKhd4sYT5qOLhFjLGDL2N3ue95Hk EFy5pni6k3Mpy5hm7QU7zqmqWx1e3bwx5OiDXcFKTzkaqfrE+vZc1sBplbcSVnGzCxvj Isy2xuF4HuGecD+NBYTJzJWLy+Y0aPCjDQTDMjmfGv4tMrCauE1NvXz65KAul19kZ1iE JQsg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=3C1gK/22v5YDqZWSd0IDSg9WTdwIcYRZJRjzwsj70z8=; b=q8hb0XhETYKC26hBzdbS5mrtRL7ijNIxIBT2JKR0xhC9ko3/ElYZRp/UsHeo2VDvG5 2hd6dbdv6HB+xFy+ImVv9TsGbHqRZPgWJ7Kv6OWLWMvhtaRo/xP1paAn8yC9UEKDxkLJ nhmIhqig8XygSSlDwzAMq8P01dThZ3cpbSPqGpo8Yk1miE4v3sBBndgGsmG/UiEpuEra 4xaPEFwEHvatSohEWxoCKSjINp4NeJVM2Ryo/t9sRh0iqTt0RjeQPx+tQCvQCJwyUK4H dhdJiRCf7dZJooq5YrE/BZV/ZSIII02bCFdRMoCNxDZFd6YqiEuMVrQRCS8hEuUA0Z05 V2hQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=fujitsu.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s11-20020a63d04b000000b0046b322237c3si12465232pgi.804.2022.11.01.03.03.16; Tue, 01 Nov 2022 03:03:30 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=fujitsu.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230468AbiKAJta (ORCPT <rfc822;kartikey406@gmail.com> + 99 others); Tue, 1 Nov 2022 05:49:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55610 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229511AbiKAJsy (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 1 Nov 2022 05:48:54 -0400 Received: from esa2.hc1455-7.c3s2.iphmx.com (esa2.hc1455-7.c3s2.iphmx.com [207.54.90.48]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4E72E1902A; Tue, 1 Nov 2022 02:48:53 -0700 (PDT) X-IronPort-AV: E=McAfee;i="6500,9779,10517"; a="94348500" X-IronPort-AV: E=Sophos;i="5.95,230,1661785200"; d="scan'208";a="94348500" Received: from unknown (HELO yto-r2.gw.nic.fujitsu.com) ([218.44.52.218]) by esa2.hc1455-7.c3s2.iphmx.com with ESMTP; 01 Nov 2022 18:48:51 +0900 Received: from yto-m2.gw.nic.fujitsu.com (yto-nat-yto-m2.gw.nic.fujitsu.com [192.168.83.65]) by yto-r2.gw.nic.fujitsu.com (Postfix) with ESMTP id F2A59D63B6; Tue, 1 Nov 2022 18:48:49 +0900 (JST) Received: from oym-om4.fujitsu.com (oym-om4.o.css.fujitsu.com [10.85.58.164]) by yto-m2.gw.nic.fujitsu.com (Postfix) with ESMTP id 587B1D35F3; Tue, 1 Nov 2022 18:48:49 +0900 (JST) Received: from cn-r05-10.example.com (n3235113.np.ts.nmh.cs.fujitsu.co.jp [10.123.235.113]) by oym-om4.fujitsu.com (Postfix) with ESMTP id 376CF40164011; Tue, 1 Nov 2022 18:48:49 +0900 (JST) From: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> To: Fenghua Yu <fenghua.yu@intel.com>, Reinette Chatre <reinette.chatre@intel.com>, Shuah Khan <shuah@kernel.org> Cc: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, tan.shaopeng@jp.fujitsu.com Subject: [PATCH v3 5/5] selftests/resctrl: Remove duplicate codes that clear each test result file Date: Tue, 1 Nov 2022 18:43:41 +0900 Message-Id: <20221101094341.3383073-6-tan.shaopeng@jp.fujitsu.com> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20221101094341.3383073-1-tan.shaopeng@jp.fujitsu.com> References: <20221101094341.3383073-1-tan.shaopeng@jp.fujitsu.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-TM-AS-GCONF: 00 X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1748287630651719317?= X-GMAIL-MSGID: =?utf-8?q?1748287630651719317?= |
Series |
Some improvements of resctrl selftest
|
|
Commit Message
Shaopeng Tan
Nov. 1, 2022, 9:43 a.m. UTC
Before exiting each test function(run_cmt/cat/mbm/mba_test()),
test results("ok","not ok") are printed by ksft_test_result() and then
temporary result files are cleaned by function
cmt/cat/mbm/mba_test_cleanup().
However, before running ksft_test_result(),
function cmt/cat/mbm/mba_test_cleanup()
has been run in each test function as follows:
cmt_resctrl_val()
cat_perf_miss_val()
mba_schemata_change()
mbm_bw_change()
Remove duplicate codes that clear each test result file.
Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
---
tools/testing/selftests/resctrl/resctrl_tests.c | 4 ----
1 file changed, 4 deletions(-)
Comments
On 11/1/22 03:43, Shaopeng Tan wrote: > Before exiting each test function(run_cmt/cat/mbm/mba_test()), > test results("ok","not ok") are printed by ksft_test_result() and then > temporary result files are cleaned by function > cmt/cat/mbm/mba_test_cleanup(). > However, before running ksft_test_result(), > function cmt/cat/mbm/mba_test_cleanup() > has been run in each test function as follows: > cmt_resctrl_val() > cat_perf_miss_val() > mba_schemata_change() > mbm_bw_change() > > Remove duplicate codes that clear each test result file. This isn't making much sense to me. Please include test report before and after this change in the change log. > > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> > --- thanks, -- Shuah
Hi Shaopeng, On 11/1/2022 2:43 AM, Shaopeng Tan wrote: > Before exiting each test function(run_cmt/cat/mbm/mba_test()), > test results("ok","not ok") are printed by ksft_test_result() and then > temporary result files are cleaned by function > cmt/cat/mbm/mba_test_cleanup(). > However, before running ksft_test_result(), > function cmt/cat/mbm/mba_test_cleanup() > has been run in each test function as follows: > cmt_resctrl_val() > cat_perf_miss_val() > mba_schemata_change() > mbm_bw_change() > > Remove duplicate codes that clear each test result file. > > Signed-off-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com> > --- > tools/testing/selftests/resctrl/resctrl_tests.c | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c > index df0d8d8526fc..8732cf736528 100644 > --- a/tools/testing/selftests/resctrl/resctrl_tests.c > +++ b/tools/testing/selftests/resctrl/resctrl_tests.c > @@ -88,7 +88,6 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span, > ksft_test_result(!res, "MBM: bw change\n"); > if ((get_vendor() == ARCH_INTEL) && res) > ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); > - mbm_test_cleanup(); > } > From what I can tell this still seem to suffer from the problem where the test files may not be cleaned. With the removal of mbm_test_cleanup() the cleanup is now expected to be done in mbm_bw_change(). Note that: mbm_bw_change() { ... ret = resctrl_val(benchmark_cmd, ¶m); if (ret) return ret; /* Test results stored in file */ ret = check_results(span); if (ret) return ret; <== Return without cleaning test result file mbm_test_cleanup(); <== Test result file cleaned only when test passed. return 0; } > static void run_mba_test(bool has_ben, char **benchmark_cmd, int span, > @@ -107,7 +106,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, int span, > sprintf(benchmark_cmd[1], "%d", span); > res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd); > ksft_test_result(!res, "MBA: schemata change\n"); > - mba_test_cleanup(); > } mba_schemata_change() has the same pattern as mbm_bw_change() so test result files may linger when test fails. > > static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no) > @@ -126,7 +124,6 @@ static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no) > ksft_test_result(!res, "CMT: test\n"); > if ((get_vendor() == ARCH_INTEL) && res) > ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); > - cmt_test_cleanup(); > } Same pattern again. > > static void run_cat_test(int cpu_no, int no_of_bits) > @@ -142,7 +139,6 @@ static void run_cat_test(int cpu_no, int no_of_bits) > > res = cat_perf_miss_val(cpu_no, no_of_bits, "L3"); > ksft_test_result(!res, "CAT: test\n"); > - cat_test_cleanup(); > } Patch 4 makes this work. Thanks for doing that. Reinette
Hi Shuah and Reinette, > On 11/1/2022 2:43 AM, Shaopeng Tan wrote: > > Before exiting each test function(run_cmt/cat/mbm/mba_test()), > > test results("ok","not ok") are printed by ksft_test_result() and then > > temporary result files are cleaned by function > > cmt/cat/mbm/mba_test_cleanup(). > > However, before running ksft_test_result(), function > > cmt/cat/mbm/mba_test_cleanup() has been run in each test function as > > follows: > > cmt_resctrl_val() > > cat_perf_miss_val() > > mba_schemata_change() > > mbm_bw_change() > > > > Remove duplicate codes that clear each test result file. > > This isn't making much sense to me. Please include test report before and after > this change in the change log. With or without this patch, there is no effect on the result message. These functions were executed twice, in brief, it runs as follows: - cmt/cat/mbm/mba_test_cleanup() - ksft_test_result() - cmt/cat/mbm/mba_test_cleanup() So, I deleted once. > From what I can tell this still seem to suffer from the problem where the test > files may not be cleaned. With the removal of mbm_test_cleanup() the cleanup > is now expected to be done in mbm_bw_change(). > > Note that: > > mbm_bw_change() > { > ... > > ret = resctrl_val(benchmark_cmd, ¶m); > if (ret) > return ret; > > /* Test results stored in file */ > > ret = check_results(span); > if (ret) > return ret; <== Return without cleaning test result file > > mbm_test_cleanup(); <== Test result file cleaned only when test > passed. > > return 0; > } I intend to avoid this problem through the following codes. mbm_bw_change() { ret = resctrl_val(benchmark_cmd, ¶m); if (ret) - return ret; + goto out; ret = check_results(span); if (ret) - return ret; + goto out; +out: mbm_test_cleanup(); - return 0; + return ret; } Best regards, Shaopeng Tan
Hi Shaopeng, On 11/8/2022 12:32 AM, Shaopeng Tan (Fujitsu) wrote: > Hi Shuah and Reinette, > >> On 11/1/2022 2:43 AM, Shaopeng Tan wrote: >>> Before exiting each test function(run_cmt/cat/mbm/mba_test()), >>> test results("ok","not ok") are printed by ksft_test_result() and then >>> temporary result files are cleaned by function >>> cmt/cat/mbm/mba_test_cleanup(). >>> However, before running ksft_test_result(), function >>> cmt/cat/mbm/mba_test_cleanup() has been run in each test function as >>> follows: >>> cmt_resctrl_val() >>> cat_perf_miss_val() >>> mba_schemata_change() >>> mbm_bw_change() >>> >>> Remove duplicate codes that clear each test result file. >> >> This isn't making much sense to me. Please include test report before and after >> this change in the change log. > > With or without this patch, there is no effect on the result message. > These functions were executed twice, in brief, it runs as follows: > - cmt/cat/mbm/mba_test_cleanup() > - ksft_test_result() > - cmt/cat/mbm/mba_test_cleanup() > So, I deleted once. > >> From what I can tell this still seem to suffer from the problem where the test >> files may not be cleaned. With the removal of mbm_test_cleanup() the cleanup >> is now expected to be done in mbm_bw_change(). >> >> Note that: >> >> mbm_bw_change() >> { >> ... >> >> ret = resctrl_val(benchmark_cmd, ¶m); >> if (ret) >> return ret; >> >> /* Test results stored in file */ >> >> ret = check_results(span); >> if (ret) >> return ret; <== Return without cleaning test result file >> >> mbm_test_cleanup(); <== Test result file cleaned only when test >> passed. >> >> return 0; >> } > > I intend to avoid this problem through the following codes. > > mbm_bw_change() > { > ret = resctrl_val(benchmark_cmd, ¶m); > if (ret) > - return ret; > + goto out; > > ret = check_results(span); > if (ret) > - return ret; > + goto out; > > +out: > mbm_test_cleanup(); > > - return 0; > + return ret; > } > Yes, even though file removal may now encounter ENOENT this does seem the most robust route and the possible error is ok since mbm_test_cleanup() does not check the return code. Could you please replicate this pattern to the other functions (mba_schemata_change() and cmt_resctrl_val()) also? Reinette
Hi Reinette, > On 11/8/2022 12:32 AM, Shaopeng Tan (Fujitsu) wrote: > > Hi Shuah and Reinette, > > > >> On 11/1/2022 2:43 AM, Shaopeng Tan wrote: > >>> Before exiting each test function(run_cmt/cat/mbm/mba_test()), > >>> test results("ok","not ok") are printed by ksft_test_result() and > >>> then temporary result files are cleaned by function > >>> cmt/cat/mbm/mba_test_cleanup(). > >>> However, before running ksft_test_result(), function > >>> cmt/cat/mbm/mba_test_cleanup() has been run in each test function as > >>> follows: > >>> cmt_resctrl_val() > >>> cat_perf_miss_val() > >>> mba_schemata_change() > >>> mbm_bw_change() > >>> > >>> Remove duplicate codes that clear each test result file. > >> > >> This isn't making much sense to me. Please include test report before > >> and after this change in the change log. > > > > With or without this patch, there is no effect on the result message. > > These functions were executed twice, in brief, it runs as follows: > > - cmt/cat/mbm/mba_test_cleanup() > > - ksft_test_result() > > - cmt/cat/mbm/mba_test_cleanup() > > So, I deleted once. > > > >> From what I can tell this still seem to suffer from the problem where > >> the test files may not be cleaned. With the removal of > >> mbm_test_cleanup() the cleanup is now expected to be done in > mbm_bw_change(). > >> > >> Note that: > >> > >> mbm_bw_change() > >> { > >> ... > >> > >> ret = resctrl_val(benchmark_cmd, ¶m); > >> if (ret) > >> return ret; > >> > >> /* Test results stored in file */ > >> > >> ret = check_results(span); > >> if (ret) > >> return ret; <== Return without cleaning test result file > >> > >> mbm_test_cleanup(); <== Test result file cleaned only when test > >> passed. > >> > >> return 0; > >> } > > > > I intend to avoid this problem through the following codes. > > > > mbm_bw_change() > > { > > ret = resctrl_val(benchmark_cmd, ¶m); > > if (ret) > > - return ret; > > + goto out; > > > > ret = check_results(span); > > if (ret) > > - return ret; > > + goto out; > > > > +out: > > mbm_test_cleanup(); > > > > - return 0; > > + return ret; > > } > > > > Yes, even though file removal may now encounter ENOENT this does seem the > most robust route and the possible error is ok since mbm_test_cleanup() does > not check the return code. > Could you please replicate this pattern to the other functions > (mba_schemata_change() and cmt_resctrl_val()) also? This is an example for MBM, I intended to replicate this pattern to other tests. Best regard, Shaopeng Tan
diff --git a/tools/testing/selftests/resctrl/resctrl_tests.c b/tools/testing/selftests/resctrl/resctrl_tests.c index df0d8d8526fc..8732cf736528 100644 --- a/tools/testing/selftests/resctrl/resctrl_tests.c +++ b/tools/testing/selftests/resctrl/resctrl_tests.c @@ -88,7 +88,6 @@ static void run_mbm_test(bool has_ben, char **benchmark_cmd, int span, ksft_test_result(!res, "MBM: bw change\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel MBM may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); - mbm_test_cleanup(); } static void run_mba_test(bool has_ben, char **benchmark_cmd, int span, @@ -107,7 +106,6 @@ static void run_mba_test(bool has_ben, char **benchmark_cmd, int span, sprintf(benchmark_cmd[1], "%d", span); res = mba_schemata_change(cpu_no, bw_report, benchmark_cmd); ksft_test_result(!res, "MBA: schemata change\n"); - mba_test_cleanup(); } static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no) @@ -126,7 +124,6 @@ static void run_cmt_test(bool has_ben, char **benchmark_cmd, int cpu_no) ksft_test_result(!res, "CMT: test\n"); if ((get_vendor() == ARCH_INTEL) && res) ksft_print_msg("Intel CMT may be inaccurate when Sub-NUMA Clustering is enabled. Check BIOS configuration.\n"); - cmt_test_cleanup(); } static void run_cat_test(int cpu_no, int no_of_bits) @@ -142,7 +139,6 @@ static void run_cat_test(int cpu_no, int no_of_bits) res = cat_perf_miss_val(cpu_no, no_of_bits, "L3"); ksft_test_result(!res, "CAT: test\n"); - cat_test_cleanup(); } int main(int argc, char **argv)