[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CO6PR11MB56353719739D822626369453EEC72@CO6PR11MB5635.namprd11.prod.outlook.com>
Date: Tue, 11 Jun 2024 17:49:10 +0000
From: "Wang, Weilin" <weilin.wang@...el.com>
To: Namhyung Kim <namhyung@...nel.org>
CC: Ian Rogers <irogers@...gle.com>, Arnaldo Carvalho de Melo
<acme@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar
<mingo@...hat.com>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, "Hunter, Adrian" <adrian.hunter@...el.com>,
"Kan Liang" <kan.liang@...ux.intel.com>, "linux-perf-users@...r.kernel.org"
<linux-perf-users@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Taylor, Perry" <perry.taylor@...el.com>,
"Alt, Samantha" <samantha.alt@...el.com>, "Biggers, Caleb"
<caleb.biggers@...el.com>
Subject: RE: [RFC PATCH v12 6/8] perf stat: Add command line option for
enabling tpebs recording
> -----Original Message-----
> From: Namhyung Kim <namhyung@...nel.org>
> Sent: Tuesday, June 11, 2024 9:50 AM
> To: Wang, Weilin <weilin.wang@...el.com>
> Cc: Ian Rogers <irogers@...gle.com>; Arnaldo Carvalho de Melo
> <acme@...nel.org>; Peter Zijlstra <peterz@...radead.org>; Ingo Molnar
> <mingo@...hat.com>; Alexander Shishkin
> <alexander.shishkin@...ux.intel.com>; Jiri Olsa <jolsa@...nel.org>; Hunter,
> Adrian <adrian.hunter@...el.com>; Kan Liang <kan.liang@...ux.intel.com>;
> linux-perf-users@...r.kernel.org; linux-kernel@...r.kernel.org; Taylor, Perry
> <perry.taylor@...el.com>; Alt, Samantha <samantha.alt@...el.com>; Biggers,
> Caleb <caleb.biggers@...el.com>
> Subject: Re: [RFC PATCH v12 6/8] perf stat: Add command line option for
> enabling tpebs recording
>
> Hello,
>
> On Mon, Jun 10, 2024 at 10:24 PM <weilin.wang@...el.com> wrote:
> >
> > From: Weilin Wang <weilin.wang@...el.com>
> >
> > With this command line option, tpebs recording is turned off in perf stat on
> > default. It will only be turned on when this option is given in perf stat
> > command.
> >
> > Exampe with --enable-tpebs-recording:
> >
> > perf stat -M tma_dtlb_store -a --enable-tpebs-recording sleep 1
> >
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.000 MB - ]
> >
> > Performance counter stats for 'system wide':
> >
> > 181,047,168 cpu_core/TOPDOWN.SLOTS/ # 0.6 %
> tma_dtlb_store
> > 3,195,608 cpu_core/topdown-retiring/
> > 40,156,649 cpu_core/topdown-mem-bound/
> > 3,550,925 cpu_core/topdown-bad-spec/
> > 117,571,818 cpu_core/topdown-fe-bound/
> > 57,118,087 cpu_core/topdown-be-bound/
> > 69,179 cpu_core/EXE_ACTIVITY.BOUND_ON_STORES/
> > 4,582 cpu_core/MEM_INST_RETIRED.STLB_HIT_STORES/
> > 30,183,104 cpu_core/CPU_CLK_UNHALTED.DISTRIBUTED/
> > 30,556,790 cpu_core/CPU_CLK_UNHALTED.THREAD/
> > 168,486 cpu_core/DTLB_STORE_MISSES.WALK_ACTIVE/
> > 0 MEM_INST_RETIRED.STLB_HIT_STORES:R
>
> Here I guess we can expect a non-zero value, right?
Hi Namhyung,
Do you mean when we have the option, we should expect non-zero value?
During the execution, it's possible that we don't capture any sample on this
event. In this case, we would have a zero value. In the future, I think we will
give it the default value instead of zero.
>
> >
> > 1.003105924 seconds time elapsed
> >
> > Exampe without --enable-tpebs-recording:
> >
> > perf stat -M tma_dtlb_store -a sleep 1
> >
> > Performance counter stats for 'system wide':
> >
> > 181,047,168 cpu_core/TOPDOWN.SLOTS/ # 0.6 %
> tma_dtlb_store
> > 3,195,608 cpu_core/topdown-retiring/
> > 40,156,649 cpu_core/topdown-mem-bound/
> > 3,550,925 cpu_core/topdown-bad-spec/
> > 117,571,818 cpu_core/topdown-fe-bound/
> > 57,118,087 cpu_core/topdown-be-bound/
> > 69,179 cpu_core/EXE_ACTIVITY.BOUND_ON_STORES/
> > 4,582 cpu_core/MEM_INST_RETIRED.STLB_HIT_STORES/
> > 30,183,104 cpu_core/CPU_CLK_UNHALTED.DISTRIBUTED/
> > 30,556,790 cpu_core/CPU_CLK_UNHALTED.THREAD/
> > 168,486 cpu_core/DTLB_STORE_MISSES.WALK_ACTIVE/
> > 0 MEM_INST_RETIRED.STLB_HIT_STORES:R
> >
> > 1.003105924 seconds time elapsed
> >
> > Signed-off-by: Weilin Wang <weilin.wang@...el.com>
> > ---
> > tools/perf/builtin-stat.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 14be132f7b6f..055139e44763 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -1235,6 +1235,8 @@ static struct option stat_options[] = {
>
> What's the base of your patchset? I think we moved this to cmd_stat().
> Please make sure to rebase on the perf-tools-next.
This was rebased on top of Ian's change for the tool event and retire_latency parser
patches. I think that was on tmp.perf-tools.next. I will rebase on perf-tools-next.
>
>
> > "disable adding events for the metric threshold calculation"),
> > OPT_BOOLEAN(0, "topdown", &topdown_run,
> > "measure top-down statistics"),
> > + OPT_BOOLEAN(0, "enable-tpebs-recording", &tpebs_recording,
> > + "enable recording for tpebs when retire_latency required"),
>
> Please update the documentation (man page) for this option.
>
> Also I'm afraid it'd fail to build on non-x86 because tpebes_recording
> is defined in intel-tpebs.c.
>
Yes, you are right. I forgot about non-x86. I will update this and also the
Documentation.
Thanks,
Weilin
> Thanks,
> Namhyung
>
>
> > OPT_UINTEGER(0, "td-level", &stat_config.topdown_level,
> > "Set the metrics level for the top-down statistics (0: max level)"),
> > OPT_BOOLEAN(0, "smi-cost", &smi_cost,
> > --
> > 2.43.0
> >
Powered by blists - more mailing lists