perf alias: Remove trailing newline when reading sysfs files

Commit-ID:  0c24d6fb7bd3578e5b9e4972d01bbe3d087ded33
Gitweb:     https://git.kernel.org/tip/0c24d6fb7bd3578e5b9e4972d01bbe3d087ded33
Author:     Thomas Richter <[email protected]>
AuthorDate: Fri, 15 Jun 2018 12:11:04 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Jun 2018 11:59:37 -0300

perf alias: Rebuild alias expression string to make it comparable

PMU alias definitions in sysfs files may have spaces, newlines and numbers with leading zeroes. Some alias definitions may also appear in JSON files without spaces, etc.

Scan alias definitions and remove leading zeroes, spaces, newlines, etc and rebuild string to make alias->str member comparable.

s390 for example has terms specified as event=0x0091 (read from files ../<PMU>/events/<FILE> and terms specified as event=0x91 (read from JSON files).

---
 tools/perf/util/pmu.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

index 2738fc8d200d..f321ce97d9ec 100644
@@ -241,9 +241,11 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 				 char *metric_expr,
 				 char *metric_name)
 {
+	struct parse_events_term *term;
 	struct perf_pmu_alias *alias;
 	int ret;
 	int num;
+	char newval[256];
 
 	alias = malloc(sizeof(*alias));
 	if (!alias)

@@ -262,6 +264,27 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 		return ret;
 	}
 
+	/* Scan event and remove leading zeroes, spaces, newlines, some
+	 * platforms have terms specified as
+	 * event=0x0091 (read from files ../<PMU>/events/<FILE>
+	 * and terms specified as event=0x91 (read from JSON files).
+	 *
+	 * Rebuild string to make alias->str member comparable.
+	 */
+	memset(newval, 0, sizeof(newval));
+	ret = 0;
+	list_for_each_entry(term, &alias->terms, list) {
+		if (ret)
+			ret += scnprintf(newval + ret, sizeof(newval) - ret,
+					 ",");
+		if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
+			ret += scnprintf(newval + ret, sizeof(newval) - ret,
+					 "%s=%#x", term->config, term->val.num);
+		else if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR)
+			ret += scnprintf(newval + ret, sizeof(newval) - ret,
+					 "%s=%s", term->config, term->val.str);
+	}
+
 	alias->name = strdup(name);
 	if (dir) {
 		/*

@@ -285,7 +308,7 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 		snprintf(alias->unit, sizeof(alias->unit), "%s", unit);
 	}
 	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
-	alias->str = strdup(val);
+	alias->str = strdup(newval);
 
 	list_add_tail(&alias->list, list);
 

Commit-ID:  6dde6429c5ff5b38d6d40a14a6ee105117e6364d
Gitweb:     https://git.kernel.org/tip/6dde6429c5ff5b38d6d40a14a6ee105117e6364d
Author:     Thomas Richter <[email protected]>
AuthorDate: Fri, 15 Jun 2018 12:11:05 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Jun 2018 11:59:37 -0300

perf stat: Remove duplicate event counting

'perf stat' shows a mismatch in perf stat regarding counter names on s390:

Run command:

[root@s35lp76 perf]# ./perf stat -e tx_nc_tend -v --

                ~/mytesttx 1 >/tmp/111

tx_nc_tend: 1 573146 573146
tx_nc_tend: 1 573146 573146

Performance counter stats for '/root/mytesttx 1':

                 3      tx_nc_tend

       0.001037252 seconds time elapsed

[root@s35lp76 perf]#

shows transaction counter tx_nc_tend with value 3 but it was triggered only once as seen by the output of mytesttx.

When looking up the event name tx_nc_tend the following function sequence is called:

parse_events_multi_pmu_add()

+--> perf_pmu__scan() being called with NULL argument
     +--> pmu_read_sysfs() scans directory ../devices/ for

all PMUs
+--> perf_pmu__find() tries to find a PMU in the
global pmu list.
+--> pmu_lookup() called to read all file
entries when not in global list.

pmu_lookup() causes the issue. It calls
+---> pmu_aliases() to read all the entries in the PMU directory.
On s390 this is named /sys/devices/cpum_cf/events.
+--> pmu_aliases_parse() reads all files and creates an
alias for each file name.

So we end up with first entry created by reading the sysfs file [root@s35lp76 perf]# cat /sys/devices/cpum_cf
/events/TX_NC_TEND
event=0x008d
[root@s35lp76 perf]#

Debug output shows this entry tx_nc_tend -> 'cpum_cf'/'event=0x008d '/
After all files in this directory have been read and aliases created this function is called:
+--> pmu_add_cpu_aliases()
This function looks up the CPU tables created by the json files.
With json files for s390 now available all the aliases are added to
the PMU alias list a second time.
The second entry is added by
reading the json file converted by jevent resulting in file pmu-events/pmu-events.c:

{
.name = "tx_nc_tend",
.event = "event=0x8d",
.desc = "Unit: cpum_cf Completed TEND \
instructions \
in non-constrained TX mode",
.topic = "extended",
.long_desc = "A TEND instruction has \
completed in a \
non-constrained \
transactional-execution mode",
.pmu = "cpum_cf",
},

Debug output shows this entry tx_nc_tend -> 'cpum_cf'/'event=0x8d'/

Function pmu_aliases_parse() and pmu_add_cpu_aliases() both use __perf_pmu__new_alias() to add an alias to the PMU alias list. There is no check if an alias already exist

So we end up with 2 entries for tx_nc_tend in the PMU alias list.

Having set up the PMU alias list for this PMU now parse_events_multi_add_pmu() reads the complete alias list and adds each alias with parse_events_add_pmu() to the global perfev_list. This causes the alias to be added multiple times to the event list.

Fix this by making __perf_pmu__new_alias() to merge alias definitions if an alias is already on the alias list. Also print a debug message when the alias has mismatches in some fields.

Output before:

  [root@s35lp76 perf]# ./perf stat -e tx_nc_tend  -v \
                        -- ~/mytesttx 1 >/tmp/111
  tx_nc_tend: 1 551446 551446

Performance counter stats for '/root/mytesttx 1':

                   3      tx_nc_tend

         0.000961134 seconds time elapsed

[root@s35lp76 perf]#

Output after:

  [root@s35lp76 perf]#  ./perf stat -e tx_nc_tend  -v \
                        -- ~/mytesttx 1 >/tmp/111
  tx_nc_tend: 1 551446 551446

Performance counter stats for '/root/mytesttx 1':

                   1      tx_nc_tend

         0.000961134 seconds time elapsed

[root@s35lp76 perf]#

---
 tools/perf/util/pmu.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

index f321ce97d9ec..3ba6a1742f91 100644
@@ -234,6 +234,74 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias,
 	return 0;
 }
 
+static void perf_pmu_assign_str(char *name, const char *field, char **old_str,
+				char **new_str)
+{
+	if (!*old_str)
+		goto set_new;
+
+	if (*new_str) {	/* Have new string, check with old */
+		if (strcasecmp(*old_str, *new_str))
+			pr_debug("alias %s differs in field '%s'\n",
+				 name, field);
+		zfree(old_str);
+	} else		/* Nothing new --> keep old string */
+		return;
+set_new:
+	*old_str = *new_str;
+	*new_str = NULL;
+}
+
+static void perf_pmu_update_alias(struct perf_pmu_alias *old,
+				  struct perf_pmu_alias *newalias)
+{
+	perf_pmu_assign_str(old->name, "desc", &old->desc, &newalias->desc);
+	perf_pmu_assign_str(old->name, "long_desc", &old->long_desc,
+			    &newalias->long_desc);
+	perf_pmu_assign_str(old->name, "topic", &old->topic, &newalias->topic);
+	perf_pmu_assign_str(old->name, "metric_expr", &old->metric_expr,
+			    &newalias->metric_expr);
+	perf_pmu_assign_str(old->name, "metric_name", &old->metric_name,
+			    &newalias->metric_name);
+	perf_pmu_assign_str(old->name, "value", &old->str, &newalias->str);
+	old->scale = newalias->scale;
+	old->per_pkg = newalias->per_pkg;
+	old->snapshot = newalias->snapshot;
+	memcpy(old->unit, newalias->unit, sizeof(old->unit));
+}
+
+/* Delete an alias entry. */
+static void perf_pmu_free_alias(struct perf_pmu_alias *newalias)
+{
+	zfree(&newalias->name);
+	zfree(&newalias->desc);
+	zfree(&newalias->long_desc);
+	zfree(&newalias->topic);
+	zfree(&newalias->str);
+	zfree(&newalias->metric_expr);
+	zfree(&newalias->metric_name);
+	parse_events_terms__purge(&newalias->terms);
+	free(newalias);
+}
+
+/* Merge an alias, search in alias list. If this name is already
+ * present merge both of them to combine all information.
+ */
+static bool perf_pmu_merge_alias(struct perf_pmu_alias *newalias,
+				 struct list_head *alist)
+{
+	struct perf_pmu_alias *a;
+
+	list_for_each_entry(a, alist, list) {
+		if (!strcasecmp(newalias->name, a->name)) {
+			perf_pmu_update_alias(a, newalias);
+			perf_pmu_free_alias(newalias);
+			return true;
+		}
+	}
+	return false;
+}
+
 static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 				 char *desc, char *val,
 				 char *long_desc, char *topic,

