| 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: <CAP-5=fXrQS=VKULkhtXjqS7Bw4CHmhNVhVPq+tV8cP=gBgcrYA@mail.gmail.com>
Date: Fri, 28 Jun 2024 15:20:13 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Kan Liang <kan.liang@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH] perf test: Print test description first
On Fri, Jun 28, 2024 at 2:57 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Currently perf test prints the description of the test when it finishes
> to run the test. But sometimes it runs long and users want to see which
> test it is running. Change it to print the info in start_test() so that
> we can see it right after the test.
>
> No functional changes intended.
The issue is around parallel testing. I sent out these similar patches
that displayed a number of remaining tests in parallel:
https://lore.kernel.org/lkml/20240405070931.1231245-1-irogers@google.com/
My thoughts are:
1) we should create a list of parallel safe tests and always just run
those in parallel before doing serial tests. Things like testing all
PMU events, which may fail due to EBUSY, can be on that list until we
add some logic to spin for EBUSY.
2) the series above doesn't work because the waitpid with WNOHANG will
cause the file output from the command to be lost if the command has
terminated. We need to buffer that output by calling read prior to
calling waitpid, so we really need some memory associated with the
run_command to read into - maybe this isn't run_command any more but a
new abstraction that takes and creates strings.
3) why is the test run serially so slow? Can we change "sleep 1" to
"sleep 0.1" or similar.
Thanks,
Ian
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/tests/builtin-test.c | 54 ++++++++++++++++++---------------
> 1 file changed, 30 insertions(+), 24 deletions(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index c3d84b67ca8e..33defd3b7185 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -243,12 +243,17 @@ static int run_test_child(struct child_process *process)
>
> static int print_test_result(struct test_suite *t, int i, int subtest, int result, int width)
> {
> - if (has_subtests(t)) {
> - int subw = width > 2 ? width - 2 : width;
> + /* If it printed other messages, print the header again. */
> + if (verbose > 1 || (verbose == 1 && result == TEST_FAIL)) {
> + if (has_subtests(t)) {
> + int subw = width > 2 ? width - 2 : width;
>
> - pr_info("%3d.%1d: %-*s:", i + 1, subtest + 1, subw, test_description(t, subtest));
> - } else
> - pr_info("%3d: %-*s:", i + 1, width, test_description(t, subtest));
> + pr_info("%3d.%1d: %-*s:", i + 1, subtest + 1, subw,
> + test_description(t, subtest));
> + } else {
> + pr_info("%3d: %-*s:", i + 1, width, test_description(t, -1));
> + }
> + }
>
> switch (result) {
> case TEST_OK:
> @@ -282,25 +287,12 @@ static int finish_test(struct child_test *child_test, int width)
> struct strbuf err_output = STRBUF_INIT;
> int ret;
>
> - /*
> - * For test suites with subtests, display the suite name ahead of the
> - * sub test names.
> - */
> - if (has_subtests(t) && subi == 0)
> - pr_info("%3d: %-*s:\n", i + 1, width, test_description(t, -1));
> -
> /*
> * Busy loop reading from the child's stdout/stderr that are set to be
> * non-blocking until EOF.
> */
> if (!err_done)
> fcntl(err, F_SETFL, O_NONBLOCK);
> - if (verbose > 1) {
> - if (has_subtests(t))
> - pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi));
> - else
> - pr_info("%3d: %s:\n", i + 1, test_description(t, -1));
> - }
> while (!err_done) {
> struct pollfd pfds[1] = {
> { .fd = err,
> @@ -330,12 +322,8 @@ static int finish_test(struct child_test *child_test, int width)
> /* Clean up child process. */
> ret = finish_command(&child_test->process);
> if (verbose == 1 && ret == TEST_FAIL) {
> - /* Add header for test that was skipped above. */
> - if (has_subtests(t))
> - pr_info("%3d.%1d: %s:\n", i + 1, subi + 1, test_description(t, subi));
> - else
> - pr_info("%3d: %s:\n", i + 1, test_description(t, -1));
> - fprintf(stderr, "%s", err_output.buf);
> + /* It printed the header first, print messages in a new line */
> + fprintf(stderr, "\n%s", err_output.buf);
> }
> strbuf_release(&err_output);
> print_test_result(t, i, subi, ret, width);
> @@ -376,6 +364,24 @@ static int start_test(struct test_suite *test, int i, int subi, struct child_tes
> (*child)->process.err = -1;
> }
> (*child)->process.no_exec_cmd = run_test_child;
> +
> + if (has_subtests(test)) {
> + int subw = width > 2 ? width - 2 : width;
> +
> + /*
> + * For test suites with subtests, display the suite name ahead of the
> + * sub test names.
> + */
> + if (has_subtests(test) && subi == 0)
> + pr_info("%3d: %-*s:\n", i + 1, width, test_description(test, -1));
> +
> + pr_info("%3d.%1d: %-*s:", i + 1, subi + 1, subw, test_description(test, subi));
> + } else
> + pr_info("%3d: %-*s:", i + 1, width, test_description(test, -1));
> +
> + if (verbose > 1)
> + pr_info("\n");
> +
> err = start_command(&(*child)->process);
> if (err || !sequential)
> return err;
> --
> 2.45.2.803.g4e1b14247a-goog
>
Powered by blists - more mailing lists