[PATCH 0/3] perf list: Improve libpfm4 event support

[PATCH 0/3] perf list: Improve libpfm4 event support

[Namhyung Kim]

Hello,

Ian and I found that some of libpfm4 events don't work well and it makes
confusing that perf list shows those unsupported events too.  Let's check
if the event is supported like other events and skip if it's not.

Also add a new test for it.  On my laptop, it runs ok like below:

  $ ./perf test -v 102
  102: perf all libpfm4 events test                                    :
  --- start ---
  test child forked, pid 3039688
  Testing ix86arch::UNHALTED_CORE_CYCLES
  Testing ix86arch::INSTRUCTION_RETIRED
  Testing ix86arch::UNHALTED_REFERENCE_CYCLES
  Testing ix86arch::LLC_REFERENCES
  Testing ix86arch::LLC_MISSES
  Testing ix86arch::BRANCH_INSTRUCTIONS_RETIRED
  Testing ix86arch::MISPREDICTED_BRANCH_RETIRED
  Testing perf_raw::r0000
  Testing icl::UNHALTED_CORE_CYCLES
  Testing icl::UNHALTED_REFERENCE_CYCLES
  Testing icl::INSTRUCTION_RETIRED
  Testing INSTRUCTION_RETIRED
  Testing icl::SQ_MISC
  Testing icl::SQ_MISC:SQ_FULL
  ...
  Testing icl::OCR:DEMAND_DATA_RD_L3_HIT_ANY
  Testing icl::OCR:DEMAND_DATA_RD_L3_HIT_SNOOP_HITM
  Testing icl::OCR:DEMAND_DATA_RD_L3_HIT_SNOOP_HIT_NO_FWD
  Testing icl::OCR:DEMAND_DATA_RD_L3_HIT_SNOOP_MISS
  Testing icl::OCR:DEMAND_DATA_RD_L3_HIT_SNOOP_NOT_NEEDED
  test child finished with 0
  ---- end ----
  perf all libpfm4 events test: Ok
  
Thanks,
Namhyung


Ian Rogers (2):
  perf list: Check arguments to show libpfm4 events
  perf test: Add test of libpfm4 events

Namhyung Kim (1):
  perf list: Check if libpfm4 event is supported

 tools/perf/builtin-list.c              | 12 +++++-
 tools/perf/tests/shell/stat_all_pfm.sh | 51 ++++++++++++++++++++++
 tools/perf/util/pfm.c                  | 58 ++++++++++++++++++++++----
 3 files changed, 112 insertions(+), 9 deletions(-)
 create mode 100755 tools/perf/tests/shell/stat_all_pfm.sh

base-commit: 634c694de4cb2d050dc312ea43e0371d84c585a5
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 1/3] perf list: Check if libpfm4 event is supported

[Namhyung Kim]

Some of its event info cannot be used directly due to missing default
attributes.  Let's check if the event is supported before printing
like we do for hw and cache events.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/pfm.c | 58 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
index 076aecc22c16..4c1024c343dd 100644
--- a/tools/perf/util/pfm.c
+++ b/tools/perf/util/pfm.c
@@ -13,6 +13,8 @@
 #include "util/pmus.h"
 #include "util/pfm.h"
 #include "util/strbuf.h"
+#include "util/cpumap.h"
+#include "util/thread_map.h"
 
 #include <string.h>
 #include <linux/kernel.h>
@@ -123,6 +125,36 @@ int parse_libpfm_events_option(const struct option *opt, const char *str,
 	return -1;
 }
 
+static bool is_libpfm_event_supported(const char *name, struct perf_cpu_map *cpus,
+				      struct perf_thread_map *threads)
+{
+	struct perf_pmu *pmu;
+	struct evsel *evsel;
+	struct perf_event_attr attr = {};
+	bool result = true;
+	int ret;
+
+	ret = pfm_get_perf_event_encoding(name, PFM_PLM0|PFM_PLM3,
+					  &attr, NULL, NULL);
+	if (ret != PFM_SUCCESS)
+		return false;
+
+	pmu = perf_pmus__find_by_type((unsigned int)attr.type);
+	evsel = parse_events__add_event(0, &attr, name, /*metric_id=*/NULL, pmu);
+	if (evsel == NULL)
+		return false;
+
+	evsel->is_libpfm_event = true;
+
+	if (evsel__open(evsel, cpus, threads) < 0)
+		result = false;
+
+	evsel__close(evsel);
+	evsel__delete(evsel);
+
+	return result;
+}
+
 static const char *srcs[PFM_ATTR_CTRL_MAX] = {
 	[PFM_ATTR_CTRL_UNKNOWN] = "???",
 	[PFM_ATTR_CTRL_PMU] = "PMU",
@@ -146,6 +178,8 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
 {
 	int j, ret;
 	char topic[80], name[80];
+	struct perf_cpu_map *cpus = perf_cpu_map__empty_new(1);
+	struct perf_thread_map *threads = thread_map__new_by_tid(0);
 
 	strbuf_setlen(buf, 0);
 	snprintf(topic, sizeof(topic), "pfm %s", pinfo->name);
@@ -185,14 +219,15 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
 				    ainfo.name, ainfo.desc);
 		}
 	}
-	print_cb->print_event(print_state,
-			pinfo->name,
-			topic,
-			name, info->equiv,
-			/*scale_unit=*/NULL,
-			/*deprecated=*/NULL, "PFM event",
-			info->desc, /*long_desc=*/NULL,
-			/*encoding_desc=*/buf->buf);
+
+	if (is_libpfm_event_supported(name, cpus, threads)) {
+		print_cb->print_event(print_state, pinfo->name, topic,
+				      name, info->equiv,
+				      /*scale_unit=*/NULL,
+				      /*deprecated=*/NULL, "PFM event",
+				      info->desc, /*long_desc=*/NULL,
+				      /*encoding_desc=*/buf->buf);
+	}
 
 	pfm_for_each_event_attr(j, info) {
 		pfm_event_attr_info_t ainfo;
@@ -215,6 +250,10 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
 			print_attr_flags(buf, &ainfo);
 			snprintf(name, sizeof(name), "%s::%s:%s",
 				 pinfo->name, info->name, ainfo.name);
+
+			if (!is_libpfm_event_supported(name, cpus, threads))
+				continue;
+
 			print_cb->print_event(print_state,
 					pinfo->name,
 					topic,
@@ -225,6 +264,9 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
 					/*encoding_desc=*/buf->buf);
 		}
 	}
+
+	perf_cpu_map__put(cpus);
+	perf_thread_map__put(threads);
 }
 
 void print_libpfm_events(const struct print_callbacks *print_cb, void *print_state)
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 2/3] perf list: Check arguments to show libpfm4 events

[Namhyung Kim]

From: Ian Rogers <irogers@google.com>

This is particularly useful for tests.

  $ perf list pfm

Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-list.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 03b5d26b2489..7fec2cca759f 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -15,6 +15,7 @@
 #include "util/pmu.h"
 #include "util/debug.h"
 #include "util/metricgroup.h"
+#include "util/pfm.h"
 #include "util/string2.h"
 #include "util/strlist.h"
 #include "util/strbuf.h"
@@ -457,7 +458,11 @@ int cmd_list(int argc, const char **argv)
 		OPT_END()
 	};
 	const char * const list_usage[] = {
+#ifdef HAVE_LIBPFM
+		"perf list [<options>] [hw|sw|cache|tracepoint|pmu|sdt|metric|metricgroup|event_glob|pfm]",
+#else
 		"perf list [<options>] [hw|sw|cache|tracepoint|pmu|sdt|metric|metricgroup|event_glob]",
+#endif
 		NULL
 	};
 
@@ -539,7 +544,12 @@ int cmd_list(int argc, const char **argv)
 			default_ps.metricgroups = true;
 			default_ps.metrics = false;
 			metricgroup__print(&print_cb, ps);
-		} else if ((sep = strchr(argv[i], ':')) != NULL) {
+		}
+#ifdef HAVE_LIBPFM
+		else if (strcmp(argv[i], "pfm") == 0)
+			print_libpfm_events(&print_cb, ps);
+#endif
+		else if ((sep = strchr(argv[i], ':')) != NULL) {
 			char *old_pmu_glob = default_ps.pmu_glob;
 
 			default_ps.event_glob = strdup(argv[i]);
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH 3/3] perf test: Add test of libpfm4 events

