1. Patchew
  2. linux
  3. View series

[PATCH v2 0/3] Fix incorrect counts when count the same uncore event multiple times

Chun-Tse Shao posted 3 patches 8 months, 3 weeks ago
There is a newer version of this series
tools/perf/builtin-record.c                   |   7 +-
tools/perf/builtin-top.c                      |   7 +-
.../tests/shell/stat+event_uniquifying.sh     |  69 ++++++++
tools/perf/util/evlist.c                      |  66 +++++---
tools/perf/util/evlist.h                      |   3 +-
tools/perf/util/evsel.c                       | 119 ++++++++++++-
tools/perf/util/evsel.h                       |  11 +-
tools/perf/util/parse-events.c                |  86 +++++++---
tools/perf/util/stat-display.c                | 160 ++----------------
tools/perf/util/stat.c                        |  40 +----
10 files changed, 329 insertions(+), 239 deletions(-)
create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh
[PATCH v2 0/3] Fix incorrect counts when count the same uncore event multiple times
Posted by Chun-Tse Shao 8 months, 3 weeks ago 
Let's take a look an example, the machine is SKX with 6 IMC devices.

  perf stat -e clockticks,clockticks -I 1000
  #           time             counts unit events
       1.001127430      6,901,503,174      uncore_imc_0/clockticks/
       1.001127430      3,940,896,301      uncore_imc_0/clockticks/
       2.002649722        988,376,876      uncore_imc_0/clockticks/
       2.002649722        988,376,141      uncore_imc_0/clockticks/
       3.004071319      1,000,292,675      uncore_imc_0/clockticks/
       3.004071319      1,000,294,160      uncore_imc_0/clockticks/

1) The events name should not be uniquified.
2) The initial count for the first `clockticks` is doubled.
3) Subsequent count only report for the first IMC device.

The first patch fixes 1) and 3), and the second patch fixes 2).

After these fix:

  perf stat -e clockticks,clockticks -I 1000
  #           time             counts unit events
       1.001127586      4,126,938,857      clockticks
       1.001127586      4,121,564,277      clockticks
       2.001686014      3,953,806,350      clockticks
       2.001686014      3,953,809,541      clockticks
       3.003121403      4,137,750,252      clockticks
       3.003121403      4,137,749,048      clockticks

I also tested `-A`, `--per-socket`, `--per-die` and `--per-core`, all
looks good.

Ian tested `hybrid-merge` and `hwmon`, all looks good as well.

Chun-Tse Shao (1):
  perf test: Add stat uniquifying test

Ian Rogers (2):
  perf evlist: Make uniquifying counter names consistent
  perf parse-events: Use wildcard processing to set an event to merge
    into

 tools/perf/builtin-record.c                   |   7 +-
 tools/perf/builtin-top.c                      |   7 +-
 .../tests/shell/stat+event_uniquifying.sh     |  69 ++++++++
 tools/perf/util/evlist.c                      |  66 +++++---
 tools/perf/util/evlist.h                      |   3 +-
 tools/perf/util/evsel.c                       | 119 ++++++++++++-
 tools/perf/util/evsel.h                       |  11 +-
 tools/perf/util/parse-events.c                |  86 +++++++---
 tools/perf/util/stat-display.c                | 160 ++----------------
 tools/perf/util/stat.c                        |  40 +----
 10 files changed, 329 insertions(+), 239 deletions(-)
 create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh

--
v2:
  - Fixes for `hwmon` and `--hybrid-merge`.
  - Add a test for event uniquifying.

v1: lore.kernel.org/20250326234758.480431-1-ctshao@google.com
2.49.0.472.ge94155a9ec-goog
Re: [PATCH v2 0/3] Fix incorrect counts when count the same uncore event multiple times
Posted by Ian Rogers 8 months, 3 weeks ago 
On Thu, Mar 27, 2025 at 3:56 PM Chun-Tse Shao <ctshao@google.com> wrote:
>
> Let's take a look an example, the machine is SKX with 6 IMC devices.
>
>   perf stat -e clockticks,clockticks -I 1000
>   #           time             counts unit events
>        1.001127430      6,901,503,174      uncore_imc_0/clockticks/
>        1.001127430      3,940,896,301      uncore_imc_0/clockticks/
>        2.002649722        988,376,876      uncore_imc_0/clockticks/
>        2.002649722        988,376,141      uncore_imc_0/clockticks/
>        3.004071319      1,000,292,675      uncore_imc_0/clockticks/
>        3.004071319      1,000,294,160      uncore_imc_0/clockticks/
>
> 1) The events name should not be uniquified.
> 2) The initial count for the first `clockticks` is doubled.
> 3) Subsequent count only report for the first IMC device.
>
> The first patch fixes 1) and 3), and the second patch fixes 2).
>
> After these fix:
>
>   perf stat -e clockticks,clockticks -I 1000
>   #           time             counts unit events
>        1.001127586      4,126,938,857      clockticks
>        1.001127586      4,121,564,277      clockticks
>        2.001686014      3,953,806,350      clockticks
>        2.001686014      3,953,809,541      clockticks
>        3.003121403      4,137,750,252      clockticks
>        3.003121403      4,137,749,048      clockticks
>
> I also tested `-A`, `--per-socket`, `--per-die` and `--per-core`, all
> looks good.
>
> Ian tested `hybrid-merge` and `hwmon`, all looks good as well.
>
> Chun-Tse Shao (1):
>   perf test: Add stat uniquifying test
>
> Ian Rogers (2):
>   perf evlist: Make uniquifying counter names consistent
>   perf parse-events: Use wildcard processing to set an event to merge
>     into

