Skip to contentSkip to conversation

Tool and hwmon PMUs

PATCH Applied Ian Rogers 4 participants 27 messages Sep 7, 2024 – Sep 11, 2024

Subsystems (3)

v1 of 3 Applied Latest is v3 →

lore
Share & tools
lore

Ian Rogers

author reviewer
#0 lore raw

cover Tool and hwmon PMUs

Rather than have fake and tool PMUs being special flags in an evsel, create special PMUs. This allows, for example, duration_time to also be tool/duration_time/. Once adding events to the tools PMU is just adding to an array, add events for nearly all the expr literals like num_cpus_online. Rather than create custom logic for finding and describing the tool events use json and add a notion of common json for the tool events.

Following the convention of the tool PMU, create a hwmon PMU that exposes hwmon data for reading. For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
     1.001153138              52.00 'C   temp_cpu
     1.001153138              2,588 rpm  fan1
     1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
     1.001153138                  8      tool/num_cpus_online/
     1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
     1.001153138      1,012,773,595      duration_time
...
```

Additional data on the hwmon events is in perf list:

```
$ perf list
...
hwmon:
...
  temp_core_0 OR temp2
       [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
...
```

Ian Rogers (15):
perf list: Avoid potential out of bounds memory read
perf pmus: Fake PMU clean up
perf evsel: Add accessor for tool_event
perf pmu: To info add event_type_desc
perf pmu: Allow hardcoded terms to be applied to attributes perf parse-events: Expose/rename config_term_name
perf tool_pmu: Factor tool events into their own PMU
perf tool_pmu: Rename enum perf_tool_event to tool_pmu_event perf tool_pmu: Rename perf_tool_event__* to tool_pmu__* perf tool_pmu: Move expr literals to tool_pmu
perf jevents: Add tool event json under a common architecture perf tool_pmu: Switch to standard pmu functions and json descriptions perf tests: Add tool PMU test
perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs perf docs: Document tool and hwmon events

 tools/perf/Documentation/perf-list.txt        |  15 +
 tools/perf/arch/arm64/util/pmu.c              |   5 +-
 tools/perf/arch/x86/util/intel-pt.c           |   3 +-
 tools/perf/arch/x86/util/tsc.c                |  16 +-
 tools/perf/builtin-list.c                     |  15 +-
 tools/perf/builtin-stat.c                     |   7 +-
 .../pmu-events/arch/common/common/tool.json   |  74 ++
 tools/perf/pmu-events/empty-pmu-events.c      | 208 +++--
 tools/perf/pmu-events/jevents.py              |  16 +-
 tools/perf/tests/Build                        |   1 +
 tools/perf/tests/builtin-test.c               |   1 +
 tools/perf/tests/parse-events.c               |   4 +-
 tools/perf/tests/pmu-events.c                 |  12 +-
 tools/perf/tests/pmu.c                        |   3 +-
 tools/perf/tests/tests.h                      |   1 +
 tools/perf/tests/tool_pmu.c                   | 111 +++
 tools/perf/util/Build                         |   2 +
 tools/perf/util/evsel.c                       | 287 +-----
 tools/perf/util/evsel.h                       |  23 +-
 tools/perf/util/expr.c                        |  93 +-
 tools/perf/util/hwmon_pmu.c                   | 879 ++++++++++++++++++
 tools/perf/util/hwmon_pmu.h                   |  30 +
 tools/perf/util/metricgroup.c                 |  45 +-
 tools/perf/util/parse-events.c                |  92 +-
 tools/perf/util/parse-events.h                |  13 +-
 tools/perf/util/parse-events.l                |  11 -
 tools/perf/util/parse-events.y                |  16 -
 tools/perf/util/pmu.c                         | 108 ++-
 tools/perf/util/pmu.h                         |  17 +-
 tools/perf/util/pmus.c                        |  25 +-
 tools/perf/util/pmus.h                        |   1 +
 tools/perf/util/print-events.c                |  36 +-
 tools/perf/util/print-events.h                |   1 -
 tools/perf/util/stat-display.c                |  14 +-
 tools/perf/util/stat-shadow.c                 |  24 +-
 tools/perf/util/tool_pmu.c                    | 508 ++++++++++
 tools/perf/util/tool_pmu.h                    |  56 ++
 tools/perf/util/tsc.h                         |   2 +-
 38 files changed, 2116 insertions(+), 659 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/common/common/tool.json
 create mode 100644 tools/perf/tests/tool_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.h
 create mode 100644 tools/perf/util/tool_pmu.c
 create mode 100644 tools/perf/util/tool_pmu.h
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#1 lore raw

PATCH 1/15 perf list: Avoid potential out of bounds memory read

If a desc string is 0 length then -1 will be out of bounds, add a check.

---
 tools/perf/builtin-list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-list.c +1 -1
index 82cb4b1010aa..65b8cba324be 100644
@@ -173,7 +173,7 @@ static void default_print_event(void *ps, const char *pmu_name, const char *topi
 		if (pmu_name && strcmp(pmu_name, "default_core")) {
 			desc_len = strlen(desc);
 			desc_len = asprintf(&desc_with_unit,
-					    desc[desc_len - 1] != '.'
+					    desc_len > 0 && desc[desc_len - 1] != '.'
 					      ? "%s. Unit: %s" : "%s Unit: %s",
 					    desc, pmu_name);
 		}
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
#2 lore raw

Re: [PATCH v1 01/15] perf list: Avoid potential out of bounds memory read

Quoted 2 lines · Ian Rogers

If a desc string is 0 length then -1 will be out of bounds, add a check.

Cherry picked this one.

- Arnaldo

Quoted 20 lines
---
 tools/perf/builtin-list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-list.c +1 -1
index 82cb4b1010aa..65b8cba324be 100644
@@ -173,7 +173,7 @@ static void default_print_event(void *ps, const char *pmu_name, const char *topi
 		if (pmu_name && strcmp(pmu_name, "default_core")) {
 			desc_len = strlen(desc);
 			desc_len = asprintf(&desc_with_unit,
-					    desc[desc_len - 1] != '.'
+					    desc_len > 0 && desc[desc_len - 1] != '.'
 					      ? "%s. Unit: %s" : "%s Unit: %s",
 					    desc, pmu_name);
 		}
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#3 lore raw

PATCH 2/15 perf pmus: Fake PMU clean up

In reply to Ian Rogers

Rather than passing a fake PMU around, just pass that the fake PMU should be used - true when doing testing. Move the fake PMU into pmus.[ch] and try to abstract the PMU's properties in pmu.c, ie so there is less "if fake_pmu" in non-PMU code. Give the fake PMU a made up type number.

---
 tools/perf/tests/parse-events.c |  4 ++--
 tools/perf/tests/pmu-events.c   | 12 +++++-------
 tools/perf/util/metricgroup.c   | 10 +++++-----
 tools/perf/util/parse-events.c  | 33 +++++++++++++++++----------------
 tools/perf/util/parse-events.h  |  8 ++++----
 tools/perf/util/pmu.c           | 21 +++++++++++++++------
 tools/perf/util/pmu.h           |  7 ++++---
 tools/perf/util/pmus.c          | 11 +++++++++++
 tools/perf/util/pmus.h          |  1 +
 9 files changed, 64 insertions(+), 43 deletions(-)
tools/perf/tests/parse-events.c +2 -2
index edc2adcf1bae..a9a77dfe0e97 100644
@@ -2500,7 +2500,7 @@ static int test_event(const struct evlist_test *e)
 		return TEST_FAIL;
 	}
 	parse_events_error__init(&err);
-	ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/NULL,
+	ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/false,
 			     /*warn_if_reordered=*/true, /*fake_tp=*/true);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
@@ -2529,7 +2529,7 @@ static int test_event_fake_pmu(const char *str)
 
 	parse_events_error__init(&err);
 	ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
-			     &perf_pmu__fake, /*warn_if_reordered=*/true,
+			     /*fake_pmu=*/true, /*warn_if_reordered=*/true,
 			     /*fake_tp=*/true);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n",
tools/perf/tests/pmu-events.c +5 -7
index ff3e7bc0a77f..db004d26fcb0 100644
@@ -819,8 +819,7 @@ static bool is_number(const char *str)
 	return errno == 0 && end_ptr != str;
 }
 
-static int check_parse_id(const char *id, struct parse_events_error *error,
-			  struct perf_pmu *fake_pmu)
+static int check_parse_id(const char *id, struct parse_events_error *error)
 {
 	struct evlist *evlist;
 	int ret;
@@ -841,7 +840,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
 	for (cur = strchr(dup, '@') ; cur; cur = strchr(++cur, '@'))
 		*cur = '/';
 
-	ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu,
+	ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, /*fake_pmu=*/true,
 			     /*warn_if_reordered=*/true, /*fake_tp=*/false);
 	free(dup);
 
@@ -855,7 +854,7 @@ static int check_parse_fake(const char *id)
 	int ret;
 
 	parse_events_error__init(&error);
-	ret = check_parse_id(id, &error, &perf_pmu__fake);
+	ret = check_parse_id(id, &error);
 	parse_events_error__exit(&error);
 	return ret;
 }
@@ -1051,9 +1050,8 @@ static int test__parsing_fake_callback(const struct pmu_metric *pm,
 }
 
 /*
- * Parse all the metrics for current architecture,
- * or all defined cpus via the 'fake_pmu'
- * in parse_events.
+ * Parse all the metrics for current architecture, or all defined cpus via the
+ * 'fake_pmu' in parse_events.
  */
 static int test__parsing_fake(struct test_suite *test __maybe_unused,
 			      int subtest __maybe_unused)
tools/perf/util/metricgroup.c +5 -5
index 69f6a46402c3..4dff3e925a47 100644
@@ -1436,7 +1436,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  * parse_ids - Build the event string for the ids and parse them creating an
  *             evlist. The encoded metric_ids are decoded.
  * @metric_no_merge: is metric sharing explicitly disabled.
- * @fake_pmu: used when testing metrics not supported by the current CPU.
+ * @fake_pmu: use a fake PMU when testing metrics not supported by the current CPU.
  * @ids: the event identifiers parsed from a metric.
  * @modifier: any modifiers added to the events.
  * @group_events: should events be placed in a weak group.
@@ -1444,7 +1444,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  *               the overall list of metrics.
  * @out_evlist: the created list of events.
  */
-static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
+static int parse_ids(bool metric_no_merge, bool fake_pmu,
 		     struct expr_parse_ctx *ids, const char *modifier,
 		     bool group_events, const bool tool_events[PERF_TOOL_MAX],
 		     struct evlist **out_evlist)
@@ -1528,7 +1528,7 @@ static int parse_groups(struct evlist *perf_evlist,
 			bool metric_no_threshold,
 			const char *user_requested_cpu_list,
 			bool system_wide,
-			struct perf_pmu *fake_pmu,
+			bool fake_pmu,
 			struct rblist *metric_events_list,
 			const struct pmu_metrics_table *table)
 {
@@ -1703,7 +1703,7 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
 
 	return parse_groups(perf_evlist, pmu, str, metric_no_group, metric_no_merge,
 			    metric_no_threshold, user_requested_cpu_list, system_wide,
-			    /*fake_pmu=*/NULL, metric_events, table);
+			    /*fake_pmu=*/false, metric_events, table);
 }
 
 int metricgroup__parse_groups_test(struct evlist *evlist,
@@ -1717,7 +1717,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
 			    /*metric_no_threshold=*/false,
 			    /*user_requested_cpu_list=*/NULL,
 			    /*system_wide=*/false,
-			    &perf_pmu__fake, metric_events, table);
+			    /*fake_pmu=*/true, metric_events, table);
 }
 
 struct metricgroup__has_metric_data {
tools/perf/util/parse-events.c +17 -16
index fab01ba54e34..081ceff467f2 100644
@@ -1478,8 +1478,8 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 	}
 
 	/* Look for event names in the terms and rewrite into format based terms. */
-	if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, &parsed_terms,
-							    &info, &alias_rewrote_terms, err)) {
+	if (perf_pmu__check_alias(pmu, &parsed_terms,
+				  &info, &alias_rewrote_terms, err)) {
 		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
 	}
@@ -1515,8 +1515,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return -ENOMEM;
 	}
 
-	if (!parse_state->fake_pmu &&
-	    perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
+	if (perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
 		free_config_terms(&config_terms);
 		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
@@ -1536,11 +1535,6 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	evsel->percore = config_term_percore(&evsel->config_terms);
 
-	if (parse_state->fake_pmu) {
-		parse_events_terms__exit(&parsed_terms);
-		return 0;
-	}
-
 	parse_events_terms__exit(&parsed_terms);
 	free((char *)evsel->unit);
 	evsel->unit = strdup(info.unit);
@@ -1616,13 +1610,13 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 	}
 
 	if (parse_state->fake_pmu) {
-		if (!parse_events_add_pmu(parse_state, list, parse_state->fake_pmu, &parsed_terms,
+		if (!parse_events_add_pmu(parse_state, list, perf_pmus__fake_pmu(), &parsed_terms,
 					  /*auto_merge_stats=*/true)) {
 			struct strbuf sb;
 
 			strbuf_init(&sb, /*hint=*/ 0);
 			parse_events_terms__to_strbuf(&parsed_terms, &sb);
-			pr_debug("%s -> %s/%s/\n", event_name, "fake_pmu", sb.buf);
+			pr_debug("%s -> fake/%s/\n", event_name, sb.buf);
 			strbuf_release(&sb);
 			ok++;
 		}
@@ -1656,11 +1650,18 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
 	INIT_LIST_HEAD(*listp);
 
 	/* Attempt to add to list assuming event_or_pmu is a PMU name. */
-	pmu = parse_state->fake_pmu ?: perf_pmus__find(event_or_pmu);
+	pmu = perf_pmus__find(event_or_pmu);
 	if (pmu && !parse_events_add_pmu(parse_state, *listp, pmu, const_parsed_terms,
 					/*auto_merge_stats=*/false))
 		return 0;
 
+	if (parse_state->fake_pmu) {
+		if (!parse_events_add_pmu(parse_state, *listp, perf_pmus__fake_pmu(),
+					  const_parsed_terms,
+					  /*auto_merge_stats=*/false))
+			return 0;
+	}
+
 	pmu = NULL;
 	/* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
 	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
@@ -1961,8 +1962,8 @@ static int evsel__compute_group_pmu_name(struct evsel *evsel,
 			}
 		}
 	}
-	/* Assign the actual name taking care that the fake PMU lacks a name. */
-	evsel->group_pmu_name = strdup(group_pmu_name ?: "fake");
+	/* Record computed name. */
+	evsel->group_pmu_name = strdup(group_pmu_name);
 	return evsel->group_pmu_name ? 0 : -ENOMEM;
 }
 
@@ -2124,7 +2125,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
 }
 
 int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
-		   struct parse_events_error *err, struct perf_pmu *fake_pmu,
+		   struct parse_events_error *err, bool fake_pmu,
 		   bool warn_if_reordered, bool fake_tp)
 {
 	struct parse_events_state parse_state = {
@@ -2343,7 +2344,7 @@ int parse_events_option(const struct option *opt, const char *str,
 
 	parse_events_error__init(&err);
 	ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err,
-			     /*fake_pmu=*/NULL, /*warn_if_reordered=*/true,
+			     /*fake_pmu=*/false, /*warn_if_reordered=*/true,
 			     /*fake_tp=*/false);
 
 	if (ret) {
tools/perf/util/parse-events.h +4 -4
index b735cd9e0acf..bfbf391451d9 100644
@@ -31,14 +31,14 @@ int parse_events_option(const struct option *opt, const char *str, int unset);
 int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset);
 __attribute__((nonnull(1, 2, 4)))
 int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
-		   struct parse_events_error *error, struct perf_pmu *fake_pmu,
+		   struct parse_events_error *error, bool fake_pmu,
 		   bool warn_if_reordered, bool fake_tp);
 
 __attribute__((nonnull(1, 2, 3)))
 static inline int parse_events(struct evlist *evlist, const char *str,
 			       struct parse_events_error *err)
 {
-	return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL,
+	return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/false,
 			      /*warn_if_reordered=*/true, /*fake_tp=*/false);
 }
 
@@ -150,8 +150,8 @@ struct parse_events_state {
 	struct parse_events_terms *terms;
 	/* Start token. */
 	int			   stoken;
-	/* Special fake PMU marker for testing. */
-	struct perf_pmu		  *fake_pmu;
+	/* Use the fake PMU marker for testing. */
+	bool			   fake_pmu;
 	/* Skip actual tracepoint processing for testing. */
 	bool			   fake_tp;
 	/* If non-null, when wildcard matching only match the given PMU. */
tools/perf/util/pmu.c +15 -6
index 0b38c51bd6eb..50e2e7abffab 100644
@@ -30,10 +30,6 @@
 #include "util/evsel_config.h"
 #include <regex.h>
 
-struct perf_pmu perf_pmu__fake = {
-	.name = "fake",
-};
-
 #define UNIT_MAX_LEN	31 /* max length for event unit name */
 
 enum event_source {
@@ -1173,6 +1169,11 @@ struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pm
 	return pmu;
 }
 
+static bool perf_pmu__is_fake(const struct perf_pmu *pmu)
+{
+	return pmu->type == PERF_PMU_TYPE_FAKE;
+}
+
 void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
 {
 	struct perf_pmu_format *format;
@@ -1183,7 +1184,7 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
 	pmu->formats_checked = true;
 
 	/* fake pmu doesn't have format list */
-	if (pmu == &perf_pmu__fake)
+	if (perf_pmu__is_fake(pmu))
 		return;
 
 	list_for_each_entry(format, &pmu->format, list) {
@@ -1511,6 +1512,10 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 {
 	bool zero = !!pmu->perf_event_attr_init_default;
 
+	/* Fake PMU doesn't have proper terms so nothing to configure in attr. */
+	if (perf_pmu__is_fake(pmu))
+		return 0;
+
 	return perf_pmu__config_terms(pmu, attr, head_terms, zero, err);
 }
 
@@ -1619,6 +1624,10 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
+	/* Fake PMU doesn't rewrite terms. */
+	if (perf_pmu__is_fake(pmu))
+		goto out;
+
 	list_for_each_entry_safe(term, h, &head_terms->terms, list) {
 		alias = pmu_find_alias(pmu, term);
 		if (!alias)
@@ -1641,7 +1650,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 		list_del_init(&term->list);
 		parse_events_term__delete(term);
 	}
-
+out:
 	/*
 	 * if no unit or scale found in aliases, then
 	 * set defaults as for evsel
tools/perf/util/pmu.h +4 -3
index b2d3fd291f02..d6f702e55a05 100644
@@ -36,6 +36,10 @@ struct perf_pmu_caps {
 	struct list_head list;
 };
 
+enum {
+	PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
+};
+
 /**
  * struct perf_pmu
  */
@@ -173,9 +177,6 @@ struct perf_pmu {
 	struct perf_mem_event *mem_events;
 };
 
-/** @perf_pmu__fake: A special global PMU used for testing. */
-extern struct perf_pmu perf_pmu__fake;
-
 struct perf_pmu_info {
 	const char *unit;
 	double scale;
tools/perf/util/pmus.c +11 -0
index 3fcabfd8fca1..864cbfa1f960 100644
@@ -720,3 +720,14 @@ struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name)
 	 */
 	return perf_pmu__lookup(&other_pmus, test_sysfs_dirfd, name, /*eager_load=*/true);
 }
+
+struct perf_pmu *perf_pmus__fake_pmu(void)
+{
+	static struct perf_pmu fake = {
+		.name = "fake",
+		.type = PERF_PMU_TYPE_FAKE,
+		.format = LIST_HEAD_INIT(fake.format),
+	};
+
+	return &fake;
+}
tools/perf/util/pmus.h +1 -0
index bdbff02324bb..e1742b56eec7 100644
@@ -30,5 +30,6 @@ bool perf_pmus__supports_extended_type(void);
 char *perf_pmus__default_pmu_name(void);
 
 struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name);
+struct perf_pmu *perf_pmus__fake_pmu(void);
 
 #endif /* __PMUS_H */
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
#4 lore raw

Re: [PATCH v1 02/15] perf pmus: Fake PMU clean up

Quoted 5 lines · Ian Rogers

Rather than passing a fake PMU around, just pass that the fake PMU should be used - true when doing testing. Move the fake PMU into pmus.[ch] and try to abstract the PMU's properties in pmu.c, ie so there is less "if fake_pmu" in non-PMU code. Give the fake PMU a made up type number.

Cherry picked this one.

- Arnaldo

Quoted 386 lines
---
 tools/perf/tests/parse-events.c |  4 ++--
 tools/perf/tests/pmu-events.c   | 12 +++++-------
 tools/perf/util/metricgroup.c   | 10 +++++-----
 tools/perf/util/parse-events.c  | 33 +++++++++++++++++----------------
 tools/perf/util/parse-events.h  |  8 ++++----
 tools/perf/util/pmu.c           | 21 +++++++++++++++------
 tools/perf/util/pmu.h           |  7 ++++---
 tools/perf/util/pmus.c          | 11 +++++++++++
 tools/perf/util/pmus.h          |  1 +
 9 files changed, 64 insertions(+), 43 deletions(-)
tools/perf/tests/parse-events.c +2 -2
index edc2adcf1bae..a9a77dfe0e97 100644
@@ -2500,7 +2500,7 @@ static int test_event(const struct evlist_test *e)
 		return TEST_FAIL;
 	}
 	parse_events_error__init(&err);
-	ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/NULL,
+	ret = __parse_events(evlist, e->name, /*pmu_filter=*/NULL, &err, /*fake_pmu=*/false,
 			     /*warn_if_reordered=*/true, /*fake_tp=*/true);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n", e->name, ret);
@@ -2529,7 +2529,7 @@ static int test_event_fake_pmu(const char *str)
 
 	parse_events_error__init(&err);
 	ret = __parse_events(evlist, str, /*pmu_filter=*/NULL, &err,
-			     &perf_pmu__fake, /*warn_if_reordered=*/true,
+			     /*fake_pmu=*/true, /*warn_if_reordered=*/true,
 			     /*fake_tp=*/true);
 	if (ret) {
 		pr_debug("failed to parse event '%s', err %d\n",
tools/perf/tests/pmu-events.c +5 -7
index ff3e7bc0a77f..db004d26fcb0 100644
@@ -819,8 +819,7 @@ static bool is_number(const char *str)
 	return errno == 0 && end_ptr != str;
 }
 
-static int check_parse_id(const char *id, struct parse_events_error *error,
-			  struct perf_pmu *fake_pmu)
+static int check_parse_id(const char *id, struct parse_events_error *error)
 {
 	struct evlist *evlist;
 	int ret;
@@ -841,7 +840,7 @@ static int check_parse_id(const char *id, struct parse_events_error *error,
 	for (cur = strchr(dup, '@') ; cur; cur = strchr(++cur, '@'))
 		*cur = '/';
 
-	ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, fake_pmu,
+	ret = __parse_events(evlist, dup, /*pmu_filter=*/NULL, error, /*fake_pmu=*/true,
 			     /*warn_if_reordered=*/true, /*fake_tp=*/false);
 	free(dup);
 
@@ -855,7 +854,7 @@ static int check_parse_fake(const char *id)
 	int ret;
 
 	parse_events_error__init(&error);
-	ret = check_parse_id(id, &error, &perf_pmu__fake);
+	ret = check_parse_id(id, &error);
 	parse_events_error__exit(&error);
 	return ret;
 }
@@ -1051,9 +1050,8 @@ static int test__parsing_fake_callback(const struct pmu_metric *pm,
 }
 
 /*
- * Parse all the metrics for current architecture,
- * or all defined cpus via the 'fake_pmu'
- * in parse_events.
+ * Parse all the metrics for current architecture, or all defined cpus via the
+ * 'fake_pmu' in parse_events.
  */
 static int test__parsing_fake(struct test_suite *test __maybe_unused,
 			      int subtest __maybe_unused)
tools/perf/util/metricgroup.c +5 -5
index 69f6a46402c3..4dff3e925a47 100644
@@ -1436,7 +1436,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  * parse_ids - Build the event string for the ids and parse them creating an
  *             evlist. The encoded metric_ids are decoded.
  * @metric_no_merge: is metric sharing explicitly disabled.
- * @fake_pmu: used when testing metrics not supported by the current CPU.
+ * @fake_pmu: use a fake PMU when testing metrics not supported by the current CPU.
  * @ids: the event identifiers parsed from a metric.
  * @modifier: any modifiers added to the events.
  * @group_events: should events be placed in a weak group.
@@ -1444,7 +1444,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  *               the overall list of metrics.
  * @out_evlist: the created list of events.
  */
-static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
+static int parse_ids(bool metric_no_merge, bool fake_pmu,
 		     struct expr_parse_ctx *ids, const char *modifier,
 		     bool group_events, const bool tool_events[PERF_TOOL_MAX],
 		     struct evlist **out_evlist)
@@ -1528,7 +1528,7 @@ static int parse_groups(struct evlist *perf_evlist,
 			bool metric_no_threshold,
 			const char *user_requested_cpu_list,
 			bool system_wide,
-			struct perf_pmu *fake_pmu,
+			bool fake_pmu,
 			struct rblist *metric_events_list,
 			const struct pmu_metrics_table *table)
 {
@@ -1703,7 +1703,7 @@ int metricgroup__parse_groups(struct evlist *perf_evlist,
 
 	return parse_groups(perf_evlist, pmu, str, metric_no_group, metric_no_merge,
 			    metric_no_threshold, user_requested_cpu_list, system_wide,
-			    /*fake_pmu=*/NULL, metric_events, table);
+			    /*fake_pmu=*/false, metric_events, table);
 }
 
 int metricgroup__parse_groups_test(struct evlist *evlist,
@@ -1717,7 +1717,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
 			    /*metric_no_threshold=*/false,
 			    /*user_requested_cpu_list=*/NULL,
 			    /*system_wide=*/false,
-			    &perf_pmu__fake, metric_events, table);
+			    /*fake_pmu=*/true, metric_events, table);
 }
 
 struct metricgroup__has_metric_data {
tools/perf/util/parse-events.c +17 -16
index fab01ba54e34..081ceff467f2 100644
@@ -1478,8 +1478,8 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 	}
 
 	/* Look for event names in the terms and rewrite into format based terms. */
-	if (!parse_state->fake_pmu && perf_pmu__check_alias(pmu, &parsed_terms,
-							    &info, &alias_rewrote_terms, err)) {
+	if (perf_pmu__check_alias(pmu, &parsed_terms,
+				  &info, &alias_rewrote_terms, err)) {
 		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
 	}
@@ -1515,8 +1515,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return -ENOMEM;
 	}
 
-	if (!parse_state->fake_pmu &&
-	    perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
+	if (perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
 		free_config_terms(&config_terms);
 		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
@@ -1536,11 +1535,6 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	evsel->percore = config_term_percore(&evsel->config_terms);
 
-	if (parse_state->fake_pmu) {
-		parse_events_terms__exit(&parsed_terms);
-		return 0;
-	}
-
 	parse_events_terms__exit(&parsed_terms);
 	free((char *)evsel->unit);
 	evsel->unit = strdup(info.unit);
@@ -1616,13 +1610,13 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 	}
 
 	if (parse_state->fake_pmu) {
-		if (!parse_events_add_pmu(parse_state, list, parse_state->fake_pmu, &parsed_terms,
+		if (!parse_events_add_pmu(parse_state, list, perf_pmus__fake_pmu(), &parsed_terms,
 					  /*auto_merge_stats=*/true)) {
 			struct strbuf sb;
 
 			strbuf_init(&sb, /*hint=*/ 0);
 			parse_events_terms__to_strbuf(&parsed_terms, &sb);
-			pr_debug("%s -> %s/%s/\n", event_name, "fake_pmu", sb.buf);
+			pr_debug("%s -> fake/%s/\n", event_name, sb.buf);
 			strbuf_release(&sb);
 			ok++;
 		}
@@ -1656,11 +1650,18 @@ int parse_events_multi_pmu_add_or_add_pmu(struct parse_events_state *parse_state
 	INIT_LIST_HEAD(*listp);
 
 	/* Attempt to add to list assuming event_or_pmu is a PMU name. */
-	pmu = parse_state->fake_pmu ?: perf_pmus__find(event_or_pmu);
+	pmu = perf_pmus__find(event_or_pmu);
 	if (pmu && !parse_events_add_pmu(parse_state, *listp, pmu, const_parsed_terms,
 					/*auto_merge_stats=*/false))
 		return 0;
 
+	if (parse_state->fake_pmu) {
+		if (!parse_events_add_pmu(parse_state, *listp, perf_pmus__fake_pmu(),
+					  const_parsed_terms,
+					  /*auto_merge_stats=*/false))
+			return 0;
+	}
+
 	pmu = NULL;
 	/* Failed to add, try wildcard expansion of event_or_pmu as a PMU name. */
 	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
@@ -1961,8 +1962,8 @@ static int evsel__compute_group_pmu_name(struct evsel *evsel,
 			}
 		}
 	}
-	/* Assign the actual name taking care that the fake PMU lacks a name. */
-	evsel->group_pmu_name = strdup(group_pmu_name ?: "fake");
+	/* Record computed name. */
+	evsel->group_pmu_name = strdup(group_pmu_name);
 	return evsel->group_pmu_name ? 0 : -ENOMEM;
 }
 
@@ -2124,7 +2125,7 @@ static int parse_events__sort_events_and_fix_groups(struct list_head *list)
 }
 
 int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
-		   struct parse_events_error *err, struct perf_pmu *fake_pmu,
+		   struct parse_events_error *err, bool fake_pmu,
 		   bool warn_if_reordered, bool fake_tp)
 {
 	struct parse_events_state parse_state = {
@@ -2343,7 +2344,7 @@ int parse_events_option(const struct option *opt, const char *str,
 
 	parse_events_error__init(&err);
 	ret = __parse_events(*args->evlistp, str, args->pmu_filter, &err,
-			     /*fake_pmu=*/NULL, /*warn_if_reordered=*/true,
+			     /*fake_pmu=*/false, /*warn_if_reordered=*/true,
 			     /*fake_tp=*/false);
 
 	if (ret) {
tools/perf/util/parse-events.h +4 -4
index b735cd9e0acf..bfbf391451d9 100644
@@ -31,14 +31,14 @@ int parse_events_option(const struct option *opt, const char *str, int unset);
 int parse_events_option_new_evlist(const struct option *opt, const char *str, int unset);
 __attribute__((nonnull(1, 2, 4)))
 int __parse_events(struct evlist *evlist, const char *str, const char *pmu_filter,
-		   struct parse_events_error *error, struct perf_pmu *fake_pmu,
+		   struct parse_events_error *error, bool fake_pmu,
 		   bool warn_if_reordered, bool fake_tp);
 
 __attribute__((nonnull(1, 2, 3)))
 static inline int parse_events(struct evlist *evlist, const char *str,
 			       struct parse_events_error *err)
 {
-	return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/NULL,
+	return __parse_events(evlist, str, /*pmu_filter=*/NULL, err, /*fake_pmu=*/false,
 			      /*warn_if_reordered=*/true, /*fake_tp=*/false);
 }
 
@@ -150,8 +150,8 @@ struct parse_events_state {
 	struct parse_events_terms *terms;
 	/* Start token. */
 	int			   stoken;
-	/* Special fake PMU marker for testing. */
-	struct perf_pmu		  *fake_pmu;
+	/* Use the fake PMU marker for testing. */
+	bool			   fake_pmu;
 	/* Skip actual tracepoint processing for testing. */
 	bool			   fake_tp;
 	/* If non-null, when wildcard matching only match the given PMU. */
tools/perf/util/pmu.c +15 -6
index 0b38c51bd6eb..50e2e7abffab 100644
@@ -30,10 +30,6 @@
 #include "util/evsel_config.h"
 #include <regex.h>
 
-struct perf_pmu perf_pmu__fake = {
-	.name = "fake",
-};
-
 #define UNIT_MAX_LEN	31 /* max length for event unit name */
 
 enum event_source {
@@ -1173,6 +1169,11 @@ struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pm
 	return pmu;
 }
 
+static bool perf_pmu__is_fake(const struct perf_pmu *pmu)
+{
+	return pmu->type == PERF_PMU_TYPE_FAKE;
+}
+
 void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
 {
 	struct perf_pmu_format *format;
@@ -1183,7 +1184,7 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
 	pmu->formats_checked = true;
 
 	/* fake pmu doesn't have format list */
-	if (pmu == &perf_pmu__fake)
+	if (perf_pmu__is_fake(pmu))
 		return;
 
 	list_for_each_entry(format, &pmu->format, list) {
@@ -1511,6 +1512,10 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 {
 	bool zero = !!pmu->perf_event_attr_init_default;
 
+	/* Fake PMU doesn't have proper terms so nothing to configure in attr. */
+	if (perf_pmu__is_fake(pmu))
+		return 0;
+
 	return perf_pmu__config_terms(pmu, attr, head_terms, zero, err);
 }
 
@@ -1619,6 +1624,10 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
+	/* Fake PMU doesn't rewrite terms. */
+	if (perf_pmu__is_fake(pmu))
+		goto out;
+
 	list_for_each_entry_safe(term, h, &head_terms->terms, list) {
 		alias = pmu_find_alias(pmu, term);
 		if (!alias)
@@ -1641,7 +1650,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 		list_del_init(&term->list);
 		parse_events_term__delete(term);
 	}
-
+out:
 	/*
 	 * if no unit or scale found in aliases, then
 	 * set defaults as for evsel
tools/perf/util/pmu.h +4 -3
index b2d3fd291f02..d6f702e55a05 100644
@@ -36,6 +36,10 @@ struct perf_pmu_caps {
 	struct list_head list;
 };
 
+enum {
+	PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
+};
+
 /**
  * struct perf_pmu
  */
@@ -173,9 +177,6 @@ struct perf_pmu {
 	struct perf_mem_event *mem_events;
 };
 
-/** @perf_pmu__fake: A special global PMU used for testing. */
-extern struct perf_pmu perf_pmu__fake;
-
 struct perf_pmu_info {
 	const char *unit;
 	double scale;
tools/perf/util/pmus.c +11 -0
index 3fcabfd8fca1..864cbfa1f960 100644
@@ -720,3 +720,14 @@ struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name)
 	 */
 	return perf_pmu__lookup(&other_pmus, test_sysfs_dirfd, name, /*eager_load=*/true);
 }
+
+struct perf_pmu *perf_pmus__fake_pmu(void)
+{
+	static struct perf_pmu fake = {
+		.name = "fake",
+		.type = PERF_PMU_TYPE_FAKE,
+		.format = LIST_HEAD_INIT(fake.format),
+	};
+
+	return &fake;
+}
tools/perf/util/pmus.h +1 -0
index bdbff02324bb..e1742b56eec7 100644
@@ -30,5 +30,6 @@ bool perf_pmus__supports_extended_type(void);
 char *perf_pmus__default_pmu_name(void);
 
 struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name);
+struct perf_pmu *perf_pmus__fake_pmu(void);
 
 #endif /* __PMUS_H */
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#5 lore raw

PATCH 3/15 perf evsel: Add accessor for tool_event

In reply to Ian Rogers

Currently tool events use a dedicated variable within the evsel. Later changes will move this to the unused struct perf_event_attr config for these events. Add an accessor to allow the later change to be well typed and avoid changing all uses.

---
 tools/perf/builtin-stat.c     |  6 +++---
 tools/perf/util/evsel.c       | 24 ++++++++++++------------
 tools/perf/util/evsel.h       |  5 +++++
 tools/perf/util/stat-shadow.c |  2 +-
 4 files changed, 21 insertions(+), 16 deletions(-)
tools/perf/builtin-stat.c +3 -3
index cf985cdb9a6e..689a3d43c258 100644
@@ -294,14 +294,14 @@ static int read_single_counter(struct evsel *counter, int cpu_map_idx, int threa
 	 * terminates. Use the wait4 values in that case.
 	 */
 	if (err && cpu_map_idx == 0 &&
-	    (counter->tool_event == PERF_TOOL_USER_TIME ||
-	     counter->tool_event == PERF_TOOL_SYSTEM_TIME)) {
+	    (evsel__tool_event(counter) == PERF_TOOL_USER_TIME ||
+	     evsel__tool_event(counter) == PERF_TOOL_SYSTEM_TIME)) {
 		u64 val, *start_time;
 		struct perf_counts_values *count =
 			perf_counts(counter->counts, cpu_map_idx, thread);
 
 		start_time = xyarray__entry(counter->start_times, cpu_map_idx, thread);
-		if (counter->tool_event == PERF_TOOL_USER_TIME)
+		if (evsel__tool_event(counter) == PERF_TOOL_USER_TIME)
 			val = ru_stats.ru_utime_usec_stat.mean;
 		else
 			val = ru_stats.ru_stime_usec_stat.mean;
tools/perf/util/evsel.c +12 -12
index 49cc71511c0c..dbf9c8cee3c5 100644
@@ -773,7 +773,7 @@ const char *evsel__name(struct evsel *evsel)
 
 	case PERF_TYPE_SOFTWARE:
 		if (evsel__is_tool(evsel))
-			evsel__tool_name(evsel->tool_event, bf, sizeof(bf));
+			evsel__tool_name(evsel__tool_event(evsel), bf, sizeof(bf));
 		else
 			evsel__sw_name(evsel, bf, sizeof(bf));
 		break;
@@ -811,7 +811,7 @@ const char *evsel__metric_id(const struct evsel *evsel)
 		return evsel->metric_id;
 
 	if (evsel__is_tool(evsel))
-		return perf_tool_event__to_str(evsel->tool_event);
+		return perf_tool_event__to_str(evsel__tool_event(evsel));
 
 	return "unknown";
 }
@@ -1503,8 +1503,8 @@ void evsel__exit(struct evsel *evsel)
 	evsel->per_pkg_mask = NULL;
 	zfree(&evsel->metric_events);
 	perf_evsel__object.fini(evsel);
-	if (evsel->tool_event == PERF_TOOL_SYSTEM_TIME ||
-	    evsel->tool_event == PERF_TOOL_USER_TIME)
+	if (evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME ||
+	    evsel__tool_event(evsel) == PERF_TOOL_USER_TIME)
 		xyarray__delete(evsel->start_times);
 }
 
@@ -1785,7 +1785,7 @@ static int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread)
 
 	count = perf_counts(evsel->counts, cpu_map_idx, thread);
 
-	switch (evsel->tool_event) {
+	switch (evsel__tool_event(evsel)) {
 	case PERF_TOOL_DURATION_TIME:
 		/*
 		 * Pretend duration_time is only on the first CPU and thread, or
@@ -1800,7 +1800,7 @@ static int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread)
 		break;
 	case PERF_TOOL_USER_TIME:
 	case PERF_TOOL_SYSTEM_TIME: {
-		bool system = evsel->tool_event == PERF_TOOL_SYSTEM_TIME;
+		bool system = evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME;
 
 		start_time = xyarray__entry(evsel->start_times, cpu_map_idx, thread);
 		fd = FD(evsel, cpu_map_idx, thread);
@@ -2072,8 +2072,8 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
 	    perf_evsel__alloc_fd(&evsel->core, perf_cpu_map__nr(cpus), nthreads) < 0)
 		return -ENOMEM;
 
-	if ((evsel->tool_event == PERF_TOOL_SYSTEM_TIME ||
-	     evsel->tool_event == PERF_TOOL_USER_TIME) &&
+	if ((evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME ||
+	     evsel__tool_event(evsel) == PERF_TOOL_USER_TIME) &&
 	    !evsel->start_times) {
 		evsel->start_times = xyarray__new(perf_cpu_map__nr(cpus), nthreads, sizeof(__u64));
 		if (!evsel->start_times)
@@ -2262,7 +2262,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 	int pid = -1, err, old_errno;
 	enum rlimit_action set_rlimit = NO_CHANGE;
 
-	if (evsel->tool_event == PERF_TOOL_DURATION_TIME) {
+	if (evsel__tool_event(evsel) == PERF_TOOL_DURATION_TIME) {
 		if (evsel->core.attr.sample_period) /* no sampling */
 			return -EINVAL;
 		evsel->start_time = rdclock();
@@ -2304,9 +2304,9 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 			if (!evsel->cgrp && !evsel->core.system_wide)
 				pid = perf_thread_map__pid(threads, thread);
 
-			if (evsel->tool_event == PERF_TOOL_USER_TIME ||
-			    evsel->tool_event == PERF_TOOL_SYSTEM_TIME) {
-				bool system = evsel->tool_event == PERF_TOOL_SYSTEM_TIME;
+			if (evsel__tool_event(evsel) == PERF_TOOL_USER_TIME ||
+			    evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME) {
+				bool system = evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME;
 				__u64 *start_time = NULL;
 
 				if (evsel->core.attr.sample_period) {
tools/perf/util/evsel.h +5 -0
index 15acf293e12a..15e745a9a798 100644
@@ -330,6 +330,11 @@ static inline bool evsel__is_retire_lat(const struct evsel *evsel)
 	return evsel->retire_lat;
 }
 
+static inline enum perf_tool_event evsel__tool_event(const struct evsel *evsel)
+{
+	return evsel->tool_event;
+}
+
 const char *evsel__group_name(struct evsel *evsel);
 int evsel__group_desc(struct evsel *evsel, char *buf, size_t size);
 
tools/perf/util/stat-shadow.c +1 -1
index 6bb975e46de3..99376c12dd8e 100644
@@ -380,7 +380,7 @@ static int prepare_metric(const struct metric_expr *mexp,
 			struct stats *stats;
 			double scale;
 
-			switch (metric_events[i]->tool_event) {
+			switch (evsel__tool_event(metric_events[i])) {
 			case PERF_TOOL_DURATION_TIME:
 				stats = &walltime_nsecs_stats;
 				scale = 1e-9;
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
maintainer Applied
#6 lore raw

Re: [PATCH v1 03/15] perf evsel: Add accessor for tool_event

Quoted 4 lines · Ian Rogers

Currently tool events use a dedicated variable within the evsel. Later changes will move this to the unused struct perf_event_attr config for these events. Add an accessor to allow the later change to be well typed and avoid changing all uses.

Thanks, applied to perf-tools-next,

- Arnaldo

Quoted 145 lines
---
 tools/perf/builtin-stat.c     |  6 +++---
 tools/perf/util/evsel.c       | 24 ++++++++++++------------
 tools/perf/util/evsel.h       |  5 +++++
 tools/perf/util/stat-shadow.c |  2 +-
 4 files changed, 21 insertions(+), 16 deletions(-)
tools/perf/builtin-stat.c +3 -3
index cf985cdb9a6e..689a3d43c258 100644
@@ -294,14 +294,14 @@ static int read_single_counter(struct evsel *counter, int cpu_map_idx, int threa
 	 * terminates. Use the wait4 values in that case.
 	 */
 	if (err && cpu_map_idx == 0 &&
-	    (counter->tool_event == PERF_TOOL_USER_TIME ||
-	     counter->tool_event == PERF_TOOL_SYSTEM_TIME)) {
+	    (evsel__tool_event(counter) == PERF_TOOL_USER_TIME ||
+	     evsel__tool_event(counter) == PERF_TOOL_SYSTEM_TIME)) {
 		u64 val, *start_time;
 		struct perf_counts_values *count =
 			perf_counts(counter->counts, cpu_map_idx, thread);
 
 		start_time = xyarray__entry(counter->start_times, cpu_map_idx, thread);
-		if (counter->tool_event == PERF_TOOL_USER_TIME)
+		if (evsel__tool_event(counter) == PERF_TOOL_USER_TIME)
 			val = ru_stats.ru_utime_usec_stat.mean;
 		else
 			val = ru_stats.ru_stime_usec_stat.mean;
tools/perf/util/evsel.c +12 -12
index 49cc71511c0c..dbf9c8cee3c5 100644
@@ -773,7 +773,7 @@ const char *evsel__name(struct evsel *evsel)
 
 	case PERF_TYPE_SOFTWARE:
 		if (evsel__is_tool(evsel))
-			evsel__tool_name(evsel->tool_event, bf, sizeof(bf));
+			evsel__tool_name(evsel__tool_event(evsel), bf, sizeof(bf));
 		else
 			evsel__sw_name(evsel, bf, sizeof(bf));
 		break;
@@ -811,7 +811,7 @@ const char *evsel__metric_id(const struct evsel *evsel)
 		return evsel->metric_id;
 
 	if (evsel__is_tool(evsel))
-		return perf_tool_event__to_str(evsel->tool_event);
+		return perf_tool_event__to_str(evsel__tool_event(evsel));
 
 	return "unknown";
 }
@@ -1503,8 +1503,8 @@ void evsel__exit(struct evsel *evsel)
 	evsel->per_pkg_mask = NULL;
 	zfree(&evsel->metric_events);
 	perf_evsel__object.fini(evsel);
-	if (evsel->tool_event == PERF_TOOL_SYSTEM_TIME ||
-	    evsel->tool_event == PERF_TOOL_USER_TIME)
+	if (evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME ||
+	    evsel__tool_event(evsel) == PERF_TOOL_USER_TIME)
 		xyarray__delete(evsel->start_times);
 }
 
@@ -1785,7 +1785,7 @@ static int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread)
 
 	count = perf_counts(evsel->counts, cpu_map_idx, thread);
 
-	switch (evsel->tool_event) {
+	switch (evsel__tool_event(evsel)) {
 	case PERF_TOOL_DURATION_TIME:
 		/*
 		 * Pretend duration_time is only on the first CPU and thread, or
@@ -1800,7 +1800,7 @@ static int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread)
 		break;
 	case PERF_TOOL_USER_TIME:
 	case PERF_TOOL_SYSTEM_TIME: {
-		bool system = evsel->tool_event == PERF_TOOL_SYSTEM_TIME;
+		bool system = evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME;
 
 		start_time = xyarray__entry(evsel->start_times, cpu_map_idx, thread);
 		fd = FD(evsel, cpu_map_idx, thread);
@@ -2072,8 +2072,8 @@ static int __evsel__prepare_open(struct evsel *evsel, struct perf_cpu_map *cpus,
 	    perf_evsel__alloc_fd(&evsel->core, perf_cpu_map__nr(cpus), nthreads) < 0)
 		return -ENOMEM;
 
-	if ((evsel->tool_event == PERF_TOOL_SYSTEM_TIME ||
-	     evsel->tool_event == PERF_TOOL_USER_TIME) &&
+	if ((evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME ||
+	     evsel__tool_event(evsel) == PERF_TOOL_USER_TIME) &&
 	    !evsel->start_times) {
 		evsel->start_times = xyarray__new(perf_cpu_map__nr(cpus), nthreads, sizeof(__u64));
 		if (!evsel->start_times)
@@ -2262,7 +2262,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 	int pid = -1, err, old_errno;
 	enum rlimit_action set_rlimit = NO_CHANGE;
 
-	if (evsel->tool_event == PERF_TOOL_DURATION_TIME) {
+	if (evsel__tool_event(evsel) == PERF_TOOL_DURATION_TIME) {
 		if (evsel->core.attr.sample_period) /* no sampling */
 			return -EINVAL;
 		evsel->start_time = rdclock();
@@ -2304,9 +2304,9 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 			if (!evsel->cgrp && !evsel->core.system_wide)
 				pid = perf_thread_map__pid(threads, thread);
 
-			if (evsel->tool_event == PERF_TOOL_USER_TIME ||
-			    evsel->tool_event == PERF_TOOL_SYSTEM_TIME) {
-				bool system = evsel->tool_event == PERF_TOOL_SYSTEM_TIME;
+			if (evsel__tool_event(evsel) == PERF_TOOL_USER_TIME ||
+			    evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME) {
+				bool system = evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME;
 				__u64 *start_time = NULL;
 
 				if (evsel->core.attr.sample_period) {
tools/perf/util/evsel.h +5 -0
index 15acf293e12a..15e745a9a798 100644
@@ -330,6 +330,11 @@ static inline bool evsel__is_retire_lat(const struct evsel *evsel)
 	return evsel->retire_lat;
 }
 
+static inline enum perf_tool_event evsel__tool_event(const struct evsel *evsel)
+{
+	return evsel->tool_event;
+}
+
 const char *evsel__group_name(struct evsel *evsel);
 int evsel__group_desc(struct evsel *evsel, char *buf, size_t size);
 
tools/perf/util/stat-shadow.c +1 -1
index 6bb975e46de3..99376c12dd8e 100644
@@ -380,7 +380,7 @@ static int prepare_metric(const struct metric_expr *mexp,
 			struct stats *stats;
 			double scale;
 
-			switch (metric_events[i]->tool_event) {
+			switch (evsel__tool_event(metric_events[i])) {
 			case PERF_TOOL_DURATION_TIME:
 				stats = &walltime_nsecs_stats;
 				scale = 1e-9;
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#7 lore raw

PATCH 4/15 perf pmu: To info add event_type_desc

In reply to Ian Rogers

All PMU events are assumed to be "Kernel PMU event", however, this isn't true for fake PMUs and won't be true with the addition of more software PMUs. Make the PMU's type description name configurable - largely for printing callbacks.

---
 tools/perf/util/pmu.c  | 1 +
 tools/perf/util/pmu.h  | 1 +
 tools/perf/util/pmus.c | 5 ++++-
 3 files changed, 6 insertions(+), 1 deletion(-)
tools/perf/util/pmu.c +1 -0
index 50e2e7abffab..72bfc321e4b3 100644
@@ -1857,6 +1857,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	struct perf_pmu_alias *event;
 	struct pmu_event_info info = {
 		.pmu = pmu,
+		.event_type_desc = "Kernel PMU event",
 	};
 	int ret = 0;
 	struct strbuf sb;
tools/perf/util/pmu.h +1 -0
index d6f702e55a05..4397c48ad569 100644
@@ -194,6 +194,7 @@ struct pmu_event_info {
 	const char *encoding_desc;
 	const char *topic;
 	const char *pmu_name;
+	const char *event_type_desc;
 	const char *str;
 	bool deprecated;
 };
tools/perf/util/pmus.c +4 -1
index 864cbfa1f960..93e5c0435ee0 100644
@@ -371,6 +371,7 @@ struct sevent {
 	const char *encoding_desc;
 	const char *topic;
 	const char *pmu_name;
+	const char *event_type_desc;
 	bool deprecated;
 };
 
@@ -444,6 +445,7 @@ static int perf_pmus__print_pmu_events__callback(void *vstate,
 	COPY_STR(encoding_desc);
 	COPY_STR(topic);
 	COPY_STR(pmu_name);
+	COPY_STR(event_type_desc);
 #undef COPY_STR
 	s->deprecated = info->deprecated;
 	state->index++;
@@ -498,7 +500,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 				aliases[j].alias,
 				aliases[j].scale_unit,
 				aliases[j].deprecated,
-				"Kernel PMU event",
+				aliases[j].event_type_desc,
 				aliases[j].desc,
 				aliases[j].long_desc,
 				aliases[j].encoding_desc);
@@ -511,6 +513,7 @@ void perf_pmus__print_pmu_events(const struct print_callbacks *print_cb, void *p
 		zfree(&aliases[j].encoding_desc);
 		zfree(&aliases[j].topic);
 		zfree(&aliases[j].pmu_name);
+		zfree(&aliases[j].event_type_desc);
 	}
 	if (printed && pager_in_use())
 		printf("\n");
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#8 lore raw

PATCH 5/15 perf pmu: Allow hardcoded terms to be applied to attributes

In reply to Ian Rogers

Hard coded terms like "config=10" are skipped by perf_pmu__config assuming they were already applied to a perf_event_attr by parse event's config_attr function. When doing a reverse number to name lookup in perf_pmu__name_from_config, as the hardcoded terms aren't applied the config value is incorrect leading to misses or false matches. Fix this by adding a parameter to have perf_pmu__config apply hardcoded terms too (not just in parse event's config_term_common).

---
 tools/perf/arch/x86/util/intel-pt.c |  3 +-
 tools/perf/tests/pmu.c              |  3 +-
 tools/perf/util/parse-events.c      |  4 ++-
 tools/perf/util/pmu.c               | 54 ++++++++++++++++++++++++-----
 tools/perf/util/pmu.h               |  4 ++-
 5 files changed, 56 insertions(+), 12 deletions(-)
tools/perf/arch/x86/util/intel-pt.c +2 -1
index ea510a7486b1..8f235d8b67b6 100644
@@ -75,7 +75,8 @@ static int intel_pt_parse_terms_with_default(const struct perf_pmu *pmu,
 		goto out_free;
 
 	attr.config = *config;
-	err = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/true, /*err=*/NULL);
+	err = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/true, /*apply_hardcoded=*/false,
+				     /*err=*/NULL);
 	if (err)
 		goto out_free;
 
tools/perf/tests/pmu.c +2 -1
index be18506f6a24..6a681e3fb552 100644
@@ -176,7 +176,8 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
 	}
 
 	memset(&attr, 0, sizeof(attr));
-	ret = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/false, /*err=*/NULL);
+	ret = perf_pmu__config_terms(pmu, &attr, &terms, /*zero=*/false,
+				     /*apply_hardcoded=*/false, /*err=*/NULL);
 	if (ret) {
 		pr_err("perf_pmu__config_terms failed");
 		goto err_out;
tools/perf/util/parse-events.c +3 -1
index 081ceff467f2..30b78fe8d704 100644
@@ -1515,7 +1515,9 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return -ENOMEM;
 	}
 
-	if (perf_pmu__config(pmu, &attr, &parsed_terms, parse_state->error)) {
+	/* Skip configuring hard coded terms that were applied by config_attr. */
+	if (perf_pmu__config(pmu, &attr, &parsed_terms, /*apply_hardcoded=*/false,
+			     parse_state->error)) {
 		free_config_terms(&config_terms);
 		parse_events_terms__exit(&parsed_terms);
 		return -EINVAL;
tools/perf/util/pmu.c +46 -8

96 lines not on this page · raw message

tools/perf/util/pmu.h +3 -1
index 4397c48ad569..af7532ca7fb1 100644
@@ -206,11 +206,13 @@ typedef int (*pmu_format_callback)(void *state, const char *name, int config,
 void pmu_add_sys_aliases(struct perf_pmu *pmu);
 int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 		     struct parse_events_terms *head_terms,
+		     bool apply_hardcoded,
 		     struct parse_events_error *error);
 int perf_pmu__config_terms(const struct perf_pmu *pmu,
 			   struct perf_event_attr *attr,
 			   struct parse_events_terms *terms,
-			   bool zero, struct parse_events_error *error);
+			   bool zero, bool apply_hardcoded,
+			   struct parse_events_error *error);
 __u64 perf_pmu__format_bits(struct perf_pmu *pmu, const char *name);
 int perf_pmu__format_type(struct perf_pmu *pmu, const char *name);
 int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms,
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
#9 lore raw

Re: [PATCH v1 05/15] perf pmu: Allow hardcoded terms to be applied to attributes

Quoted 7 lines · Ian Rogers

Hard coded terms like "config=10" are skipped by perf_pmu__config assuming they were already applied to a perf_event_attr by parse event's config_attr function. When doing a reverse number to name lookup in perf_pmu__name_from_config, as the hardcoded terms aren't applied the config value is incorrect leading to misses or false matches. Fix this by adding a parameter to have perf_pmu__config apply hardcoded terms too (not just in parse event's config_term_common).

Stopped here:

CC /tmp/build/perf-tools-next/util/stat.o
util/pmu.c: In function ‘pmu_config_term’:
util/pmu.c:1397:17: error: switch missing default case [-Werror=switch-default]

 1397 |                 switch (term->type_term) {
      |                 ^~~~~~

CC /tmp/build/perf-tools-next/util/stat-shadow.o

author reviewer
#10 lore raw

PATCH 6/15 perf parse-events: Expose/rename config_term_name

In reply to Ian Rogers

Expose config_term_name as parse_events__term_type_str so that PMUs not in pmu.c may access it.

---
 tools/perf/util/parse-events.c | 20 +++++++++++---------
 tools/perf/util/parse-events.h |  2 ++
 2 files changed, 13 insertions(+), 9 deletions(-)
tools/perf/util/parse-events.c +11 -9
index 30b78fe8d704..b68a744f41ba 100644
@@ -772,7 +772,7 @@ static int check_type_val(struct parse_events_term *term,
 
 static bool config_term_shrinked;
 
-static const char *config_term_name(enum parse_events__term_type term_type)
+const char *parse_events__term_type_str(enum parse_events__term_type term_type)
 {
 	/*
 	 * Update according to parse-events.l
@@ -858,7 +858,7 @@ config_term_avail(enum parse_events__term_type term_type, struct parse_events_er
 
 		/* term_type is validated so indexing is safe */
 		if (asprintf(&err_str, "'%s' is not usable in 'perf stat'",
-			     config_term_name(term_type)) >= 0)
+			     parse_events__term_type_str(term_type)) >= 0)
 			parse_events_error__handle(err, -1, err_str, NULL);
 		return false;
 	}
@@ -982,7 +982,7 @@ do {									   \
 	case PARSE_EVENTS__TERM_TYPE_HARDWARE:
 	default:
 		parse_events_error__handle(err, term->err_term,
-					strdup(config_term_name(term->type_term)),
+					strdup(parse_events__term_type_str(term->type_term)),
 					parse_events_formats_error_string(NULL));
 		return -EINVAL;
 	}
@@ -1105,8 +1105,9 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
 	default:
 		if (err) {
 			parse_events_error__handle(err, term->err_term,
-						   strdup(config_term_name(term->type_term)),
-				strdup("valid terms: call-graph,stack-size\n"));
+					strdup(parse_events__term_type_str(term->type_term)),
+					strdup("valid terms: call-graph,stack-size\n")
+				);
 		}
 		return -EINVAL;
 	}
@@ -2546,7 +2547,7 @@ int parse_events_term__num(struct parse_events_term **term,
 	struct parse_events_term temp = {
 		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
 		.type_term = type_term,
-		.config    = config ? : strdup(config_term_name(type_term)),
+		.config    = config ? : strdup(parse_events__term_type_str(type_term)),
 		.no_value  = no_value,
 		.err_term  = loc_term ? loc_term->first_column : 0,
 		.err_val   = loc_val  ? loc_val->first_column  : 0,
@@ -2580,7 +2581,7 @@ int parse_events_term__term(struct parse_events_term **term,
 			    void *loc_term, void *loc_val)
 {
 	return parse_events_term__str(term, term_lhs, NULL,
-				      strdup(config_term_name(term_rhs)),
+				      strdup(parse_events__term_type_str(term_rhs)),
 				      loc_term, loc_val);
 }
 
@@ -2687,7 +2688,8 @@ int parse_events_terms__to_strbuf(const struct parse_events_terms *terms, struct
 				if (ret < 0)
 					return ret;
 			} else if ((unsigned int)term->type_term < __PARSE_EVENTS__TERM_TYPE_NR) {
-				ret = strbuf_addf(sb, "%s=", config_term_name(term->type_term));
+				ret = strbuf_addf(sb, "%s=",
+						  parse_events__term_type_str(term->type_term));
 				if (ret < 0)
 					return ret;
 			}
@@ -2707,7 +2709,7 @@ static void config_terms_list(char *buf, size_t buf_sz)
 
 	buf[0] = '\0';
 	for (i = 0; i < __PARSE_EVENTS__TERM_TYPE_NR; i++) {
-		const char *name = config_term_name(i);
+		const char *name = parse_events__term_type_str(i);
 
 		if (!config_term_avail(i, NULL))
 			continue;
tools/perf/util/parse-events.h +2 -0
index bfbf391451d9..cf3f07db0295 100644
@@ -162,6 +162,8 @@ struct parse_events_state {
 	bool			   wild_card_pmus;
 };
 
+const char *parse_events__term_type_str(enum parse_events__term_type term_type);
+
 bool parse_events__filter_pmu(const struct parse_events_state *parse_state,
 			      const struct perf_pmu *pmu);
 void parse_events__shrink_config_terms(void);
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#11 lore raw

PATCH 7/15 perf tool_pmu: Factor tool events into their own PMU

In reply to Ian Rogers

Rather than treat tool events as a special kind of event, create a tool only PMU where the events/aliases match the existing duration_time, user_time and system_time events. Remove special parsing and printing support for the tool events, but add function calls for when PMU functions are called on a tool_pmu.

Move the tool PMU code in evsel into tool_pmu.c to better encapsulate the tool event behavior in that file.

---
 tools/perf/builtin-list.c      |  13 +-
 tools/perf/builtin-stat.c      |   1 +
 tools/perf/util/Build          |   1 +
 tools/perf/util/evsel.c        | 272 ++--------------------
 tools/perf/util/evsel.h        |  28 +--
 tools/perf/util/metricgroup.c  |   1 +
 tools/perf/util/parse-events.c |  37 ---
 tools/perf/util/parse-events.h |   3 -
 tools/perf/util/parse-events.l |  11 -
 tools/perf/util/parse-events.y |  16 --
 tools/perf/util/pmu.c          |  20 +-
 tools/perf/util/pmu.h          |   2 +
 tools/perf/util/pmus.c         |   9 +
 tools/perf/util/print-events.c |  36 +--
 tools/perf/util/print-events.h |   1 -
 tools/perf/util/stat-display.c |   6 +-
 tools/perf/util/stat-shadow.c  |   1 +
 tools/perf/util/tool_pmu.c     | 411 +++++++++++++++++++++++++++++++++
 tools/perf/util/tool_pmu.h     |  51 ++++
 19 files changed, 530 insertions(+), 390 deletions(-)
 create mode 100644 tools/perf/util/tool_pmu.c
 create mode 100644 tools/perf/util/tool_pmu.h
tools/perf/builtin-list.c +11 -2
index 65b8cba324be..b8378ba18c28 100644
@@ -19,6 +19,7 @@
 #include "util/string2.h"
 #include "util/strlist.h"
 #include "util/strbuf.h"
+#include "util/tool_pmu.h"
 #include <subcmd/pager.h>
 #include <subcmd/parse-options.h>
 #include <linux/zalloc.h>
@@ -614,9 +615,18 @@ int cmd_list(int argc, const char **argv)
 					event_symbols_hw, PERF_COUNT_HW_MAX);
 		else if (strcmp(argv[i], "sw") == 0 ||
 			 strcmp(argv[i], "software") == 0) {
+			char *old_pmu_glob = default_ps.pmu_glob;
+
 			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, ps);
+			default_ps.pmu_glob = strdup("tool");
+			if (!default_ps.pmu_glob) {
+				ret = -1;
+				goto out;
+			}
+			perf_pmus__print_pmu_events(&print_cb, ps);
+			zfree(&default_ps.pmu_glob);
+			default_ps.pmu_glob = old_pmu_glob;
 		} else if (strcmp(argv[i], "cache") == 0 ||
 			 strcmp(argv[i], "hwcache") == 0)
 			print_hwcache_events(&print_cb, ps);
@@ -664,7 +674,6 @@ int cmd_list(int argc, const char **argv)
 					event_symbols_hw, PERF_COUNT_HW_MAX);
 			print_symbol_events(&print_cb, ps, PERF_TYPE_SOFTWARE,
 					event_symbols_sw, PERF_COUNT_SW_MAX);
-			print_tool_events(&print_cb, ps);
 			print_hwcache_events(&print_cb, ps);
 			perf_pmus__print_pmu_events(&print_cb, ps);
 			print_tracepoint_events(&print_cb, ps);
tools/perf/builtin-stat.c +1 -0
index 689a3d43c258..ae525fdcfbde 100644
@@ -46,6 +46,7 @@
 #include "util/parse-events.h"
 #include "util/pmus.h"
 #include "util/pmu.h"
+#include "util/tool_pmu.h"
 #include "util/event.h"
 #include "util/evlist.h"
 #include "util/evsel.h"
tools/perf/util/Build +1 -0
index 260cec2f6c0b..80187e3a52be 100644
@@ -83,6 +83,7 @@ perf-util-y += pmu.o
 perf-util-y += pmus.o
 perf-util-y += pmu-flex.o
 perf-util-y += pmu-bison.o
+perf-util-y += tool_pmu.o
 perf-util-y += svghelper.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
 perf-util-y += trace-event-scripting.o
tools/perf/util/evsel.c +17 -255

361 lines not on this page · raw message

tools/perf/util/evsel.h +1 -27
index 15e745a9a798..bc30491bd490 100644
@@ -11,6 +11,7 @@
 #include <perf/evsel.h>
 #include "symbol_conf.h"
 #include "pmus.h"
+#include "pmu.h"
 
 struct bpf_object;
 struct cgroup;
@@ -22,25 +23,9 @@ struct target;
 struct hashmap;
 struct bperf_leader_bpf;
 struct bperf_follower_bpf;
-struct perf_pmu;
 
 typedef int (evsel__sb_cb_t)(union perf_event *event, void *data);
 
-enum perf_tool_event {
-	PERF_TOOL_NONE		= 0,
-	PERF_TOOL_DURATION_TIME = 1,
-	PERF_TOOL_USER_TIME = 2,
-	PERF_TOOL_SYSTEM_TIME = 3,
-
-	PERF_TOOL_MAX,
-};
-
-const char *perf_tool_event__to_str(enum perf_tool_event ev);
-enum perf_tool_event perf_tool_event__from_str(const char *str);
-
-#define perf_tool_event__for_each_event(ev)		\
-	for ((ev) = PERF_TOOL_DURATION_TIME; (ev) < PERF_TOOL_MAX; ev++)
-
 /** struct evsel - event selector
  *
  * @evlist - evlist this evsel is in, if it is in one.
@@ -83,7 +68,6 @@ struct evsel {
 		const char		*unit;
 		struct cgroup		*cgrp;
 		const char		*metric_id;
-		enum perf_tool_event	tool_event;
 		/* parse modifier helper */
 		int			exclude_GH;
 		int			sample_read;
@@ -320,21 +304,11 @@ const char *evsel__name(struct evsel *evsel);
 bool evsel__name_is(struct evsel *evsel, const char *name);
 const char *evsel__metric_id(const struct evsel *evsel);
 
-static inline bool evsel__is_tool(const struct evsel *evsel)
-{
-	return evsel->tool_event != PERF_TOOL_NONE;
-}
-
 static inline bool evsel__is_retire_lat(const struct evsel *evsel)
 {
 	return evsel->retire_lat;
 }
 
-static inline enum perf_tool_event evsel__tool_event(const struct evsel *evsel)
-{
-	return evsel->tool_event;
-}
-
 const char *evsel__group_name(struct evsel *evsel);
 int evsel__group_desc(struct evsel *evsel, char *buf, size_t size);
 
tools/perf/util/metricgroup.c +1 -0
index 4dff3e925a47..9181548e8881 100644
@@ -14,6 +14,7 @@
 #include "pmus.h"
 #include "print-events.h"
 #include "smt.h"
+#include "tool_pmu.h"
 #include "expr.h"
 #include "rblist.h"
 #include <string.h>
tools/perf/util/parse-events.c +0 -37
index b68a744f41ba..80e2b4e06ab0 100644
@@ -297,36 +297,6 @@ static int add_event(struct list_head *list, int *idx,
 			   /*auto_merge_stats=*/false, /*cpu_list=*/NULL) ? 0 : -ENOMEM;
 }
 
-static int add_event_tool(struct list_head *list, int *idx,
-			  enum perf_tool_event tool_event)
-{
-	struct evsel *evsel;
-	struct perf_event_attr attr = {
-		.type = PERF_TYPE_SOFTWARE,
-		.config = PERF_COUNT_SW_DUMMY,
-	};
-	const char *cpu_list = NULL;
-
-	if (tool_event == PERF_TOOL_DURATION_TIME) {
-		/* Duration time is gathered globally, pretend it is only on CPU0. */
-		cpu_list = "0";
-	}
-	evsel = __add_event(list, idx, &attr, /*init_attr=*/true, /*name=*/NULL,
-			    /*metric_id=*/NULL, /*pmu=*/NULL,
-			    /*config_terms=*/NULL, /*auto_merge_stats=*/false,
-			    cpu_list);
-	if (!evsel)
-		return -ENOMEM;
-	evsel->tool_event = tool_event;
-	if (tool_event == PERF_TOOL_DURATION_TIME
-	    || tool_event == PERF_TOOL_USER_TIME
-	    || tool_event == PERF_TOOL_SYSTEM_TIME) {
-		free((char *)evsel->unit);
-		evsel->unit = strdup("ns");
-	}
-	return 0;
-}
-
 /**
  * parse_aliases - search names for entries beginning or equalling str ignoring
  *                 case. If mutliple entries in names match str then the longest
@@ -1400,13 +1370,6 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
 					type, /*extended_type=*/0, config, head_config);
 }
 
-int parse_events_add_tool(struct parse_events_state *parse_state,
-			  struct list_head *list,
-			  int tool_event)
-{
-	return add_event_tool(list, &parse_state->idx, tool_event);
-}
-
 static bool config_term_percore(struct list_head *config_terms)
 {
 	struct evsel_config_term *term;
tools/perf/util/parse-events.h +0 -3
index cf3f07db0295..d77be173af22 100644
@@ -223,9 +223,6 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
 			     u32 type, u64 config,
 			     const struct parse_events_terms *head_config,
 			     bool wildcard);
-int parse_events_add_tool(struct parse_events_state *parse_state,
-			  struct list_head *list,
-			  int tool_event);
 int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 			   struct parse_events_state *parse_state,
 			   struct parse_events_terms *parsed_terms);
tools/perf/util/parse-events.l +0 -11
index 5a0bcd7f166a..14e5bd856a18 100644
@@ -121,14 +121,6 @@ static int sym(yyscan_t scanner, int type, int config)
 	return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW;
 }
 
-static int tool(yyscan_t scanner, enum perf_tool_event event)
-{
-	YYSTYPE *yylval = parse_events_get_lval(scanner);
-
-	yylval->num = event;
-	return PE_VALUE_SYM_TOOL;
-}
-
 static int term(yyscan_t scanner, enum parse_events__term_type type)
 {
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
@@ -404,9 +396,6 @@ cpu-migrations|migrations			{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COU
 alignment-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
 emulation-faults				{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
 dummy						{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
-duration_time					{ return tool(yyscanner, PERF_TOOL_DURATION_TIME); }
-user_time						{ return tool(yyscanner, PERF_TOOL_USER_TIME); }
-system_time						{ return tool(yyscanner, PERF_TOOL_SYSTEM_TIME); }
 bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
 cgroup-switches					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); }
 
tools/perf/util/parse-events.y +0 -16
index b3c51f06cbdc..5c4669264177 100644
@@ -56,7 +56,6 @@ static void free_list_evsel(struct list_head* list_evsel)
 
 %token PE_START_EVENTS PE_START_TERMS
 %token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_TERM
-%token PE_VALUE_SYM_TOOL
 %token PE_EVENT_NAME
 %token PE_RAW PE_NAME
 %token PE_MODIFIER_EVENT PE_MODIFIER_BP PE_BP_COLON PE_BP_SLASH
@@ -68,7 +67,6 @@ static void free_list_evsel(struct list_head* list_evsel)
 %type <num> PE_VALUE
 %type <num> PE_VALUE_SYM_HW
 %type <num> PE_VALUE_SYM_SW
-%type <num> PE_VALUE_SYM_TOOL
 %type <mod> PE_MODIFIER_EVENT
 %type <term_type> PE_TERM
 %type <num> value_sym
@@ -350,20 +348,6 @@ value_sym sep_slash_slash_dc
 		PE_ABORT(err);
 	$$ = list;
 }
-|
-PE_VALUE_SYM_TOOL sep_slash_slash_dc
-{
-	struct list_head *list;
-	int err;
-
-	list = alloc_list();
-	if (!list)
-		YYNOMEM;
-	err = parse_events_add_tool(_parse_state, list, $1);
-	if (err)
-		YYNOMEM;
-	$$ = list;
-}
 
 event_legacy_cache:
 PE_LEGACY_CACHE opt_event_config
tools/perf/util/pmu.c +18 -2
index 29bd0fa9f88c..bbb8e207c6b9 100644
@@ -19,6 +19,7 @@
 #include "evsel.h"
 #include "pmu.h"
 #include "pmus.h"
+#include "tool_pmu.h"
 #include <util/pmu-bison.h>
 #include <util/pmu-flex.h>
 #include "parse-events.h"
@@ -1548,6 +1549,9 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 {
 	bool zero = !!pmu->perf_event_attr_init_default;
 
+	if (perf_pmu__is_tool(pmu))
+		return tool_pmu__config_terms(attr, head_terms, err);
+
 	/* Fake PMU doesn't have proper terms so nothing to configure in attr. */
 	if (perf_pmu__is_fake(pmu))
 		return 0;
@@ -1660,8 +1664,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
-	/* Fake PMU doesn't rewrite terms. */
-	if (perf_pmu__is_fake(pmu))
+	/* Tool/fake PMU doesn't rewrite terms. */
+	if (perf_pmu__is_tool(pmu) || perf_pmu__is_fake(pmu))
 		goto out;
 
 	list_for_each_entry_safe(term, h, &head_terms->terms, list) {
@@ -1827,6 +1831,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 {
 	if (!name)
 		return false;
+	if (perf_pmu__is_tool(pmu))
+		return perf_tool_event__from_str(name) != PERF_TOOL_NONE;
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1838,6 +1844,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
 	size_t nr;
 
+	if (perf_pmu__is_tool(pmu))
+		return tool_pmu__num_events();
+
 	pmu_aliases_parse(pmu);
 	nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1898,6 +1907,9 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct strbuf sb;
 
+	if (perf_pmu__is_tool(pmu))
+		return tool_pmu__for_each_event_cb(pmu, state, cb);
+
 	strbuf_init(&sb, /*hint=*/ 0);
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
@@ -1986,6 +1998,7 @@ bool perf_pmu__is_software(const struct perf_pmu *pmu)
 	case PERF_TYPE_HW_CACHE:	return false;
 	case PERF_TYPE_RAW:		return false;
 	case PERF_TYPE_BREAKPOINT:	return true;
+	case PERF_PMU_TYPE_TOOL:	return true;
 	default: break;
 	}
 	for (size_t i = 0; i < ARRAY_SIZE(known_sw_pmus); i++) {
@@ -2313,6 +2326,9 @@ const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
 	if (!pmu)
 		return NULL;
 
+	if (perf_pmu__is_tool(pmu))
+		return perf_tool_event__to_str(config);
+
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
 	list_for_each_entry(event, &pmu->aliases, list) {
tools/perf/util/pmu.h +2 -0
index af7532ca7fb1..880a8dc661b7 100644
@@ -37,6 +37,7 @@ struct perf_pmu_caps {
 };
 
 enum {
+	PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
 	PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
 };
 
@@ -282,6 +283,7 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pmus);
 void perf_pmu__delete(struct perf_pmu *pmu);
 struct perf_pmu *perf_pmus__find_core_pmu(void);
+
 const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config);
 
 #endif /* __PMU_H */
tools/perf/util/pmus.c +9 -0
index 93e5c0435ee0..36329bc77316 100644
@@ -15,6 +15,7 @@
 #include "evsel.h"
 #include "pmus.h"
 #include "pmu.h"
+#include "tool_pmu.h"
 #include "print-events.h"
 #include "strbuf.h"
 
@@ -200,6 +201,7 @@ static void pmu_read_sysfs(bool core_only)
 	int fd;
 	DIR *dir;
 	struct dirent *dent;
+	struct perf_pmu *tool_pmu;
 
 	if (read_sysfs_all_pmus || (core_only && read_sysfs_core_pmus))
 		return;
@@ -229,6 +231,10 @@ static void pmu_read_sysfs(bool core_only)
 			pr_err("Failure to set up any core PMUs\n");
 	}
 	list_sort(NULL, &core_pmus, pmus_cmp);
+	if (!core_only) {
+		tool_pmu = perf_pmus__tool_pmu();
+		list_add_tail(&tool_pmu->list, &other_pmus);
+	}
 	list_sort(NULL, &other_pmus, pmus_cmp);
 	if (!list_empty(&core_pmus)) {
 		read_sysfs_core_pmus = true;
@@ -584,6 +590,9 @@ void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, voi
 		int len = pmu_name_len_no_suffix(pmu->name);
 		const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
 
+		if (perf_pmu__is_tool(pmu))
+			continue;
+
 		if (!pmu->is_core)
 			desc = NULL;
 
tools/perf/util/print-events.c +1 -35
index 81e0135cddf0..a786cbfb0ff5 100644
@@ -29,6 +29,7 @@
 #include "tracepoint.h"
 #include "pfm.h"
 #include "thread_map.h"
+#include "tool_pmu.h"
 #include "util.h"
 
 #define MAX_NAME_LEN 100
@@ -43,21 +44,6 @@ static const char * const event_type_descriptors[] = {
 	"Hardware breakpoint",
 };
 
-static const struct event_symbol event_symbols_tool[PERF_TOOL_MAX] = {
-	[PERF_TOOL_DURATION_TIME] = {
-		.symbol = "duration_time",
-		.alias  = "",
-	},
-	[PERF_TOOL_USER_TIME] = {
-		.symbol = "user_time",
-		.alias  = "",
-	},
-	[PERF_TOOL_SYSTEM_TIME] = {
-		.symbol = "system_time",
-		.alias  = "",
-	},
-};
-
 /*
  * Print the events from <debugfs_mount_point>/tracing/events
  */
@@ -341,24 +327,6 @@ int print_hwcache_events(const struct print_callbacks *print_cb, void *print_sta
 	return 0;
 }
 
-void print_tool_events(const struct print_callbacks *print_cb, void *print_state)
-{
-	// Start at 1 because the first enum entry means no tool event.
-	for (int i = 1; i < PERF_TOOL_MAX; ++i) {
-		print_cb->print_event(print_state,
-				"tool",
-				/*pmu_name=*/NULL,
-				event_symbols_tool[i].symbol,
-				event_symbols_tool[i].alias,
-				/*scale_unit=*/NULL,
-				/*deprecated=*/false,
-				"Tool event",
-				/*desc=*/NULL,
-				/*long_desc=*/NULL,
-				/*encoding_desc=*/NULL);
-	}
-}
-
 void print_symbol_events(const struct print_callbacks *print_cb, void *print_state,
 			 unsigned int type, const struct event_symbol *syms,
 			 unsigned int max)
@@ -422,8 +390,6 @@ void print_events(const struct print_callbacks *print_cb, void *print_state)
 	print_symbol_events(print_cb, print_state, PERF_TYPE_SOFTWARE,
 			event_symbols_sw, PERF_COUNT_SW_MAX);
 
-	print_tool_events(print_cb, print_state);
-
 	print_hwcache_events(print_cb, print_state);
 
 	perf_pmus__print_pmu_events(print_cb, print_state);
tools/perf/util/print-events.h +0 -1
index bf4290bef0cd..445efa1636c1 100644
@@ -36,7 +36,6 @@ void print_sdt_events(const struct print_callbacks *print_cb, void *print_state)
 void print_symbol_events(const struct print_callbacks *print_cb, void *print_state,
 			 unsigned int type, const struct event_symbol *syms,
 			 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);
 
tools/perf/util/stat-display.c +5 -1
index ea96e4ebad8c..a82a8ec79b39 100644
@@ -21,6 +21,7 @@
 #include "iostat.h"
 #include "pmu.h"
 #include "pmus.h"
+#include "tool_pmu.h"
 
 #define CNTR_NOT_SUPPORTED	"<not supported>"
 #define CNTR_NOT_COUNTED	"<not counted>"
@@ -946,7 +947,10 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
 	if (config->aggr_mode == AGGR_THREAD && config->system_wide)
 		return true;
 
-	/* Tool events have the software PMU but are only gathered on 1. */
+	/*
+	 * Many tool events are only gathered on the first index, skip other
+	 * zero values.
+	 */
 	if (evsel__is_tool(counter))
 		return true;
 
tools/perf/util/stat-shadow.c +1 -0
index 99376c12dd8e..2d653f91400e 100644
@@ -15,6 +15,7 @@
 #include <linux/zalloc.h>
 #include "iostat.h"
 #include "util/hashmap.h"
+#include "tool_pmu.h"
 
 struct stats walltime_nsecs_stats;
 struct rusage_stats ru_stats;
tools/perf/util/tool_pmu.c new file +411 -0

411 lines not on this page · raw message

tools/perf/util/tool_pmu.h new file +51 -0

51 lines not on this page · raw message

Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#12 lore raw

PATCH 8/15 perf tool_pmu: Rename enum perf_tool_event to tool_pmu_event

In reply to Ian Rogers

To better reflect the events listed are from the tool PMU. Rename the enum values from PERF_TOOL_* to TOOL_PMU__EVENT_*.

---
 tools/perf/builtin-stat.c     |  6 ++---
 tools/perf/util/evsel.c       |  4 ++--
 tools/perf/util/metricgroup.c | 12 +++++-----
 tools/perf/util/pmu.c         |  2 +-
 tools/perf/util/stat-shadow.c | 10 ++++----
 tools/perf/util/tool_pmu.c    | 44 +++++++++++++++++------------------
 tools/perf/util/tool_pmu.h    | 22 +++++++++---------
 7 files changed, 50 insertions(+), 50 deletions(-)
tools/perf/builtin-stat.c +3 -3
index ae525fdcfbde..a6b4d1b98b3a 100644
@@ -295,14 +295,14 @@ static int read_single_counter(struct evsel *counter, int cpu_map_idx, int threa
 	 * terminates. Use the wait4 values in that case.
 	 */
 	if (err && cpu_map_idx == 0 &&
-	    (evsel__tool_event(counter) == PERF_TOOL_USER_TIME ||
-	     evsel__tool_event(counter) == PERF_TOOL_SYSTEM_TIME)) {
+	    (evsel__tool_event(counter) == TOOL_PMU__EVENT_USER_TIME ||
+	     evsel__tool_event(counter) == TOOL_PMU__EVENT_SYSTEM_TIME)) {
 		u64 val, *start_time;
 		struct perf_counts_values *count =
 			perf_counts(counter->counts, cpu_map_idx, thread);
 
 		start_time = xyarray__entry(counter->start_times, cpu_map_idx, thread);
-		if (evsel__tool_event(counter) == PERF_TOOL_USER_TIME)
+		if (evsel__tool_event(counter) == TOOL_PMU__EVENT_USER_TIME)
 			val = ru_stats.ru_utime_usec_stat.mean;
 		else
 			val = ru_stats.ru_stime_usec_stat.mean;
tools/perf/util/evsel.c +2 -2
index b75c4a4c7aa7..c0b49c5dd3cf 100644
@@ -1471,8 +1471,8 @@ void evsel__exit(struct evsel *evsel)
 	evsel->per_pkg_mask = NULL;
 	zfree(&evsel->metric_events);
 	perf_evsel__object.fini(evsel);
-	if (evsel__tool_event(evsel) == PERF_TOOL_SYSTEM_TIME ||
-	    evsel__tool_event(evsel) == PERF_TOOL_USER_TIME)
+	if (evsel__tool_event(evsel) == TOOL_PMU__EVENT_SYSTEM_TIME ||
+	    evsel__tool_event(evsel) == TOOL_PMU__EVENT_USER_TIME)
 		xyarray__delete(evsel->start_times);
 }
 
tools/perf/util/metricgroup.c +6 -6
index 9181548e8881..0e21e2f9c170 100644
@@ -674,20 +674,20 @@ static int metricgroup__build_event_string(struct strbuf *events,
 	struct hashmap_entry *cur;
 	size_t bkt;
 	bool no_group = true, has_tool_events = false;
-	bool tool_events[PERF_TOOL_MAX] = {false};
+	bool tool_events[TOOL_PMU__EVENT_MAX] = {false};
 	int ret = 0;
 
 #define RETURN_IF_NON_ZERO(x) do { if (x) return x; } while (0)
 
 	hashmap__for_each_entry(ctx->ids, cur, bkt) {
 		const char *sep, *rsep, *id = cur->pkey;
-		enum perf_tool_event ev;
+		enum tool_pmu_event ev;
 
 		pr_debug("found event %s\n", id);
 
 		/* Always move tool events outside of the group. */
 		ev = perf_tool_event__from_str(id);
-		if (ev != PERF_TOOL_NONE) {
+		if (ev != TOOL_PMU__EVENT_NONE) {
 			has_tool_events = true;
 			tool_events[ev] = true;
 			continue;
@@ -1375,7 +1375,7 @@ static void metricgroup__free_metrics(struct list_head *metric_list)
  *               to true if tool event is found.
  */
 static void find_tool_events(const struct list_head *metric_list,
-			     bool tool_events[PERF_TOOL_MAX])
+			     bool tool_events[TOOL_PMU__EVENT_MAX])
 {
 	struct metric *m;
 
@@ -1447,7 +1447,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  */
 static int parse_ids(bool metric_no_merge, bool fake_pmu,
 		     struct expr_parse_ctx *ids, const char *modifier,
-		     bool group_events, const bool tool_events[PERF_TOOL_MAX],
+		     bool group_events, const bool tool_events[TOOL_PMU__EVENT_MAX],
 		     struct evlist **out_evlist)
 {
 	struct parse_events_error parse_error;
@@ -1536,7 +1536,7 @@ static int parse_groups(struct evlist *perf_evlist,
 	struct evlist *combined_evlist = NULL;
 	LIST_HEAD(metric_list);
 	struct metric *m;
-	bool tool_events[PERF_TOOL_MAX] = {false};
+	bool tool_events[TOOL_PMU__EVENT_MAX] = {false};
 	bool is_default = !strcmp(str, "Default");
 	int ret;
 
tools/perf/util/pmu.c +1 -1
index bbb8e207c6b9..34766151cfc3 100644
@@ -1832,7 +1832,7 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 	if (!name)
 		return false;
 	if (perf_pmu__is_tool(pmu))
-		return perf_tool_event__from_str(name) != PERF_TOOL_NONE;
+		return perf_tool_event__from_str(name) != TOOL_PMU__EVENT_NONE;
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
tools/perf/util/stat-shadow.c +5 -5
index 2d653f91400e..e56969d1e39d 100644
@@ -382,22 +382,22 @@ static int prepare_metric(const struct metric_expr *mexp,
 			double scale;
 
 			switch (evsel__tool_event(metric_events[i])) {
-			case PERF_TOOL_DURATION_TIME:
+			case TOOL_PMU__EVENT_DURATION_TIME:
 				stats = &walltime_nsecs_stats;
 				scale = 1e-9;
 				break;
-			case PERF_TOOL_USER_TIME:
+			case TOOL_PMU__EVENT_USER_TIME:
 				stats = &ru_stats.ru_utime_usec_stat;
 				scale = 1e-6;
 				break;
-			case PERF_TOOL_SYSTEM_TIME:
+			case TOOL_PMU__EVENT_SYSTEM_TIME:
 				stats = &ru_stats.ru_stime_usec_stat;
 				scale = 1e-6;
 				break;
-			case PERF_TOOL_NONE:
+			case TOOL_PMU__EVENT_NONE:
 				pr_err("Invalid tool event 'none'");
 				abort();
-			case PERF_TOOL_MAX:
+			case TOOL_PMU__EVENT_MAX:
 				pr_err("Invalid tool event 'max'");
 				abort();
 			default:
tools/perf/util/tool_pmu.c +22 -22

122 lines not on this page · raw message

tools/perf/util/tool_pmu.h +11 -11
index 05a4052c8b9d..b27a26133927 100644
@@ -8,25 +8,25 @@ struct evsel;
 struct perf_thread_map;
 struct print_callbacks;
 
-enum perf_tool_event {
-	PERF_TOOL_NONE = 0,
-	PERF_TOOL_DURATION_TIME = 1,
-	PERF_TOOL_USER_TIME = 2,
-	PERF_TOOL_SYSTEM_TIME = 3,
+enum tool_pmu_event {
+	TOOL_PMU__EVENT_NONE = 0,
+	TOOL_PMU__EVENT_DURATION_TIME = 1,
+	TOOL_PMU__EVENT_USER_TIME = 2,
+	TOOL_PMU__EVENT_SYSTEM_TIME = 3,
 
-	PERF_TOOL_MAX,
+	TOOL_PMU__EVENT_MAX,
 };
 
 #define perf_tool_event__for_each_event(ev)				\
-	for ((ev) = PERF_TOOL_DURATION_TIME; (ev) < PERF_TOOL_MAX; ev++)
+	for ((ev) = TOOL_PMU__EVENT_DURATION_TIME; (ev) < TOOL_PMU__EVENT_MAX; ev++)
 
 static inline size_t tool_pmu__num_events(void)
 {
-	return PERF_TOOL_MAX - 1;
+	return TOOL_PMU__EVENT_MAX - 1;
 }
 
-const char *perf_tool_event__to_str(enum perf_tool_event ev);
-enum perf_tool_event perf_tool_event__from_str(const char *str);
+const char *perf_tool_event__to_str(enum tool_pmu_event ev);
+enum tool_pmu_event perf_tool_event__from_str(const char *str);
 int tool_pmu__config_terms(struct perf_event_attr *attr,
 			   struct parse_events_terms *terms,
 			   struct parse_events_error *err);
@@ -36,7 +36,7 @@ bool perf_pmu__is_tool(const struct perf_pmu *pmu);
 
 
 bool evsel__is_tool(const struct evsel *evsel);
-enum perf_tool_event evsel__tool_event(const struct evsel *evsel);
+enum tool_pmu_event evsel__tool_event(const struct evsel *evsel);
 const char *evsel__tool_pmu_event_name(const struct evsel *evsel);
 int evsel__tool_pmu_prepare_open(struct evsel *evsel,
 				 struct perf_cpu_map *cpus,
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#13 lore raw

PATCH 9/15 perf tool_pmu: Rename perf_tool_event__* to tool_pmu__*

In reply to Ian Rogers

Now the events are associated with the tool PMU, rename the functions to reflect this.

---
 tools/perf/util/evsel.c       |  2 +-
 tools/perf/util/metricgroup.c | 22 +++++++++++-----------
 tools/perf/util/pmu.c         |  4 ++--
 tools/perf/util/tool_pmu.c    | 16 ++++++++--------
 tools/perf/util/tool_pmu.h    |  8 ++++----
 5 files changed, 26 insertions(+), 26 deletions(-)
tools/perf/util/evsel.c +1 -1
index c0b49c5dd3cf..9e748ed20988 100644
@@ -1655,7 +1655,7 @@ static int evsel__read_group(struct evsel *leader, int cpu_map_idx, int thread)
 int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread)
 {
 	if (evsel__is_tool(evsel))
-		return evsel__read_tool(evsel, cpu_map_idx, thread);
+		return evsel__tool_pmu_read(evsel, cpu_map_idx, thread);
 
 	if (evsel__is_retire_lat(evsel))
 		return evsel__read_retire_lat(evsel, cpu_map_idx, thread);
tools/perf/util/metricgroup.c +11 -11
index 0e21e2f9c170..e5dd932e4d3f 100644
@@ -686,7 +686,7 @@ static int metricgroup__build_event_string(struct strbuf *events,
 		pr_debug("found event %s\n", id);
 
 		/* Always move tool events outside of the group. */
-		ev = perf_tool_event__from_str(id);
+		ev = tool_pmu__str_to_event(id);
 		if (ev != TOOL_PMU__EVENT_NONE) {
 			has_tool_events = true;
 			tool_events[ev] = true;
@@ -755,14 +755,14 @@ static int metricgroup__build_event_string(struct strbuf *events,
 	if (has_tool_events) {
 		int i;
 
-		perf_tool_event__for_each_event(i) {
+		tool_pmu__for_each_event(i) {
 			if (tool_events[i]) {
 				if (!no_group) {
 					ret = strbuf_addch(events, ',');
 					RETURN_IF_NON_ZERO(ret);
 				}
 				no_group = false;
-				ret = strbuf_addstr(events, perf_tool_event__to_str(i));
+				ret = strbuf_addstr(events, tool_pmu__event_to_str(i));
 				RETURN_IF_NON_ZERO(ret);
 			}
 		}
@@ -1148,14 +1148,14 @@ static int metric_list_cmp(void *priv __maybe_unused, const struct list_head *l,
 	int i, left_count, right_count;
 
 	left_count = hashmap__size(left->pctx->ids);
-	perf_tool_event__for_each_event(i) {
-		if (!expr__get_id(left->pctx, perf_tool_event__to_str(i), &data))
+	tool_pmu__for_each_event(i) {
+		if (!expr__get_id(left->pctx, tool_pmu__event_to_str(i), &data))
 			left_count--;
 	}
 
 	right_count = hashmap__size(right->pctx->ids);
-	perf_tool_event__for_each_event(i) {
-		if (!expr__get_id(right->pctx, perf_tool_event__to_str(i), &data))
+	tool_pmu__for_each_event(i) {
+		if (!expr__get_id(right->pctx, tool_pmu__event_to_str(i), &data))
 			right_count--;
 	}
 
@@ -1382,11 +1382,11 @@ static void find_tool_events(const struct list_head *metric_list,
 	list_for_each_entry(m, metric_list, nd) {
 		int i;
 
-		perf_tool_event__for_each_event(i) {
+		tool_pmu__for_each_event(i) {
 			struct expr_id_data *data;
 
 			if (!tool_events[i] &&
-			    !expr__get_id(m->pctx, perf_tool_event__to_str(i), &data))
+			    !expr__get_id(m->pctx, tool_pmu__event_to_str(i), &data))
 				tool_events[i] = true;
 		}
 	}
@@ -1472,9 +1472,9 @@ static int parse_ids(bool metric_no_merge, bool fake_pmu,
 		 *    event1 if #smt_on else 0
 		 * Add a tool event to avoid a parse error on an empty string.
 		 */
-		perf_tool_event__for_each_event(i) {
+		tool_pmu__for_each_event(i) {
 			if (tool_events[i]) {
-				char *tmp = strdup(perf_tool_event__to_str(i));
+				char *tmp = strdup(tool_pmu__event_to_str(i));
 
 				if (!tmp)
 					return -ENOMEM;
tools/perf/util/pmu.c +2 -2
index 34766151cfc3..69c26dad3e6c 100644
@@ -1832,7 +1832,7 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 	if (!name)
 		return false;
 	if (perf_pmu__is_tool(pmu))
-		return perf_tool_event__from_str(name) != TOOL_PMU__EVENT_NONE;
+		return tool_pmu__str_to_event(name) != TOOL_PMU__EVENT_NONE;
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -2327,7 +2327,7 @@ const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
 		return NULL;
 
 	if (perf_pmu__is_tool(pmu))
-		return perf_tool_event__to_str(config);
+		return tool_pmu__event_to_str(config);
 
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
tools/perf/util/tool_pmu.c +8 -8
index c56fc0c13899..fb08b0400c81 100644
@@ -20,7 +20,7 @@ static const char *const tool_pmu__event_names[TOOL_PMU__EVENT_MAX] = {
 };
 
 
-const char *perf_tool_event__to_str(enum tool_pmu_event ev)
+const char *tool_pmu__event_to_str(enum tool_pmu_event ev)
 {
 	if (ev > TOOL_PMU__EVENT_NONE && ev < TOOL_PMU__EVENT_MAX)
 		return tool_pmu__event_names[ev];
@@ -28,11 +28,11 @@ const char *perf_tool_event__to_str(enum tool_pmu_event ev)
 	return NULL;
 }
 
-enum tool_pmu_event perf_tool_event__from_str(const char *str)
+enum tool_pmu_event tool_pmu__str_to_event(const char *str)
 {
 	int i;
 
-	perf_tool_event__for_each_event(i) {
+	tool_pmu__for_each_event(i) {
 		if (!strcasecmp(str, tool_pmu__event_names[i]))
 			return i;
 	}
@@ -44,7 +44,7 @@ static int tool_pmu__config_term(struct perf_event_attr *attr,
 				 struct parse_events_error *err)
 {
 	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
-		enum tool_pmu_event ev = perf_tool_event__from_str(term->config);
+		enum tool_pmu_event ev = tool_pmu__str_to_event(term->config);
 
 		if (ev == TOOL_PMU__EVENT_NONE)
 			goto err_out;
@@ -91,10 +91,10 @@ int tool_pmu__for_each_event_cb(struct perf_pmu *pmu, void *state, pmu_event_cal
 	};
 	int i;
 
-	perf_tool_event__for_each_event(i) {
+	tool_pmu__for_each_event(i) {
 		int ret;
 
-		info.name = perf_tool_event__to_str(i);
+		info.name = tool_pmu__event_to_str(i);
 		info.alias = NULL;
 		info.scale_unit = NULL;
 		info.desc = NULL;
@@ -130,7 +130,7 @@ enum tool_pmu_event evsel__tool_event(const struct evsel *evsel)
 
 const char *evsel__tool_pmu_event_name(const struct evsel *evsel)
 {
-	return perf_tool_event__to_str(evsel->core.attr.config);
+	return tool_pmu__event_to_str(evsel->core.attr.config);
 }
 
 static bool read_until_char(struct io *io, char e)
@@ -330,7 +330,7 @@ int evsel__tool_pmu_open(struct evsel *evsel,
 	return err;
 }
 
-int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread)
+int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread)
 {
 	__u64 *start_time, cur_time, delta_start;
 	int fd, err = 0;
tools/perf/util/tool_pmu.h +4 -4
index b27a26133927..b156645206c4 100644
@@ -17,7 +17,7 @@ enum tool_pmu_event {
 	TOOL_PMU__EVENT_MAX,
 };
 
-#define perf_tool_event__for_each_event(ev)				\
+#define tool_pmu__for_each_event(ev)					\
 	for ((ev) = TOOL_PMU__EVENT_DURATION_TIME; (ev) < TOOL_PMU__EVENT_MAX; ev++)
 
 static inline size_t tool_pmu__num_events(void)
@@ -25,8 +25,8 @@ static inline size_t tool_pmu__num_events(void)
 	return TOOL_PMU__EVENT_MAX - 1;
 }
 
-const char *perf_tool_event__to_str(enum tool_pmu_event ev);
-enum tool_pmu_event perf_tool_event__from_str(const char *str);
+const char *tool_pmu__event_to_str(enum tool_pmu_event ev);
+enum tool_pmu_event tool_pmu__str_to_event(const char *str);
 int tool_pmu__config_terms(struct perf_event_attr *attr,
 			   struct parse_events_terms *terms,
 			   struct parse_events_error *err);
@@ -44,7 +44,7 @@ int evsel__tool_pmu_prepare_open(struct evsel *evsel,
 int evsel__tool_pmu_open(struct evsel *evsel,
 			 struct perf_thread_map *threads,
 			 int start_cpu_map_idx, int end_cpu_map_idx);
-int evsel__read_tool(struct evsel *evsel, int cpu_map_idx, int thread);
+int evsel__tool_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread);
 
 struct perf_pmu *perf_pmus__tool_pmu(void);
 
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#14 lore raw

PATCH 10/15 perf tool_pmu: Move expr literals to tool_pmu

In reply to Ian Rogers

Add the expr literals like "#smt_on" as tool events, this allows stat events to give the values. On my laptop with hyperthreading enabled:

```
$ perf stat -e "has_pmem,num_cores,num_cpus,num_cpus_online,num_dies,num_packages,smt_on,system_tsc_freq" true

 Performance counter stats for 'true':

                 0      has_pmem
                 8      num_cores
                16      num_cpus
                16      num_cpus_online
                 1      num_dies
                 1      num_packages
                 1      smt_on
     2,496,000,000      system_tsc_freq

       0.001113637 seconds time elapsed

       0.001218000 seconds user
       0.000000000 seconds sys
```

And with hyperthreading disabled:

```
$ perf stat -e "has_pmem,num_cores,num_cpus,num_cpus_online,num_dies,num_packages,smt_on,system_tsc_freq" true

 Performance counter stats for 'true':

                 0      has_pmem
                 8      num_cores
                16      num_cpus
                 8      num_cpus_online
                 1      num_dies
                 1      num_packages
                 0      smt_on
     2,496,000,000      system_tsc_freq

       0.000802115 seconds time elapsed

       0.000000000 seconds user
       0.000806000 seconds sys
```

As zero matters for these values, in stat-display should_skip_zero_counter only skip the zero value if it is not the first aggregation index.

The tool event implementations are used in expr but not evaluated as events for simplicity. Also core_wide isn't made a tool event as it requires command line parameters.

---
 tools/perf/arch/arm64/util/pmu.c |   5 +-
 tools/perf/arch/x86/util/tsc.c   |  16 ++--
 tools/perf/util/expr.c           |  93 +++-----------------
 tools/perf/util/pmu.c            |   5 --
 tools/perf/util/pmu.h            |   1 -
 tools/perf/util/stat-display.c   |   8 +-
 tools/perf/util/stat-shadow.c    |  11 ++-
 tools/perf/util/tool_pmu.c       | 146 ++++++++++++++++++++++++++++++-
 tools/perf/util/tool_pmu.h       |  19 +++-
 tools/perf/util/tsc.h            |   2 +-
 10 files changed, 200 insertions(+), 106 deletions(-)
tools/perf/arch/arm64/util/pmu.c +3 -2
index 2a4eab2d160e..a0964b191fcb 100644
@@ -5,6 +5,7 @@
 #include "../../../util/header.h"
 #include "../../../util/pmu.h"
 #include "../../../util/pmus.h"
+#include "../../../util/tool_pmu.h"
 #include <api/fs/fs.h>
 #include <math.h>
 
@@ -24,7 +25,7 @@ const struct pmu_metrics_table *pmu_metrics_table__find(void)
 	return NULL;
 }
 
-double perf_pmu__cpu_slots_per_cycle(void)
+u64 tool_pmu__cpu_slots_per_cycle(void)
 {
 	char path[PATH_MAX];
 	unsigned long long slots = 0;
@@ -41,5 +42,5 @@ double perf_pmu__cpu_slots_per_cycle(void)
 		filename__read_ull(path, &slots);
 	}
 
-	return slots ? (double)slots : NAN;
+	return slots;
 }
tools/perf/arch/x86/util/tsc.c +9 -7
index e2d6cfe21057..c79415b3cc5d 100644
@@ -24,9 +24,9 @@ u64 rdtsc(void)
  * ...
  * will return 3000000000.
  */
-static double cpuinfo_tsc_freq(void)
+static u64 cpuinfo_tsc_freq(void)
 {
-	double result = 0;
+	u64 result = 0;
 	FILE *cpuinfo;
 	char *line = NULL;
 	size_t len = 0;
@@ -39,15 +39,17 @@ static double cpuinfo_tsc_freq(void)
 	while (getline(&line, &len, cpuinfo) > 0) {
 		if (!strncmp(line, "model name", 10)) {
 			char *pos = strstr(line + 11, " @ ");
+			double float_result;
 
-			if (pos && sscanf(pos, " @ %lfGHz", &result) == 1) {
-				result *= 1000000000;
+			if (pos && sscanf(pos, " @ %lfGHz", &float_result) == 1) {
+				float_result *= 1000000000;
+				result = (u64)float_result;
 				goto out;
 			}
 		}
 	}
 out:
-	if (fpclassify(result) == FP_ZERO)
+	if (result == 0)
 		pr_err("Failed to find TSC frequency in /proc/cpuinfo\n");
 
 	free(line);
@@ -55,7 +57,7 @@ static double cpuinfo_tsc_freq(void)
 	return result;
 }
 
-double arch_get_tsc_freq(void)
+u64 arch_get_tsc_freq(void)
 {
 	unsigned int a, b, c, d, lvl;
 	static bool cached;
@@ -86,6 +88,6 @@ double arch_get_tsc_freq(void)
 		return tsc;
 	}
 
-	tsc = (double)c * (double)b / (double)a;
+	tsc = (u64)c * (u64)b / (u64)a;
 	return tsc;
 }
tools/perf/util/expr.c +13 -80

128 lines not on this page · raw message

tools/perf/util/pmu.c +0 -5
index 69c26dad3e6c..a412c9c62f9c 100644
@@ -2250,11 +2250,6 @@ bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok)
 		(need_fnmatch && !fnmatch(tok, name, 0));
 }
 
-double __weak perf_pmu__cpu_slots_per_cycle(void)
-{
-	return NAN;
-}
-
 int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size)
 {
 	const char *sysfs = sysfs__mountpoint();
tools/perf/util/pmu.h +0 -1
index 880a8dc661b7..c4ca359d4215 100644
@@ -271,7 +271,6 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
 
 bool perf_pmu__match(const struct perf_pmu *pmu, const char *tok);
 
-double perf_pmu__cpu_slots_per_cycle(void);
 int perf_pmu__event_source_devices_scnprintf(char *pathname, size_t size);
 int perf_pmu__pathname_scnprintf(char *buf, size_t size,
 				 const char *pmu_name, const char *filename);
tools/perf/util/stat-display.c +6 -2
index a82a8ec79b39..ad8baf33fdba 100644
@@ -951,8 +951,12 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
 	 * Many tool events are only gathered on the first index, skip other
 	 * zero values.
 	 */
-	if (evsel__is_tool(counter))
-		return true;
+	if (evsel__is_tool(counter)) {
+		struct aggr_cpu_id own_id =
+			config->aggr_get_id(config, (struct perf_cpu){ .cpu = 0 });
+
+		return !aggr_cpu_id__equal(id, &own_id);
+	}
 
 	/*
 	 * Skip value 0 when it's an uncore event and the given aggr id
tools/perf/util/stat-shadow.c +10 -1
index e56969d1e39d..aa367c667926 100644
@@ -400,8 +400,17 @@ static int prepare_metric(const struct metric_expr *mexp,
 			case TOOL_PMU__EVENT_MAX:
 				pr_err("Invalid tool event 'max'");
 				abort();
+			case TOOL_PMU__EVENT_HAS_PMEM:
+			case TOOL_PMU__EVENT_NUM_CORES:
+			case TOOL_PMU__EVENT_NUM_CPUS:
+			case TOOL_PMU__EVENT_NUM_CPUS_ONLINE:
+			case TOOL_PMU__EVENT_NUM_DIES:
+			case TOOL_PMU__EVENT_NUM_PACKAGES:
+			case TOOL_PMU__EVENT_SLOTS:
+			case TOOL_PMU__EVENT_SMT_ON:
+			case TOOL_PMU__EVENT_SYSTEM_TSC_FREQ:
 			default:
-				pr_err("Unknown tool event '%s'", evsel__name(metric_events[i]));
+				pr_err("Unexpected tool event '%s'", evsel__name(metric_events[i]));
 				abort();
 			}
 			val = avg_stats(stats) * scale;
tools/perf/util/tool_pmu.c +143 -3

190 lines not on this page · raw message

tools/perf/util/tool_pmu.h +15 -4
index b156645206c4..ecdf316525bb 100644
@@ -10,9 +10,18 @@ struct print_callbacks;
 
 enum tool_pmu_event {
 	TOOL_PMU__EVENT_NONE = 0,
-	TOOL_PMU__EVENT_DURATION_TIME = 1,
-	TOOL_PMU__EVENT_USER_TIME = 2,
-	TOOL_PMU__EVENT_SYSTEM_TIME = 3,
+	TOOL_PMU__EVENT_DURATION_TIME,
+	TOOL_PMU__EVENT_USER_TIME,
+	TOOL_PMU__EVENT_SYSTEM_TIME,
+	TOOL_PMU__EVENT_HAS_PMEM,
+	TOOL_PMU__EVENT_NUM_CORES,
+	TOOL_PMU__EVENT_NUM_CPUS,
+	TOOL_PMU__EVENT_NUM_CPUS_ONLINE,
+	TOOL_PMU__EVENT_NUM_DIES,
+	TOOL_PMU__EVENT_NUM_PACKAGES,
+	TOOL_PMU__EVENT_SLOTS,
+	TOOL_PMU__EVENT_SMT_ON,
+	TOOL_PMU__EVENT_SYSTEM_TSC_FREQ,
 
 	TOOL_PMU__EVENT_MAX,
 };
@@ -31,9 +40,11 @@ int tool_pmu__config_terms(struct perf_event_attr *attr,
 			   struct parse_events_terms *terms,
 			   struct parse_events_error *err);
 int tool_pmu__for_each_event_cb(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+bool tool_pmu__read_event(enum tool_pmu_event ev, u64 *result);
 
-bool perf_pmu__is_tool(const struct perf_pmu *pmu);
+u64 tool_pmu__cpu_slots_per_cycle(void);
 
+bool perf_pmu__is_tool(const struct perf_pmu *pmu);
 
 bool evsel__is_tool(const struct evsel *evsel);
 enum tool_pmu_event evsel__tool_event(const struct evsel *evsel);
tools/perf/util/tsc.h +1 -1
index 88fd1c4c1cb8..57ce8449647f 100644
@@ -25,7 +25,7 @@ int perf_read_tsc_conversion(const struct perf_event_mmap_page *pc,
 u64 perf_time_to_tsc(u64 ns, struct perf_tsc_conversion *tc);
 u64 tsc_to_perf_time(u64 cyc, struct perf_tsc_conversion *tc);
 u64 rdtsc(void);
-double arch_get_tsc_freq(void);
+u64 arch_get_tsc_freq(void);
 
 size_t perf_event__fprintf_time_conv(union perf_event *event, FILE *fp);
 
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#15 lore raw

PATCH 11/15 perf jevents: Add tool event json under a common architecture

In reply to Ian Rogers

Introduce the notion of a common architecture/model that can be used to find event tables for common PMUs like the tool PMU. By having tool events be json standard PMU attribute configuration, descriptions, etc. can be used and these routines are already optimized for things like binary searching.

---
 .../pmu-events/arch/common/common/tool.json   |  74 +++++++
 tools/perf/pmu-events/empty-pmu-events.c      | 208 +++++++++++-------
 tools/perf/pmu-events/jevents.py              |  16 +-
 3 files changed, 215 insertions(+), 83 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/common/common/tool.json
tools/perf/pmu-events/arch/common/common/tool.json new file +74 -0
new file mode 100644
index 000000000000..12f2ef1813a6
@@ -0,0 +1,74 @@
+[
+  {
+    "Unit": "tool",
+    "EventName": "duration_time",
+    "BriefDescription": "Wall clock interval time in nanoseconds",
+    "ConfigCode": "1"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "user_time",
+    "BriefDescription": "User (non-kernel) time in nanoseconds",
+    "ConfigCode": "2"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "system_time",
+    "BriefDescription": "System/kernel time in nanoseconds",
+    "ConfigCode": "3"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "has_pmem",
+    "BriefDescription": "1 if persistent memory installed otherwise 0",
+    "ConfigCode": "4"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "num_cores",
+    "BriefDescription": "Number of cores. A core consists of 1 or more thread, with each thread being associated with a logical Linux CPU",
+    "ConfigCode": "5"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "num_cpus",
+    "BriefDescription": "Number of logical Linux CPUs. There may be multiple such CPUs on a core",
+    "ConfigCode": "6"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "num_cpus_online",
+    "BriefDescription": "Number of online logical Linux CPUs. There may be multiple such CPUs on a core",
+    "ConfigCode": "7"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "num_dies",
+    "BriefDescription": "Number of dies. Each die has 1 or more cores",
+    "ConfigCode": "8"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "num_packages",
+    "BriefDescription": "Number of packages. Each package has 1 or more die",
+    "ConfigCode": "9"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "slots",
+    "BriefDescription": "Number of functional units that in parallel can execute parts of an instruction",
+    "ConfigCode": "10"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "smt_on",
+    "BriefDescription": "1 if simultaneous multithreading (aka hyperthreading) is enable otherwise 0",
+    "ConfigCode": "11"
+  },
+  {
+    "Unit": "tool",
+    "EventName": "system_tsc_freq",
+    "BriefDescription": "The amount a Time Stamp Counter (TSC) increases per second",
+    "ConfigCode": "12"
+  }
+]
tools/perf/pmu-events/empty-pmu-events.c +127 -81

291 lines not on this page · raw message

tools/perf/pmu-events/jevents.py +14 -2
index 1d96b2204e52..c932543bdc77 100755
@@ -292,6 +292,7 @@ class JsonEvent:
           'cpu_atom': 'cpu_atom',
           'ali_drw': 'ali_drw',
           'arm_cmn': 'arm_cmn',
+          'tool': 'tool',
       }
       return table[unit] if unit in table else f'uncore_{unit.lower()}'
 
@@ -718,6 +719,17 @@ const struct pmu_events_map pmu_events_map[] = {
 \t\t.num_pmus = ARRAY_SIZE(pmu_metrics__test_soc_cpu),
 \t}
 },
+""")
+    elif arch == 'common':
+      _args.output_file.write("""{
+\t.arch = "common",
+\t.cpuid = "common",
+\t.event_table = {
+\t\t.pmus = pmu_events__common,
+\t\t.num_pmus = ARRAY_SIZE(pmu_events__common),
+\t},
+\t.metric_table = {},
+},
 """)
     else:
       with open(f'{_args.starting_dir}/{arch}/mapfile.csv') as csvfile:
@@ -1238,7 +1250,7 @@ def main() -> None:
         if len(parents) == _args.model.split(',')[0].count('/'):
           # We're testing the correct directory.
           item_path = '/'.join(parents) + ('/' if len(parents) > 0 else '') + item.name
-          if 'test' not in item_path and item_path not in _args.model.split(','):
+          if 'test' not in item_path and 'common' not in item_path and item_path not in _args.model.split(','):
             continue
       action(parents, item)
       if item.is_dir():
@@ -1286,7 +1298,7 @@ struct pmu_table_entry {
   for item in os.scandir(_args.starting_dir):
     if not item.is_dir():
       continue
-    if item.name == _args.arch or _args.arch == 'all' or item.name == 'test':
+    if item.name == _args.arch or _args.arch == 'all' or item.name == 'test' or item.name == 'common':
       archs.append(item.name)
 
   if len(archs) < 2 and _args.arch != 'none':
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#16 lore raw

PATCH 12/15 perf tool_pmu: Switch to standard pmu functions and json descriptions

In reply to Ian Rogers

Use the regular PMU approaches with tool json events to reduce the amount of special tool_pmu code - tool_pmu__config_terms and tool_pmu__for_each_event_cb are removed. Some functions remain, like tool_pmu__str_to_event, as conveniences to metricgroups. Add tool_pmu__skip_event/tool_pmu__num_skip_events to handle the case that tool json events shouldn't appear on certain architectures. This isn't done in jevents.py due to complexity in the empty-pmu-events.c and when all vendor json is built into the tool.

---
 tools/perf/util/pmu.c      |  26 ++++-----
 tools/perf/util/pmus.c     |   4 +-
 tools/perf/util/tool_pmu.c | 111 ++++++++++++-------------------------
 tools/perf/util/tool_pmu.h |  12 +---
 4 files changed, 48 insertions(+), 105 deletions(-)
tools/perf/util/pmu.c +10 -16
index a412c9c62f9c..5ff6556292fd 100644
@@ -1549,9 +1549,6 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 {
 	bool zero = !!pmu->perf_event_attr_init_default;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__config_terms(attr, head_terms, err);
-
 	/* Fake PMU doesn't have proper terms so nothing to configure in attr. */
 	if (perf_pmu__is_fake(pmu))
 		return 0;
@@ -1664,8 +1661,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
-	/* Tool/fake PMU doesn't rewrite terms. */
-	if (perf_pmu__is_tool(pmu) || perf_pmu__is_fake(pmu))
+	/* Fake PMU doesn't rewrite terms. */
+	if (perf_pmu__is_fake(pmu))
 		goto out;
 
 	list_for_each_entry_safe(term, h, &head_terms->terms, list) {
@@ -1831,8 +1828,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 {
 	if (!name)
 		return false;
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__str_to_event(name) != TOOL_PMU__EVENT_NONE;
+	if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(name))
+		return false;
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1844,9 +1841,6 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
 	size_t nr;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__num_events();
-
 	pmu_aliases_parse(pmu);
 	nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1857,6 +1851,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 	else
 		assert(pmu->cpu_json_aliases == 0);
 
+	if (perf_pmu__is_tool(pmu))
+		nr -= tool_pmu__num_skip_events();
+
 	return pmu->selectable ? nr + 1 : nr;
 }
 
@@ -1907,15 +1904,15 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct strbuf sb;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__for_each_event_cb(pmu, state, cb);
-
 	strbuf_init(&sb, /*hint=*/ 0);
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
 	list_for_each_entry(event, &pmu->aliases, list) {
 		size_t buf_used, pmu_name_len;
 
+		if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(event->name))
+			continue;
+
 		info.pmu_name = event->pmu_name ?: pmu->name;
 		pmu_name_len = pmu_deduped_name_len(pmu, info.pmu_name,
 						    skip_duplicate_pmus);
@@ -2321,9 +2318,6 @@ const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
 	if (!pmu)
 		return NULL;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__event_to_str(config);
-
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
 	list_for_each_entry(event, &pmu->aliases, list) {
tools/perf/util/pmus.c +1 -3
index 36329bc77316..19673b9991c6 100644
@@ -440,6 +440,7 @@ static int perf_pmus__print_pmu_events__callback(void *vstate,
 		pr_err("Unexpected event %s/%s/\n", info->pmu->name, info->name);
 		return 1;
 	}
+	assert(info->pmu != NULL || info->name != NULL);
 	s = &state->aliases[state->index];
 	s->pmu = info->pmu;
 #define COPY_STR(str) s->str = info->str ? strdup(info->str) : NULL
@@ -590,9 +591,6 @@ void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, voi
 		int len = pmu_name_len_no_suffix(pmu->name);
 		const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
 
-		if (perf_pmu__is_tool(pmu))
-			continue;
-
 		if (!pmu->is_core)
 			desc = NULL;
 
tools/perf/util/tool_pmu.c +34 -77

143 lines not on this page · raw message

tools/perf/util/tool_pmu.h +3 -9
index ecdf316525bb..a60184859080 100644
@@ -29,17 +29,11 @@ enum tool_pmu_event {
 #define tool_pmu__for_each_event(ev)					\
 	for ((ev) = TOOL_PMU__EVENT_DURATION_TIME; (ev) < TOOL_PMU__EVENT_MAX; ev++)
 
-static inline size_t tool_pmu__num_events(void)
-{
-	return TOOL_PMU__EVENT_MAX - 1;
-}
-
 const char *tool_pmu__event_to_str(enum tool_pmu_event ev);
 enum tool_pmu_event tool_pmu__str_to_event(const char *str);
-int tool_pmu__config_terms(struct perf_event_attr *attr,
-			   struct parse_events_terms *terms,
-			   struct parse_events_error *err);
-int tool_pmu__for_each_event_cb(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+bool tool_pmu__skip_event(const char *name);
+int tool_pmu__num_skip_events(void);
+
 bool tool_pmu__read_event(enum tool_pmu_event ev, u64 *result);
 
 u64 tool_pmu__cpu_slots_per_cycle(void);
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
maintainer
#17 lore raw

Re: [PATCH v1 12/15] perf tool_pmu: Switch to standard pmu functions and json descriptions

Quoted 133 lines · Ian Rogers

Use the regular PMU approaches with tool json events to reduce the amount of special tool_pmu code - tool_pmu__config_terms and tool_pmu__for_each_event_cb are removed. Some functions remain, like tool_pmu__str_to_event, as conveniences to metricgroups. Add tool_pmu__skip_event/tool_pmu__num_skip_events to handle the case that tool json events shouldn't appear on certain architectures. This isn't done in jevents.py due to complexity in the empty-pmu-events.c and when all vendor json is built into the tool.

---
 tools/perf/util/pmu.c      |  26 ++++-----
 tools/perf/util/pmus.c     |   4 +-
 tools/perf/util/tool_pmu.c | 111 ++++++++++++-------------------------
 tools/perf/util/tool_pmu.h |  12 +---
 4 files changed, 48 insertions(+), 105 deletions(-)
tools/perf/util/pmu.c +10 -16
index a412c9c62f9c..5ff6556292fd 100644
@@ -1549,9 +1549,6 @@ int perf_pmu__config(struct perf_pmu *pmu, struct perf_event_attr *attr,
 {
 	bool zero = !!pmu->perf_event_attr_init_default;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__config_terms(attr, head_terms, err);
-
 	/* Fake PMU doesn't have proper terms so nothing to configure in attr. */
 	if (perf_pmu__is_fake(pmu))
 		return 0;
@@ -1664,8 +1661,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
-	/* Tool/fake PMU doesn't rewrite terms. */
-	if (perf_pmu__is_tool(pmu) || perf_pmu__is_fake(pmu))
+	/* Fake PMU doesn't rewrite terms. */
+	if (perf_pmu__is_fake(pmu))
 		goto out;
 
 	list_for_each_entry_safe(term, h, &head_terms->terms, list) {
@@ -1831,8 +1828,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 {
 	if (!name)
 		return false;
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__str_to_event(name) != TOOL_PMU__EVENT_NONE;
+	if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(name))
+		return false;
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1844,9 +1841,6 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
 	size_t nr;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__num_events();
-
 	pmu_aliases_parse(pmu);
 	nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1857,6 +1851,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 	else
 		assert(pmu->cpu_json_aliases == 0);
 
+	if (perf_pmu__is_tool(pmu))
+		nr -= tool_pmu__num_skip_events();
+
 	return pmu->selectable ? nr + 1 : nr;
 }
 
@@ -1907,15 +1904,15 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct strbuf sb;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__for_each_event_cb(pmu, state, cb);
-
 	strbuf_init(&sb, /*hint=*/ 0);
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
 	list_for_each_entry(event, &pmu->aliases, list) {
 		size_t buf_used, pmu_name_len;
 
+		if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(event->name))
+			continue;
+
 		info.pmu_name = event->pmu_name ?: pmu->name;
 		pmu_name_len = pmu_deduped_name_len(pmu, info.pmu_name,
 						    skip_duplicate_pmus);
@@ -2321,9 +2318,6 @@ const char *perf_pmu__name_from_config(struct perf_pmu *pmu, u64 config)
 	if (!pmu)
 		return NULL;
 
-	if (perf_pmu__is_tool(pmu))
-		return tool_pmu__event_to_str(config);
-
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
 	list_for_each_entry(event, &pmu->aliases, list) {
tools/perf/util/pmus.c +1 -3
index 36329bc77316..19673b9991c6 100644
@@ -440,6 +440,7 @@ static int perf_pmus__print_pmu_events__callback(void *vstate,
 		pr_err("Unexpected event %s/%s/\n", info->pmu->name, info->name);
 		return 1;
 	}
+	assert(info->pmu != NULL || info->name != NULL);
 	s = &state->aliases[state->index];
 	s->pmu = info->pmu;
 #define COPY_STR(str) s->str = info->str ? strdup(info->str) : NULL
@@ -590,9 +591,6 @@ void perf_pmus__print_raw_pmu_events(const struct print_callbacks *print_cb, voi
 		int len = pmu_name_len_no_suffix(pmu->name);
 		const char *desc = "(see 'man perf-list' or 'man perf-record' on how to encode it)";
 
-		if (perf_pmu__is_tool(pmu))
-			continue;
-
 		if (!pmu->is_core)
 			desc = NULL;
 
tools/perf/util/tool_pmu.c +0 -1
index 3dc7cc7b8d32..f2d26e9e51f5 100644
@@ -33,101 +33,54 @@ static const char *const tool_pmu__event_names[TOOL_PMU__EVENT_MAX] = {
 	"system_tsc_freq",
 };
 
-

tools/perf/util/tool_pmu.c static const char *const tool_pmu__event_names[TOOL_PMU__EVENT_MAX] = { · line 36

Quoted 10 lines
tools/perf/util/tool_pmu.c +1 -9
-const char *tool_pmu__event_to_str(enum tool_pmu_event ev)
-{
-	if (ev > TOOL_PMU__EVENT_NONE && ev < TOOL_PMU__EVENT_MAX)
-		return tool_pmu__event_names[ev];
-
-	return NULL;
-}
-
-enum tool_pmu_event tool_pmu__str_to_event(const char *str)
+bool tool_pmu__skip_event(const char *name)

I think you need __maybe_unused.

Quoted 118 lines
tools/perf/util/tool_pmu.c +29 -67
 {
-	int i;
-
-	tool_pmu__for_each_event(i) {
-		if (!strcasecmp(str, tool_pmu__event_names[i])) {
 #if !defined(__aarch64__)
-			/* The slots event should only appear on arm64. */
-			if (i == TOOL_PMU__EVENT_SLOTS)
-				return TOOL_PMU__EVENT_NONE;
+	/* The slots event should only appear on arm64. */
+	if (strcasecmp(name, "slots") == 0)
+		return true;
 #endif
-			return i;
-		}
-	}
-	return TOOL_PMU__EVENT_NONE;
+#if !defined(__i386__) && !defined(__x86_64__)
+	/* The system_tsc_freq event should only appear on x86. */
+	if (strcasecmp(name, "system_tsc_freq") == 0)
+		return true;
+#endif
+	return false;
 }
 
-static int tool_pmu__config_term(struct perf_event_attr *attr,
-				 struct parse_events_term *term,
-				 struct parse_events_error *err)
+int tool_pmu__num_skip_events(void)
 {
-	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
-		enum tool_pmu_event ev = tool_pmu__str_to_event(term->config);
+	int num = 0;
 
-		if (ev == TOOL_PMU__EVENT_NONE)
-			goto err_out;
-
-		attr->config = ev;
-		return 0;
-	}
-err_out:
-	if (err) {
-		char *err_str;
-
-		parse_events_error__handle(err, term->err_val,
-					asprintf(&err_str,
-						"unexpected tool event term (%s) %s",
-						parse_events__term_type_str(term->type_term),
-						term->config) < 0
-					? strdup("unexpected tool event term")
-					: err_str,
-					NULL);
-	}
-	return -EINVAL;
+#if !defined(__aarch64__)
+	num++;
+#endif
+#if !defined(__i386__) && !defined(__x86_64__)
+	num++;
+#endif
+	return num;
 }
 
-int tool_pmu__config_terms(struct perf_event_attr *attr,
-			   struct parse_events_terms *terms,
-			   struct parse_events_error *err)
+const char *tool_pmu__event_to_str(enum tool_pmu_event ev)
 {
-	struct parse_events_term *term;
-
-	list_for_each_entry(term, &terms->terms, list) {
-		if (tool_pmu__config_term(attr, term, err))
-			return -EINVAL;
-	}
-
-	return 0;
+	if (ev > TOOL_PMU__EVENT_NONE && ev < TOOL_PMU__EVENT_MAX)
+		return tool_pmu__event_names[ev];
 
+	return NULL;
 }
 
-int tool_pmu__for_each_event_cb(struct perf_pmu *pmu, void *state, pmu_event_callback cb)
+enum tool_pmu_event tool_pmu__str_to_event(const char *str)
 {
-	struct pmu_event_info info = {
-		.pmu = pmu,
-		.event_type_desc = "Tool event",
-	};
 	int i;
 
+	if (tool_pmu__skip_event(str))
+		return TOOL_PMU__EVENT_NONE;
+
 	tool_pmu__for_each_event(i) {
-		int ret;
-
-		info.name = tool_pmu__event_to_str(i);
-		info.alias = NULL;
-		info.scale_unit = NULL;
-		info.desc = NULL;
-		info.long_desc = NULL;
-		info.encoding_desc = NULL;
-		info.topic = NULL;
-		info.pmu_name = pmu->name;
-		info.deprecated = false;
-		ret = cb(state, &info);
-		if (ret)
-			return ret;
+		if (!strcasecmp(str, tool_pmu__event_names[i]))
+			return i;
 	}
-	return 0;
+	return TOOL_PMU__EVENT_NONE;
 }
 
 bool perf_pmu__is_tool(const struct perf_pmu *pmu)

tools/perf/util/tool_pmu.c struct perf_pmu *perf_pmus__tool_pmu(void) · line 501

Quoted 5 lines
tools/perf/util/tool_pmu.c +2 -0
@@ -544,8 +497,12 @@ struct perf_pmu *perf_pmus__tool_pmu(void)
 	static struct perf_pmu tool = {
 		.name = "tool",
 		.type = PERF_PMU_TYPE_TOOL,
+		.aliases = LIST_HEAD_INIT(tool.aliases),
+		.caps = LIST_HEAD_INIT(tool.caps),

This can be moved to the previous patch.

Thanks,
Namhyung

Quoted 35 lines
tools/perf/util/tool_pmu.c +2 -0
 		.format = LIST_HEAD_INIT(tool.format),
 	};
+	if (!tool.events_table)
+		tool.events_table = find_core_events_table("common", "common");
 
 	return &tool;
 }
tools/perf/util/tool_pmu.h +3 -9
index ecdf316525bb..a60184859080 100644
@@ -29,17 +29,11 @@ enum tool_pmu_event {
 #define tool_pmu__for_each_event(ev)					\
 	for ((ev) = TOOL_PMU__EVENT_DURATION_TIME; (ev) < TOOL_PMU__EVENT_MAX; ev++)
 
-static inline size_t tool_pmu__num_events(void)
-{
-	return TOOL_PMU__EVENT_MAX - 1;
-}
-
 const char *tool_pmu__event_to_str(enum tool_pmu_event ev);
 enum tool_pmu_event tool_pmu__str_to_event(const char *str);
-int tool_pmu__config_terms(struct perf_event_attr *attr,
-			   struct parse_events_terms *terms,
-			   struct parse_events_error *err);
-int tool_pmu__for_each_event_cb(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+bool tool_pmu__skip_event(const char *name);
+int tool_pmu__num_skip_events(void);
+
 bool tool_pmu__read_event(enum tool_pmu_event ev, u64 *result);
 
 u64 tool_pmu__cpu_slots_per_cycle(void);
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#18 lore raw

PATCH 13/15 perf tests: Add tool PMU test

In reply to Ian Rogers

Ensure parsing with and without PMU creates events with the expected config values. This ensures the tool.json doesn't get out of sync with tool_pmu_event enum.

---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   1 +
 tools/perf/tests/tests.h        |   1 +
 tools/perf/tests/tool_pmu.c     | 111 ++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+)
 create mode 100644 tools/perf/tests/tool_pmu.c
tools/perf/tests/Build +1 -0
index 5671ee530019..a771e4928247 100644
@@ -67,6 +67,7 @@ perf-test-y += sigtrap.o
 perf-test-y += event_groups.o
 perf-test-y += symbols.o
 perf-test-y += util.o
+perf-test-y += tool_pmu.o
 
 ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
 perf-test-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
tools/perf/tests/builtin-test.c +1 -0
index 470a9709427d..3b30f258c395 100644
@@ -73,6 +73,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__PERF_RECORD,
 	&suite__pmu,
 	&suite__pmu_events,
+	&suite__tool_pmu,
 	&suite__dso_data,
 	&suite__perf_evsel__roundtrip_name_test,
 #ifdef HAVE_LIBTRACEEVENT
tools/perf/tests/tests.h +1 -0
index 6ea2be86b7bf..1ed76d4156b6 100644
@@ -83,6 +83,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
 DECLARE_SUITE(syscall_openat_tp_fields);
 DECLARE_SUITE(pmu);
 DECLARE_SUITE(pmu_events);
+DECLARE_SUITE(tool_pmu);
 DECLARE_SUITE(attr);
 DECLARE_SUITE(dso_data);
 DECLARE_SUITE(dso_data_cache);
tools/perf/tests/tool_pmu.c new file +111 -0

111 lines not on this page · raw message

Signature · 1 line
2.46.0.469.g59c65b2a67-goog
maintainer
#19 lore raw

Re: [PATCH v1 13/15] perf tests: Add tool PMU test

Quoted 150 lines · Ian Rogers

Ensure parsing with and without PMU creates events with the expected config values. This ensures the tool.json doesn't get out of sync with tool_pmu_event enum.

---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   1 +
 tools/perf/tests/tests.h        |   1 +
 tools/perf/tests/tool_pmu.c     | 111 ++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+)
 create mode 100644 tools/perf/tests/tool_pmu.c
tools/perf/tests/Build +1 -0
index 5671ee530019..a771e4928247 100644
@@ -67,6 +67,7 @@ perf-test-y += sigtrap.o
 perf-test-y += event_groups.o
 perf-test-y += symbols.o
 perf-test-y += util.o
+perf-test-y += tool_pmu.o
 
 ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
 perf-test-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
tools/perf/tests/builtin-test.c +1 -0
index 470a9709427d..3b30f258c395 100644
@@ -73,6 +73,7 @@ static struct test_suite *generic_tests[] = {
 	&suite__PERF_RECORD,
 	&suite__pmu,
 	&suite__pmu_events,
+	&suite__tool_pmu,
 	&suite__dso_data,
 	&suite__perf_evsel__roundtrip_name_test,
 #ifdef HAVE_LIBTRACEEVENT
tools/perf/tests/tests.h +1 -0
index 6ea2be86b7bf..1ed76d4156b6 100644
@@ -83,6 +83,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
 DECLARE_SUITE(syscall_openat_tp_fields);
 DECLARE_SUITE(pmu);
 DECLARE_SUITE(pmu_events);
+DECLARE_SUITE(tool_pmu);
 DECLARE_SUITE(attr);
 DECLARE_SUITE(dso_data);
 DECLARE_SUITE(dso_data_cache);
tools/perf/tests/tool_pmu.c new file +95 -0
new file mode 100644
index 000000000000..94d0dd8fd3cb
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "debug.h"
+#include "evlist.h"
+#include "parse-events.h"
+#include "tests.h"
+#include "tool_pmu.h"
+
+static int do_test(enum tool_pmu_event ev, bool with_pmu)
+{
+	struct evlist *evlist = evlist__new();
+	struct evsel *evsel;
+	struct parse_events_error err;
+	int ret;
+	char str[128];
+	bool found = false;
+
+	if (!evlist) {
+		pr_err("evlist allocation failed\n");
+		return TEST_FAIL;
+	}
+
+	if (with_pmu)
+		snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev));
+	else
+		strncpy(str, tool_pmu__event_to_str(ev), sizeof(str));
+
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, str, &err);
+	if (ret) {
+		evlist__delete(evlist);
+		if (tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+			ret = TEST_OK;
+			goto out;
+		}
+
+		pr_debug("FAILED %s:%d failed to parse event '%s', err %d\n",
+			 __FILE__, __LINE__, str, ret);
+		parse_events_error__print(&err, str);
+		ret = TEST_FAIL;
+		goto out;
+	}
+
+	ret = TEST_OK;
+	if (with_pmu ? (evlist->core.nr_entries != 1) : (evlist->core.nr_entries < 1)) {
+		pr_debug("FAILED %s:%d Unexpected number of events for '%s' of %d\n",
+			 __FILE__, __LINE__, str, evlist->core.nr_entries);
+		ret = TEST_FAIL;
+		goto out;
+	}
+
+	evlist__for_each_entry(evlist, evsel) {
+		if (perf_pmu__is_tool(evsel->pmu)) {
+			if (evsel->core.attr.config != ev) {
+				pr_debug("FAILED %s:%d Unexpected config for '%s', %lld != %d\n",
+					__FILE__, __LINE__, str, evsel->core.attr.config, ev);
+				ret = TEST_FAIL;
+				goto out;
+			}
+			found = true;
+		}
+	}
+
+	if (!found && !tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+		pr_debug("FAILED %s:%d Didn't find tool event '%s' in parsed evsels\n",
+			 __FILE__, __LINE__, str);
+		ret = TEST_FAIL;
+	}
+
+out:
+	evlist__delete(evlist);
+	return ret;
+}
+
+static int test__tool_pmu_without_pmu(struct test_suite *test __maybe_unused,
+				      int subtest __maybe_unused)
+{
+	int i;
+
+	tool_pmu__for_each_event(i) {
+		int ret = do_test(i, /*with_pmu=*/false);
+
+		if (ret != TEST_OK)
+			return ret;
+	}
+	return TEST_OK;
+}
+
+static int test__tool_pmu_with_pmu(struct test_suite *test __maybe_unused,
+				   int subtest __maybe_unused)
+{
+	int i;
+
+	tool_pmu__for_each_event(i) {
+		int ret = do_test(i, /*with_pmu=*/true);
+

tools/perf/tests/tool_pmu.c line 105

Quoted 10 lines
tools/perf/tests/tool_pmu.c new file +10 -0
+		if (ret != TEST_OK)
+			return ret;
+	}
+	return TEST_OK;
+}
+
+static struct test_case tests__tool_pmu[] = {
+	TEST_CASE("Parsing without PMU name", tool_pmu_without_pmu),
+	TEST_CASE("Parsing with PMU name", tool_pmu_with_pmu),
+	{	.name = NULL, }

An unusual indentation.

Thanks,
Namhyung

Quoted 9 lines
tools/perf/tests/tool_pmu.c new file +6 -0
+};
+
+struct test_suite suite__tool_pmu = {
+	.desc = "Tool PMU",
+	.test_cases = tests__tool_pmu,
+};
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#20 lore raw

Re: [PATCH v1 13/15] perf tests: Add tool PMU test

Quoted 156 lines · Namhyung Kim
Quoted 154 lines · Ian Rogers

Ensure parsing with and without PMU creates events with the expected config values. This ensures the tool.json doesn't get out of sync with tool_pmu_event enum.

---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   1 +
 tools/perf/tests/tests.h        |   1 +
 tools/perf/tests/tool_pmu.c     | 111 ++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+)
 create mode 100644 tools/perf/tests/tool_pmu.c
tools/perf/tests/Build +1 -0
index 5671ee530019..a771e4928247 100644
@@ -67,6 +67,7 @@ perf-test-y += sigtrap.o
 perf-test-y += event_groups.o
 perf-test-y += symbols.o
 perf-test-y += util.o
+perf-test-y += tool_pmu.o
 
 ifeq ($(SRCARCH),$(filter $(SRCARCH),x86 arm arm64 powerpc))
 perf-test-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
tools/perf/tests/builtin-test.c +1 -0
index 470a9709427d..3b30f258c395 100644
@@ -73,6 +73,7 @@ static struct test_suite *generic_tests[] = {
      &suite__PERF_RECORD,
      &suite__pmu,
      &suite__pmu_events,
+     &suite__tool_pmu,
      &suite__dso_data,
      &suite__perf_evsel__roundtrip_name_test,
 #ifdef HAVE_LIBTRACEEVENT
tools/perf/tests/tests.h +1 -0
index 6ea2be86b7bf..1ed76d4156b6 100644
@@ -83,6 +83,7 @@ DECLARE_SUITE(perf_evsel__tp_sched_test);
 DECLARE_SUITE(syscall_openat_tp_fields);
 DECLARE_SUITE(pmu);
 DECLARE_SUITE(pmu_events);
+DECLARE_SUITE(tool_pmu);
 DECLARE_SUITE(attr);
 DECLARE_SUITE(dso_data);
 DECLARE_SUITE(dso_data_cache);
tools/perf/tests/tool_pmu.c new file +99 -0
new file mode 100644
index 000000000000..94d0dd8fd3cb
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "debug.h"
+#include "evlist.h"
+#include "parse-events.h"
+#include "tests.h"
+#include "tool_pmu.h"
+
+static int do_test(enum tool_pmu_event ev, bool with_pmu)
+{
+     struct evlist *evlist = evlist__new();
+     struct evsel *evsel;
+     struct parse_events_error err;
+     int ret;
+     char str[128];
+     bool found = false;
+
+     if (!evlist) {
+             pr_err("evlist allocation failed\n");
+             return TEST_FAIL;
+     }
+
+     if (with_pmu)
+             snprintf(str, sizeof(str), "tool/%s/", tool_pmu__event_to_str(ev));
+     else
+             strncpy(str, tool_pmu__event_to_str(ev), sizeof(str));
+
+     parse_events_error__init(&err);
+     ret = parse_events(evlist, str, &err);
+     if (ret) {
+             evlist__delete(evlist);
+             if (tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+                     ret = TEST_OK;
+                     goto out;
+             }
+
+             pr_debug("FAILED %s:%d failed to parse event '%s', err %d\n",
+                      __FILE__, __LINE__, str, ret);
+             parse_events_error__print(&err, str);
+             ret = TEST_FAIL;
+             goto out;
+     }
+
+     ret = TEST_OK;
+     if (with_pmu ? (evlist->core.nr_entries != 1) : (evlist->core.nr_entries < 1)) {
+             pr_debug("FAILED %s:%d Unexpected number of events for '%s' of %d\n",
+                      __FILE__, __LINE__, str, evlist->core.nr_entries);
+             ret = TEST_FAIL;
+             goto out;
+     }
+
+     evlist__for_each_entry(evlist, evsel) {
+             if (perf_pmu__is_tool(evsel->pmu)) {
+                     if (evsel->core.attr.config != ev) {
+                             pr_debug("FAILED %s:%d Unexpected config for '%s', %lld != %d\n",
+                                     __FILE__, __LINE__, str, evsel->core.attr.config, ev);
+                             ret = TEST_FAIL;
+                             goto out;
+                     }
+                     found = true;
+             }
+     }
+
+     if (!found && !tool_pmu__skip_event(tool_pmu__event_to_str(ev))) {
+             pr_debug("FAILED %s:%d Didn't find tool event '%s' in parsed evsels\n",
+                      __FILE__, __LINE__, str);
+             ret = TEST_FAIL;
+     }
+
+out:
+     evlist__delete(evlist);
+     return ret;
+}
+
+static int test__tool_pmu_without_pmu(struct test_suite *test __maybe_unused,
+                                   int subtest __maybe_unused)
+{
+     int i;
+
+     tool_pmu__for_each_event(i) {
+             int ret = do_test(i, /*with_pmu=*/false);
+
+             if (ret != TEST_OK)
+                     return ret;
+     }
+     return TEST_OK;
+}
+
+static int test__tool_pmu_with_pmu(struct test_suite *test __maybe_unused,
+                                int subtest __maybe_unused)
+{
+     int i;
+
+     tool_pmu__for_each_event(i) {
+             int ret = do_test(i, /*with_pmu=*/true);
+
+             if (ret != TEST_OK)
+                     return ret;
+     }
+     return TEST_OK;
Quoted 8 lines

tools/perf/tests/tool_pmu.c line 105

Quoted 6 lines
tools/perf/tests/tool_pmu.c new file +6 -0
+}
+
+static struct test_case tests__tool_pmu[] = {
+     TEST_CASE("Parsing without PMU name", tool_pmu_without_pmu),
+     TEST_CASE("Parsing with PMU name", tool_pmu_with_pmu),
+     {       .name = NULL, }

An unusual indentation.

Agreed. It matches the other tests though:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/mmap-basic.c?h=perf-tools-next#n303
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/pmu.c?h=perf-tools-next#n539
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/parse-events.c?h=perf-tools-next#n2876

Thanks,
Ian

Quoted 13 lines

Thanks,
Namhyung

Quoted 9 lines
tools/perf/tests/tool_pmu.c new file +6 -1
+};
+
+struct test_suite suite__tool_pmu = {
+     .desc = "Tool PMU",
+     .test_cases = tests__tool_pmu,
+};
--

2.46.0.469.g59c65b2a67-goog

author reviewer
#21 lore raw

PATCH 14/15 perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs

In reply to Ian Rogers

The hwmon sysfs ABI is defined in Documentation/hwmon/sysfs-interface.rst. Create a PMU that reads the hwmon input and can be used in perf stat and metrics much as an uncore PMU can.

For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
     1.001153138              52.00 'C   temp_cpu
     1.001153138              2,588 rpm  fan1
     1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
     1.001153138                  8      tool/num_cpus_online/
     1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
     1.001153138      1,012,773,595      duration_time
...
```

The PMUs are named from /sys/class/hwmon/hwmon<num>/name and have an alias of hwmon<num>. The events are naned using the _label files as well as the <type><num> prefix, the latter guaranteed to be unique.

In perf list the other hwmon files are used to give a richer description, for example:

```
hwmon:
  temp1
       [Temperature in unit acpitz named temp1. Unit: hwmon_acpitz]
  in0
       [Voltage in unit bat0 named in0. Unit: hwmon_bat0]
  temp_core_0 OR temp2
       [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
  temp_core_1 OR temp3
       [Temperature in unit coretemp named Core 1. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
...
  temp_package_id_0 OR temp1
       [Temperature in unit coretemp named Package id 0. crit=100'C,max=100'C crit_alarm=0'C.
        Unit: hwmon_coretemp]
  temp1
       [Temperature in unit iwlwifi_1 named temp1. Unit: hwmon_iwlwifi_1]
  temp_composite OR temp1
       [Temperature in unit nvme named Composite. alarm=0'C,crit=86.85'C,max=75.85'C,
        min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_1 OR temp2
       [Temperature in unit nvme named Sensor 1. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_2 OR temp3
       [Temperature in unit nvme named Sensor 2. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  fan1
       [Fan in unit thinkpad named fan1. Unit: hwmon_thinkpad]
  fan2
       [Fan in unit thinkpad named fan2. Unit: hwmon_thinkpad]
...
  temp_cpu OR temp1
       [Temperature in unit thinkpad named CPU. Unit: hwmon_thinkpad]
  temp_gpu OR temp2
       [Temperature in unit thinkpad named GPU. Unit: hwmon_thinkpad]
  curr1
       [Current in unit ucsi_source_psy_usbc000_0 named curr1. max=1.5A. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
  in0
       [Voltage in unit ucsi_source_psy_usbc000_0 named in0. max=5V,min=5V. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
```

As there may be multiple hwmon devices a range of PMU types are reserved for their use and to identify the PMU as belonging to the hwmon types.

---
 tools/perf/util/Build       |   1 +
 tools/perf/util/evsel.c     |   9 +
 tools/perf/util/hwmon_pmu.c | 879 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/hwmon_pmu.h |  30 ++
 tools/perf/util/pmu.c       |  17 +
 tools/perf/util/pmu.h       |   2 +
 tools/perf/util/pmus.c      |   2 +
 7 files changed, 940 insertions(+)
 create mode 100644 tools/perf/util/hwmon_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.h
tools/perf/util/Build +1 -0
index 80187e3a52be..b1dd5d176d1c 100644
@@ -83,6 +83,7 @@ perf-util-y += pmu.o
 perf-util-y += pmus.o
 perf-util-y += pmu-flex.o
 perf-util-y += pmu-bison.o
+perf-util-y += hwmon_pmu.o
 perf-util-y += tool_pmu.o
 perf-util-y += svghelper.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
tools/perf/util/evsel.c +9 -0
index 9e748ed20988..64883d2aa1bb 100644
@@ -50,6 +50,7 @@
 #include "off_cpu.h"
 #include "pmu.h"
 #include "pmus.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "rlimit.h"
 #include "../perf-sys.h"
@@ -1657,6 +1658,9 @@ int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread)
 	if (evsel__is_tool(evsel))
 		return evsel__tool_pmu_read(evsel, cpu_map_idx, thread);
 
