redonkable
/
remarkable-linux
1
0
Fork 0
Code Releases Activity

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>
zero-colors
Thomas Richter 2018-06-15 12:11:05 +02:00 committed by Arnaldo Carvalho de Melo
parent 0c24d6fb7b
commit 6dde6429c5
1 changed files with 70 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
tools/perf/util/pmu.c
View File

@ -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;
}