Commit 9c880c24 authored by John Garry's avatar John Garry Committed by Arnaldo Carvalho de Melo

perf metricgroup: Fix for metrics containing duration_time

Metrics containing duration_time cause a segfault:

  $ perf stat -v -M L1D_Cache_Fill_BW sleep 1
  Using CPUID GenuineIntel-6-3D-4
  metric expr 64 * l1d.replacement / 1000000000 / duration_time for L1D_Cache_Fill_BW
  found event duration_time
  found event l1d.replacement
  adding {l1d.replacement}:W,duration_time
  l1d.replacement -> cpu/umask=0x1,(null)=0x1e8483,event=0x51/
  Segmentation fault
  $

In commit c2337d67 ("perf metricgroup: Fix metrics using aliases
covering multiple PMUs"), the logic in find_evsel_group() when iter'ing
events was changed to not only select events in same group, but also for
aliased PMUs.

Checking whether events were for aliased PMUs was done by comparing the
event PMU name. This was not safe for duration_time event, which has no
associated PMU (and no PMU name), so fix by checking if the event PMU name
is set also.

Committer testing:

Reproduced the bug, then, on a:

  $ grep -m1 ^'model name' /proc/cpuinfo
  model name	: Intel(R) Core(TM) i7-8650U CPU @ 1.90GHz
  $

We now get:

  $ perf stat -M L1D_Cache_Fill_BW sleep 1

   Performance counter stats for 'sleep 1':

               4,141      l1d.replacement:u
       1,001,285,107 ns   duration_time:u

         1.001285107 seconds time elapsed

         0.000000000 seconds user
         0.001119000 seconds sys

  $

Detais from -v:

  Using CPUID GenuineIntel-6-8E-A
  metric expr 64 * l1d.replacement / 1000000000 / duration_time for L1D_Cache_Fill_BW
  found event duration_time
  found event l1d.replacement
  adding {l1d.replacement}:W,duration_time
  l1d.replacement -> cpu/(null)=0x1e8483,umask=0x1,event=0x51/
  Control descriptor is not initialized
  Warning:
  kernel.perf_event_paranoid=2, trying to fall back to excluding kernel and hypervisor  samples
  Warning:
  kernel.perf_event_paranoid=2, trying to fall back to excluding kernel and hypervisor  samples
  l1d.replacement:u: 4592 612201 612201
  duration_time:u: 1001478621 1001478621 1001478621

Fixes: c2337d67 ("perf metricgroup: Fix metrics using aliases covering multiple PMUs")
Reported-by: default avatarJoakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: default avatarJohn Garry <john.garry@huawei.com>
Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: default avatarJiri Olsa <jolsa@redhat.com>
Acked-by: default avatarIan Rogers <irogers@google.com>
Acked-by: default avatarJiri Olsa <jolsa@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linuxarm@openeuler.org
Link: https://lore.kernel.org/r/1611159518-226883-1-git-send-email-john.garry@huawei.comSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent fc705fec
...@@ -162,6 +162,14 @@ static bool contains_event(struct evsel **metric_events, int num_events, ...@@ -162,6 +162,14 @@ static bool contains_event(struct evsel **metric_events, int num_events,
return false; return false;
} }
static bool evsel_same_pmu(struct evsel *ev1, struct evsel *ev2)
{
if (!ev1->pmu_name || !ev2->pmu_name)
return false;
return !strcmp(ev1->pmu_name, ev2->pmu_name);
}
/** /**
* Find a group of events in perf_evlist that correspond to those from a parsed * Find a group of events in perf_evlist that correspond to those from a parsed
* metric expression. Note, as find_evsel_group is called in the same order as * metric expression. Note, as find_evsel_group is called in the same order as
...@@ -280,8 +288,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, ...@@ -280,8 +288,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
*/ */
if (!has_constraint && if (!has_constraint &&
ev->leader != metric_events[i]->leader && ev->leader != metric_events[i]->leader &&
!strcmp(ev->leader->pmu_name, evsel_same_pmu(ev->leader, metric_events[i]->leader))
metric_events[i]->leader->pmu_name))
break; break;
if (!strcmp(metric_events[i]->name, ev->name)) { if (!strcmp(metric_events[i]->name, ev->name)) {
set_bit(ev->idx, evlist_used); set_bit(ev->idx, evlist_used);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment