[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210208185521.GJ920417@kernel.org>
Date: Mon, 8 Feb 2021 15:55:21 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: kan.liang@...ux.intel.com
Cc: peterz@...radead.org, mingo@...nel.org,
linux-kernel@...r.kernel.org, tglx@...utronix.de, bp@...en8.de,
namhyung@...nel.org, jolsa@...hat.com, ak@...ux.intel.com,
yao.jin@...ux.intel.com, alexander.shishkin@...ux.intel.com,
adrian.hunter@...el.com
Subject: Re: [PATCH 28/49] perf pmu: Save detected hybrid pmus to a global
pmu list
Em Mon, Feb 08, 2021 at 07:25:25AM -0800, kan.liang@...ux.intel.com escreveu:
> From: Jin Yao <yao.jin@...ux.intel.com>
>
> We identify the cpu_core pmu and cpu_atom pmu by explicitly
> checking following files:
>
> For cpu_core, check:
> "/sys/bus/event_source/devices/cpu_core/cpus"
>
> For cpu_atom, check:
> "/sys/bus/event_source/devices/cpu_atom/cpus"
>
> If the 'cpus' file exists, the pmu exists.
>
> But in order not to hardcode the "cpu_core" and "cpu_atom",
> and make the code generic, if the path
> "/sys/bus/event_source/devices/cpu_xxx/cpus" exists, the hybrid
> pmu exists. All the detected hybrid pmus are linked to a
> global list 'perf_pmu__hybrid_pmus' and then next we just need
> to iterate the list by using perf_pmu__for_each_hybrid_pmus.
>
> Reviewed-by: Andi Kleen <ak@...ux.intel.com>
> Signed-off-by: Jin Yao <yao.jin@...ux.intel.com>
> ---
> tools/perf/util/pmu.c | 21 +++++++++++++++++++++
> tools/perf/util/pmu.h | 7 +++++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 0c25457..e97b121 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -27,6 +27,7 @@
> #include "fncache.h"
>
> struct perf_pmu perf_pmu__fake;
> +LIST_HEAD(perf_pmu__hybrid_pmus);
>
> struct perf_pmu_format {
> char *name;
> @@ -633,11 +634,27 @@ static struct perf_cpu_map *pmu_cpumask(const char *name)
> return NULL;
> }
>
> +static bool pmu_is_hybrid(const char *name)
> +{
> + char path[PATH_MAX];
> + const char *sysfs;
> +
> + if (strncmp(name, "cpu_", 4))
> + return false;
> +
> + sysfs = sysfs__mountpoint();
Its extremely unlikely that sysfs isn't mounted, but if so, this will
NULL deref, so please do as other sysfs__mountpoint() uses in
tools/perf/util/pmu.c and check if sysfs is NULL, returning false, i.e.
file isn't available.
> + snprintf(path, PATH_MAX, CPUS_TEMPLATE_CPU, sysfs, name);
> + return file_available(path);
> +}
> +
> static bool pmu_is_uncore(const char *name)
> {
> char path[PATH_MAX];
> const char *sysfs;
>
> + if (pmu_is_hybrid(name))
> + return false;
> +
> sysfs = sysfs__mountpoint();
> snprintf(path, PATH_MAX, CPUS_TEMPLATE_UNCORE, sysfs, name);
> return file_available(path);
> @@ -951,6 +968,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
> pmu->is_uncore = pmu_is_uncore(name);
> if (pmu->is_uncore)
> pmu->id = pmu_id(name);
> + pmu->is_hybrid = pmu_is_hybrid(name);
> pmu->max_precise = pmu_max_precise(name);
> pmu_add_cpu_aliases(&aliases, pmu);
> pmu_add_sys_aliases(&aliases, pmu);
> @@ -962,6 +980,9 @@ static struct perf_pmu *pmu_lookup(const char *name)
> list_splice(&aliases, &pmu->aliases);
> list_add_tail(&pmu->list, &pmus);
>
> + if (pmu->is_hybrid)
> + list_add_tail(&pmu->hybrid_list, &perf_pmu__hybrid_pmus);
> +
> pmu->default_config = perf_pmu__get_default_config(pmu);
>
> return pmu;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 0e724d5..99bdb5d 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -5,6 +5,7 @@
> #include <linux/bitmap.h>
> #include <linux/compiler.h>
> #include <linux/perf_event.h>
> +#include <linux/list.h>
> #include <stdbool.h>
> #include "parse-events.h"
> #include "pmu-events/pmu-events.h"
> @@ -34,6 +35,7 @@ struct perf_pmu {
> __u32 type;
> bool selectable;
> bool is_uncore;
> + bool is_hybrid;
> bool auxtrace;
> int max_precise;
> struct perf_event_attr *default_config;
> @@ -42,9 +44,11 @@ struct perf_pmu {
> struct list_head aliases; /* HEAD struct perf_pmu_alias -> list */
> struct list_head caps; /* HEAD struct perf_pmu_caps -> list */
> struct list_head list; /* ELEM */
> + struct list_head hybrid_list;
> };
>
> extern struct perf_pmu perf_pmu__fake;
> +extern struct list_head perf_pmu__hybrid_pmus;
>
> struct perf_pmu_info {
> const char *unit;
> @@ -124,4 +128,7 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
>
> int perf_pmu__caps_parse(struct perf_pmu *pmu);
>
> +#define perf_pmu__for_each_hybrid_pmus(pmu) \
singular, i.e.
#define perf_pmu__for_each_hybrid_pmu(pmu) \
> + list_for_each_entry(pmu, &perf_pmu__hybrid_pmus, hybrid_list)
> +
> #endif /* __PMU_H */
> --
> 2.7.4
>
--
- Arnaldo
Powered by blists - more mailing lists