+	if (evsel__is_hwmon(evsel))
+		return evsel__hwmon_pmu_read(evsel, cpu_map_idx, thread);
+
 	if (evsel__is_retire_lat(evsel))
 		return evsel__read_retire_lat(evsel, cpu_map_idx, thread);
 
@@ -2094,6 +2098,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 					    start_cpu_map_idx,
 					    end_cpu_map_idx);
 	}
+	if (evsel__is_hwmon(evsel)) {
+		return evsel__hwmon_pmu_open(evsel, threads,
+					     start_cpu_map_idx,
+					     end_cpu_map_idx);
+	}
 
 	for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
 
tools/perf/util/hwmon_pmu.c new file +879 -0

879 lines not on this page · raw message

tools/perf/util/hwmon_pmu.h new file +30 -0
new file mode 100644
index 000000000000..49e1683d5bce
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __HWMON_PMU_H
+#define __HWMON_PMU_H
+
+#include "pmu.h"
+
+struct list_head;
+
+bool perf_pmu__is_hwmon(const struct perf_pmu *pmu);
+bool evsel__is_hwmon(const struct evsel *evsel);
+
+int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+size_t hwmon_pmu__num_events(struct perf_pmu *pmu);
+bool hwmon_pmu__have_event(struct perf_pmu *pmu, const char *name);
+int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
+			    struct perf_event_attr *attr,
+			    struct parse_events_terms *terms,
+			    struct parse_events_error *err);
+int hwmon_pmu__check_alias(struct parse_events_terms *terms, struct perf_pmu_info *info,
+			   struct parse_events_error *err);
+
+int perf_pmus__read_hwmon_pmus(struct list_head *pmus);
+
+
+int evsel__hwmon_pmu_open(struct evsel *evsel,
+			 struct perf_thread_map *threads,
+			 int start_cpu_map_idx, int end_cpu_map_idx);
+int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread);
+
+#endif /* __HWMON_PMU_H */
tools/perf/util/pmu.c +17 -0
index 5ff6556292fd..c31d3409c787 100644
@@ -18,6 +18,7 @@
 #include "debug.h"
 #include "evsel.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "pmus.h"
 #include "tool_pmu.h"
 #include <util/pmu-bison.h>
