linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files
@ 2018-06-15 10:11 Thomas Richter
  2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Thomas Richter @ 2018-06-15 10:11 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users, acme
  Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter, Jiri Olsa

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'/

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/pmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 7878934ebb23..6d2012405f2b 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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);
 }
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable
  2018-06-15 10:11 [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Thomas Richter
@ 2018-06-15 10:11 ` Thomas Richter
  2018-06-19 15:16   ` Arnaldo Carvalho de Melo
  2018-06-26  6:58   ` [tip:perf/urgent] " tip-bot for Thomas Richter
  2018-06-15 10:11 ` [PATCH 3/3 v2] perf stat: Remove duplicate event counting Thomas Richter
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 8+ messages in thread
From: Thomas Richter @ 2018-06-15 10:11 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users, acme
  Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter, Jiri Olsa

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).

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/pmu.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6d2012405f2b..208e427dc038 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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);
 
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3 v2] perf stat: Remove duplicate event counting
  2018-06-15 10:11 [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Thomas Richter
  2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
@ 2018-06-15 10:11 ` Thomas Richter
  2018-06-26  6:58   ` [tip:perf/urgent] " tip-bot for Thomas Richter
  2018-06-19 15:13 ` [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Arnaldo Carvalho de Melo
  2018-06-26  6:57 ` [tip:perf/urgent] " tip-bot for Thomas Richter
  3 siblings, 1 reply; 8+ messages in thread
From: Thomas Richter @ 2018-06-15 10:11 UTC (permalink / raw)
  To: linux-kernel, linux-perf-users, acme
  Cc: brueckner, schwidefsky, heiko.carstens, Thomas Richter, Jiri Olsa

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]#

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/util/pmu.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 208e427dc038..afd68524ffa9 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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;
 }
-- 
2.14.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files
  2018-06-15 10:11 [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Thomas Richter
  2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
  2018-06-15 10:11 ` [PATCH 3/3 v2] perf stat: Remove duplicate event counting Thomas Richter
@ 2018-06-19 15:13 ` Arnaldo Carvalho de Melo
  2018-06-26  6:57 ` [tip:perf/urgent] " tip-bot for Thomas Richter
  3 siblings, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-19 15:13 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky,
	heiko.carstens, Jiri Olsa

Em Fri, Jun 15, 2018 at 12:11:03PM +0200, Thomas Richter escreveu:
> 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
> '/

Thanks, applied.

- Arnaldo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable
  2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
@ 2018-06-19 15:16   ` Arnaldo Carvalho de Melo
  2018-06-26  6:58   ` [tip:perf/urgent] " tip-bot for Thomas Richter
  1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-19 15:16 UTC (permalink / raw)
  To: Thomas Richter
  Cc: linux-kernel, linux-perf-users, brueckner, schwidefsky,
	heiko.carstens, Jiri Olsa

Em Fri, Jun 15, 2018 at 12:11:04PM +0200, Thomas Richter escreveu:
> 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).

Applying, I think Jiri agreed with this one in the v1 post, right?

- Arnaldo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip:perf/urgent] perf alias: Remove trailing newline when reading sysfs files
  2018-06-15 10:11 [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Thomas Richter
                   ` (2 preceding siblings ...)
  2018-06-19 15:13 ` [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Arnaldo Carvalho de Melo
@ 2018-06-26  6:57 ` tip-bot for Thomas Richter
  3 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Thomas Richter @ 2018-06-26  6:57 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, tmricht, jolsa, hpa, acme, heiko.carstens,
	schwidefsky, tglx, brueckner, mingo

Commit-ID:  ea23ac73085743a4f1682d6605fe019577c82e1e
Gitweb:     https://git.kernel.org/tip/ea23ac73085743a4f1682d6605fe019577c82e1e
Author:     Thomas Richter <tmricht@linux.ibm.com>
AuthorDate: Fri, 15 Jun 2018 12:11:03 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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'/

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20180615101105.47047-1-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index d2fb597c9a8c..2738fc8d200d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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);
 }

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [tip:perf/urgent] perf alias: Rebuild alias expression string to make it comparable
  2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
  2018-06-19 15:16   ` Arnaldo Carvalho de Melo
@ 2018-06-26  6:58   ` tip-bot for Thomas Richter
  1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Thomas Richter @ 2018-06-26  6:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tmricht, brueckner, schwidefsky, jolsa, acme, mingo,
	linux-kernel, heiko.carstens, tglx, hpa

Commit-ID:  0c24d6fb7bd3578e5b9e4972d01bbe3d087ded33
Gitweb:     https://git.kernel.org/tip/0c24d6fb7bd3578e5b9e4972d01bbe3d087ded33
Author:     Thomas Richter <tmricht@linux.ibm.com>
AuthorDate: Fri, 15 Jun 2018 12:11:04 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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).

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20180615101105.47047-2-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 2738fc8d200d..f321ce97d9ec 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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);
 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [tip:perf/urgent] perf stat: Remove duplicate event counting
  2018-06-15 10:11 ` [PATCH 3/3 v2] perf stat: Remove duplicate event counting Thomas Richter
@ 2018-06-26  6:58   ` tip-bot for Thomas Richter
  0 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Thomas Richter @ 2018-06-26  6:58 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tmricht, linux-kernel, heiko.carstens, brueckner, hpa, tglx,
	schwidefsky, acme, jolsa, mingo

Commit-ID:  6dde6429c5ff5b38d6d40a14a6ee105117e6364d
Gitweb:     https://git.kernel.org/tip/6dde6429c5ff5b38d6d40a14a6ee105117e6364d
Author:     Thomas Richter <tmricht@linux.ibm.com>
AuthorDate: Fri, 15 Jun 2018 12:11:05 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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]#

Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
Reviewed-by: Jiri Olsa <jolsa@redhat.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20180615101105.47047-3-tmricht@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/pmu.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index f321ce97d9ec..3ba6a1742f91 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -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;
 }

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-06-26  6:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15 10:11 [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Thomas Richter
2018-06-15 10:11 ` [PATCH 2/3 v2] perf alias: Rebuild alias expression string to make it comparable Thomas Richter
2018-06-19 15:16   ` Arnaldo Carvalho de Melo
2018-06-26  6:58   ` [tip:perf/urgent] " tip-bot for Thomas Richter
2018-06-15 10:11 ` [PATCH 3/3 v2] perf stat: Remove duplicate event counting Thomas Richter
2018-06-26  6:58   ` [tip:perf/urgent] " tip-bot for Thomas Richter
2018-06-19 15:13 ` [PATCH 1/3 v2] perf alias: Remove trailing newline when reading sysfs files Arnaldo Carvalho de Melo
2018-06-26  6:57 ` [tip:perf/urgent] " tip-bot for Thomas Richter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).