From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>,
Arnaldo Carvalho de Melo <acme@redhat.com>,
James Clark <james.clark@arm.com>, Jiri Olsa <jolsa@kernel.org>,
Kan Liang <kan.liang@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/1] perf test: Reintroduce -p/--parallel and make -S/--sequential the default
Date: Fri, 26 Apr 2024 22:32:43 -0300 [thread overview]
Message-ID: <ZixVuyDSvTETMvi6@x1> (raw)
In-Reply-To: <CAP-5=fXKsW_cD6=k7yAkNwwdmKqOSMga8Y7o4CwMd+9MOSw0zA@mail.gmail.com>
On Fri, Apr 26, 2024 at 03:19:22PM -0700, Ian Rogers wrote:
> On Fri, Apr 26, 2024 at 3:12 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > We can't default to doing parallel tests as there are tests that compete
> > for the same resources and thus clash, for instance tests that put in
> > place 'perf probe' probes, that clean the probes without regard to other
> > tests needs, ARM64 coresight tests, Intel PT ones, etc.
> >
> > So reintroduce --p/--parallel and make -S/--sequential the default.
> >
> > We need to come up with infrastructure that state which tests can't run
> > in parallel because they need exclusive access to some resource,
> > something as simple as "probes" that would then avoid 'perf probe' tests
> > from running while other such test is running, or make the tests more
> > resilient, till then we can't use parallel mode as default.
> >
> > While at it, document all these options in the 'perf test' man page.
>
> Looks good to me, LKML?
Right, CCed now.
> Reviewed-by: Ian Rogers <irogers@google.com>
Thanks!
- Arnaldo
> Thanks,
> Ian
>
>
> > Reported-by: Adrian Hunter <adrian.hunter@intel.com>
> > Reported-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Reported-by: James Clark <james.clark@arm.com>
> > Acked-by: Ian Rogers <irogers@google.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Link: https://lore.kernel.org/lkml/
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> > tools/perf/Documentation/perf-test.txt | 13 ++++++++++++-
> > tools/perf/tests/builtin-test.c | 8 +++++++-
> > 2 files changed, 19 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/Documentation/perf-test.txt b/tools/perf/Documentation/perf-test.txt
> > index 951a2f2628727a95..9acb8d1f658890e9 100644
> > --- a/tools/perf/Documentation/perf-test.txt
> > +++ b/tools/perf/Documentation/perf-test.txt
> > @@ -31,9 +31,20 @@ OPTIONS
> > --verbose::
> > Be more verbose.
> >
> > +-S::
> > +--sequential::
> > + Run tests one after the other, this is the default mode.
> > +
> > +-p::
> > +--parallel::
> > + Run tests in parallel, speeds up the whole process but is not safe with
> > + the current infrastructure, where some tests that compete for some resources,
> > + for instance, 'perf probe' tests that add/remove probes or clean all probes, etc.
> > +
> > -F::
> > --dont-fork::
> > - Do not fork child for each test, run all tests within single process.
> > + Do not fork child for each test, run all tests within single process, this
> > + sets sequential mode.
> >
> > --dso::
> > Specify a DSO for the "Symbols" test.
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index 73f53b02f7334692..c3d84b67ca8e775d 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -40,7 +40,10 @@
> > */
> > static bool dont_fork;
> > /* Don't fork the tests in parallel and wait for their completion. */
> > -static bool sequential;
> > +static bool sequential = true;
> > +/* Do it in parallel, lacks infrastructure to avoid running tests that clash for resources,
> > + * So leave it as the developers choice to enable while working on the needed infra */
> > +static bool parallel;
> > const char *dso_to_test;
> > const char *test_objdump_path = "objdump";
> >
> > @@ -536,6 +539,7 @@ int cmd_test(int argc, const char **argv)
> > "be more verbose (show symbol address, etc)"),
> > OPT_BOOLEAN('F', "dont-fork", &dont_fork,
> > "Do not fork for testcase"),
> > + OPT_BOOLEAN('p', "parallel", ¶llel, "Run the tests in parallel"),
> > OPT_BOOLEAN('S', "sequential", &sequential,
> > "Run the tests one after another rather than in parallel"),
> > OPT_STRING('w', "workload", &workload, "work", "workload to run for testing"),
> > @@ -566,6 +570,8 @@ int cmd_test(int argc, const char **argv)
> >
> > if (dont_fork)
> > sequential = true;
> > + else if (parallel)
> > + sequential = false;
> >
> > symbol_conf.priv_size = sizeof(int);
> > symbol_conf.try_vmlinux_path = true;
> > --
> > 2.44.0
> >
parent reply other threads:[~2024-04-27 1:32 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <CAP-5=fXKsW_cD6=k7yAkNwwdmKqOSMga8Y7o4CwMd+9MOSw0zA@mail.gmail.com>]
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=ZixVuyDSvTETMvi6@x1 \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=irogers@google.com \
--cc=james.clark@arm.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=namhyung@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).