Received-SPF: pass (google.com: domain of linux-kernel+bounces-110095-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 21 Mar 2024 10:31:33 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Yang Jihong <yangjihong1@huawei.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	James Clark <james.clark@arm.com>,
	Ravi Bangoria <ravi.bangoria@amd.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/6] perf list: Give more details about raw event
 encodings
Message-ID: <Zfw2tR6R-Uvf6ity@x1>
References: <20240308001915.4060155-1-irogers@google.com>
 <20240308001915.4060155-5-irogers@google.com>
 <CAP-5=fVE3f-Q3Zcdi2qWb5EJSadp5c7xbhzWeq8PFw2NOZEEJw@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAP-5=fVE3f-Q3Zcdi2qWb5EJSadp5c7xbhzWeq8PFw2NOZEEJw@mail.gmail.com>

On Wed, Mar 20, 2024 at 07:59:06PM -0700, Ian Rogers wrote:
> On Thu, Mar 7, 2024 at 4:19 PM Ian Rogers <irogers@google.com> wrote:
> >
> > List all the PMUs, not just the first core one, and list real format
> > specifiers with value ranges.
> >
> > Before:
> > ```
> > $ perf list
> > ...
> >   rNNN                                               [Raw hardware event descriptor]
> >   cpu/t1=v1[,t2=v2,t3 ...]/modifier                  [Raw hardware event descriptor]
> >        [(see 'man perf-list' on how to encode it)]
> >   mem:<addr>[/len][:access]                          [Hardware breakpoint]
> > ...
> > ```
> >
> > After:
> > ```
> > $ perf list
> > ...
> >   rNNN                                               [Raw event descriptor]
> >   cpu/event=0..255,pc,edge,.../modifier              [Raw event descriptor]
> >        [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> >   breakpoint//modifier                               [Raw event descriptor]
> >   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
> >   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
> >   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
> >   intel_bts//modifier                                [Raw event descriptor]
> >   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw event descriptor]
> >   kprobe/retprobe/modifier                           [Raw event descriptor]
> >   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
> >   power/event=0..255/modifier                        [Raw event descriptor]
> >   software//modifier                                 [Raw event descriptor]
> >   tracepoint//modifier                               [Raw event descriptor]
> >   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw event descriptor]
> >   uncore_cbox/event=0..255,edge,inv,.../modifier     [Raw event descriptor]
> >   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
> >   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> >   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> >   mem:<addr>[/len][:access]                          [Hardware breakpoint]
> > ...
> > ```
> >
> > With '--details' provide more details on the formats encoding:
> > ```
> >   cpu/event=0..255,pc,edge,.../modifier              [Raw event descriptor]
> >        [(see 'man perf-list' or 'man perf-record' on how to encode it)]
> >         cpu/event=0..255,pc,edge,offcore_rsp=0..0xffffffffffffffff,ldlat=0..0xffff,inv,
> >         umask=0..255,frontend=0..0xffffff,cmask=0..255,config=0..0xffffffffffffffff,
> >         config1=0..0xffffffffffffffff,config2=0..0xffffffffffffffff,config3=0..0xffffffffffffffff,
> >         name=string,period=number,freq=number,branch_type=(u|k|hv|any|...),time,
> >         call-graph=(fp|dwarf|lbr),stack-size=number,max-stack=number,nr=number,inherit,no-inherit,
> >         overwrite,no-overwrite,percore,aux-output,aux-sample-size=number/modifier
> >   breakpoint//modifier                               [Raw event descriptor]
> >         breakpoint//modifier
> >   cstate_core/event=0..0xffffffffffffffff/modifier   [Raw event descriptor]
> >         cstate_core/event=0..0xffffffffffffffff/modifier
> >   cstate_pkg/event=0..0xffffffffffffffff/modifier    [Raw event descriptor]
> >         cstate_pkg/event=0..0xffffffffffffffff/modifier
> >   i915/i915_eventid=0..0x1fffff/modifier             [Raw event descriptor]
> >         i915/i915_eventid=0..0x1fffff/modifier
> >   intel_bts//modifier                                [Raw event descriptor]
> >         intel_bts//modifier
> >   intel_pt/ptw,event,cyc_thresh=0..15,.../modifier   [Raw event descriptor]
> >         intel_pt/ptw,event,cyc_thresh=0..15,pt,notnt,branch,tsc,pwr_evt,fup_on_ptw,cyc,noretcomp,
> >         mtc,psb_period=0..15,mtc_period=0..15/modifier
> >   kprobe/retprobe/modifier                           [Raw event descriptor]
> >         kprobe/retprobe/modifier
> >   msr/event=0..0xffffffffffffffff/modifier           [Raw event descriptor]
> >         msr/event=0..0xffffffffffffffff/modifier
> >   power/event=0..255/modifier                        [Raw event descriptor]
> >         power/event=0..255/modifier
> >   software//modifier                                 [Raw event descriptor]
> >         software//modifier
> >   tracepoint//modifier                               [Raw event descriptor]
> >         tracepoint//modifier
> >   uncore_arb/event=0..255,edge,inv,.../modifier      [Raw event descriptor]
> >         uncore_arb/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> >   uncore_cbox/event=0..255,edge,inv,.../modifier     [Raw event descriptor]
> >         uncore_cbox/event=0..255,edge,inv,umask=0..255,cmask=0..31/modifier
> >   uncore_clock/event=0..255/modifier                 [Raw event descriptor]
> >         uncore_clock/event=0..255/modifier
> >   uncore_imc_free_running/event=0..255,umask=0..255/modifier[Raw event descriptor]
> >         uncore_imc_free_running/event=0..255,umask=0..255/modifier
> >   uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier[Raw event descriptor]
> >         uprobe/ref_ctr_offset=0..0xffffffff,retprobe/modifier
> > ```
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/util/pmu.c          | 61 +++++++++++++++++++++-
> >  tools/perf/util/pmu.h          |  3 ++
> >  tools/perf/util/pmus.c         | 94 ++++++++++++++++++++++++++++++++++
> >  tools/perf/util/pmus.h         |  1 +
> >  tools/perf/util/print-events.c | 20 +-------
> >  5 files changed, 160 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 24be587e3537..e24bc3b8f696 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -1603,6 +1603,61 @@ bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name)
> >         return false;
> >  }
> >
> > +int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_callback cb)
> > +{
> > +       static const char *const terms[] = {
> > +               "config=0..0xffffffffffffffff",
> > +               "config1=0..0xffffffffffffffff",
> > +               "config2=0..0xffffffffffffffff",
> > +               "config3=0..0xffffffffffffffff",
> > +               "name=string",
> > +               "period=number",
> > +               "freq=number",
> > +               "branch_type=(u|k|hv|any|...)",
> > +               "time",
> > +               "call-graph=(fp|dwarf|lbr)",
> > +               "stack-size=number",
> > +               "max-stack=number",
> > +               "nr=number",
> > +               "inherit",
> > +               "no-inherit",
> > +               "overwrite",
> > +               "no-overwrite",
> > +               "percore",
> > +               "aux-output",
> > +               "aux-sample-size=number",
> > +       };
> > +       struct perf_pmu_format *format;
> > +       int ret;
> > +
> > +       /*
> > +        * max-events and driver-config are missing above as are the internal
> > +        * types user, metric-id, raw, legacy cache and hardware. Assert against
> > +        * the enum parse_events__term_type so they are kept in sync.
> > +        */
> > +       _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
> 
> For C11 it is required that a 2nd message argument be passed:
> https://en.cppreference.com/w/c/language/_Static_assert
> 
> This line needs to be something like:
> _Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
> "Unexpected array size");
> 
> Let me know if we I should send a fix, resend all the patches or if
> you can fix in tmp.perf-tools-next.

I added this, no need to resend.

- Arnaldo

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index e24bc3b8f696a9f5..81952c6cd3c6f5e8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1635,7 +1635,8 @@ int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_call
 	 * types user, metric-id, raw, legacy cache and hardware. Assert against
 	 * the enum parse_events__term_type so they are kept in sync.
 	 */
-	_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6);
+	_Static_assert(ARRAY_SIZE(terms) == __PARSE_EVENTS__TERM_TYPE_NR - 6,
+		       "perf_pmu__for_each_format()'s terms must be kept in sync with enum parse_events__term_type");
 	list_for_each_entry(format, &pmu->format, list) {
 		perf_pmu_format__load(pmu, format);
 		ret = cb(state, format->name, (int)format->value, format->bits);