[v2] Fix incorrect counts when count the same uncore event multiple times

[PATCH v2 3/3] perf test: Add stat uniquifying test

Posted by Chun-Tse Shao 8 months, 3 weeks ago

The `stat+uniquify.sh` test retrieves all uniquified `clockticks` events
from `perf list -v clockticks` and check if `perf stat -e clockticks -A`
contains all of them.

Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
 .../tests/shell/stat+event_uniquifying.sh     | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh

diff --git a/tools/perf/tests/shell/stat+event_uniquifying.sh b/tools/perf/tests/shell/stat+event_uniquifying.sh
new file mode 100755
index 000000000000..5ec35c52b7d9
--- /dev/null
+++ b/tools/perf/tests/shell/stat+event_uniquifying.sh
@@ -0,0 +1,69 @@
+#!/bin/bash
+# perf stat events uniquifying
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+stat_output=$(mktemp /tmp/__perf_test.stat_output.XXXXX)
+perf_tool=perf
+err=0
+
+test_event_uniquifying() {
+  # We use `clockticks` to verify the uniquify behavior.
+  event="clockticks"
+
+  # If the `-A` option is added, the event should be uniquified.
+  #
+  # $perf list -v clockticks
+  #
+  # List of pre-defined events (to be used in -e or -M):
+  #
+  #   uncore_imc_0/clockticks/                           [Kernel PMU event]
+  #   uncore_imc_1/clockticks/                           [Kernel PMU event]
+  #   uncore_imc_2/clockticks/                           [Kernel PMU event]
+  #   uncore_imc_3/clockticks/                           [Kernel PMU event]
+  #   uncore_imc_4/clockticks/                           [Kernel PMU event]
+  #   uncore_imc_5/clockticks/                           [Kernel PMU event]
+  #
+  #   ...
+  #
+  # $perf stat -e clockticks -A -- true
+  #
+  #  Performance counter stats for 'system wide':
+  #
+  # CPU0            3,773,018      uncore_imc_0/clockticks/
+  # CPU0            3,609,025      uncore_imc_1/clockticks/
+  # CPU0                    0      uncore_imc_2/clockticks/
+  # CPU0            3,230,009      uncore_imc_3/clockticks/
+  # CPU0            3,049,897      uncore_imc_4/clockticks/
+  # CPU0                    0      uncore_imc_5/clockticks/
+  #
+  #        0.002029828 seconds time elapsed
+
+  echo "stat event uniquifying test"
+  uniquified_event_array=()
+
+  # Check how many uniquified events.
+  while IFS= read -r line; do
+    uniquified_event=$(echo "$line" | awk '{print $1}')
+    uniquified_event_array+=("${uniquified_event}")
+  done < <(${perf_tool} list -v ${event} | grep "\[Kernel PMU event\]")
+
+  perf_command="${perf_tool} stat -e $event -A -o ${stat_output} -- true"
+  $perf_command
+
+  # Check the output contains all uniquified events.
+  for uniquified_event in "${uniquified_event_array[@]}"; do
+    if ! cat "${stat_output}" | grep -q "${uniquified_event}"; then
+      echo "Event is not uniquified [Failed]"
+      echo "${perf_command}"
+      cat "${stat_output}"
+      err=1
+      break
+    fi
+  done
+}
+
+test_event_uniquifying
+rm -f "${stat_output}"
+exit $err
-- 
2.49.0.472.ge94155a9ec-goog

Re: [PATCH v2 3/3] perf test: Add stat uniquifying test

Posted by Ian Rogers 8 months, 2 weeks ago

