mbox series

[RFC,0/9] perf tool: sys event metric support re-write

Message ID 20230628102949.2598096-1-john.g.garry@oracle.com (mailing list archive)
Headers show
Series perf tool: sys event metric support re-write | expand

Message

John Garry June 28, 2023, 10:29 a.m. UTC
The current sys event metric support has some issues, like:
- It is broken that we only match a metric based on PMU compat, but not
  Unit as well, as reported by Jing Zhang <renyu.zj@linux.alibaba.com>
- No real self-test support
- Not able to use resolvable metrics
- Need to specify event PMU Unit and Compat for metric, which should not
  be necessary

This series changes sys event metric support to match metrics based on
evaluating each term in the metric expression and then ensuring it
matches an event from the same associated pmu_sys_events table.

Why an RFC?
- Even though main motivation here was to improve self-test support, that
  has proved quite tricky and nothing has been added yet.
  My desire is to test the feature that we match metrics for a specific
  SoC when PMUs with matching HW identifier are present. So I would hope
  to add sys metrics for many SoCs in ../pmu-events/arch/test/
- I still need to suppress logs from metricgroup_sys_metric_supported()
  indirect calls to functions like parse_events_multi_pmu_add(),
  generating logs like
  "smmuv3_pmcg.wr_sent_sp -> smmuv3_pmcg_50/event=0x86/" - we should only
  see those logs for when really adding the metric in calling add_metric()

Based on 82fe2e45cdb0 (acme/tmp.perf/core, acme/tmp.perf-tools-next, acme/perf/core, acme/perf-tools-next) perf pmus: Check if we can encode the PMU number in perf_event_attr.type

John Garry (9):
  perf metrics: Delete metricgroup_add_iter_data.table
  perf metrics: Don't iter sys metrics if we already found a CPU match
  perf metrics: Pass cpu and sys tables to metricgroup__add_metric()
  perf jevents: Add sys_events_find_events_table()
  perf pmu: Refactor pmu_add_sys_aliases_iter_fn()
  perf metrics: Add metricgroup_sys_metric_supported()
  perf metrics: Test metric match in metricgroup__sys_event_iter()
  perf metrics: Stop metricgroup__add_metric_sys_event_iter if already
    matched
  perf vendor events arm64: Remove unnecessary metric Unit and Compat
    specifiers

 .../arm64/freescale/imx8mm/sys/metrics.json   |   4 -
 .../arm64/freescale/imx8mn/sys/metrics.json   |   4 -
 .../arm64/freescale/imx8mq/sys/metrics.json   |   4 -
 .../arm64/hisilicon/hip09/sys/uncore-cpa.json |   4 -
 tools/perf/pmu-events/empty-pmu-events.c      |   6 +
 tools/perf/pmu-events/jevents.py              |  11 ++
 tools/perf/pmu-events/pmu-events.h            |   3 +
 tools/perf/tests/expand-cgroup.c              |   2 +-
 tools/perf/tests/parse-metric.c               |   2 +-
 tools/perf/tests/pmu-events.c                 |  29 ++-
 tools/perf/util/metricgroup.c                 | 182 +++++++++++++++---
 tools/perf/util/metricgroup.h                 |   3 +-
 tools/perf/util/pmu.c                         |  20 +-
 tools/perf/util/pmu.h                         |   2 +
 14 files changed, 220 insertions(+), 56 deletions(-)

Comments

Namhyung Kim June 29, 2023, 10:08 p.m. UTC | #1
Hello,

On Wed, Jun 28, 2023 at 3:30 AM John Garry <john.g.garry@oracle.com> wrote:
>
> The current sys event metric support has some issues, like:
> - It is broken that we only match a metric based on PMU compat, but not
>   Unit as well, as reported by Jing Zhang <renyu.zj@linux.alibaba.com>
> - No real self-test support
> - Not able to use resolvable metrics
> - Need to specify event PMU Unit and Compat for metric, which should not
>   be necessary
>
> This series changes sys event metric support to match metrics based on
> evaluating each term in the metric expression and then ensuring it
> matches an event from the same associated pmu_sys_events table.
>
> Why an RFC?
> - Even though main motivation here was to improve self-test support, that
>   has proved quite tricky and nothing has been added yet.
>   My desire is to test the feature that we match metrics for a specific
>   SoC when PMUs with matching HW identifier are present. So I would hope
>   to add sys metrics for many SoCs in ../pmu-events/arch/test/
> - I still need to suppress logs from metricgroup_sys_metric_supported()
>   indirect calls to functions like parse_events_multi_pmu_add(),
>   generating logs like
>   "smmuv3_pmcg.wr_sent_sp -> smmuv3_pmcg_50/event=0x86/" - we should only
>   see those logs for when really adding the metric in calling add_metric()
>
> Based on 82fe2e45cdb0 (acme/tmp.perf/core, acme/tmp.perf-tools-next, acme/perf/core, acme/perf-tools-next) perf pmus: Check if we can encode the PMU number in perf_event_attr.type