@@ -1529,6 +1530,9 @@ int perf_pmu__config_terms(const struct perf_pmu *pmu,
 {
 	struct parse_events_term *term;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__config_terms(pmu, attr, terms, err);
+
 	list_for_each_entry(term, &terms->terms, list) {
 		if (pmu_config_term(pmu, attr, term, terms, zero, apply_hardcoded, err))
 			return -EINVAL;
@@ -1661,6 +1665,11 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
+	if (perf_pmu__is_hwmon(pmu)) {
+		ret = hwmon_pmu__check_alias(head_terms, info, err);
+		goto out;
+	}
+
 	/* Fake PMU doesn't rewrite terms. */
 	if (perf_pmu__is_fake(pmu))
 		goto out;
@@ -1830,6 +1839,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 		return false;
 	if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(name))
 		return false;
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__have_event(pmu, name);
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1841,6 +1852,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
 	size_t nr;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__num_events(pmu);
+
 	pmu_aliases_parse(pmu);
 	nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1904,6 +1918,9 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct strbuf sb;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__for_each_event(pmu, state, cb);
+
 	strbuf_init(&sb, /*hint=*/ 0);
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
tools/perf/util/pmu.h +2 -0
index c4ca359d4215..32f95a1060d8 100644
@@ -37,6 +37,8 @@ struct perf_pmu_caps {
 };
 
 enum {
+	PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
+	PERF_PMU_TYPE_HWMON_END   = 0xFFFFFFFD,
 	PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
 	PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
 };
tools/perf/util/pmus.c +2 -0
index 19673b9991c6..4e3b19a2db00 100644
@@ -15,6 +15,7 @@
 #include "evsel.h"
 #include "pmus.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "print-events.h"
 #include "strbuf.h"
@@ -234,6 +235,7 @@ static void pmu_read_sysfs(bool core_only)
 	if (!core_only) {
 		tool_pmu = perf_pmus__tool_pmu();
 		list_add_tail(&tool_pmu->list, &other_pmus);
+		perf_pmus__read_hwmon_pmus(&other_pmus);
 	}
 	list_sort(NULL, &other_pmus, pmus_cmp);
 	if (!list_empty(&core_pmus)) {
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
maintainer
#22 lore raw

Re: [PATCH v1 14/15] perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs

Quoted 7 lines · Ian Rogers

The hwmon sysfs ABI is defined in Documentation/hwmon/sysfs-interface.rst. Create a PMU that reads the hwmon input and can be used in perf stat and metrics much as an uncore PMU can.

For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

Quoted 10 lines
```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
     1.001153138              52.00 'C   temp_cpu
     1.001153138              2,588 rpm  fan1
     1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
     1.001153138                  8      tool/num_cpus_online/
     1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
     1.001153138      1,012,773,595      duration_time
...
```

Interesting! While they don't seem to be counters, it'd be useful to see the values from various sources/PMUs especially for interval mode.

Quoted 139 lines

The PMUs are named from /sys/class/hwmon/hwmon<num>/name and have an alias of hwmon<num>. The events are naned using the _label files as well as the <type><num> prefix, the latter guaranteed to be unique.

In perf list the other hwmon files are used to give a richer description, for example:

```
hwmon:
  temp1
       [Temperature in unit acpitz named temp1. Unit: hwmon_acpitz]
  in0
       [Voltage in unit bat0 named in0. Unit: hwmon_bat0]
  temp_core_0 OR temp2
       [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
  temp_core_1 OR temp3
       [Temperature in unit coretemp named Core 1. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
...
  temp_package_id_0 OR temp1
       [Temperature in unit coretemp named Package id 0. crit=100'C,max=100'C crit_alarm=0'C.
        Unit: hwmon_coretemp]
  temp1
       [Temperature in unit iwlwifi_1 named temp1. Unit: hwmon_iwlwifi_1]
  temp_composite OR temp1
       [Temperature in unit nvme named Composite. alarm=0'C,crit=86.85'C,max=75.85'C,
        min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_1 OR temp2
       [Temperature in unit nvme named Sensor 1. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_2 OR temp3
       [Temperature in unit nvme named Sensor 2. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  fan1
       [Fan in unit thinkpad named fan1. Unit: hwmon_thinkpad]
  fan2
       [Fan in unit thinkpad named fan2. Unit: hwmon_thinkpad]
...
  temp_cpu OR temp1
       [Temperature in unit thinkpad named CPU. Unit: hwmon_thinkpad]
  temp_gpu OR temp2
       [Temperature in unit thinkpad named GPU. Unit: hwmon_thinkpad]
  curr1
       [Current in unit ucsi_source_psy_usbc000_0 named curr1. max=1.5A. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
  in0
       [Voltage in unit ucsi_source_psy_usbc000_0 named in0. max=5V,min=5V. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
```

As there may be multiple hwmon devices a range of PMU types are reserved for their use and to identify the PMU as belonging to the hwmon types.

---
 tools/perf/util/Build       |   1 +
 tools/perf/util/evsel.c     |   9 +
 tools/perf/util/hwmon_pmu.c | 879 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/hwmon_pmu.h |  30 ++
 tools/perf/util/pmu.c       |  17 +
 tools/perf/util/pmu.h       |   2 +
 tools/perf/util/pmus.c      |   2 +
 7 files changed, 940 insertions(+)
 create mode 100644 tools/perf/util/hwmon_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.h
tools/perf/util/Build +1 -0
index 80187e3a52be..b1dd5d176d1c 100644
@@ -83,6 +83,7 @@ perf-util-y += pmu.o
 perf-util-y += pmus.o
 perf-util-y += pmu-flex.o
 perf-util-y += pmu-bison.o
+perf-util-y += hwmon_pmu.o
 perf-util-y += tool_pmu.o
 perf-util-y += svghelper.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
tools/perf/util/evsel.c +9 -0
index 9e748ed20988..64883d2aa1bb 100644
@@ -50,6 +50,7 @@
 #include "off_cpu.h"
 #include "pmu.h"
 #include "pmus.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "rlimit.h"
 #include "../perf-sys.h"
@@ -1657,6 +1658,9 @@ int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread)
 	if (evsel__is_tool(evsel))
 		return evsel__tool_pmu_read(evsel, cpu_map_idx, thread);
 
+	if (evsel__is_hwmon(evsel))
+		return evsel__hwmon_pmu_read(evsel, cpu_map_idx, thread);
+
 	if (evsel__is_retire_lat(evsel))
 		return evsel__read_retire_lat(evsel, cpu_map_idx, thread);
 
@@ -2094,6 +2098,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 					    start_cpu_map_idx,
 					    end_cpu_map_idx);
 	}