[Namhyung Kim]

From: Ian Rogers <irogers@google.com>

  $ ./perf test -v 102
  102: perf all libpfm4 events test                                    :
  --- start ---
  test child forked, pid 3030994
  Testing ix86arch::UNHALTED_CORE_CYCLES
  Testing ix86arch::INSTRUCTION_RETIRED
  Testing ix86arch::UNHALTED_REFERENCE_CYCLES
  Testing ix86arch::LLC_REFERENCES
  Testing ix86arch::LLC_MISSES
  Testing ix86arch::BRANCH_INSTRUCTIONS_RETIRED
  Testing ix86arch::MISPREDICTED_BRANCH_RETIRED
  Testing perf_raw::r0000
  Testing icl::UNHALTED_CORE_CYCLES
  Testing icl::UNHALTED_REFERENCE_CYCLES
  ...
  test child finished with 0
  ---- end ----
  perf all libpfm4 events test: Ok

Signed-off-by: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/stat_all_pfm.sh | 51 ++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100755 tools/perf/tests/shell/stat_all_pfm.sh

diff --git a/tools/perf/tests/shell/stat_all_pfm.sh b/tools/perf/tests/shell/stat_all_pfm.sh
new file mode 100755
index 000000000000..4d004f777a6e
--- /dev/null
+++ b/tools/perf/tests/shell/stat_all_pfm.sh
@@ -0,0 +1,51 @@
+#!/bin/sh
+# perf all libpfm4 events test
+# SPDX-License-Identifier: GPL-2.0
+
+if perf version --build-options | grep HAVE_LIBPFM | grep -q OFF
+then
+  echo "Skipping, no libpfm4 support"
+  exit 2
+fi
+
+err=0
+for p in $(perf list --raw-dump pfm)
+do
+  if echo "$p" | grep -q unc_
+  then
+    echo "Skipping uncore event '$p' that may require additional options."
+    continue
+  fi
+  echo "Testing $p"
+  result=$(perf stat --pfm-events "$p" true 2>&1)
+  x=$?
+  if echo "$result" | grep -q "failed to parse event $p : invalid or missing unit mask"
+  then
+    continue
+  fi
+  if test "$x" -ne "0"
+  then
+    echo "Unexpected exit code '$x'"
+    err=1
+  fi
+  if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>"
+  then
+    # We failed to see the event and it is supported. Possibly the workload was
+    # too small so retry with something longer.
+    result=$(perf stat --pfm-events "$p" perf bench internals synthesize 2>&1)
+    x=$?
+    if test "$x" -ne "0"
+    then
+      echo "Unexpected exit code '$x'"
+      err=1
+    fi
+    if ! echo "$result" | grep -q "$p"
+    then
+      echo "Event '$p' not printed in:"
+      echo "$result"
+      err=1
+    fi
+  fi
+done
+
+exit "$err"
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH 1/3] perf list: Check if libpfm4 event is supported

[Ian Rogers]

On Thu, Jun 8, 2023 at 4:24 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Some of its event info cannot be used directly due to missing default
> attributes.  Let's check if the event is supported before printing
> like we do for hw and cache events.
>
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

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

Thanks,
Ian

