Re: Hybrid PMU issues on aarch64. was: Re: perf test failures in linux-next on s390

[Ian Rogers <irogers@google.com>]

On Fri, Jun 16, 2023 at 2:47 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Fri, Jun 16, 2023 at 01:53:41PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > presumably with the #ifdef you just get 1 PMU - shame. I think rather
> > > than do an #ifdef we can do something like call is_event_supported:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/print-events.c?h=perf-tools-next#n232
> > > so:
> > > bool perf_pmus__supports_extended_type(void)
> > >   struct perf_pmu *pmu = NULL;
> > >   if (perf_pmus__num_core_pmus() <= 1)
> > >      return false;
> > >   while((pmu = perf_pmus__scan_core(pmu) != NULL) {
> > >     return is_event_supported(PERF_TYPE_HARDWARE,
> > > PERF_COUNT_HW_CPU_CYCLES | ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT);
> > >  }
> > > return false;
> > > }
> > > We probably don't want to do this for each call of
> > > perf_pmus__supports_extended_type so you could use a static and
> > > pthread_once, etc.
> > >
> > > This would mean if this regression is introduced elsewhere than ARM it
> > > will self heal. It will also mean that when ARM support extended types
> > > in the kernel, they will get the normal heterogeneous behavior.
> >
> > That looks better, I'll try it when I get back to my office.
>
> End result, Ack?
>

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

Thanks,
Ian

> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index a2032c1b7644..d891d72c824e 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -4,6 +4,7 @@
>  #include <subcmd/pager.h>
>  #include <sys/types.h>
>  #include <dirent.h>
> +#include <pthread.h>
>  #include <string.h>
>  #include <unistd.h>
>  #include "debug.h"
> @@ -492,9 +493,35 @@ int perf_pmus__num_core_pmus(void)
>         return count;
>  }
>
> +static bool __perf_pmus__supports_extended_type(void)
> +{
> +       struct perf_pmu *pmu = NULL;
> +
> +       if (perf_pmus__num_core_pmus() <= 1)
> +               return false;
> +
> +       while ((pmu = perf_pmus__scan_core(pmu)) != NULL) {
> +               if (!is_event_supported(PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES | ((__u64)pmu->type << PERF_PMU_TYPE_SHIFT)))
> +                       return false;
> +       }
> +
> +       return true;
> +}
> +
> +static bool perf_pmus__do_support_extended_type;
> +
> +static void perf_pmus__init_supports_extended_type(void)
> +{
> +       perf_pmus__do_support_extended_type = __perf_pmus__supports_extended_type();
> +}
> +
>  bool perf_pmus__supports_extended_type(void)
>  {
> -       return perf_pmus__num_core_pmus() > 1;
> +       static pthread_once_t extended_type_once = PTHREAD_ONCE_INIT;
> +
> +       pthread_once(&extended_type_once, perf_pmus__init_supports_extended_type);
> +
> +       return perf_pmus__do_support_extended_type;
>  }
>
>  struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index 7a5f87392720..a7566edc86a3 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -229,7 +229,7 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state)
>         strlist__delete(sdtlist);
>  }
>
> -static bool is_event_supported(u8 type, u64 config)
> +bool is_event_supported(u8 type, u64 config)
>  {
>         bool ret = true;
>         int open_return;
> diff --git a/tools/perf/util/print-events.h b/tools/perf/util/print-events.h
> index e75a3d7e3fe3..d7fab411e75c 100644
> --- a/tools/perf/util/print-events.h
> +++ b/tools/perf/util/print-events.h
> @@ -3,6 +3,7 @@
>  #define __PERF_PRINT_EVENTS_H
>
>  #include <linux/perf_event.h>
> +#include <linux/types.h>
>  #include <stdbool.h>
>
>  struct event_symbol;
> @@ -36,5 +37,6 @@ void print_symbol_events(const struct print_callbacks *print_cb, void *print_sta
>                          unsigned int max);
>  void print_tool_events(const struct print_callbacks *print_cb, void *print_state);
>  void print_tracepoint_events(const struct print_callbacks *print_cb, void *print_state);
> +bool is_event_supported(u8 type, u64 config);
>
>  #endif /* __PERF_PRINT_EVENTS_H */