+	if (evsel__is_hwmon(evsel)) {
+		return evsel__hwmon_pmu_open(evsel, threads,
+					     start_cpu_map_idx,
+					     end_cpu_map_idx);
+	}
 
 	for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
 
tools/perf/util/hwmon_pmu.c new file +21 -0
new file mode 100644
index 000000000000..cc8816b787ca
@@ -0,0 +1,879 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "counts.h"
+#include "debug.h"
+#include "evsel.h"
+#include "hashmap.h"
+#include "hwmon_pmu.h"
+#include "pmu.h"
+#include <internal/xyarray.h>
+#include <internal/threadmap.h>
+#include <perf/threadmap.h>
+#include <sys/types.h>
+#include <ctype.h>
+#include <dirent.h>
+#include <fcntl.h>
+#include <api/fs/fs.h>
+#include <api/io.h>
+#include <linux/zalloc.h>
+
+enum hwmon_type {
+	HWMON_TYPE_NONE,
+

tools/perf/util/hwmon_pmu.c line 31

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+	HWMON_TYPE_CPU,
+	HWMON_TYPE_CURR,
+	HWMON_TYPE_ENERGY,
+	HWMON_TYPE_FAN,
+	HWMON_TYPE_HUMIDITY,
+	HWMON_TYPE_IN,
+	HWMON_TYPE_INTRUSION,
+	HWMON_TYPE_POWER,
+	HWMON_TYPE_PWM,
+	HWMON_TYPE_TEMP,

Where does this list come from? Is it exhaustive?

Quoted 136 lines
tools/perf/util/hwmon_pmu.c new file +136 -0
+
+	HWMON_TYPE_MAX
+};
+
+static const char * const hwmon_type_strs[HWMON_TYPE_MAX] = {
+	NULL,
+	"cpu",
+	"curr",
+	"energy",
+	"fan",
+	"humidity",
+	"in",
+	"intrusion",
+	"power",
+	"pwm",
+	"temp",
+};
+
+static const char *const hwmon_units[HWMON_TYPE_MAX] = {
+	NULL,
+	"V",   /* cpu */
+	"A",   /* curr */
+	"J",   /* energy */
+	"rpm", /* fan */
+	"%",   /* humidity */
+	"V",   /* in */
+	"",    /* intrusion */
+	"W",   /* power */
+	"Hz",  /* pwm */
+	"'C",  /* temp */
+};
+
+enum hwmon_item {
+	HWMON_ITEM_NONE,
+
+	HWMON_ITEM_ACCURACY,
+	HWMON_ITEM_ALARM,
+	HWMON_ITEM_AUTO_CHANNELS_TEMP,
+	HWMON_ITEM_AVERAGE,
+	HWMON_ITEM_AVERAGE_HIGHEST,
+	HWMON_ITEM_AVERAGE_INTERVAL,
+	HWMON_ITEM_AVERAGE_INTERVAL_MAX,
+	HWMON_ITEM_AVERAGE_INTERVAL_MIN,
+	HWMON_ITEM_AVERAGE_LOWEST,
+	HWMON_ITEM_AVERAGE_MAX,
+	HWMON_ITEM_AVERAGE_MIN,
+	HWMON_ITEM_BEEP,
+	HWMON_ITEM_CAP,
+	HWMON_ITEM_CAP_HYST,
+	HWMON_ITEM_CAP_MAX,
+	HWMON_ITEM_CAP_MIN,
+	HWMON_ITEM_CRIT,
+	HWMON_ITEM_CRIT_HYST,
+	HWMON_ITEM_DIV,
+	HWMON_ITEM_EMERGENCY,
+	HWMON_ITEM_EMERGENCY_HIST,
+	HWMON_ITEM_ENABLE,
+	HWMON_ITEM_FAULT,
+	HWMON_ITEM_FREQ,
+	HWMON_ITEM_HIGHEST,
+	HWMON_ITEM_INPUT,
+	HWMON_ITEM_LABEL,
+	HWMON_ITEM_LCRIT,
+	HWMON_ITEM_LCRIT_HYST,
+	HWMON_ITEM_LOWEST,
+	HWMON_ITEM_MAX,
+	HWMON_ITEM_MAX_HYST,
+	HWMON_ITEM_MIN,
+	HWMON_ITEM_MIN_HYST,
+	HWMON_ITEM_MOD,
+	HWMON_ITEM_OFFSET,
+	HWMON_ITEM_PULSES,
+	HWMON_ITEM_RATED_MAX,
+	HWMON_ITEM_RATED_MIN,
+	HWMON_ITEM_RESET_HISTORY,
+	HWMON_ITEM_TARGET,
+	HWMON_ITEM_TYPE,
+	HWMON_ITEM_VID,
+
+	HWMON_ITEM__MAX,
+};
+
+static const char * const hwmon_item_strs[HWMON_ITEM__MAX] = {
+	NULL,
+	"accuracy",
+	"alarm",
+	"auto_channels_temp",
+	"average",
+	"average_highest",
+	"average_interval",
+	"average_interval_max",
+	"average_interval_min",
+	"average_lowest",
+	"average_max",
+	"average_min",
+	"beep",
+	"cap",
+	"cap_hyst",
+	"cap_max",
+	"cap_min",
+	"crit",
+	"crit_hyst",
+	"div",
+	"emergency",
+	"emergency_hist",
+	"enable",
+	"fault",
+	"freq",
+	"highest",
+	"input",
+	"label",
+	"lcrit",
+	"lcrit_hyst",
+	"lowest",
+	"max",
+	"max_hyst",
+	"min",
+	"min_hyst",
+	"mod",
+	"offset",
+	"pulses",
+	"rated_max",
+	"rated_min",
+	"reset_history",
+	"target",
+	"type",
+	"vid",
+};
+
+struct hwmon_pmu {
+	struct perf_pmu pmu;
+	struct hashmap events;
+	int pmu_dir_fd;
+};
+
+/**

tools/perf/util/hwmon_pmu.c line 177

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+ * union hwmon_pmu_event_key: Key for hwmon_pmu->events as such each key
+ * represents an event.
+ *
+ * Related hwmon files start <type><number> that this key represents.
+ */
+union hwmon_pmu_event_key {
+	long type_and_num;
+	struct {
+		int num :16;
+		enum hwmon_type type :8;

Why not plain int types? I'm not sure how much we care about 32 bits but you could use short int then.

Quoted 40 lines
tools/perf/util/hwmon_pmu.c new file +40 -0
+	};
+};
+
+/**
+ * struct hwmon_pmu_event_value: Value in hwmon_pmu->events.
+ *
+ * Hwmon files are of the form <type><number>_<item> and may have a suffix
+ * _alarm.
+ */
+struct hwmon_pmu_event_value {
+	/** @items: which item files are present. */
+	DECLARE_BITMAP(items, HWMON_ITEM__MAX);
+	/** @alarm_items: which item files are present. */
+	DECLARE_BITMAP(alarm_items, HWMON_ITEM__MAX);
+	/** @label: contents of <type><number>_label if present. */
+	char *label;
+	/** @name: name computed from label of the form <type>_<label>. */
+	char *name;
+};
+
+bool perf_pmu__is_hwmon(const struct perf_pmu *pmu)
+{
+	return pmu && pmu->type >= PERF_PMU_TYPE_HWMON_START &&
+		pmu->type <= PERF_PMU_TYPE_HWMON_END;
+}
+
+bool evsel__is_hwmon(const struct evsel *evsel)
+{
+	return perf_pmu__is_hwmon(evsel->pmu);
+}
+
+static size_t hwmon_pmu__event_hashmap_hash(long key, void *ctx __maybe_unused)
+{
+	return ((union hwmon_pmu_event_key)key).type_and_num;
+}
+
+static bool hwmon_pmu__event_hashmap_equal(long key1, long key2, void *ctx __maybe_unused)
+{
+	return ((union hwmon_pmu_event_key)key1).type_and_num ==
+	       ((union hwmon_pmu_event_key)key2).type_and_num;

tools/perf/util/hwmon_pmu.c line 227

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+}
+
+static int hwmon_strcmp(const void *a, const void *b)
+{
+	const char *sa = a;
+	const char * const *sb = b;
+
+	return strcmp(sa, *sb);
+}
+