> ---
>  tools/perf/util/pfm.c | 58 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 50 insertions(+), 8 deletions(-)
>
> diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> index 076aecc22c16..4c1024c343dd 100644
> --- a/tools/perf/util/pfm.c
> +++ b/tools/perf/util/pfm.c
> @@ -13,6 +13,8 @@
>  #include "util/pmus.h"
>  #include "util/pfm.h"
>  #include "util/strbuf.h"
> +#include "util/cpumap.h"
> +#include "util/thread_map.h"
>
>  #include <string.h>
>  #include <linux/kernel.h>
> @@ -123,6 +125,36 @@ int parse_libpfm_events_option(const struct option *opt, const char *str,
>         return -1;
>  }
>
> +static bool is_libpfm_event_supported(const char *name, struct perf_cpu_map *cpus,
> +                                     struct perf_thread_map *threads)
> +{
> +       struct perf_pmu *pmu;
> +       struct evsel *evsel;
> +       struct perf_event_attr attr = {};
> +       bool result = true;
> +       int ret;
> +
> +       ret = pfm_get_perf_event_encoding(name, PFM_PLM0|PFM_PLM3,
> +                                         &attr, NULL, NULL);
> +       if (ret != PFM_SUCCESS)
> +               return false;
> +
> +       pmu = perf_pmus__find_by_type((unsigned int)attr.type);
> +       evsel = parse_events__add_event(0, &attr, name, /*metric_id=*/NULL, pmu);
> +       if (evsel == NULL)
> +               return false;
> +
> +       evsel->is_libpfm_event = true;
> +
> +       if (evsel__open(evsel, cpus, threads) < 0)
> +               result = false;
> +
> +       evsel__close(evsel);
> +       evsel__delete(evsel);
> +
> +       return result;
> +}
> +
>  static const char *srcs[PFM_ATTR_CTRL_MAX] = {
>         [PFM_ATTR_CTRL_UNKNOWN] = "???",
>         [PFM_ATTR_CTRL_PMU] = "PMU",
> @@ -146,6 +178,8 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
>  {
>         int j, ret;
>         char topic[80], name[80];
> +       struct perf_cpu_map *cpus = perf_cpu_map__empty_new(1);
> +       struct perf_thread_map *threads = thread_map__new_by_tid(0);
>
>         strbuf_setlen(buf, 0);
>         snprintf(topic, sizeof(topic), "pfm %s", pinfo->name);
> @@ -185,14 +219,15 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
>                                     ainfo.name, ainfo.desc);
>                 }
>         }
> -       print_cb->print_event(print_state,
> -                       pinfo->name,
> -                       topic,
> -                       name, info->equiv,
> -                       /*scale_unit=*/NULL,
> -                       /*deprecated=*/NULL, "PFM event",
> -                       info->desc, /*long_desc=*/NULL,
> -                       /*encoding_desc=*/buf->buf);
> +
> +       if (is_libpfm_event_supported(name, cpus, threads)) {
> +               print_cb->print_event(print_state, pinfo->name, topic,
> +                                     name, info->equiv,
> +                                     /*scale_unit=*/NULL,
> +                                     /*deprecated=*/NULL, "PFM event",
> +                                     info->desc, /*long_desc=*/NULL,
> +                                     /*encoding_desc=*/buf->buf);
> +       }
>
>         pfm_for_each_event_attr(j, info) {
>                 pfm_event_attr_info_t ainfo;
> @@ -215,6 +250,10 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
>                         print_attr_flags(buf, &ainfo);
>                         snprintf(name, sizeof(name), "%s::%s:%s",
>                                  pinfo->name, info->name, ainfo.name);
> +
> +                       if (!is_libpfm_event_supported(name, cpus, threads))
> +                               continue;
> +
>                         print_cb->print_event(print_state,
>                                         pinfo->name,
>                                         topic,
> @@ -225,6 +264,9 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
>                                         /*encoding_desc=*/buf->buf);
>                 }
>         }
> +
> +       perf_cpu_map__put(cpus);
> +       perf_thread_map__put(threads);
>  }
>
>  void print_libpfm_events(const struct print_callbacks *print_cb, void *print_state)
> --
> 2.41.0.162.gfafddb0af9-goog
>

Re: [PATCH 1/3] perf list: Check if libpfm4 event is supported

[Arnaldo Carvalho de Melo]

Em Thu, Jun 08, 2023 at 04:38:06PM -0700, Ian Rogers escreveu:
> On Thu, Jun 8, 2023 at 4:24 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Some of its event info cannot be used directly due to missing default
> > attributes.  Let's check if the event is supported before printing
> > like we do for hw and cache events.
> >
> > Cc: Stephane Eranian <eranian@google.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers>@google.com>

Thanks, applied the series.

- Arnaldo

 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/pfm.c | 58 +++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 50 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/perf/util/pfm.c b/tools/perf/util/pfm.c