We moved to new repos from acme to perf/perf-tools and perf/perf-tools-next.
You'd better rebase the series onto perf-tools-next (branch name is the same).

Thanks,
Namhyung


>
> John Garry (9):
>   perf metrics: Delete metricgroup_add_iter_data.table
>   perf metrics: Don't iter sys metrics if we already found a CPU match
>   perf metrics: Pass cpu and sys tables to metricgroup__add_metric()
>   perf jevents: Add sys_events_find_events_table()
>   perf pmu: Refactor pmu_add_sys_aliases_iter_fn()
>   perf metrics: Add metricgroup_sys_metric_supported()
>   perf metrics: Test metric match in metricgroup__sys_event_iter()
>   perf metrics: Stop metricgroup__add_metric_sys_event_iter if already
>     matched
>   perf vendor events arm64: Remove unnecessary metric Unit and Compat
>     specifiers
>
>  .../arm64/freescale/imx8mm/sys/metrics.json   |   4 -
>  .../arm64/freescale/imx8mn/sys/metrics.json   |   4 -
>  .../arm64/freescale/imx8mq/sys/metrics.json   |   4 -
>  .../arm64/hisilicon/hip09/sys/uncore-cpa.json |   4 -
>  tools/perf/pmu-events/empty-pmu-events.c      |   6 +
>  tools/perf/pmu-events/jevents.py              |  11 ++
>  tools/perf/pmu-events/pmu-events.h            |   3 +
>  tools/perf/tests/expand-cgroup.c              |   2 +-
>  tools/perf/tests/parse-metric.c               |   2 +-
>  tools/perf/tests/pmu-events.c                 |  29 ++-
>  tools/perf/util/metricgroup.c                 | 182 +++++++++++++++---
>  tools/perf/util/metricgroup.h                 |   3 +-
>  tools/perf/util/pmu.c                         |  20 +-
>  tools/perf/util/pmu.h                         |   2 +
>  14 files changed, 220 insertions(+), 56 deletions(-)
>
> --
> 2.35.3
>
John Garry June 30, 2023, 9:35 a.m. UTC | #2
>>
>> Based on 82fe2e45cdb0 (acme/tmp.perf/core, acme/tmp.perf-tools-next, acme/perf/core, acme/perf-tools-next) perf pmus: Check if we can encode the PMU number in perf_event_attr.type
> 
> We moved to new repos from acme to perf/perf-tools and perf/perf-tools-next.
> You'd better rebase the series onto perf-tools-next (branch name is the same).

Is that in the MAINTAINERS file? I could not see it.

And I was hoping that Ian could first have a look, since this is just an 
RFC.

Cheers,
John
Namhyung Kim June 30, 2023, 9:07 p.m. UTC | #3
On Fri, Jun 30, 2023 at 2:35 AM John Garry <john.g.garry@oracle.com> wrote:
>
>
> >>
> >> Based on 82fe2e45cdb0 (acme/tmp.perf/core, acme/tmp.perf-tools-next, acme/perf/core, acme/perf-tools-next) perf pmus: Check if we can encode the PMU number in perf_event_attr.type
> >
> > We moved to new repos from acme to perf/perf-tools and perf/perf-tools-next.
> > You'd better rebase the series onto perf-tools-next (branch name is the same).
>
> Is that in the MAINTAINERS file? I could not see it.

No it's not.  But it seems acme/perf is not there either.
Probably we need to add one and split the tooling part.

>
> And I was hoping that Ian could first have a look, since this is just an
> RFC.

Ok, makes sense.

Thanks,
Namhyung