Can you please add some examples of typical filename in a comment here?

tools/perf/util/hwmon_pmu.c line 239

Quoted 12 lines
tools/perf/util/hwmon_pmu.c new file +12 -0
+static bool parse_hwmon_filename(const char *filename,
+				 enum hwmon_type *type,
+				 int *number,
+				 enum hwmon_item *item,
+				 bool *alarm)
+{
+	char fn_type[24];
+	const char **elem;
+	const char *fn_item = NULL;
+	size_t fn_item_len;
+
+	assert(strlen("intrusion") < sizeof(fn_type));

Why intrusion?

Quoted 12 lines
tools/perf/util/hwmon_pmu.c new file +12 -0
+	strncpy(fn_type, filename, sizeof(fn_type));
+	for (size_t i = 0; i < sizeof(fn_type); i++) {
+		if (fn_type[i] >= '0' && fn_type[i] <= '9') {
+			fn_type[i] = '\0';
+			*number = strtoul(&filename[i], (char **)&fn_item, 10);
+			if (*fn_item == '_')
+				fn_item++;
+			break;
+		}
+		if (fn_type[i] == '_') {
+			fn_type[i] = '\0';
+			*number = -1;

tools/perf/util/hwmon_pmu.c line 261

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+			fn_item = &filename[i + 1];
+			break;
+		}
+	}
+	if (fn_item == NULL || fn_type[0] == '\0' || (item != NULL && fn_item[0] == '\0')) {
+		pr_debug("hwmon_pmu: not a hwmon file '%s'\n", filename);
+		return false;
+	}
+	elem = bsearch(&fn_type, hwmon_type_strs + 1, ARRAY_SIZE(hwmon_type_strs) - 1,
+		       sizeof(hwmon_type_strs[0]), hwmon_strcmp);

Can we make the first entry an empty string rather than NULL to make this simpler?

Quoted 8 lines
tools/perf/util/hwmon_pmu.c new file +8 -0
+	if (!elem) {
+		pr_debug("hwmon_pmu: not a hwmon type '%s' in file name '%s'\n",
+			 fn_type, filename);
+		return false;
+	}
+
+	*type = elem - &hwmon_type_strs[0];
+	if (!item)

tools/perf/util/hwmon_pmu.c line 279

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+		return true;
+
+	*alarm = false;
+	fn_item_len = strlen(fn_item);
+	if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
+		assert(strlen("average_interval_max") < sizeof(fn_type));
+		strncpy(fn_type, fn_item, fn_item_len - 6);
+		fn_item = fn_type;
+		*alarm = true;
+	}