Tested-by: Ian Rogers <irogers@google.com>

There could be minor conflict with this unreviewed series:
https://lore.kernel.org/lkml/20250318041442.321230-1-irogers@google.com/

Thanks,
Ian

>  tools/perf/builtin-record.c                   |   7 +-
>  tools/perf/builtin-top.c                      |   7 +-
>  .../tests/shell/stat+event_uniquifying.sh     |  69 ++++++++
>  tools/perf/util/evlist.c                      |  66 +++++---
>  tools/perf/util/evlist.h                      |   3 +-
>  tools/perf/util/evsel.c                       | 119 ++++++++++++-
>  tools/perf/util/evsel.h                       |  11 +-
>  tools/perf/util/parse-events.c                |  86 +++++++---
>  tools/perf/util/stat-display.c                | 160 ++----------------
>  tools/perf/util/stat.c                        |  40 +----
>  10 files changed, 329 insertions(+), 239 deletions(-)
>  create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh
>
> --
> v2:
>   - Fixes for `hwmon` and `--hybrid-merge`.
>   - Add a test for event uniquifying.
>
> v1: lore.kernel.org/20250326234758.480431-1-ctshao@google.com
> 2.49.0.472.ge94155a9ec-goog
>
Re: [PATCH v2 0/3] Fix incorrect counts when count the same uncore event multiple times
Posted by Chun-Tse Shao 7 months, 3 weeks ago 
Ping.

Thanks,
CT

On Thu, Mar 27, 2025 at 9:20 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Mar 27, 2025 at 3:56 PM Chun-Tse Shao <ctshao@google.com> wrote:
> >
> > Let's take a look an example, the machine is SKX with 6 IMC devices.
> >
> >   perf stat -e clockticks,clockticks -I 1000
> >   #           time             counts unit events
> >        1.001127430      6,901,503,174      uncore_imc_0/clockticks/
> >        1.001127430      3,940,896,301      uncore_imc_0/clockticks/
> >        2.002649722        988,376,876      uncore_imc_0/clockticks/
> >        2.002649722        988,376,141      uncore_imc_0/clockticks/
> >        3.004071319      1,000,292,675      uncore_imc_0/clockticks/
> >        3.004071319      1,000,294,160      uncore_imc_0/clockticks/
> >
> > 1) The events name should not be uniquified.
> > 2) The initial count for the first `clockticks` is doubled.
> > 3) Subsequent count only report for the first IMC device.
> >
> > The first patch fixes 1) and 3), and the second patch fixes 2).
> >
> > After these fix:
> >
> >   perf stat -e clockticks,clockticks -I 1000
> >   #           time             counts unit events
> >        1.001127586      4,126,938,857      clockticks
> >        1.001127586      4,121,564,277      clockticks
> >        2.001686014      3,953,806,350      clockticks
> >        2.001686014      3,953,809,541      clockticks
> >        3.003121403      4,137,750,252      clockticks
> >        3.003121403      4,137,749,048      clockticks
> >
> > I also tested `-A`, `--per-socket`, `--per-die` and `--per-core`, all
> > looks good.
> >
> > Ian tested `hybrid-merge` and `hwmon`, all looks good as well.
> >
> > Chun-Tse Shao (1):
> >   perf test: Add stat uniquifying test
> >
> > Ian Rogers (2):
> >   perf evlist: Make uniquifying counter names consistent
> >   perf parse-events: Use wildcard processing to set an event to merge
> >     into
>
> Tested-by: Ian Rogers <irogers@google.com>
>
> There could be minor conflict with this unreviewed series:
> https://lore.kernel.org/lkml/20250318041442.321230-1-irogers@google.com/
>
> Thanks,
> Ian
>
> >  tools/perf/builtin-record.c                   |   7 +-
> >  tools/perf/builtin-top.c                      |   7 +-
> >  .../tests/shell/stat+event_uniquifying.sh     |  69 ++++++++
> >  tools/perf/util/evlist.c                      |  66 +++++---
> >  tools/perf/util/evlist.h                      |   3 +-
> >  tools/perf/util/evsel.c                       | 119 ++++++++++++-
> >  tools/perf/util/evsel.h                       |  11 +-
> >  tools/perf/util/parse-events.c                |  86 +++++++---
> >  tools/perf/util/stat-display.c                | 160 ++----------------
> >  tools/perf/util/stat.c                        |  40 +----
> >  10 files changed, 329 insertions(+), 239 deletions(-)
> >  create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh
> >
> > --
> > v2:
> >   - Fixes for `hwmon` and `--hybrid-merge`.
> >   - Add a test for event uniquifying.
> >
> > v1: lore.kernel.org/20250326234758.480431-1-ctshao@google.com
> > 2.49.0.472.ge94155a9ec-goog
> >

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.