[PATCH v1 1/2] perf pmus: Add notion of default PMU for JSON events

[PATCH v1 1/2] perf pmus: Add notion of default PMU for JSON events

[Ian Rogers]

JSON events created in pmu-events.c by jevents.py may not specify a
PMU they are associated with, in which case it is implied that it is
the first core PMU. Care is needed to select this for regular 'cpu',
s390 'cpum_cf' and ARMs many names as at the point the name is first
needed the core PMUs list hasn't been initialized. Add a helper in
perf_pmus to create this value, in the worst case by scanning sysfs.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmu.c  | 35 ++++++++++++++++-------------------
 tools/perf/util/pmus.c | 31 +++++++++++++++++++++++++++++++
 tools/perf/util/pmus.h |  1 +
 3 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6142e4710a2f..963c12f910c5 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -741,9 +741,11 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
 }
 
 struct pmu_add_cpu_aliases_map_data {
+	/* List being added to. */
 	struct list_head *head;
-	const char *name;
-	const char *cpu_name;
+	/* If a pmu_event lacks a given PMU the default used. */
+	char *default_pmu_name;
+	/* The PMU that we're searching for events for. */
 	struct perf_pmu *pmu;
 };
 
@@ -752,37 +754,32 @@ static int pmu_add_cpu_aliases_map_callback(const struct pmu_event *pe,
 					void *vdata)
 {
 	struct pmu_add_cpu_aliases_map_data *data = vdata;
-	const char *pname = pe->pmu ? pe->pmu : data->cpu_name;
+	const char *pname = pe->pmu ?: data->default_pmu_name;
 
-	if (data->pmu->is_uncore && pmu_uncore_alias_match(pname, data->name))
-		goto new_alias;
-
-	if (strcmp(pname, data->name))
-		return 0;
-
-new_alias:
-	/* need type casts to override 'const' */
-	__perf_pmu__new_alias(data->head, -1, (char *)pe->name, (char *)pe->desc,
-			      (char *)pe->event, pe);
+	if (!strcmp(pname, data->pmu->name) ||
+	    (data->pmu->is_uncore && pmu_uncore_alias_match(pname, data->pmu->name))) {
+		/* need type casts to override 'const' */
+		__perf_pmu__new_alias(data->head, -1, (char *)pe->name, (char *)pe->desc,
+				      (char *)pe->event, pe);
+	}
 	return 0;
 }
 
 /*
- * From the pmu_events_map, find the table of PMU events that corresponds
- * to the current running CPU. Then, add all PMU events from that table
- * as aliases.
+ * From the pmu_events_table, find the events that correspond to the given
+ * PMU and add them to the list 'head'.
  */
 void pmu_add_cpu_aliases_table(struct list_head *head, struct perf_pmu *pmu,
-			       const struct pmu_events_table *table)
+			const struct pmu_events_table *table)
 {
 	struct pmu_add_cpu_aliases_map_data data = {
 		.head = head,
-		.name = pmu->name,
-		.cpu_name = is_sysfs_pmu_core(pmu->name) ? pmu->name : "cpu",
+		.default_pmu_name = perf_pmus__default_pmu_name(),
 		.pmu = pmu,
 	};
 
 	pmu_events_table_for_each_event(table, pmu_add_cpu_aliases_map_callback, &data);
+	free(data.default_pmu_name);
 }
 
 static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index d891d72c824e..80797e1b862f 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -524,6 +524,37 @@ bool perf_pmus__supports_extended_type(void)
 	return perf_pmus__do_support_extended_type;
 }
 
+char *perf_pmus__default_pmu_name(void)
+{
+	int fd;
+	DIR *dir;
+	struct dirent *dent;
+	char *result = NULL;
+
+	if (!list_empty(&core_pmus))
+		return strdup(list_first_entry(&core_pmus, struct perf_pmu, list)->name);
+
+	fd = perf_pmu__event_source_devices_fd();
+	if (fd < 0)
+		return strdup("cpu");
+
+	dir = fdopendir(fd);
+	if (!dir)
+		return strdup("cpu");
+
+	while ((dent = readdir(dir))) {
+		if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
+			continue;
+		if (is_pmu_core(dent->d_name)) {
+			result = strdup(dent->d_name);
+			break;
+		}
+	}
+
+	closedir(dir);
+	return result ?: strdup("cpu");
+}
+
 struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
 {
 	struct perf_pmu *pmu = evsel->pmu;
diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
index d02ffea5d3a4..a21464432d0f 100644
--- a/tools/perf/util/pmus.h
+++ b/tools/perf/util/pmus.h
@@ -20,5 +20,6 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 bool perf_pmus__have_event(const char *pname, const char *name);
 int perf_pmus__num_core_pmus(void);
 bool perf_pmus__supports_extended_type(void);
+char *perf_pmus__default_pmu_name(void);
 
 #endif /* __PMUS_H */
-- 
2.41.0.162.gfafddb0af9-goog

[PATCH v1 2/2] perf pmu: Remove a hard coded cpu PMU assumption

[Ian Rogers]

The property of "cpu" when it has no cpu map is true on S390 with the
PMU cpum_cf. Rather than maintain a list of such PMUs, reuse the
is_core test result from the caller.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/pmu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 963c12f910c5..64fa568a5426 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -551,7 +551,7 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias,
  * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
  * may have a "cpus" file.
  */
-static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name)
+static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name, bool is_core)
 {
 	struct perf_cpu_map *cpus;
 	const char *templates[] = {
@@ -575,7 +575,8 @@ static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name)
 			return cpus;
 	}
 
-	return !strcmp(name, "cpu") ? perf_cpu_map__get(cpu_map__online()) : NULL;
+	/* Nothing found, for core PMUs assume this means all CPUs. */
+	return is_core ? perf_cpu_map__get(cpu_map__online()) : NULL;
 }
 
 static bool pmu_is_uncore(int dirfd, const char *name)
@@ -886,7 +887,8 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 	if (!pmu)
 		return NULL;
 
-	pmu->cpus = pmu_cpumask(dirfd, name);
+	pmu->is_core = is_pmu_core(name);
+	pmu->cpus = pmu_cpumask(dirfd, name, pmu->is_core);
 	pmu->name = strdup(name);
 	if (!pmu->name)
 		goto err;
@@ -903,7 +905,6 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 	}
 
 	pmu->type = type;
-	pmu->is_core = is_pmu_core(name);
 	pmu->is_uncore = pmu_is_uncore(dirfd, name);
 	if (pmu->is_uncore)
 		pmu->id = pmu_id(name);
-- 
2.41.0.162.gfafddb0af9-goog

Re: [PATCH v1 2/2] perf pmu: Remove a hard coded cpu PMU assumption

[Thomas Richter]

On 6/22/23 07:03, Ian Rogers wrote:
> The property of "cpu" when it has no cpu map is true on S390 with the
> PMU cpum_cf. Rather than maintain a list of such PMUs, reuse the
> is_core test result from the caller.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/util/pmu.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 963c12f910c5..64fa568a5426 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -551,7 +551,7 @@ static int pmu_alias_terms(struct perf_pmu_alias *alias,
>   * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
>   * may have a "cpus" file.
>   */
> -static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name)
> +static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name, bool is_core)
>  {
>  	struct perf_cpu_map *cpus;
>  	const char *templates[] = {
> @@ -575,7 +575,8 @@ static struct perf_cpu_map *pmu_cpumask(int dirfd, const char *name)
>  			return cpus;
>  	}
>  
> -	return !strcmp(name, "cpu") ? perf_cpu_map__get(cpu_map__online()) : NULL;
> +	/* Nothing found, for core PMUs assume this means all CPUs. */
> +	return is_core ? perf_cpu_map__get(cpu_map__online()) : NULL;
>  }
>  
>  static bool pmu_is_uncore(int dirfd, const char *name)
> @@ -886,7 +887,8 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
>  	if (!pmu)
>  		return NULL;
>  
> -	pmu->cpus = pmu_cpumask(dirfd, name);
> +	pmu->is_core = is_pmu_core(name);
> +	pmu->cpus = pmu_cpumask(dirfd, name, pmu->is_core);
>  	pmu->name = strdup(name);
>  	if (!pmu->name)
>  		goto err;
> @@ -903,7 +905,6 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
>  	}
>  
>  	pmu->type = type;
> -	pmu->is_core = is_pmu_core(name);
>  	pmu->is_uncore = pmu_is_uncore(dirfd, name);
>  	if (pmu->is_uncore)
>  		pmu->id = pmu_id(name);

Looks good to me, for both patches:

Tested-by: Thomas Richter <tmricht@linux.ibm.com>
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

Re: [PATCH v1 1/2] perf pmus: Add notion of default PMU for JSON events

[Namhyung Kim]

On Wed, Jun 21, 2023 at 10:03 PM Ian Rogers <irogers@google.com> wrote:
>
> JSON events created in pmu-events.c by jevents.py may not specify a
> PMU they are associated with, in which case it is implied that it is
> the first core PMU. Care is needed to select this for regular 'cpu',
> s390 'cpum_cf' and ARMs many names as at the point the name is first
> needed the core PMUs list hasn't been initialized. Add a helper in
> perf_pmus to create this value, in the worst case by scanning sysfs.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---

[SNIP]
> diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
> index d891d72c824e..80797e1b862f 100644
> --- a/tools/perf/util/pmus.c
> +++ b/tools/perf/util/pmus.c
> @@ -524,6 +524,37 @@ bool perf_pmus__supports_extended_type(void)
>         return perf_pmus__do_support_extended_type;
>  }
>
> +char *perf_pmus__default_pmu_name(void)
> +{
> +       int fd;
> +       DIR *dir;
> +       struct dirent *dent;
> +       char *result = NULL;
> +
> +       if (!list_empty(&core_pmus))
> +               return strdup(list_first_entry(&core_pmus, struct perf_pmu, list)->name);
> +
> +       fd = perf_pmu__event_source_devices_fd();
> +       if (fd < 0)
> +               return strdup("cpu");
> +
> +       dir = fdopendir(fd);
> +       if (!dir)
> +               return strdup("cpu");

close(fd) ?

Thanks,
Namhyung


> +
> +       while ((dent = readdir(dir))) {
> +               if (!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, ".."))
> +                       continue;
> +               if (is_pmu_core(dent->d_name)) {
> +                       result = strdup(dent->d_name);
> +                       break;
> +               }
> +       }
> +
> +       closedir(dir);
> +       return result ?: strdup("cpu");
> +}
> +
>  struct perf_pmu *evsel__find_pmu(const struct evsel *evsel)
>  {
>         struct perf_pmu *pmu = evsel->pmu;
> diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
> index d02ffea5d3a4..a21464432d0f 100644
> --- a/tools/perf/util/pmus.h
> +++ b/tools/perf/util/pmus.h
> @@ -20,5 +20,6 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
>  bool perf_pmus__have_event(const char *pname, const char *name);
>  int perf_pmus__num_core_pmus(void);
>  bool perf_pmus__supports_extended_type(void);
> +char *perf_pmus__default_pmu_name(void);
>
>  #endif /* __PMUS_H */
> --
> 2.41.0.162.gfafddb0af9-goog
>