On Thu, Mar 27, 2025 at 4:01 PM Chun-Tse Shao <ctshao@google.com> wrote:
>
> The `stat+uniquify.sh` test retrieves all uniquified `clockticks` events
> from `perf list -v clockticks` and check if `perf stat -e clockticks -A`
> contains all of them.
>
> Signed-off-by: Chun-Tse Shao <ctshao@google.com>
> ---
>  .../tests/shell/stat+event_uniquifying.sh     | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh
>
> diff --git a/tools/perf/tests/shell/stat+event_uniquifying.sh b/tools/perf/tests/shell/stat+event_uniquifying.sh
> new file mode 100755
> index 000000000000..5ec35c52b7d9
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat+event_uniquifying.sh
> @@ -0,0 +1,69 @@
> +#!/bin/bash
> +# perf stat events uniquifying
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +stat_output=$(mktemp /tmp/__perf_test.stat_output.XXXXX)
> +perf_tool=perf
> +err=0
> +
> +test_event_uniquifying() {
> +  # We use `clockticks` to verify the uniquify behavior.
> +  event="clockticks"

This event is generally only available on Intel, not AMD or ARM, so we
will need to skip if it isn't present.

> +  # If the `-A` option is added, the event should be uniquified.
> +  #
> +  # $perf list -v clockticks
> +  #
> +  # List of pre-defined events (to be used in -e or -M):
> +  #
> +  #   uncore_imc_0/clockticks/                           [Kernel PMU event]
> +  #   uncore_imc_1/clockticks/                           [Kernel PMU event]
> +  #   uncore_imc_2/clockticks/                           [Kernel PMU event]
> +  #   uncore_imc_3/clockticks/                           [Kernel PMU event]
> +  #   uncore_imc_4/clockticks/                           [Kernel PMU event]
> +  #   uncore_imc_5/clockticks/                           [Kernel PMU event]
> +  #
> +  #   ...
> +  #
> +  # $perf stat -e clockticks -A -- true
> +  #
> +  #  Performance counter stats for 'system wide':
> +  #
> +  # CPU0            3,773,018      uncore_imc_0/clockticks/
> +  # CPU0            3,609,025      uncore_imc_1/clockticks/
> +  # CPU0                    0      uncore_imc_2/clockticks/
> +  # CPU0            3,230,009      uncore_imc_3/clockticks/
> +  # CPU0            3,049,897      uncore_imc_4/clockticks/
> +  # CPU0                    0      uncore_imc_5/clockticks/
> +  #
> +  #        0.002029828 seconds time elapsed
> +
> +  echo "stat event uniquifying test"
> +  uniquified_event_array=()
> +
> +  # Check how many uniquified events.
> +  while IFS= read -r line; do
> +    uniquified_event=$(echo "$line" | awk '{print $1}')
> +    uniquified_event_array+=("${uniquified_event}")
> +  done < <(${perf_tool} list -v ${event} | grep "\[Kernel PMU event\]")

Shouldn't the array contain every sysfs event that doesn't have a json
component? They may or may not be uniquified so I think the array name
is misleading.

> +  perf_command="${perf_tool} stat -e $event -A -o ${stat_output} -- true"
> +  $perf_command
> +
> +  # Check the output contains all uniquified events.
> +  for uniquified_event in "${uniquified_event_array[@]}"; do
> +    if ! cat "${stat_output}" | grep -q "${uniquified_event}"; then

Why not pass the file directly to grep?
Should some of the events not show in the stat output as you only
asked for the clockticks event? I'm not sure how this test can pass
currently.

Thanks,
Ian

> +      echo "Event is not uniquified [Failed]"
> +      echo "${perf_command}"
> +      cat "${stat_output}"
> +      err=1
> +      break
> +    fi
> +  done
> +}
> +
> +test_event_uniquifying
> +rm -f "${stat_output}"
> +exit $err
> --
> 2.49.0.472.ge94155a9ec-goog
>

Re: [PATCH v2 3/3] perf test: Add stat uniquifying test

Posted by Chun-Tse Shao 7 months, 3 weeks ago

On Thu, Apr 3, 2025 at 12:25 PM Ian Rogers <irogers@google.com> wrote:
>
> On Thu, Mar 27, 2025 at 4:01 PM Chun-Tse Shao <ctshao@google.com> wrote:
> >
> > The `stat+uniquify.sh` test retrieves all uniquified `clockticks` events
> > from `perf list -v clockticks` and check if `perf stat -e clockticks -A`
> > contains all of them.
> >
> > Signed-off-by: Chun-Tse Shao <ctshao@google.com>
> > ---
> >  .../tests/shell/stat+event_uniquifying.sh     | 69 +++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100755 tools/perf/tests/shell/stat+event_uniquifying.sh
> >
> > diff --git a/tools/perf/tests/shell/stat+event_uniquifying.sh b/tools/perf/tests/shell/stat+event_uniquifying.sh
> > new file mode 100755
> > index 000000000000..5ec35c52b7d9
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/stat+event_uniquifying.sh
> > @@ -0,0 +1,69 @@
> > +#!/bin/bash
> > +# perf stat events uniquifying
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +set -e
> > +
> > +stat_output=$(mktemp /tmp/__perf_test.stat_output.XXXXX)
> > +perf_tool=perf
> > +err=0
> > +
> > +test_event_uniquifying() {
> > +  # We use `clockticks` to verify the uniquify behavior.
> > +  event="clockticks"
>
> This event is generally only available on Intel, not AMD or ARM, so we
> will need to skip if it isn't present.
>
> > +  # If the `-A` option is added, the event should be uniquified.
> > +  #
> > +  # $perf list -v clockticks
> > +  #
> > +  # List of pre-defined events (to be used in -e or -M):
> > +  #
> > +  #   uncore_imc_0/clockticks/                           [Kernel PMU event]
> > +  #   uncore_imc_1/clockticks/                           [Kernel PMU event]
> > +  #   uncore_imc_2/clockticks/                           [Kernel PMU event]
> > +  #   uncore_imc_3/clockticks/                           [Kernel PMU event]
> > +  #   uncore_imc_4/clockticks/                           [Kernel PMU event]
> > +  #   uncore_imc_5/clockticks/                           [Kernel PMU event]
> > +  #
> > +  #   ...
> > +  #
> > +  # $perf stat -e clockticks -A -- true
> > +  #
> > +  #  Performance counter stats for 'system wide':
> > +  #
> > +  # CPU0            3,773,018      uncore_imc_0/clockticks/
> > +  # CPU0            3,609,025      uncore_imc_1/clockticks/
> > +  # CPU0                    0      uncore_imc_2/clockticks/
> > +  # CPU0            3,230,009      uncore_imc_3/clockticks/
> > +  # CPU0            3,049,897      uncore_imc_4/clockticks/
> > +  # CPU0                    0      uncore_imc_5/clockticks/
> > +  #
> > +  #        0.002029828 seconds time elapsed
> > +
> > +  echo "stat event uniquifying test"
> > +  uniquified_event_array=()
> > +
> > +  # Check how many uniquified events.
> > +  while IFS= read -r line; do
> > +    uniquified_event=$(echo "$line" | awk '{print $1}')
> > +    uniquified_event_array+=("${uniquified_event}")
> > +  done < <(${perf_tool} list -v ${event} | grep "\[Kernel PMU event\]")
>
> Shouldn't the array contain every sysfs event that doesn't have a json
> component? They may or may not be uniquified so I think the array name
> is misleading.
>
> > +  perf_command="${perf_tool} stat -e $event -A -o ${stat_output} -- true"
> > +  $perf_command
> > +
> > +  # Check the output contains all uniquified events.
> > +  for uniquified_event in "${uniquified_event_array[@]}"; do
> > +    if ! cat "${stat_output}" | grep -q "${uniquified_event}"; then
>
> Why not pass the file directly to grep?
> Should some of the events not show in the stat output as you only
> asked for the clockticks event? I'm not sure how this test can pass
> currently.
>
> Thanks,
> Ian
>
Thanks for your comment, Ian.

I tried to run `perf list -v clockticks` and grep for all events which
have `[Kernel PMU event]` tag.
Ideally, that should retrieve all uniquified clockticks events.
Then I ran `perf stat -e clockticks` to check if all events from `perf
list` are showing.

I randomly picked some machines from different arches and it seems
work. Let me know if you have a better idea.

-CT

> > +      echo "Event is not uniquified [Failed]"
> > +      echo "${perf_command}"
> > +      cat "${stat_output}"
> > +      err=1
> > +      break
> > +    fi
> > +  done
> > +}
> > +
> > +test_event_uniquifying
> > +rm -f "${stat_output}"
> > +exit $err
> > --
> > 2.49.0.472.ge94155a9ec-goog
> >

[PATCH v2 1/3] perf evlist: Make uniquifying counter names consistent
[PATCH v2 2/3] perf parse-events: Use wildcard processing to set an event to merge into
[PATCH v2 3/3] perf test: Add stat uniquifying test