A comment to explain why it handles the "alarm" differently would be great.

Quoted 18 lines
tools/perf/util/hwmon_pmu.c new file +18 -0
+	elem = bsearch(fn_item, hwmon_item_strs + 1, ARRAY_SIZE(hwmon_item_strs) - 1,
+		       sizeof(hwmon_item_strs[0]), hwmon_strcmp);
+	if (!elem) {
+		pr_debug("hwmon_pmu: not a hwmon item '%s' in file name '%s'\n",
+			 fn_item, filename);
+		return false;
+	}
+	*item = elem - &hwmon_item_strs[0];
+	return true;
+}
+
+static void fix_name(char *p)
+{
+	char *s = strchr(p, '\n');
+
+	if (s)
+		*s = '\0';
+

tools/perf/util/hwmon_pmu.c line 307

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+	while (*p != '\0') {
+		if (strchr(" :,/\n\t", *p))
+			*p = '_';
+		else
+			*p = tolower(*p);
+		p++;
+	}
+}
+
+static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)

Maybe better to use other short name ('hwm'?) than 'pmu'.

Quoted 87 lines
tools/perf/util/hwmon_pmu.c new file +87 -0
+{
+	DIR *dir;
+	struct dirent *ent;
+	int dup_fd, err = 0;
+	struct hashmap_entry *cur, *tmp;
+	size_t bkt;
+
+	if (pmu->pmu.sysfs_aliases_loaded)
+		return 0;
+
+	/* Use a dup-ed fd as closedir will close it. */
+	dup_fd = dup(pmu->pmu_dir_fd);
+	if (dup_fd == -1)
+		return -ENOMEM;
+
+	dir = fdopendir(dup_fd);
+	if (!dir) {
+		close(dup_fd);
+		return -ENOMEM;
+	}
+
+	while ((ent = readdir(dir)) != NULL) {
+		enum hwmon_type type;
+		int number;
+		enum hwmon_item item;
+		bool alarm;
+		union hwmon_pmu_event_key key;
+		struct hwmon_pmu_event_value *value;
+
+		if (ent->d_type != DT_REG)
+			continue;
+
+		if (!parse_hwmon_filename(ent->d_name, &type, &number, &item, &alarm)) {
+			pr_debug("Not a hwmon file '%s'\n", ent->d_name);
+			continue;
+		}
+		key.num = number;
+		key.type = type;
+		if (!hashmap__find(&pmu->events, key.type_and_num, &value)) {
+			value = zalloc(sizeof(*value));
+			if (!value) {
+				err = -ENOMEM;
+				goto err_out;
+			}
+			err = hashmap__add(&pmu->events, key.type_and_num, value);
+			if (err) {
+				free(value);
+				err = -ENOMEM;
+				goto err_out;
+			}
+		}
+		__set_bit(item, alarm ? value->alarm_items : value->items);
+		if (item == HWMON_ITEM_LABEL) {
+			char buf[128];
+			int fd = openat(pmu->pmu_dir_fd, ent->d_name, O_RDONLY);
+			ssize_t read_len;
+
+			if (fd < 0)
+				continue;
+
+			read_len = read(fd, buf, sizeof(buf));
+
+			while (read_len > 0 && buf[read_len - 1] == '\n')
+				read_len--;
+
+			if (read_len > 0)
+				buf[read_len] = '\0';
+
+			if (buf[0] == '\0') {
+				pr_debug("hwmon_pmu: empty label file %s %s\n",
+					 pmu->pmu.name, ent->d_name);
+				close(fd);
+				continue;
+			}
+			value->label = strdup(buf);
+			if (!value->label) {
+				pr_debug("hwmon_pmu: memory allocation failure\n");
+				close(fd);
+				continue;
+			}
+			snprintf(buf, sizeof(buf), "%s_%s", hwmon_type_strs[type], value->label);
+			fix_name(buf);
+			value->name = strdup(buf);
+			if (!value->name)
+				pr_debug("hwmon_pmu: memory allocation failure\n");
+			close(fd);
+		}

tools/perf/util/hwmon_pmu.c line 404

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+	}
+	hashmap__for_each_entry_safe((&pmu->events), cur, tmp, bkt) {
+		union hwmon_pmu_event_key key = {
+			.type_and_num = cur->key,
+		};
+		struct hwmon_pmu_event_value *value = cur->pvalue;
+
+		if (!test_bit(HWMON_ITEM_INPUT, value->items)) {
+			pr_debug("hwmon_pmu: removing event '%s%d' that has no input file",
+				hwmon_type_strs[key.type], key.num);

What is the input file and why is it required?

Quoted 11 lines
tools/perf/util/hwmon_pmu.c new file +11 -0
+			hashmap__delete(&pmu->events, key.type_and_num, &key, &value);
+			zfree(&value->label);
+			zfree(&value->name);
+			free(value);
+		}
+	}
+	pmu->pmu.sysfs_aliases_loaded = true;
+
+err_out:
+	closedir(dir);
+	return err;

tools/perf/util/hwmon_pmu.c line 425

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+}
+
+static int hwmon_pmu__new(struct list_head *pmus, int pmu_dir, const char *sysfs_name,
+			  const char *name)
+{
+	char buf[32];
+	struct hwmon_pmu *pmu;
+	int err = 0;
+
+	pr_debug("Hi Ian, '%s' -> '%s'\n", sysfs_name, name);

Forgot to remove? :)

Quoted 315 lines
tools/perf/util/hwmon_pmu.c new file +315 -0
+
+	pmu = zalloc(sizeof(*pmu));
+	if (!pmu)
+		return -ENOMEM;
+
+
+	pmu->pmu_dir_fd = pmu_dir;
+	pmu->pmu.type = PERF_PMU_TYPE_HWMON_START + strtoul(sysfs_name + 5, NULL, 10);
+	if (pmu->pmu.type > PERF_PMU_TYPE_HWMON_END) {
+		err = -EINVAL;
+		goto err_out;
+	}
+	snprintf(buf, sizeof(buf), "hwmon_%s", name);
+	fix_name(buf + 6);
+	pmu->pmu.name = strdup(buf);
+	if (!pmu->pmu.name) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+	pmu->pmu.alias_name = strdup(sysfs_name);
+	if (!pmu->pmu.alias_name) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+	pmu->pmu.cpus = perf_cpu_map__new("0");
+	if (!pmu->pmu.cpus) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+	INIT_LIST_HEAD(&pmu->pmu.format);
+	INIT_LIST_HEAD(&pmu->pmu.aliases);
+	INIT_LIST_HEAD(&pmu->pmu.caps);
+	hashmap__init(&pmu->events, hwmon_pmu__event_hashmap_hash,
+		      hwmon_pmu__event_hashmap_equal, /*ctx=*/NULL);
+
+	list_add_tail(&pmu->pmu.list, pmus);
+err_out:
+	if (err) {
+		free((char *)pmu->pmu.name);
+		free(pmu->pmu.alias_name);
+		free(pmu);
+		close(pmu_dir);
+	}
+	return err;
+}
+
+static size_t hwmon_pmu__describe_items(struct hwmon_pmu *pmu, char *out_buf, size_t out_buf_len,
+					union hwmon_pmu_event_key key,
+					const unsigned long *items, bool is_alarm)
+{
+	size_t bit;
+	char buf[64];
+	size_t len = 0;
+
+	for_each_set_bit(bit, items, HWMON_ITEM__MAX) {
+		int fd;
+
+		if (bit == HWMON_ITEM_LABEL || bit == HWMON_ITEM_INPUT)
+			continue;
+
+		snprintf(buf, sizeof(buf), "%s%d_%s%s",
+			hwmon_type_strs[key.type],
+			key.num,
+			hwmon_item_strs[bit],
+			is_alarm ? "_alarm" : "");
+		fd = openat(pmu->pmu_dir_fd, buf, O_RDONLY);
+		if (fd > 0) {
+			ssize_t read_len = read(fd, buf, sizeof(buf));
+
+			while (read_len > 0 && buf[read_len - 1] == '\n')
+				read_len--;
+
+			if (read_len > 0) {
+				long long val;
+
+				buf[read_len] = '\0';
+				val = strtoll(buf, /*endptr=*/NULL, 10);
+				len += snprintf(out_buf + len, out_buf_len - len, "%s%s%s=%g%s",
+						len == 0 ? " " : ", ",
+						hwmon_item_strs[bit],
+						is_alarm ? "_alarm" : "",
+						(double)val / 1000.0,
+						hwmon_units[key.type]);
+			}
+			close(fd);
+		}
+	}
+	return len;
+}
+
+int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb)
+{
+	struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+	struct hashmap_entry *cur;
+	size_t bkt;
+
+	if (hwmon_pmu__read_events(hwmon_pmu))
+		return false;
+
+	hashmap__for_each_entry((&hwmon_pmu->events), cur, bkt) {
+		static const char *const hwmon_scale_units[HWMON_TYPE_MAX] = {
+			NULL,
+			"0.001V", /* cpu */
+			"0.001A", /* curr */
+			"0.001J", /* energy */
+			"1rpm",   /* fan */
+			"0.001%", /* humidity */
+			"0.001V", /* in */
+			NULL,     /* intrusion */
+			"0.001W", /* power */
+			"1Hz",    /* pwm */
+			"0.001'C", /* temp */
+		};
+		static const char *const hwmon_desc[HWMON_TYPE_MAX] = {
+			NULL,
+			"CPU core reference voltage",   /* cpu */
+			"Current",                      /* curr */
+			"Cumulative energy use",        /* energy */
+			"Fan",                          /* fan */
+			"Humidity",                     /* humidity */
+			"Voltage",                      /* in */
+			"Chassis intrusion detection",  /* intrusion */
+			"Power use",                    /* power */
+			"Pulse width modulation fan control", /* pwm */
+			"Temperature",                  /* temp */
+		};
+		char alias_buf[64];
+		char desc_buf[256];
+		char encoding_buf[128];
+		union hwmon_pmu_event_key key = {
+			.type_and_num = cur->key,
+		};
+		struct hwmon_pmu_event_value *value = cur->pvalue;
+		struct pmu_event_info info = {
+			.pmu = pmu,
+			.name = value->name,
+			.alias = alias_buf,
+			.scale_unit = hwmon_scale_units[key.type],
+			.desc = desc_buf,
+			.long_desc = NULL,
+			.encoding_desc = encoding_buf,
+			.topic = "hwmon",
+			.pmu_name = pmu->name,
+			.event_type_desc = "Hwmon event",
+		};
+		int ret;
+		size_t len;
+
+		len = snprintf(alias_buf, sizeof(alias_buf), "%s%d",
+			       hwmon_type_strs[key.type], key.num);
+		if (!info.name) {
+			info.name = info.alias;
+			info.alias = NULL;
+		}
+
+		len = snprintf(desc_buf, sizeof(desc_buf), "%s in unit %s named %s.",
+			hwmon_desc[key.type],
+			pmu->name + 6,
+			value->label ?: info.name);
+
+		len += hwmon_pmu__describe_items(hwmon_pmu, desc_buf + len, sizeof(desc_buf) - len,
+						key, value->items, /*is_alarm=*/false);
+
+		len += hwmon_pmu__describe_items(hwmon_pmu, desc_buf + len, sizeof(desc_buf) - len,
+						key, value->alarm_items, /*is_alarm=*/true);
+
+		snprintf(encoding_buf, sizeof(encoding_buf), "%s/config=0x%lx/",
+			 pmu->name, cur->key);
+
+		ret = cb(state, &info);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+size_t hwmon_pmu__num_events(struct perf_pmu *pmu)
+{
+	struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+
+	hwmon_pmu__read_events(hwmon_pmu);
+	return hashmap__size(&hwmon_pmu->events);
+}
+
+bool hwmon_pmu__have_event(struct perf_pmu *pmu, const char *name)
+{
+	struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+	enum hwmon_type type;
+	int number;
+	union hwmon_pmu_event_key key;
+	struct hashmap_entry *cur;
+	size_t bkt;
+
+	if (!parse_hwmon_filename(name, &type, &number, /*item=*/NULL, /*is_alarm=*/NULL))
+		return false;
+
+	if (hwmon_pmu__read_events(hwmon_pmu))
+		return false;
+
+	key.type = type;
+	key.num = number;
+	if (hashmap_find(&hwmon_pmu->events, key.type_and_num, /*value=*/NULL))
+		return true;
+	if (key.num != -1)
+		return false;
+	/* Item is of form <type>_ which means we should match <type>_<label>. */
+	hashmap__for_each_entry((&hwmon_pmu->events), cur, bkt) {
+		struct hwmon_pmu_event_value *value = cur->pvalue;
+
+		key.type_and_num = cur->key;
+		if (key.type == type && value->name && !strcasecmp(name, value->name))
+			return true;
+	}
+	return false;
+}
+
+static int hwmon_pmu__config_term(const struct hwmon_pmu *pmu,
+				  struct perf_event_attr *attr,
+				  struct parse_events_term *term,
+				  struct parse_events_error *err)
+{
+	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
+		enum hwmon_type type;
+		int number;
+
+		if (parse_hwmon_filename(term->config, &type, &number,
+					 /*item=*/NULL, /*is_alarm=*/NULL)) {
+			if (number == -1) {
+				/*
+				 * Item is of form <type>_ which means we should
+				 * match <type>_<label>.
+				 */
+				struct hashmap_entry *cur;
+				size_t bkt;
+
+				attr->config = 0;
+				hashmap__for_each_entry((&pmu->events), cur, bkt) {
+					union hwmon_pmu_event_key key = {
+						.type_and_num = cur->key,
+					};
+					struct hwmon_pmu_event_value *value = cur->pvalue;
+
+					if (key.type == type && value->name &&
+					    !strcasecmp(term->config, value->name)) {
+						attr->config = key.type_and_num;
+						break;
+					}
+				}
+				if (attr->config == 0)
+					return -EINVAL;
+			} else {
+				union hwmon_pmu_event_key key = {
+					.type = type,
+					.num = number,
+				};
+
+				attr->config = key.type_and_num;
+			}
+			return 0;
+		}
+	}
+	if (err) {
+		char *err_str;
+
+		parse_events_error__handle(err, term->err_val,
+					asprintf(&err_str,
+						"unexpected hwmon event term (%s) %s",
+						parse_events__term_type_str(term->type_term),
+						term->config) < 0
+					? strdup("unexpected hwmon event term")
+					: err_str,
+					NULL);
+	}
+	return -EINVAL;
+}
+
+int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
+			    struct perf_event_attr *attr,
+			    struct parse_events_terms *terms,
+			    struct parse_events_error *err)
+{
+	const struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+	struct parse_events_term *term;
+
+	assert(pmu->sysfs_aliases_loaded);
+	list_for_each_entry(term, &terms->terms, list) {
+		if (hwmon_pmu__config_term(hwmon_pmu, attr, term, err))
+			return -EINVAL;
+	}
+
+	return 0;
+
+}
+
+int hwmon_pmu__check_alias(struct parse_events_terms *terms, struct perf_pmu_info *info,
+			   struct parse_events_error *err)
+{
+	struct parse_events_term *term =
+		list_first_entry(&terms->terms, struct parse_events_term, list);
+
+	if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
+		enum hwmon_type type;
+		int number;
+
+		if (parse_hwmon_filename(term->config, &type, &number,
+					 /*item=*/NULL, /*is_alarm=*/NULL)) {
+			info->unit = hwmon_units[type];
+			if (type == HWMON_TYPE_FAN || type == HWMON_TYPE_PWM ||
+			    type == HWMON_TYPE_INTRUSION)
+				info->scale = 1;
+			else
+				info->scale = 0.001;
+		}
+		return 0;
+	}