@@ -310,7 +378,8 @@ static int __perf_pmu__new_alias(struct list_head *list, char *dir, char *name,
 	alias->per_pkg = perpkg && sscanf(perpkg, "%d", &num) == 1 && num == 1;
 	alias->str = strdup(newval);
 
-	list_add_tail(&alias->list, list);
+	if (!perf_pmu_merge_alias(alias, list))
+		list_add_tail(&alias->list, list);
 
 	return 0;
 }

Commit-ID:  ea23ac73085743a4f1682d6605fe019577c82e1e
Gitweb:     https://git.kernel.org/tip/ea23ac73085743a4f1682d6605fe019577c82e1e
Author:     Thomas Richter <[email protected]>
AuthorDate: Fri, 15 Jun 2018 12:11:03 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 25 Jun 2018 11:59:37 -0300

perf alias: Remove trailing newline when reading sysfs files

Remove a trailing newline when reading sysfs file contents such as /sys/devices/cpum_cf/events/TX_NC_TEND. This shows when verbose option -v is used.

Output before:

tx_nc_tend -> 'cpum_cf'/'event=0x008d
'/

Output after:

tx_nc_tend -> 'cpum_cf'/'event=0x8d'/

---
 tools/perf/util/pmu.c | 3 +++
 1 file changed, 3 insertions(+)

index d2fb597c9a8c..2738fc8d200d 100644
@@ -303,6 +303,9 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI
 
 	buf[ret] = 0;
 
+	/* Remove trailing newline from sysfs file */
+	rtrim(buf);
+
 	return __perf_pmu__new_alias(list, dir, name, NULL, buf, NULL, NULL, NULL,
 				     NULL, NULL, NULL);
 }