> > index 076aecc22c16..4c1024c343dd 100644
> > --- a/tools/perf/util/pfm.c
> > +++ b/tools/perf/util/pfm.c
> > @@ -13,6 +13,8 @@
> >  #include "util/pmus.h"
> >  #include "util/pfm.h"
> >  #include "util/strbuf.h"
> > +#include "util/cpumap.h"
> > +#include "util/thread_map.h"
> >
> >  #include <string.h>
> >  #include <linux/kernel.h>
> > @@ -123,6 +125,36 @@ int parse_libpfm_events_option(const struct option *opt, const char *str,
> >         return -1;
> >  }
> >
> > +static bool is_libpfm_event_supported(const char *name, struct perf_cpu_map *cpus,
> > +                                     struct perf_thread_map *threads)
> > +{
> > +       struct perf_pmu *pmu;
> > +       struct evsel *evsel;
> > +       struct perf_event_attr attr = {};
> > +       bool result = true;
> > +       int ret;
> > +
> > +       ret = pfm_get_perf_event_encoding(name, PFM_PLM0|PFM_PLM3,
> > +                                         &attr, NULL, NULL);
> > +       if (ret != PFM_SUCCESS)
> > +               return false;
> > +
> > +       pmu = perf_pmus__find_by_type((unsigned int)attr.type);
> > +       evsel = parse_events__add_event(0, &attr, name, /*metric_id=*/NULL, pmu);
> > +       if (evsel == NULL)
> > +               return false;
> > +
> > +       evsel->is_libpfm_event = true;
> > +
> > +       if (evsel__open(evsel, cpus, threads) < 0)
> > +               result = false;
> > +
> > +       evsel__close(evsel);
> > +       evsel__delete(evsel);
> > +
> > +       return result;
> > +}
> > +
> >  static const char *srcs[PFM_ATTR_CTRL_MAX] = {
> >         [PFM_ATTR_CTRL_UNKNOWN] = "???",
> >         [PFM_ATTR_CTRL_PMU] = "PMU",
> > @@ -146,6 +178,8 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> >  {
> >         int j, ret;
> >         char topic[80], name[80];
> > +       struct perf_cpu_map *cpus = perf_cpu_map__empty_new(1);
> > +       struct perf_thread_map *threads = thread_map__new_by_tid(0);
> >
> >         strbuf_setlen(buf, 0);
> >         snprintf(topic, sizeof(topic), "pfm %s", pinfo->name);
> > @@ -185,14 +219,15 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> >                                     ainfo.name, ainfo.desc);
> >                 }
> >         }
> > -       print_cb->print_event(print_state,
> > -                       pinfo->name,
> > -                       topic,
> > -                       name, info->equiv,
> > -                       /*scale_unit=*/NULL,
> > -                       /*deprecated=*/NULL, "PFM event",
> > -                       info->desc, /*long_desc=*/NULL,
> > -                       /*encoding_desc=*/buf->buf);
> > +
> > +       if (is_libpfm_event_supported(name, cpus, threads)) {
> > +               print_cb->print_event(print_state, pinfo->name, topic,
> > +                                     name, info->equiv,
> > +                                     /*scale_unit=*/NULL,
> > +                                     /*deprecated=*/NULL, "PFM event",
> > +                                     info->desc, /*long_desc=*/NULL,
> > +                                     /*encoding_desc=*/buf->buf);
> > +       }
> >
> >         pfm_for_each_event_attr(j, info) {
> >                 pfm_event_attr_info_t ainfo;
> > @@ -215,6 +250,10 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> >                         print_attr_flags(buf, &ainfo);
> >                         snprintf(name, sizeof(name), "%s::%s:%s",
> >                                  pinfo->name, info->name, ainfo.name);
> > +
> > +                       if (!is_libpfm_event_supported(name, cpus, threads))
> > +                               continue;
> > +
> >                         print_cb->print_event(print_state,
> >                                         pinfo->name,
> >                                         topic,
> > @@ -225,6 +264,9 @@ print_libpfm_event(const struct print_callbacks *print_cb, void *print_state,
> >                                         /*encoding_desc=*/buf->buf);
> >                 }
> >         }
> > +
> > +       perf_cpu_map__put(cpus);
> > +       perf_thread_map__put(threads);
> >  }
> >
> >  void print_libpfm_events(const struct print_callbacks *print_cb, void *print_state)
> > --
> > 2.41.0.162.gfafddb0af9-goog
> >

-- 

- Arnaldo