Messages in this thread | ![/](/images/icornerl.gif) | | From | "Wang, Weilin" <> | Subject | RE: [RFC PATCH v8 3/7] perf stat: Fork and launch perf record when perf stat needs to get retire latency value for a metric. | Date | Fri, 17 May 2024 21:57:52 +0000 |
| |
> -----Original Message----- > From: Namhyung Kim <namhyung@kernel.org> > Sent: Friday, May 17, 2024 2:43 PM > To: Wang, Weilin <weilin.wang@intel.com> > Cc: Ian Rogers <irogers@google.com>; Arnaldo Carvalho de Melo > <acme@kernel.org>; Peter Zijlstra <peterz@infradead.org>; Ingo Molnar > <mingo@redhat.com>; Alexander Shishkin > <alexander.shishkin@linux.intel.com>; Jiri Olsa <jolsa@kernel.org>; Hunter, > Adrian <adrian.hunter@intel.com>; Kan Liang <kan.liang@linux.intel.com>; > linux-perf-users@vger.kernel.org; linux-kernel@vger.kernel.org; Taylor, Perry > <perry.taylor@intel.com>; Alt, Samantha <samantha.alt@intel.com>; Biggers, > Caleb <caleb.biggers@intel.com> > Subject: Re: [RFC PATCH v8 3/7] perf stat: Fork and launch perf record when > perf stat needs to get retire latency value for a metric. > > On Tue, May 14, 2024 at 10:44 PM <weilin.wang@intel.com> wrote: > > > > From: Weilin Wang <weilin.wang@intel.com> > > > > When retire_latency value is used in a metric formula, perf stat would fork a > > perf record process with "-e" and "-W" options. Perf record will collect > > required retire_latency values in parallel while perf stat is collecting > > counting values. > > > > At the point of time that perf stat stops counting, it would send sigterm > signal > > to perf record process and receiving sampling data back from perf record > from a > > pipe. Perf stat will then process the received data to get retire latency data > > and calculate metric result. > > > > Another thread is required to synchronize between perf stat and perf record > > when we pass data through pipe. > > > > Signed-off-by: Weilin Wang <weilin.wang@intel.com> > > Reviewed-by: Ian Rogers <irogers@google.com> > > --- > [SNIP] > > diff --git a/tools/perf/util/intel-tpebs.c b/tools/perf/util/intel-tpebs.c > > new file mode 100644 > > index 000000000000..4b7a98794fae > > --- /dev/null > > +++ b/tools/perf/util/intel-tpebs.c > > @@ -0,0 +1,285 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * intel_pt.c: Intel Processor Trace support > > + * Copyright (c) 2013-2015, Intel Corporation. > > This needs some updates. :) > > > > + */ > > + > > + > > +#include <sys/param.h> > > +#include <subcmd/run-command.h> > > +#include <thread.h> > > +#include "intel-tpebs.h" > > +#include <linux/list.h> > > +#include <linux/zalloc.h> > > +#include <linux/err.h> > > +#include "sample.h" > > +#include "debug.h" > > +#include "evlist.h" > > +#include "evsel.h" > > +#include "session.h" > > +#include "tool.h" > > +#include "metricgroup.h" > > +#include <sys/stat.h> > > +#include <sys/file.h> > > + > > + > > + > > +#define PERF_DATA "-" > > +#define CONTROL "/tmp/control" > > +#define ACK "/tmp/ack" > > +pthread_t reader_thread; > > +struct child_process *cmd; > > +struct perf_stat_config *stat_config; > > static ? > > > + > > +static int get_perf_record_args(const char **record_argv) > > +{ > > + int i = 0; > > + struct tpebs_retire_lat *e; > > + > > + pr_debug("Prepare perf record for retire_latency\n"); > > + > > + record_argv[i++] = "perf"; > > + record_argv[i++] = "record"; > > + record_argv[i++] = "-W"; > > + record_argv[i++] = "--synth=no"; > > Unfortunately this still synthesizes MMAP records for the kernel > and modules. As we don't care about them and just want to > minimize the overhead at the beginning, we can add > "--tail-synthesize" too. > > > > + record_argv[i++] = "--control=fifo:/tmp/control,/tmp/ack"; > > This hard-coded path won't work well when more than one users > want to run the perf command at the same time. > Hi Namhyung,
Yes, this is not a good implementation. I'm going to update this named pipe and all the related code.
Thanks, Weilin
> Thanks, > Namhyung > > > + > > + if (stat_config->user_requested_cpu_list) { > > + record_argv[i++] = "-C"; > > + record_argv[i++] = stat_config->user_requested_cpu_list; > > + } > > + > > + if (stat_config->system_wide) > > + record_argv[i++] = "-a"; > > + > > + if (!stat_config->system_wide && !stat_config- > >user_requested_cpu_list) { > > + pr_err("Require -a or -C option to run sampling.\n"); > > + return -ECANCELED; > > + } > > + > > + list_for_each_entry(e, &stat_config->tpebs_results, nd) { > > + record_argv[i++] = "-e"; > > + record_argv[i++] = e->name; > > + } > > + > > + record_argv[i++] = "-o"; > > + record_argv[i++] = PERF_DATA; > > + > > + return 0; > > +} > > + > >
| ![\](/images/icornerr.gif) |