tools/perf/util/hwmon_pmu.c line 750

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+	if (err) {
+		char *err_str;
+
+		parse_events_error__handle(err, term->err_val,
+					asprintf(&err_str,
+						"unexpected hwmon event term (%s) %s",
+						parse_events__term_type_str(term->type_term),
+						term->config) < 0
+					? strdup("unexpected hwmon event term")
+					: err_str,

Probably better to move the asprintf() out to a statement.

Quoted 31 lines
tools/perf/util/hwmon_pmu.c new file +31 -0
+					NULL);
+	}
+	return -EINVAL;
+}
+
+int perf_pmus__read_hwmon_pmus(struct list_head *pmus)
+{
+	char *line = NULL;
+	DIR *hwmon_dir;
+	struct dirent *hwmon_ent;
+	char buf[PATH_MAX];
+	const char *sysfs = sysfs__mountpoint();
+
+	if (!sysfs)
+		return 0;
+
+	scnprintf(buf, sizeof(buf), "%s/class/hwmon/", sysfs);
+	hwmon_dir = opendir(buf);
+	if (!hwmon_dir)
+		return 0;
+
+	while ((hwmon_ent = readdir(hwmon_dir)) != NULL) {
+		size_t line_len;
+		int pmu_dir, name_fd;
+		struct io io;
+
+		if (hwmon_ent->d_type != DT_LNK)
+			continue;
+
+		scnprintf(buf, sizeof(buf), "%s/class/hwmon/%s", sysfs, hwmon_ent->d_name);
+		pmu_dir = open(buf, O_DIRECTORY);

tools/perf/util/hwmon_pmu.c line 791

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+		if (pmu_dir == -1) {
+			pr_debug("hwmon_pmu: not a directory: '%s/class/hwmon/%s'\n",
+				 sysfs, hwmon_ent->d_name);
+			continue;
+		}
+		name_fd = openat(pmu_dir, "name", O_RDONLY);
+		if (name_fd == -1) {
+			pr_debug("hwmon_pmu: failure to open '%s/class/hwmon/%s/name'\n",
+				  sysfs, hwmon_ent->d_name);
+			continue;

close(pmu_dir)?

Quoted 62 lines
tools/perf/util/hwmon_pmu.c new file +62 -0
+		}
+		io__init(&io, name_fd, buf, sizeof(buf));
+		io__getline(&io, &line, &line_len);
+		if (line_len > 0 && line[line_len - 1] == '\n')
+			line[line_len - 1] = '\0';
+		hwmon_pmu__new(pmus, pmu_dir, hwmon_ent->d_name, line);
+		close(name_fd);
+	}
+	free(line);
+	closedir(hwmon_dir);
+	return 0;
+}
+
+#define FD(e, x, y) (*(int *)xyarray__entry(e->core.fd, x, y))
+
+int evsel__hwmon_pmu_open(struct evsel *evsel,
+			 struct perf_thread_map *threads,
+			 int start_cpu_map_idx, int end_cpu_map_idx)
+{
+	struct hwmon_pmu *hwmon_pmu = container_of(evsel->pmu, struct hwmon_pmu, pmu);
+	union hwmon_pmu_event_key key = {
+		.type_and_num = evsel->core.attr.config,
+	};
+	int idx = 0, thread = 0, nthreads, err = 0;
+
+	nthreads = perf_thread_map__nr(threads);
+	for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
+		for (thread = 0; thread < nthreads; thread++) {
+			char buf[64];
+			int fd;
+
+			snprintf(buf, sizeof(buf), "%s%d_input",
+				 hwmon_type_strs[key.type], key.num);
+
+			fd = openat(hwmon_pmu->pmu_dir_fd, buf, O_RDONLY);
+			FD(evsel, idx, thread) = fd;
+			if (fd < 0) {
+				err = -errno;
+				goto out_close;
+			}
+		}
+	}
+	return 0;
+out_close:
+	if (err)
+		threads->err_thread = thread;
+
+	do {
+		while (--thread >= 0) {
+			if (FD(evsel, idx, thread) >= 0)
+				close(FD(evsel, idx, thread));
+			FD(evsel, idx, thread) = -1;
+		}
+		thread = nthreads;
+	} while (--idx >= 0);
+	return err;
+}
+
+int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread)
+{
+	char buf[32];
+	int fd;

tools/perf/util/hwmon_pmu.c line 863

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+	ssize_t len;
+	struct perf_counts_values *count, *old_count = NULL;
+
+	if (evsel->prev_raw_counts)
+		old_count = perf_counts(evsel->prev_raw_counts, cpu_map_idx, thread);
+
+	count = perf_counts(evsel->counts, cpu_map_idx, thread);
+	fd = FD(evsel, cpu_map_idx, thread);
+	lseek(fd, SEEK_SET, 0);
+	len = read(fd, buf, sizeof(buf));

pread() instead?

tools/perf/util/hwmon_pmu.c line 872

Quoted 9 lines
tools/perf/util/hwmon_pmu.c new file +9 -0
+	if (len <= 0) {
+		count->lost++;
+		return -EINVAL;
+	}
+	buf[len] = '\0';
+	if (old_count) {
+		count->val = old_count->val + strtoll(buf, NULL, 10);
+		count->run = old_count->run + 1;
+		count->ena = old_count->ena + 1;

I'm not sure if it's meaningful to use the old value for hwmon. Oh.. is it just to make interval logic work correctly?

Thanks,
Namhyung

Quoted 142 lines
tools/perf/util/hwmon_pmu.c new file +7 -0
+	} else {
+		count->val = strtoll(buf, NULL, 10);
+		count->run++;
+		count->ena++;
+	}
+	return 0;
+}
tools/perf/util/hwmon_pmu.h new file +30 -0
new file mode 100644
index 000000000000..49e1683d5bce
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __HWMON_PMU_H
+#define __HWMON_PMU_H
+
+#include "pmu.h"
+
+struct list_head;
+
+bool perf_pmu__is_hwmon(const struct perf_pmu *pmu);
+bool evsel__is_hwmon(const struct evsel *evsel);
+
+int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+size_t hwmon_pmu__num_events(struct perf_pmu *pmu);
+bool hwmon_pmu__have_event(struct perf_pmu *pmu, const char *name);
+int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
+			    struct perf_event_attr *attr,
+			    struct parse_events_terms *terms,
+			    struct parse_events_error *err);
+int hwmon_pmu__check_alias(struct parse_events_terms *terms, struct perf_pmu_info *info,
+			   struct parse_events_error *err);
+
+int perf_pmus__read_hwmon_pmus(struct list_head *pmus);
+
+
+int evsel__hwmon_pmu_open(struct evsel *evsel,
+			 struct perf_thread_map *threads,
+			 int start_cpu_map_idx, int end_cpu_map_idx);
+int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread);
+
+#endif /* __HWMON_PMU_H */
tools/perf/util/pmu.c +17 -0
index 5ff6556292fd..c31d3409c787 100644
@@ -18,6 +18,7 @@
 #include "debug.h"
 #include "evsel.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "pmus.h"
 #include "tool_pmu.h"
 #include <util/pmu-bison.h>
@@ -1529,6 +1530,9 @@ int perf_pmu__config_terms(const struct perf_pmu *pmu,
 {
 	struct parse_events_term *term;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__config_terms(pmu, attr, terms, err);
+
 	list_for_each_entry(term, &terms->terms, list) {
 		if (pmu_config_term(pmu, attr, term, terms, zero, apply_hardcoded, err))
 			return -EINVAL;
@@ -1661,6 +1665,11 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
 	info->scale    = 0.0;
 	info->snapshot = false;
 
+	if (perf_pmu__is_hwmon(pmu)) {
+		ret = hwmon_pmu__check_alias(head_terms, info, err);
+		goto out;
+	}
+
 	/* Fake PMU doesn't rewrite terms. */
 	if (perf_pmu__is_fake(pmu))
 		goto out;
@@ -1830,6 +1839,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
 		return false;
 	if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(name))
 		return false;
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__have_event(pmu, name);
 	if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
 		return true;
 	if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1841,6 +1852,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
 	size_t nr;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__num_events(pmu);
+
 	pmu_aliases_parse(pmu);
 	nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1904,6 +1918,9 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 	int ret = 0;
 	struct strbuf sb;
 
+	if (perf_pmu__is_hwmon(pmu))
+		return hwmon_pmu__for_each_event(pmu, state, cb);
+
 	strbuf_init(&sb, /*hint=*/ 0);
 	pmu_aliases_parse(pmu);
 	pmu_add_cpu_aliases(pmu);
tools/perf/util/pmu.h +2 -0
index c4ca359d4215..32f95a1060d8 100644
@@ -37,6 +37,8 @@ struct perf_pmu_caps {
 };
 
 enum {
+	PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
+	PERF_PMU_TYPE_HWMON_END   = 0xFFFFFFFD,
 	PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
 	PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
 };
tools/perf/util/pmus.c +2 -0
index 19673b9991c6..4e3b19a2db00 100644
@@ -15,6 +15,7 @@
 #include "evsel.h"
 #include "pmus.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "print-events.h"
 #include "strbuf.h"
@@ -234,6 +235,7 @@ static void pmu_read_sysfs(bool core_only)
 	if (!core_only) {
 		tool_pmu = perf_pmus__tool_pmu();
 		list_add_tail(&tool_pmu->list, &other_pmus);
+		perf_pmus__read_hwmon_pmus(&other_pmus);
 	}
 	list_sort(NULL, &other_pmus, pmus_cmp);
 	if (!list_empty(&core_pmus)) {
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#23 lore raw

Re: [PATCH v1 14/15] perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs

Quoted 166 lines · Namhyung Kim
Quoted 7 lines · Ian Rogers

The hwmon sysfs ABI is defined in Documentation/hwmon/sysfs-interface.rst. Create a PMU that reads the hwmon input and can be used in perf stat and metrics much as an uncore PMU can.

For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

Quoted 10 lines
```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
     1.001153138              52.00 'C   temp_cpu
     1.001153138              2,588 rpm  fan1
     1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
     1.001153138                  8      tool/num_cpus_online/
     1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
     1.001153138      1,012,773,595      duration_time
...
```

Interesting! While they don't seem to be counters, it'd be useful to see the values from various sources/PMUs especially for interval mode.

Quoted 143 lines

The PMUs are named from /sys/class/hwmon/hwmon<num>/name and have an alias of hwmon<num>. The events are naned using the _label files as well as the <type><num> prefix, the latter guaranteed to be unique.

In perf list the other hwmon files are used to give a richer description, for example:

```
hwmon:
  temp1
       [Temperature in unit acpitz named temp1. Unit: hwmon_acpitz]
  in0
       [Voltage in unit bat0 named in0. Unit: hwmon_bat0]
  temp_core_0 OR temp2
       [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
  temp_core_1 OR temp3
       [Temperature in unit coretemp named Core 1. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
...
  temp_package_id_0 OR temp1
       [Temperature in unit coretemp named Package id 0. crit=100'C,max=100'C crit_alarm=0'C.
        Unit: hwmon_coretemp]
  temp1
       [Temperature in unit iwlwifi_1 named temp1. Unit: hwmon_iwlwifi_1]
  temp_composite OR temp1
       [Temperature in unit nvme named Composite. alarm=0'C,crit=86.85'C,max=75.85'C,
        min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_1 OR temp2
       [Temperature in unit nvme named Sensor 1. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  temp_sensor_2 OR temp3
       [Temperature in unit nvme named Sensor 2. max=65261.8'C,min=-273.15'C. Unit: hwmon_nvme]
  fan1
       [Fan in unit thinkpad named fan1. Unit: hwmon_thinkpad]
  fan2
       [Fan in unit thinkpad named fan2. Unit: hwmon_thinkpad]
...
  temp_cpu OR temp1
       [Temperature in unit thinkpad named CPU. Unit: hwmon_thinkpad]
  temp_gpu OR temp2
       [Temperature in unit thinkpad named GPU. Unit: hwmon_thinkpad]
  curr1
       [Current in unit ucsi_source_psy_usbc000_0 named curr1. max=1.5A. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
  in0
       [Voltage in unit ucsi_source_psy_usbc000_0 named in0. max=5V,min=5V. Unit:
        hwmon_ucsi_source_psy_usbc000_0]
```

As there may be multiple hwmon devices a range of PMU types are reserved for their use and to identify the PMU as belonging to the hwmon types.

---
 tools/perf/util/Build       |   1 +
 tools/perf/util/evsel.c     |   9 +
 tools/perf/util/hwmon_pmu.c | 879 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/hwmon_pmu.h |  30 ++
 tools/perf/util/pmu.c       |  17 +
 tools/perf/util/pmu.h       |   2 +
 tools/perf/util/pmus.c      |   2 +
 7 files changed, 940 insertions(+)
 create mode 100644 tools/perf/util/hwmon_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.h
tools/perf/util/Build +1 -0
index 80187e3a52be..b1dd5d176d1c 100644
@@ -83,6 +83,7 @@ perf-util-y += pmu.o
 perf-util-y += pmus.o
 perf-util-y += pmu-flex.o
 perf-util-y += pmu-bison.o
+perf-util-y += hwmon_pmu.o
 perf-util-y += tool_pmu.o
 perf-util-y += svghelper.o
 perf-util-$(CONFIG_LIBTRACEEVENT) += trace-event-info.o
tools/perf/util/evsel.c +9 -0
index 9e748ed20988..64883d2aa1bb 100644
@@ -50,6 +50,7 @@
 #include "off_cpu.h"
 #include "pmu.h"
 #include "pmus.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "rlimit.h"
 #include "../perf-sys.h"
@@ -1657,6 +1658,9 @@ int evsel__read_counter(struct evsel *evsel, int cpu_map_idx, int thread)
      if (evsel__is_tool(evsel))
              return evsel__tool_pmu_read(evsel, cpu_map_idx, thread);
 
+     if (evsel__is_hwmon(evsel))
+             return evsel__hwmon_pmu_read(evsel, cpu_map_idx, thread);
+
      if (evsel__is_retire_lat(evsel))
              return evsel__read_retire_lat(evsel, cpu_map_idx, thread);
 
@@ -2094,6 +2098,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
                                          start_cpu_map_idx,
                                          end_cpu_map_idx);
      }
+     if (evsel__is_hwmon(evsel)) {
+             return evsel__hwmon_pmu_open(evsel, threads,
+                                          start_cpu_map_idx,
+                                          end_cpu_map_idx);
+     }
 
      for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
 
tools/perf/util/hwmon_pmu.c new file +25 -0
new file mode 100644
index 000000000000..cc8816b787ca
@@ -0,0 +1,879 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+#include "counts.h"
+#include "debug.h"
+#include "evsel.h"
+#include "hashmap.h"
+#include "hwmon_pmu.h"
+#include "pmu.h"
+#include <internal/xyarray.h>
+#include <internal/threadmap.h>
+#include <perf/threadmap.h>
+#include <sys/types.h>
+#include <ctype.h>
+#include <dirent.h>
+#include <fcntl.h>
+#include <api/fs/fs.h>
+#include <api/io.h>
+#include <linux/zalloc.h>
+
+enum hwmon_type {
+     HWMON_TYPE_NONE,
+
+     HWMON_TYPE_CPU,
+     HWMON_TYPE_CURR,
+     HWMON_TYPE_ENERGY,
+     HWMON_TYPE_FAN,
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 31

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+     HWMON_TYPE_HUMIDITY,
+     HWMON_TYPE_IN,
+     HWMON_TYPE_INTRUSION,
+     HWMON_TYPE_POWER,
+     HWMON_TYPE_PWM,
+     HWMON_TYPE_TEMP,

Where does this list come from? Is it exhaustive?

The list incorporates everything in:
Documentation/hwmon/sysfs-interface.rst
It's exhaustive in that it covers everything that is documented. It can't cover future expansion and undocumented types. There are debug messages so hopefully we can catch these and remedy the situation. Originally I was looking into a parse-events.[ylc] style parser but it was overkill. The lists fell out from creating the lists of tokens. I'll add some comments.

Quoted 141 lines
Quoted 141 lines
tools/perf/util/hwmon_pmu.c new file +141 -0
+
+     HWMON_TYPE_MAX
+};
+
+static const char * const hwmon_type_strs[HWMON_TYPE_MAX] = {
+     NULL,
+     "cpu",
+     "curr",
+     "energy",
+     "fan",
+     "humidity",
+     "in",
+     "intrusion",
+     "power",
+     "pwm",
+     "temp",
+};
+
+static const char *const hwmon_units[HWMON_TYPE_MAX] = {
+     NULL,
+     "V",   /* cpu */
+     "A",   /* curr */
+     "J",   /* energy */
+     "rpm", /* fan */
+     "%",   /* humidity */
+     "V",   /* in */
+     "",    /* intrusion */
+     "W",   /* power */
+     "Hz",  /* pwm */
+     "'C",  /* temp */
+};
+
+enum hwmon_item {
+     HWMON_ITEM_NONE,
+
+     HWMON_ITEM_ACCURACY,
+     HWMON_ITEM_ALARM,
+     HWMON_ITEM_AUTO_CHANNELS_TEMP,
+     HWMON_ITEM_AVERAGE,
+     HWMON_ITEM_AVERAGE_HIGHEST,
+     HWMON_ITEM_AVERAGE_INTERVAL,
+     HWMON_ITEM_AVERAGE_INTERVAL_MAX,
+     HWMON_ITEM_AVERAGE_INTERVAL_MIN,
+     HWMON_ITEM_AVERAGE_LOWEST,
+     HWMON_ITEM_AVERAGE_MAX,
+     HWMON_ITEM_AVERAGE_MIN,
+     HWMON_ITEM_BEEP,
+     HWMON_ITEM_CAP,
+     HWMON_ITEM_CAP_HYST,
+     HWMON_ITEM_CAP_MAX,
+     HWMON_ITEM_CAP_MIN,
+     HWMON_ITEM_CRIT,
+     HWMON_ITEM_CRIT_HYST,
+     HWMON_ITEM_DIV,
+     HWMON_ITEM_EMERGENCY,
+     HWMON_ITEM_EMERGENCY_HIST,
+     HWMON_ITEM_ENABLE,
+     HWMON_ITEM_FAULT,
+     HWMON_ITEM_FREQ,
+     HWMON_ITEM_HIGHEST,
+     HWMON_ITEM_INPUT,
+     HWMON_ITEM_LABEL,
+     HWMON_ITEM_LCRIT,
+     HWMON_ITEM_LCRIT_HYST,
+     HWMON_ITEM_LOWEST,
+     HWMON_ITEM_MAX,
+     HWMON_ITEM_MAX_HYST,
+     HWMON_ITEM_MIN,
+     HWMON_ITEM_MIN_HYST,
+     HWMON_ITEM_MOD,
+     HWMON_ITEM_OFFSET,
+     HWMON_ITEM_PULSES,
+     HWMON_ITEM_RATED_MAX,
+     HWMON_ITEM_RATED_MIN,
+     HWMON_ITEM_RESET_HISTORY,
+     HWMON_ITEM_TARGET,
+     HWMON_ITEM_TYPE,
+     HWMON_ITEM_VID,
+
+     HWMON_ITEM__MAX,
+};
+
+static const char * const hwmon_item_strs[HWMON_ITEM__MAX] = {
+     NULL,
+     "accuracy",
+     "alarm",
+     "auto_channels_temp",
+     "average",
+     "average_highest",
+     "average_interval",
+     "average_interval_max",
+     "average_interval_min",
+     "average_lowest",
+     "average_max",
+     "average_min",
+     "beep",
+     "cap",
+     "cap_hyst",
+     "cap_max",
+     "cap_min",
+     "crit",
+     "crit_hyst",
+     "div",
+     "emergency",
+     "emergency_hist",
+     "enable",
+     "fault",
+     "freq",
+     "highest",
+     "input",
+     "label",
+     "lcrit",
+     "lcrit_hyst",
+     "lowest",
+     "max",
+     "max_hyst",
+     "min",
+     "min_hyst",
+     "mod",
+     "offset",
+     "pulses",
+     "rated_max",
+     "rated_min",
+     "reset_history",
+     "target",
+     "type",
+     "vid",
+};
+
+struct hwmon_pmu {
+     struct perf_pmu pmu;
+     struct hashmap events;
+     int pmu_dir_fd;
+};
+
+/**
+ * union hwmon_pmu_event_key: Key for hwmon_pmu->events as such each key
+ * represents an event.
+ *
+ * Related hwmon files start <type><number> that this key represents.
+ */
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 177

Quoted 5 lines
tools/perf/util/hwmon_pmu.c new file +5 -0
+union hwmon_pmu_event_key {
+     long type_and_num;
+     struct {
+             int num :16;
+             enum hwmon_type type :8;

Why not plain int types? I'm not sure how much we care about 32 bits but you could use short int then.

I've not seen the number be in the 20s so 16-bits is sufficient and as you point out avoids issues around 32-bit builds for i386, arm32 and the like. Switching to a short is more intention revealing so I'll do that.

Quoted 44 lines
Quoted 44 lines
tools/perf/util/hwmon_pmu.c new file +44 -0
+     };
+};
+
+/**
+ * struct hwmon_pmu_event_value: Value in hwmon_pmu->events.
+ *
+ * Hwmon files are of the form <type><number>_<item> and may have a suffix
+ * _alarm.
+ */
+struct hwmon_pmu_event_value {
+     /** @items: which item files are present. */
+     DECLARE_BITMAP(items, HWMON_ITEM__MAX);
+     /** @alarm_items: which item files are present. */
+     DECLARE_BITMAP(alarm_items, HWMON_ITEM__MAX);
+     /** @label: contents of <type><number>_label if present. */
+     char *label;
+     /** @name: name computed from label of the form <type>_<label>. */
+     char *name;
+};
+
+bool perf_pmu__is_hwmon(const struct perf_pmu *pmu)
+{
+     return pmu && pmu->type >= PERF_PMU_TYPE_HWMON_START &&
+             pmu->type <= PERF_PMU_TYPE_HWMON_END;
+}
+
+bool evsel__is_hwmon(const struct evsel *evsel)
+{
+     return perf_pmu__is_hwmon(evsel->pmu);
+}
+
+static size_t hwmon_pmu__event_hashmap_hash(long key, void *ctx __maybe_unused)
+{
+     return ((union hwmon_pmu_event_key)key).type_and_num;
+}
+
+static bool hwmon_pmu__event_hashmap_equal(long key1, long key2, void *ctx __maybe_unused)
+{
+     return ((union hwmon_pmu_event_key)key1).type_and_num ==
+            ((union hwmon_pmu_event_key)key2).type_and_num;
+}
+
+static int hwmon_strcmp(const void *a, const void *b)
+{
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 227

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+     const char *sa = a;
+     const char * const *sb = b;
+
+     return strcmp(sa, *sb);
+}
+

Can you please add some examples of typical filename in a comment here?

Good idea. This prompts me to think it'd be good to have tests for "fan0_input" maps to type "fan", number 0 and item "input". I'll move things around and add a test patch.

Quoted 6 lines
Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+static bool parse_hwmon_filename(const char *filename,
+                              enum hwmon_type *type,
+                              int *number,
+                              enum hwmon_item *item,
+                              bool *alarm)
+{
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 239

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+     char fn_type[24];
+     const char **elem;
+     const char *fn_item = NULL;
+     size_t fn_item_len;
+
+     assert(strlen("intrusion") < sizeof(fn_type));

Why intrusion?

It is the longest string in hwmon_type_strs, ie the assert is making sure the buffer is sized big enough to hold the biggest parse-able thing. Worth adding a comment.

Quoted 23 lines
Quoted 12 lines
tools/perf/util/hwmon_pmu.c new file +12 -0
+     strncpy(fn_type, filename, sizeof(fn_type));
+     for (size_t i = 0; i < sizeof(fn_type); i++) {
+             if (fn_type[i] >= '0' && fn_type[i] <= '9') {
+                     fn_type[i] = '\0';
+                     *number = strtoul(&filename[i], (char **)&fn_item, 10);
+                     if (*fn_item == '_')
+                             fn_item++;
+                     break;
+             }
+             if (fn_type[i] == '_') {
+                     fn_type[i] = '\0';
+                     *number = -1;

tools/perf/util/hwmon_pmu.c line 261

Quoted 10 lines
tools/perf/util/hwmon_pmu.c new file +10 -0
+                     fn_item = &filename[i + 1];
+                     break;
+             }
+     }
+     if (fn_item == NULL || fn_type[0] == '\0' || (item != NULL && fn_item[0] == '\0')) {
+             pr_debug("hwmon_pmu: not a hwmon file '%s'\n", filename);
+             return false;
+     }
+     elem = bsearch(&fn_type, hwmon_type_strs + 1, ARRAY_SIZE(hwmon_type_strs) - 1,
+                    sizeof(hwmon_type_strs[0]), hwmon_strcmp);
Quoted 2 lines

Can we make the first entry an empty string rather than NULL to make this simpler?

It would remove the +1 and -1 but both of those are to constants that I'd expect the compiler to find ways to fold away. So we'd be increasing the "n" feeding into the O(log n) of the binary search. It is a minimal thing but I'd prefer to avoid it.

Quoted 13 lines
Quoted 13 lines
tools/perf/util/hwmon_pmu.c new file +13 -0
+     if (!elem) {
+             pr_debug("hwmon_pmu: not a hwmon type '%s' in file name '%s'\n",
+                      fn_type, filename);
+             return false;
+     }
+
+     *type = elem - &hwmon_type_strs[0];
+     if (!item)
+             return true;
+
+     *alarm = false;
+     fn_item_len = strlen(fn_item);
+     if (fn_item_len > 6 && !strcmp(&fn_item[fn_item_len - 6], "_alarm")) {
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 279

Quoted 5 lines
tools/perf/util/hwmon_pmu.c new file +5 -0
+             assert(strlen("average_interval_max") < sizeof(fn_type));
+             strncpy(fn_type, fn_item, fn_item_len - 6);
+             fn_item = fn_type;
+             *alarm = true;
+     }

A comment to explain why it handles the "alarm" differently would be great.

Sure, but the short answer is that it has to give that _alarm is a potential suffix.

Quoted 22 lines
Quoted 22 lines
tools/perf/util/hwmon_pmu.c new file +22 -0
+     elem = bsearch(fn_item, hwmon_item_strs + 1, ARRAY_SIZE(hwmon_item_strs) - 1,
+                    sizeof(hwmon_item_strs[0]), hwmon_strcmp);
+     if (!elem) {
+             pr_debug("hwmon_pmu: not a hwmon item '%s' in file name '%s'\n",
+                      fn_item, filename);
+             return false;
+     }
+     *item = elem - &hwmon_item_strs[0];
+     return true;
+}
+
+static void fix_name(char *p)
+{
+     char *s = strchr(p, '\n');
+
+     if (s)
+             *s = '\0';
+
+     while (*p != '\0') {
+             if (strchr(" :,/\n\t", *p))
+                     *p = '_';
+             else
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 307

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+                     *p = tolower(*p);
+             p++;
+     }
+}
+
+static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)

Maybe better to use other short name ('hwm'?) than 'pmu'.

SGTM, I'll try to clean this up consistently.

Quoted 91 lines
Quoted 91 lines
tools/perf/util/hwmon_pmu.c new file +91 -0
+{
+     DIR *dir;
+     struct dirent *ent;
+     int dup_fd, err = 0;
+     struct hashmap_entry *cur, *tmp;
+     size_t bkt;
+
+     if (pmu->pmu.sysfs_aliases_loaded)
+             return 0;
+
+     /* Use a dup-ed fd as closedir will close it. */
+     dup_fd = dup(pmu->pmu_dir_fd);
+     if (dup_fd == -1)
+             return -ENOMEM;
+
+     dir = fdopendir(dup_fd);
+     if (!dir) {
+             close(dup_fd);
+             return -ENOMEM;
+     }
+
+     while ((ent = readdir(dir)) != NULL) {
+             enum hwmon_type type;
+             int number;
+             enum hwmon_item item;
+             bool alarm;
+             union hwmon_pmu_event_key key;
+             struct hwmon_pmu_event_value *value;
+
+             if (ent->d_type != DT_REG)
+                     continue;
+
+             if (!parse_hwmon_filename(ent->d_name, &type, &number, &item, &alarm)) {
+                     pr_debug("Not a hwmon file '%s'\n", ent->d_name);
+                     continue;
+             }
+             key.num = number;
+             key.type = type;
+             if (!hashmap__find(&pmu->events, key.type_and_num, &value)) {
+                     value = zalloc(sizeof(*value));
+                     if (!value) {
+                             err = -ENOMEM;
+                             goto err_out;
+                     }
+                     err = hashmap__add(&pmu->events, key.type_and_num, value);
+                     if (err) {
+                             free(value);
+                             err = -ENOMEM;
+                             goto err_out;
+                     }
+             }
+             __set_bit(item, alarm ? value->alarm_items : value->items);
+             if (item == HWMON_ITEM_LABEL) {
+                     char buf[128];
+                     int fd = openat(pmu->pmu_dir_fd, ent->d_name, O_RDONLY);
+                     ssize_t read_len;
+
+                     if (fd < 0)
+                             continue;
+
+                     read_len = read(fd, buf, sizeof(buf));
+
+                     while (read_len > 0 && buf[read_len - 1] == '\n')
+                             read_len--;
+
+                     if (read_len > 0)
+                             buf[read_len] = '\0';
+
+                     if (buf[0] == '\0') {
+                             pr_debug("hwmon_pmu: empty label file %s %s\n",
+                                      pmu->pmu.name, ent->d_name);
+                             close(fd);
+                             continue;
+                     }
+                     value->label = strdup(buf);
+                     if (!value->label) {
+                             pr_debug("hwmon_pmu: memory allocation failure\n");
+                             close(fd);
+                             continue;
+                     }
+                     snprintf(buf, sizeof(buf), "%s_%s", hwmon_type_strs[type], value->label);
+                     fix_name(buf);
+                     value->name = strdup(buf);
+                     if (!value->name)
+                             pr_debug("hwmon_pmu: memory allocation failure\n");
+                     close(fd);
+             }
+     }
+     hashmap__for_each_entry_safe((&pmu->events), cur, tmp, bkt) {
+             union hwmon_pmu_event_key key = {
+                     .type_and_num = cur->key,
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 404

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+             };
+             struct hwmon_pmu_event_value *value = cur->pvalue;
+
+             if (!test_bit(HWMON_ITEM_INPUT, value->items)) {
+                     pr_debug("hwmon_pmu: removing event '%s%d' that has no input file",
+                             hwmon_type_strs[key.type], key.num);

What is the input file and why is it required?

The input file is the one that actually tells you the fan speed, incoming voltage, etc. If you don't have one there is nothing to read. There are other files like max and average that we could potentially read but I'm ignoring that for now (those values will appear in perf-list though).

Quoted 15 lines
Quoted 15 lines
tools/perf/util/hwmon_pmu.c new file +15 -0
+                     hashmap__delete(&pmu->events, key.type_and_num, &key, &value);
+                     zfree(&value->label);
+                     zfree(&value->name);
+                     free(value);
+             }
+     }
+     pmu->pmu.sysfs_aliases_loaded = true;
+
+err_out:
+     closedir(dir);
+     return err;
+}
+
+static int hwmon_pmu__new(struct list_head *pmus, int pmu_dir, const char *sysfs_name,
+                       const char *name)
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 425

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+{
+     char buf[32];
+     struct hwmon_pmu *pmu;
+     int err = 0;
+
+     pr_debug("Hi Ian, '%s' -> '%s'\n", sysfs_name, name);

Forgot to remove? :)

