Re: [PATCH 1/5] perf metricgroup: Support printing metrics for arm64

From: Jiri Olsa <jolsa@redhat.com>
To: John Garry <john.garry@huawei.com>
Cc: will@kernel.org, mathieu.poirier@linaro.org, leo.yan@linaro.org,
	peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
	namhyung@kernel.org, irogers@google.com,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linuxarm@huawei.com,
	zhangshaokun@hisilicon.com, qiangqing.zhang@nxp.com,
	kjain@linux.ibm.com
Subject: Re: [PATCH 1/5] perf metricgroup: Support printing metrics for arm64
Date: Thu, 4 Mar 2021 21:05:52 +0100	[thread overview]
Message-ID: <YEE9oInI38txHWmo@krava> (raw)
In-Reply-To: <1614784938-27080-2-git-send-email-john.garry@huawei.com>

On Wed, Mar 03, 2021 at 11:22:14PM +0800, John Garry wrote:
> Calling perf_pmu__find_map(NULL) returns the cpumap for the common CPU
> PMU. However arm64 supports heterogeneous-CPU based systems, and so there
> may be no common CPU PMU. As such, perf_pmu__find_map(NULL) returns NULL
> for arm64.
> 
> To support printing metrics for arm64, iterate through all PMUs, looking
> for a CPU PMU, and use the cpumap there for determining supported metrics.
> 
> For heterogeneous systems (like arm big.LITTLE), supporting metrics has
> potential challenges, like not all CPUs in a system not supporting a
> specific metric event. So just don't support it for now.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/util/metricgroup.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 26c990e32378..9a2a23093961 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -6,6 +6,7 @@
>  /* Manage metrics and groups of metrics from JSON files */
>  
>  #include "metricgroup.h"
> +#include "cpumap.h"
>  #include "debug.h"
>  #include "evlist.h"
>  #include "evsel.h"
> @@ -615,10 +616,31 @@ static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
>  				     d->details, d->groups, d->metriclist);
>  }
>  
> +static struct pmu_events_map *find_cpumap(void)
> +{
> +	struct perf_pmu *pmu = NULL;
> +
> +	while ((pmu = perf_pmu__scan(pmu))) {
> +		if (!is_pmu_core(pmu->name))
> +			continue;
> +
> +		/*
> +		 * The cpumap should cover all CPUs. Otherwise, some CPUs may
> +		 * not support some events or have different event IDs.
> +		 */
> +		if (pmu->cpus && pmu->cpus->nr != cpu__max_cpu())
> +			return NULL;
> +
> +		return perf_pmu__find_map(pmu);
> +	}
> +
> +	return NULL;
> +}
> +
>  void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>  			bool raw, bool details)
>  {
> -	struct pmu_events_map *map = perf_pmu__find_map(NULL);
> +	struct pmu_events_map *map = find_cpumap();

so this is just for arm at the moment right?

could we rather make this arch specific code, so we don't need
to do the scanning on archs where this is not needed?

like marking perf_pmu__find_map as __weak and add arm specific
version?

thanks,
jirka

>  	struct pmu_event *pe;
>  	int i;
>  	struct rblist groups;
> -- 
> 2.26.2
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel