Received-SPF: pass (google.com: domain of linux-kernel+bounces-108295-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240308001915.4060155-1-irogers@google.com> <89249fc5-6102-45d4-9345-5dd80d06e001@linux.intel.com>
 <CAP-5=fWGhZHnsBWo4=+9PdfaAPNEnx7u40G+BHAWR+4rPC2Udw@mail.gmail.com>
In-Reply-To: <CAP-5=fWGhZHnsBWo4=+9PdfaAPNEnx7u40G+BHAWR+4rPC2Udw@mail.gmail.com>
From: Ian Rogers <irogers@google.com>
Date: Tue, 19 Mar 2024 18:01:35 -0700
Message-ID: <CAP-5=fXXACwattr9fEcmxoDfGNNi8zmzrDUc8RjUtuX5sV4M9w@mail.gmail.com>
Subject: Re: [PATCH v3 0/6] Extra verbose/perf-list details
To: "Liang, Kan" <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Arnaldo Carvalho de Melo <acme@kernel.org>, 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>, 
	James Clark <james.clark@arm.com>, Ravi Bangoria <ravi.bangoria@amd.com>, 
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Mar 8, 2024 at 8:34=E2=80=AFAM Ian Rogers <irogers@google.com> wrot=
e:
>
> On Fri, Mar 8, 2024 at 7:39=E2=80=AFAM Liang, Kan <kan.liang@linux.intel.=
com> wrote:
>>
>>
>>
>> On 2024-03-07 7:19 p.m., Ian Rogers wrote:
>> > Add more encoding detail and raw event details in perf list. Add PMU
>> > name and reverse lookup from config to event name to
>> > perf_event_attr_fprintf. This makes the verbose output easier to read,
>> > and the perf list information more specific.
>> >
>> > v3. Fix to reverse lookup to ensure or aliases are loaded and if
>> >     getting the config value fails for an event/alias just continue to
>> >     the next one.
>> > v2. Address feedback from Kan Liang, "Raw hardware event descriptor"
>> >     becomes "Raw event descriptor" add assert to keep term numbers in
>> >     sync, fix a commit message.
>> >
>> > Ian Rogers (6):
>> >   perf list: Add tracepoint encoding to detailed output
>> >   perf pmu: Drop "default_core" from alias names
>> >   perf list: Allow wordwrap to wrap on commas
>> >   perf list: Give more details about raw event encodings
>> >   perf tools: Use pmus to describe type from attribute
>> >   perf tools: Add/use PMU reverse lookup from config to name
>>
>> The patch series look good to me.
>> I verified it on a hybrid machine. The new format is the same as the
>> advertise.
>>
>> Tested-by: Kan Liang <kan.liang@linux.intel.com>

Ping.

Thanks,
Ian

>
> Thanks Kan! Just to add another feature of perf_event_attr_fprintf change=
s is that it shows up in perf trace output:
>
> ```
> $ perf trace -e perf_event_open perf stat -e data_read true
>      0.000 ( 0.011 ms): :36857/36857 perf_event_open(attr_uptr: { type: 2=
4 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_cou=
nt_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL=
_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_=
fd: -1, flags: FD_CLOEXEC) =3D -1 EINVAL (Invalid argument)
>      0.014 ( 0.002 ms): :36857/36857 perf_event_open(attr_uptr: { type: 2=
4 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_cou=
nt_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL=
_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_=
fd: -1) =3D -1 EINVAL (Invalid argument)
>      0.020 ( 0.008 ms): :36857/36857 perf_event_open(attr_uptr: { type: 2=
4 (uncore_imc_free_running_0), size: 136, config: 0x20ff (unc_mc0_rdcas_cou=
nt_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL=
_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) =3D 8
>      0.031 ( 0.006 ms): :36857/36857 perf_event_open(attr_uptr: { type: 2=
5 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_cou=
nt_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL=
_TIME_RUNNING, disabled: 1, inherit: 1, exclude_guest: 1 }, pid: -1, group_=
fd: -1) =3D -1 EINVAL (Invalid argument)
>      0.044 ( 0.004 ms): :36857/36857 perf_event_open(attr_uptr: { type: 2=
5 (uncore_imc_free_running_1), size: 136, config: 0x20ff (unc_mc1_rdcas_cou=
nt_freerun), sample_type: IDENTIFIER, read_format: TOTAL_TIME_ENABLED|TOTAL=
_TIME_RUNNING, disabled: 1, inherit: 1 }, pid: -1, group_fd: -1) =3D 9
>
>  Performance counter stats for 'system wide':
>
>               5.07 MiB  data_read
>
>        0.001178187 seconds time elapsed
> ```
>
> Thanks,
> Ian
>
>>
>> Thanks,
>> Kan
>> >
>> >  tools/perf/builtin-list.c                 | 21 ++++-
>> >  tools/perf/util/perf_event_attr_fprintf.c | 26 +++++--
>> >  tools/perf/util/pmu.c                     | 82 +++++++++++++++++++-
>> >  tools/perf/util/pmu.h                     |  4 +
>> >  tools/perf/util/pmus.c                    | 94 ++++++++++++++++++++++=
+
>> >  tools/perf/util/pmus.h                    |  1 +
>> >  tools/perf/util/print-events.c            | 55 +++++++------
>> >  7 files changed, 242 insertions(+), 41 deletions(-)
>> >