All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chengdong Li <brytonlee01@gmail.com>
To: adrian.hunter@intel.com, linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org, peterz@infradead.org,
	mingo@redhat.com, acme@kernel.org, mark.rutland@arm.com,
	alexander.shishkin@linux.intel.com, jolsa@redhat.com,
	namhyung@kernel.org
Cc: ak@linux.intel.com, likexu@tencent.com, chengdongli@tencent.com
Subject: [PATCH v4] perf test tsc: Fix error message report when not supported.
Date: Fri,  8 Apr 2022 16:47:48 +0800	[thread overview]
Message-ID: <20220408084748.43707-1-chengdongli@tencent.com> (raw)
In-Reply-To: <CAC2pzGdMFmtV8YNR4DszoATjM80uYNwrnW5As6vgBsyXVcWueA@mail.gmail.com>

From: Chengdong Li <chengdongli@tencent.com>

By default `perf test tsc` does not return the error message
when the child process detected kernel does not support. Instead,
the child process print error message to stderr, unfortunately
the stderr is redirected to /dev/null when verbose <= 0.

This patch did three things:
- returns TEST_SKIP to the parent process instead of TEST_OK when
  perf_read_tsc_conversion() is not supported.
- add a new subtest of testing if TSC is supported on current
  architecture by moving exist code to a separate function.
  It avoids two places in test__perf_time_to_tsc() that return
  TEST_SKIP by doing this.
- extended test suite definition to contain above two subtests.
  Current test_suite and test_case structs do not support printing
  skip reason when the number of subtest less than 1. To print skip
  reason, it is necessary to extend current test suite definition.

Changes since v3:
- refine commit message again to make it clear. 

Changes since v2:
- refine error message format.
- refine commit log message according to Adrian's review comments.

Changes since v1 (thanks for the feedback from Adrian Hunter):
- rebase commit to current source.

Signed-off-by: Chengdong Li <chengdongli@tencent.com>
---
 tools/perf/tests/perf-time-to-tsc.c | 36 +++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
index d12d0ad81801..cc6df49a65a1 100644
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ b/tools/perf/tests/perf-time-to-tsc.c
@@ -47,6 +47,17 @@
 	}					\
 }
 
+static int test__tsc_is_supported(struct test_suite *test __maybe_unused,
+				  int subtest __maybe_unused)
+{
+	if (!TSC_IS_SUPPORTED) {
+		pr_debug("Test not supported on this architecture\n");
+		return TEST_SKIP;
+	}
+
+	return TEST_OK;
+}
+
 /**
  * test__perf_time_to_tsc - test converting perf time to TSC.
  *
@@ -70,7 +81,7 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	struct perf_cpu_map *cpus = NULL;
 	struct evlist *evlist = NULL;
 	struct evsel *evsel = NULL;
-	int err = -1, ret, i;
+	int err = TEST_FAIL, ret, i;
 	const char *comm1, *comm2;
 	struct perf_tsc_conversion tc;
 	struct perf_event_mmap_page *pc;
@@ -79,10 +90,6 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	u64 test_time, comm1_time = 0, comm2_time = 0;
 	struct mmap *md;
 
-	if (!TSC_IS_SUPPORTED) {
-		pr_debug("Test not supported on this architecture");
-		return TEST_SKIP;
-	}
 
 	threads = thread_map__new(-1, getpid(), UINT_MAX);
 	CHECK_NOT_NULL__(threads);
@@ -124,8 +131,8 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	ret = perf_read_tsc_conversion(pc, &tc);
 	if (ret) {
 		if (ret == -EOPNOTSUPP) {
-			fprintf(stderr, " (not supported)");
-			return 0;
+			pr_debug("perf_read_tsc_conversion is not supported in current kernel\n");
+			err = TEST_SKIP;
 		}
 		goto out_err;
 	}
@@ -191,7 +198,7 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	    test_tsc >= comm2_tsc)
 		goto out_err;
 
-	err = 0;
+	err = TEST_OK;
 
 out_err:
 	evlist__delete(evlist);
@@ -200,4 +207,15 @@ static int test__perf_time_to_tsc(struct test_suite *test __maybe_unused, int su
 	return err;
 }
 
-DEFINE_SUITE("Convert perf time to TSC", perf_time_to_tsc);
+static struct test_case time_to_tsc_tests[] = {
+	TEST_CASE_REASON("TSC support", tsc_is_supported,
+			 "This architecture does not support"),
+	TEST_CASE_REASON("Perf time to TSC", perf_time_to_tsc,
+			 "perf_read_tsc_conversion is not supported"),
+	{ .name = NULL, }
+};
+
+struct test_suite suite__perf_time_to_tsc = {
+	.desc = "Convert perf time to TSC",
+	.test_cases = time_to_tsc_tests,
+};
-- 
2.27.0


  reply	other threads:[~2022-04-08  8:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-06 10:06 [PATCH v2] perf test tsc: Fix error message report when not supported Chengdong Li
2022-04-07  6:59 ` Adrian Hunter
2022-04-07 11:34   ` Bryton Lee
2022-04-07 12:34     ` Adrian Hunter
2022-04-08  2:14       ` [PATCH v3] " Chengdong Li
2022-04-08  7:58         ` Adrian Hunter
2022-04-08  8:41           ` Bryton Lee
2022-04-08  8:47             ` Chengdong Li [this message]
2022-04-08  9:16               ` [PATCH v4] " Adrian Hunter
2022-04-09 14:43                 ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220408084748.43707-1-chengdongli@tencent.com \
    --to=brytonlee01@gmail.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=chengdongli@tencent.com \
    --cc=jolsa@redhat.com \
    --cc=likexu@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.