Whoops :-)

Quoted 319 lines
Quoted 319 lines
tools/perf/util/hwmon_pmu.c new file +319 -0
+
+     pmu = zalloc(sizeof(*pmu));
+     if (!pmu)
+             return -ENOMEM;
+
+
+     pmu->pmu_dir_fd = pmu_dir;
+     pmu->pmu.type = PERF_PMU_TYPE_HWMON_START + strtoul(sysfs_name + 5, NULL, 10);
+     if (pmu->pmu.type > PERF_PMU_TYPE_HWMON_END) {
+             err = -EINVAL;
+             goto err_out;
+     }
+     snprintf(buf, sizeof(buf), "hwmon_%s", name);
+     fix_name(buf + 6);
+     pmu->pmu.name = strdup(buf);
+     if (!pmu->pmu.name) {
+             err = -ENOMEM;
+             goto err_out;
+     }
+     pmu->pmu.alias_name = strdup(sysfs_name);
+     if (!pmu->pmu.alias_name) {
+             err = -ENOMEM;
+             goto err_out;
+     }
+     pmu->pmu.cpus = perf_cpu_map__new("0");
+     if (!pmu->pmu.cpus) {
+             err = -ENOMEM;
+             goto err_out;
+     }
+     INIT_LIST_HEAD(&pmu->pmu.format);
+     INIT_LIST_HEAD(&pmu->pmu.aliases);
+     INIT_LIST_HEAD(&pmu->pmu.caps);
+     hashmap__init(&pmu->events, hwmon_pmu__event_hashmap_hash,
+                   hwmon_pmu__event_hashmap_equal, /*ctx=*/NULL);
+
+     list_add_tail(&pmu->pmu.list, pmus);
+err_out:
+     if (err) {
+             free((char *)pmu->pmu.name);
+             free(pmu->pmu.alias_name);
+             free(pmu);
+             close(pmu_dir);
+     }
+     return err;
+}
+
+static size_t hwmon_pmu__describe_items(struct hwmon_pmu *pmu, char *out_buf, size_t out_buf_len,
+                                     union hwmon_pmu_event_key key,
+                                     const unsigned long *items, bool is_alarm)
+{
+     size_t bit;
+     char buf[64];
+     size_t len = 0;
+
+     for_each_set_bit(bit, items, HWMON_ITEM__MAX) {
+             int fd;
+
+             if (bit == HWMON_ITEM_LABEL || bit == HWMON_ITEM_INPUT)
+                     continue;
+
+             snprintf(buf, sizeof(buf), "%s%d_%s%s",
+                     hwmon_type_strs[key.type],
+                     key.num,
+                     hwmon_item_strs[bit],
+                     is_alarm ? "_alarm" : "");
+             fd = openat(pmu->pmu_dir_fd, buf, O_RDONLY);
+             if (fd > 0) {
+                     ssize_t read_len = read(fd, buf, sizeof(buf));
+
+                     while (read_len > 0 && buf[read_len - 1] == '\n')
+                             read_len--;
+
+                     if (read_len > 0) {
+                             long long val;
+
+                             buf[read_len] = '\0';
+                             val = strtoll(buf, /*endptr=*/NULL, 10);
+                             len += snprintf(out_buf + len, out_buf_len - len, "%s%s%s=%g%s",
+                                             len == 0 ? " " : ", ",
+                                             hwmon_item_strs[bit],
+                                             is_alarm ? "_alarm" : "",
+                                             (double)val / 1000.0,
+                                             hwmon_units[key.type]);
+                     }
+                     close(fd);
+             }
+     }
+     return len;
+}
+
+int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb)
+{
+     struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+     struct hashmap_entry *cur;
+     size_t bkt;
+
+     if (hwmon_pmu__read_events(hwmon_pmu))
+             return false;
+
+     hashmap__for_each_entry((&hwmon_pmu->events), cur, bkt) {
+             static const char *const hwmon_scale_units[HWMON_TYPE_MAX] = {
+                     NULL,
+                     "0.001V", /* cpu */
+                     "0.001A", /* curr */
+                     "0.001J", /* energy */
+                     "1rpm",   /* fan */
+                     "0.001%", /* humidity */
+                     "0.001V", /* in */
+                     NULL,     /* intrusion */
+                     "0.001W", /* power */
+                     "1Hz",    /* pwm */
+                     "0.001'C", /* temp */
+             };
+             static const char *const hwmon_desc[HWMON_TYPE_MAX] = {
+                     NULL,
+                     "CPU core reference voltage",   /* cpu */
+                     "Current",                      /* curr */
+                     "Cumulative energy use",        /* energy */
+                     "Fan",                          /* fan */
+                     "Humidity",                     /* humidity */
+                     "Voltage",                      /* in */
+                     "Chassis intrusion detection",  /* intrusion */
+                     "Power use",                    /* power */
+                     "Pulse width modulation fan control", /* pwm */
+                     "Temperature",                  /* temp */
+             };
+             char alias_buf[64];
+             char desc_buf[256];
+             char encoding_buf[128];
+             union hwmon_pmu_event_key key = {
+                     .type_and_num = cur->key,
+             };
+             struct hwmon_pmu_event_value *value = cur->pvalue;
+             struct pmu_event_info info = {
+                     .pmu = pmu,
+                     .name = value->name,
+                     .alias = alias_buf,
+                     .scale_unit = hwmon_scale_units[key.type],
+                     .desc = desc_buf,
+                     .long_desc = NULL,
+                     .encoding_desc = encoding_buf,
+                     .topic = "hwmon",
+                     .pmu_name = pmu->name,
+                     .event_type_desc = "Hwmon event",
+             };
+             int ret;
+             size_t len;
+
+             len = snprintf(alias_buf, sizeof(alias_buf), "%s%d",
+                            hwmon_type_strs[key.type], key.num);
+             if (!info.name) {
+                     info.name = info.alias;
+                     info.alias = NULL;
+             }
+
+             len = snprintf(desc_buf, sizeof(desc_buf), "%s in unit %s named %s.",
+                     hwmon_desc[key.type],
+                     pmu->name + 6,
+                     value->label ?: info.name);
+
+             len += hwmon_pmu__describe_items(hwmon_pmu, desc_buf + len, sizeof(desc_buf) - len,
+                                             key, value->items, /*is_alarm=*/false);
+
+             len += hwmon_pmu__describe_items(hwmon_pmu, desc_buf + len, sizeof(desc_buf) - len,
+                                             key, value->alarm_items, /*is_alarm=*/true);
+
+             snprintf(encoding_buf, sizeof(encoding_buf), "%s/config=0x%lx/",
+                      pmu->name, cur->key);
+
+             ret = cb(state, &info);
+             if (ret)
+                     return ret;
+     }
+     return 0;
+}
+
+size_t hwmon_pmu__num_events(struct perf_pmu *pmu)
+{
+     struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+
+     hwmon_pmu__read_events(hwmon_pmu);
+     return hashmap__size(&hwmon_pmu->events);
+}
+
+bool hwmon_pmu__have_event(struct perf_pmu *pmu, const char *name)
+{
+     struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+     enum hwmon_type type;
+     int number;
+     union hwmon_pmu_event_key key;
+     struct hashmap_entry *cur;
+     size_t bkt;
+
+     if (!parse_hwmon_filename(name, &type, &number, /*item=*/NULL, /*is_alarm=*/NULL))
+             return false;
+
+     if (hwmon_pmu__read_events(hwmon_pmu))
+             return false;
+
+     key.type = type;
+     key.num = number;
+     if (hashmap_find(&hwmon_pmu->events, key.type_and_num, /*value=*/NULL))
+             return true;
+     if (key.num != -1)
+             return false;
+     /* Item is of form <type>_ which means we should match <type>_<label>. */
+     hashmap__for_each_entry((&hwmon_pmu->events), cur, bkt) {
+             struct hwmon_pmu_event_value *value = cur->pvalue;
+
+             key.type_and_num = cur->key;
+             if (key.type == type && value->name && !strcasecmp(name, value->name))
+                     return true;
+     }
+     return false;
+}
+
+static int hwmon_pmu__config_term(const struct hwmon_pmu *pmu,
+                               struct perf_event_attr *attr,
+                               struct parse_events_term *term,
+                               struct parse_events_error *err)
+{
+     if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
+             enum hwmon_type type;
+             int number;
+
+             if (parse_hwmon_filename(term->config, &type, &number,
+                                      /*item=*/NULL, /*is_alarm=*/NULL)) {
+                     if (number == -1) {
+                             /*
+                              * Item is of form <type>_ which means we should
+                              * match <type>_<label>.
+                              */
+                             struct hashmap_entry *cur;
+                             size_t bkt;
+
+                             attr->config = 0;
+                             hashmap__for_each_entry((&pmu->events), cur, bkt) {
+                                     union hwmon_pmu_event_key key = {
+                                             .type_and_num = cur->key,
+                                     };
+                                     struct hwmon_pmu_event_value *value = cur->pvalue;
+
+                                     if (key.type == type && value->name &&
+                                         !strcasecmp(term->config, value->name)) {
+                                             attr->config = key.type_and_num;
+                                             break;
+                                     }
+                             }
+                             if (attr->config == 0)
+                                     return -EINVAL;
+                     } else {
+                             union hwmon_pmu_event_key key = {
+                                     .type = type,
+                                     .num = number,
+                             };
+
+                             attr->config = key.type_and_num;
+                     }
+                     return 0;
+             }
+     }
+     if (err) {
+             char *err_str;
+
+             parse_events_error__handle(err, term->err_val,
+                                     asprintf(&err_str,
+                                             "unexpected hwmon event term (%s) %s",
+                                             parse_events__term_type_str(term->type_term),
+                                             term->config) < 0
+                                     ? strdup("unexpected hwmon event term")
+                                     : err_str,
+                                     NULL);
+     }
+     return -EINVAL;
+}
+
+int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
+                         struct perf_event_attr *attr,
+                         struct parse_events_terms *terms,
+                         struct parse_events_error *err)
+{
+     const struct hwmon_pmu *hwmon_pmu = container_of(pmu, struct hwmon_pmu, pmu);
+     struct parse_events_term *term;
+
+     assert(pmu->sysfs_aliases_loaded);
+     list_for_each_entry(term, &terms->terms, list) {
+             if (hwmon_pmu__config_term(hwmon_pmu, attr, term, err))
+                     return -EINVAL;
+     }
+
+     return 0;
+
+}
+
+int hwmon_pmu__check_alias(struct parse_events_terms *terms, struct perf_pmu_info *info,
+                        struct parse_events_error *err)
+{
+     struct parse_events_term *term =
+             list_first_entry(&terms->terms, struct parse_events_term, list);
+
+     if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
+             enum hwmon_type type;
+             int number;
+
+             if (parse_hwmon_filename(term->config, &type, &number,
+                                      /*item=*/NULL, /*is_alarm=*/NULL)) {
+                     info->unit = hwmon_units[type];
+                     if (type == HWMON_TYPE_FAN || type == HWMON_TYPE_PWM ||
+                         type == HWMON_TYPE_INTRUSION)
+                             info->scale = 1;
+                     else
+                             info->scale = 0.001;
+             }
+             return 0;
+     }
+     if (err) {
+             char *err_str;
+
+             parse_events_error__handle(err, term->err_val,
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 696

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+                                     asprintf(&err_str,
+                                             "unexpected hwmon event term (%s) %s",
+                                             parse_events__term_type_str(term->type_term),
+                                             term->config) < 0
+                                     ? strdup("unexpected hwmon event term")
+                                     : err_str,

Probably better to move the asprintf() out to a statement.

Okay, it a c&p from parse-events.c so I'll need to add in a refactor patch.

Quoted 36 lines
Quoted 36 lines
tools/perf/util/hwmon_pmu.c new file +35 -0
+                                     NULL);
+     }
+     return -EINVAL;
+}
+
+int perf_pmus__read_hwmon_pmus(struct list_head *pmus)
+{
+     char *line = NULL;
+     DIR *hwmon_dir;
+     struct dirent *hwmon_ent;
+     char buf[PATH_MAX];
+     const char *sysfs = sysfs__mountpoint();
+
+     if (!sysfs)
+             return 0;
+
+     scnprintf(buf, sizeof(buf), "%s/class/hwmon/", sysfs);
+     hwmon_dir = opendir(buf);
+     if (!hwmon_dir)
+             return 0;
+
+     while ((hwmon_ent = readdir(hwmon_dir)) != NULL) {
+             size_t line_len;
+             int pmu_dir, name_fd;
+             struct io io;
+
+             if (hwmon_ent->d_type != DT_LNK)
+                     continue;
+
+             scnprintf(buf, sizeof(buf), "%s/class/hwmon/%s", sysfs, hwmon_ent->d_name);
+             pmu_dir = open(buf, O_DIRECTORY);
+             if (pmu_dir == -1) {
+                     pr_debug("hwmon_pmu: not a directory: '%s/class/hwmon/%s'\n",
+                              sysfs, hwmon_ent->d_name);
+                     continue;
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 791

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+             }
+             name_fd = openat(pmu_dir, "name", O_RDONLY);
+             if (name_fd == -1) {
+                     pr_debug("hwmon_pmu: failure to open '%s/class/hwmon/%s/name'\n",
+                               sysfs, hwmon_ent->d_name);
+                     continue;

close(pmu_dir)?

Done.

Quoted 67 lines
Quoted 67 lines
tools/perf/util/hwmon_pmu.c new file +66 -0
+             }
+             io__init(&io, name_fd, buf, sizeof(buf));
+             io__getline(&io, &line, &line_len);
+             if (line_len > 0 && line[line_len - 1] == '\n')
+                     line[line_len - 1] = '\0';
+             hwmon_pmu__new(pmus, pmu_dir, hwmon_ent->d_name, line);
+             close(name_fd);
+     }
+     free(line);
+     closedir(hwmon_dir);
+     return 0;
+}
+
+#define FD(e, x, y) (*(int *)xyarray__entry(e->core.fd, x, y))
+
+int evsel__hwmon_pmu_open(struct evsel *evsel,
+                      struct perf_thread_map *threads,
+                      int start_cpu_map_idx, int end_cpu_map_idx)
+{
+     struct hwmon_pmu *hwmon_pmu = container_of(evsel->pmu, struct hwmon_pmu, pmu);
+     union hwmon_pmu_event_key key = {
+             .type_and_num = evsel->core.attr.config,
+     };
+     int idx = 0, thread = 0, nthreads, err = 0;
+
+     nthreads = perf_thread_map__nr(threads);
+     for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
+             for (thread = 0; thread < nthreads; thread++) {
+                     char buf[64];
+                     int fd;
+
+                     snprintf(buf, sizeof(buf), "%s%d_input",
+                              hwmon_type_strs[key.type], key.num);
+
+                     fd = openat(hwmon_pmu->pmu_dir_fd, buf, O_RDONLY);
+                     FD(evsel, idx, thread) = fd;
+                     if (fd < 0) {
+                             err = -errno;
+                             goto out_close;
+                     }
+             }
+     }
+     return 0;
+out_close:
+     if (err)
+             threads->err_thread = thread;
+
+     do {
+             while (--thread >= 0) {
+                     if (FD(evsel, idx, thread) >= 0)
+                             close(FD(evsel, idx, thread));
+                     FD(evsel, idx, thread) = -1;
+             }
+             thread = nthreads;
+     } while (--idx >= 0);
+     return err;
+}
+
+int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread)
+{
+     char buf[32];
+     int fd;
+     ssize_t len;
+     struct perf_counts_values *count, *old_count = NULL;
+
+     if (evsel->prev_raw_counts)
Quoted 8 lines

tools/perf/util/hwmon_pmu.c line 863

Quoted 6 lines
tools/perf/util/hwmon_pmu.c new file +6 -0
+             old_count = perf_counts(evsel->prev_raw_counts, cpu_map_idx, thread);
+
+     count = perf_counts(evsel->counts, cpu_map_idx, thread);
+     fd = FD(evsel, cpu_map_idx, thread);
+     lseek(fd, SEEK_SET, 0);
+     len = read(fd, buf, sizeof(buf));

pread() instead?

Done.

Quoted 10 lines

tools/perf/util/hwmon_pmu.c line 872

Quoted 9 lines
tools/perf/util/hwmon_pmu.c new file +9 -0
+     if (len <= 0) {
+             count->lost++;
+             return -EINVAL;
+     }
+     buf[len] = '\0';
+     if (old_count) {
+             count->val = old_count->val + strtoll(buf, NULL, 10);
+             count->run = old_count->run + 1;
+             count->ena = old_count->ena + 1;
Quoted 2 lines

I'm not sure if it's meaningful to use the old value for hwmon. Oh.. is it just to make interval logic work correctly?

Yep. Otherwise the interval logic sees values not moving or going backward and other bad things. I'm not a fan of this logic but I don't like interval mode not working more. The logic also needs to use old_count, not just count, otherwise 1 in 3 intervals break.

Thanks,
Ian

Quoted 146 lines

Thanks,
Namhyung

Quoted 142 lines
tools/perf/util/hwmon_pmu.c new file +7 -0
+     } else {
+             count->val = strtoll(buf, NULL, 10);
+             count->run++;
+             count->ena++;
+     }
+     return 0;
+}
tools/perf/util/hwmon_pmu.h new file +30 -0
new file mode 100644
index 000000000000..49e1683d5bce
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) */
+#ifndef __HWMON_PMU_H
+#define __HWMON_PMU_H
+
+#include "pmu.h"
+
+struct list_head;
+
+bool perf_pmu__is_hwmon(const struct perf_pmu *pmu);
+bool evsel__is_hwmon(const struct evsel *evsel);
+
+int hwmon_pmu__for_each_event(struct perf_pmu *pmu, void *state, pmu_event_callback cb);
+size_t hwmon_pmu__num_events(struct perf_pmu *pmu);
+bool hwmon_pmu__have_event(struct perf_pmu *pmu, const char *name);
+int hwmon_pmu__config_terms(const struct perf_pmu *pmu,
+                         struct perf_event_attr *attr,
+                         struct parse_events_terms *terms,
+                         struct parse_events_error *err);
+int hwmon_pmu__check_alias(struct parse_events_terms *terms, struct perf_pmu_info *info,
+                        struct parse_events_error *err);
+
+int perf_pmus__read_hwmon_pmus(struct list_head *pmus);
+
+
+int evsel__hwmon_pmu_open(struct evsel *evsel,
+                      struct perf_thread_map *threads,
+                      int start_cpu_map_idx, int end_cpu_map_idx);
+int evsel__hwmon_pmu_read(struct evsel *evsel, int cpu_map_idx, int thread);
+
+#endif /* __HWMON_PMU_H */
tools/perf/util/pmu.c +17 -0
index 5ff6556292fd..c31d3409c787 100644
@@ -18,6 +18,7 @@
 #include "debug.h"
 #include "evsel.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "pmus.h"
 #include "tool_pmu.h"
 #include <util/pmu-bison.h>
@@ -1529,6 +1530,9 @@ int perf_pmu__config_terms(const struct perf_pmu *pmu,
 {
      struct parse_events_term *term;
 
+     if (perf_pmu__is_hwmon(pmu))
+             return hwmon_pmu__config_terms(pmu, attr, terms, err);
+
      list_for_each_entry(term, &terms->terms, list) {
              if (pmu_config_term(pmu, attr, term, terms, zero, apply_hardcoded, err))
                      return -EINVAL;
@@ -1661,6 +1665,11 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_
      info->scale    = 0.0;
      info->snapshot = false;
 
+     if (perf_pmu__is_hwmon(pmu)) {
+             ret = hwmon_pmu__check_alias(head_terms, info, err);
+             goto out;
+     }
+
      /* Fake PMU doesn't rewrite terms. */
      if (perf_pmu__is_fake(pmu))
              goto out;
@@ -1830,6 +1839,8 @@ bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name)
              return false;
      if (perf_pmu__is_tool(pmu) && tool_pmu__skip_event(name))
              return false;
+     if (perf_pmu__is_hwmon(pmu))
+             return hwmon_pmu__have_event(pmu, name);
      if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL)
              return true;
      if (pmu->cpu_aliases_added || !pmu->events_table)
@@ -1841,6 +1852,9 @@ size_t perf_pmu__num_events(struct perf_pmu *pmu)
 {
      size_t nr;
 
+     if (perf_pmu__is_hwmon(pmu))
+             return hwmon_pmu__num_events(pmu);
+
      pmu_aliases_parse(pmu);
      nr = pmu->sysfs_aliases + pmu->sys_json_aliases;
 
@@ -1904,6 +1918,9 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
      int ret = 0;
      struct strbuf sb;
 
+     if (perf_pmu__is_hwmon(pmu))
+             return hwmon_pmu__for_each_event(pmu, state, cb);
+
      strbuf_init(&sb, /*hint=*/ 0);
      pmu_aliases_parse(pmu);
      pmu_add_cpu_aliases(pmu);
tools/perf/util/pmu.h +2 -0
index c4ca359d4215..32f95a1060d8 100644
@@ -37,6 +37,8 @@ struct perf_pmu_caps {
 };
 
 enum {
+     PERF_PMU_TYPE_HWMON_START = 0xFFFF0000,
+     PERF_PMU_TYPE_HWMON_END   = 0xFFFFFFFD,
      PERF_PMU_TYPE_TOOL = 0xFFFFFFFE,
      PERF_PMU_TYPE_FAKE = 0xFFFFFFFF,
 };
tools/perf/util/pmus.c +2 -1
index 19673b9991c6..4e3b19a2db00 100644
@@ -15,6 +15,7 @@
 #include "evsel.h"
 #include "pmus.h"
 #include "pmu.h"
+#include "hwmon_pmu.h"
 #include "tool_pmu.h"
 #include "print-events.h"
 #include "strbuf.h"
@@ -234,6 +235,7 @@ static void pmu_read_sysfs(bool core_only)
      if (!core_only) {
              tool_pmu = perf_pmus__tool_pmu();
              list_add_tail(&tool_pmu->list, &other_pmus);
+             perf_pmus__read_hwmon_pmus(&other_pmus);
      }
      list_sort(NULL, &other_pmus, pmus_cmp);
      if (!list_empty(&core_pmus)) {
--

2.46.0.469.g59c65b2a67-goog

author reviewer
#24 lore raw

PATCH 15/15 perf docs: Document tool and hwmon events

In reply to Ian Rogers

Add a few paragraphs on tool and hwmon events.

---
 tools/perf/Documentation/perf-list.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
tools/perf/Documentation/perf-list.txt +15 -0
index dea005410ec0..7e3cd6c5e35d 100644
@@ -243,6 +243,21 @@ For accessing trace point events perf needs to have read access to
 /sys/kernel/tracing, even when perf_event_paranoid is in a relaxed
 setting.
 
+TOOL/HWMON EVENTS
+-----------------
+
+Some events don't have an associated PMU instead reading values
+available to software without perf_event_open. As these events don't
+support sampling they can only really be read by tools like perf stat.
+
+Tool events provide times and certain system parameters. Examples
+include duration_time, user_time, system_time and num_cpus_online.
+
+Hwmon events provide easy access to hwmon sysfs data typically in
+/sys/class/hwmon. This information includes temperatures, fan speeds
+and energy usage.
+
+
 TRACING
 -------
 
Signature · 1 line
2.46.0.469.g59c65b2a67-goog
author reviewer
#25 lore raw
In reply to Ian Rogers
Quoted 13 lines · Ian Rogers

Rather than have fake and tool PMUs being special flags in an evsel, create special PMUs. This allows, for example, duration_time to also be tool/duration_time/. Once adding events to the tools PMU is just adding to an array, add events for nearly all the expr literals like num_cpus_online. Rather than create custom logic for finding and describing the tool events use json and add a notion of common json for the tool events.

Following the convention of the tool PMU, create a hwmon PMU that exposes hwmon data for reading. For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

Quoted 10 lines
```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
     1.001153138              52.00 'C   temp_cpu
     1.001153138              2,588 rpm  fan1
     1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
     1.001153138                  8      tool/num_cpus_online/
     1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
     1.001153138      1,012,773,595      duration_time
...
```

[email protected] as a heads up.

Thanks,
Ian

Quoted 78 lines

Additional data on the hwmon events is in perf list:

```
$ perf list
...
hwmon:
...
  temp_core_0 OR temp2
       [Temperature in unit coretemp named Core 0. crit=100'C,max=100'C crit_alarm=0'C. Unit:
        hwmon_coretemp]
...
```

Ian Rogers (15):
perf list: Avoid potential out of bounds memory read
perf pmus: Fake PMU clean up
perf evsel: Add accessor for tool_event
perf pmu: To info add event_type_desc
perf pmu: Allow hardcoded terms to be applied to attributes perf parse-events: Expose/rename config_term_name
perf tool_pmu: Factor tool events into their own PMU
perf tool_pmu: Rename enum perf_tool_event to tool_pmu_event perf tool_pmu: Rename perf_tool_event__* to tool_pmu__* perf tool_pmu: Move expr literals to tool_pmu
perf jevents: Add tool event json under a common architecture perf tool_pmu: Switch to standard pmu functions and json descriptions perf tests: Add tool PMU test
perf hwmon_pmu: Add a tool PMU exposing events from hwmon in sysfs perf docs: Document tool and hwmon events

 tools/perf/Documentation/perf-list.txt        |  15 +
 tools/perf/arch/arm64/util/pmu.c              |   5 +-
 tools/perf/arch/x86/util/intel-pt.c           |   3 +-
 tools/perf/arch/x86/util/tsc.c                |  16 +-
 tools/perf/builtin-list.c                     |  15 +-
 tools/perf/builtin-stat.c                     |   7 +-
 .../pmu-events/arch/common/common/tool.json   |  74 ++
 tools/perf/pmu-events/empty-pmu-events.c      | 208 +++--
 tools/perf/pmu-events/jevents.py              |  16 +-
 tools/perf/tests/Build                        |   1 +
 tools/perf/tests/builtin-test.c               |   1 +
 tools/perf/tests/parse-events.c               |   4 +-
 tools/perf/tests/pmu-events.c                 |  12 +-
 tools/perf/tests/pmu.c                        |   3 +-
 tools/perf/tests/tests.h                      |   1 +
 tools/perf/tests/tool_pmu.c                   | 111 +++
 tools/perf/util/Build                         |   2 +
 tools/perf/util/evsel.c                       | 287 +-----
 tools/perf/util/evsel.h                       |  23 +-
 tools/perf/util/expr.c                        |  93 +-
 tools/perf/util/hwmon_pmu.c                   | 879 ++++++++++++++++++
 tools/perf/util/hwmon_pmu.h                   |  30 +
 tools/perf/util/metricgroup.c                 |  45 +-
 tools/perf/util/parse-events.c                |  92 +-
 tools/perf/util/parse-events.h                |  13 +-
 tools/perf/util/parse-events.l                |  11 -
 tools/perf/util/parse-events.y                |  16 -
 tools/perf/util/pmu.c                         | 108 ++-
 tools/perf/util/pmu.h                         |  17 +-
 tools/perf/util/pmus.c                        |  25 +-
 tools/perf/util/pmus.h                        |   1 +
 tools/perf/util/print-events.c                |  36 +-
 tools/perf/util/print-events.h                |   1 -
 tools/perf/util/stat-display.c                |  14 +-
 tools/perf/util/stat-shadow.c                 |  24 +-
 tools/perf/util/tool_pmu.c                    | 508 ++++++++++
 tools/perf/util/tool_pmu.h                    |  56 ++
 tools/perf/util/tsc.h                         |   2 +-
 38 files changed, 2116 insertions(+), 659 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/common/common/tool.json
 create mode 100644 tools/perf/tests/tool_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.c
 create mode 100644 tools/perf/util/hwmon_pmu.h
 create mode 100644 tools/perf/util/tool_pmu.c
 create mode 100644 tools/perf/util/tool_pmu.h

--
2.46.0.469.g59c65b2a67-goog

maintainer
#26 lore raw
Quoted 25 lines · Ian Rogers
Quoted 13 lines · Ian Rogers

Rather than have fake and tool PMUs being special flags in an evsel, create special PMUs. This allows, for example, duration_time to also be tool/duration_time/. Once adding events to the tools PMU is just adding to an array, add events for nearly all the expr literals like num_cpus_online. Rather than create custom logic for finding and describing the tool events use json and add a notion of common json for the tool events.

Following the convention of the tool PMU, create a hwmon PMU that exposes hwmon data for reading. For example, the following shows reading the CPU temperature and 2 fan speeds alongside the uncore frequency:

Quoted 10 lines
```
$ perf stat -e temp_cpu,fan1,hwmon_thinkpad/fan2/,tool/num_cpus_online/ -M UNCORE_FREQ -I 1000
      1.001153138              52.00 'C   temp_cpu
      1.001153138              2,588 rpm  fan1
      1.001153138              2,482 rpm  hwmon_thinkpad/fan2/
      1.001153138                  8      tool/num_cpus_online/
      1.001153138      1,077,101,397      UNC_CLOCK.SOCKET                 #     1.08 UNCORE_FREQ
      1.001153138      1,012,773,595      duration_time
...
```
Quoted 2 lines

[email protected] as a heads up.

No idea what you are doing, or why. I am not sure if I care, unless you introduce dependencies on hwmon internals (or, in other words, define undocumented APIs into hwmon). Which I'd argue would be a no-go.

Guenter