| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAC2pzGdMFmtV8YNR4DszoATjM80uYNwrnW5As6vgBsyXVcWueA@mail.gmail.com>
Date: Fri, 8 Apr 2022 16:41:43 +0800
From: Bryton Lee <brytonlee01@...il.com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
peterz@...radead.org, mingo@...hat.com,
Arnaldo Carvalho de Melo <acme@...nel.org>,
mark.rutland@....com, alexander.shishkin@...ux.intel.com,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>, ak@...ux.intel.com,
likexu@...cent.com, chengdongli@...cent.com
Subject: Re: [PATCH v3] perf test tsc: Fix error message report when not supported.
On Fri, Apr 8, 2022 at 3:58 PM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> On 08/04/2022 5.14, Chengdong Li wrote:
> > 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 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@...cent.com>
>
> It is better to try to consistently use the same email address
> for the same person for git logs, unless you have a sensible
> reason not to.
>
Sorry for the inconvenience. I can not use my company email account to send
patches via `git send-email` due to lack of email protocol support in
our email system.
> So in this case it looks like you should use the same email
> address for the "From:" and "Signed-off-by:". Note you can
> optionally put the "From:" line before the commit message e.g.
>
> From: Chengdong Li <chengdongli@...cent.com>
>
> By default `perf test tsc` does not return the error message
> ...
Thanks for your guidance, I will resend this patch again.
>
> Otherwise you can add:
>
> Reviewed-by: Adrian Hunter <adrian.hunter@...el.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,
> > +};
>
--
Best Regards
Bryton.Lee
Powered by blists - more mailing lists