On Tue, Oct 19, 2021 at 8:18 AM Arnaldo Carvalho de Melo
[off-list ref] wrote:

Em Tue, Oct 19, 2021 at 12:17:10PM -0300, Arnaldo Carvalho de Melo escreveu:

quoted

Em Tue, Oct 19, 2021 at 12:13:52PM -0300, Arnaldo Carvalho de Melo escreveu:

quoted

Em Tue, Oct 19, 2021 at 12:06:17PM -0300, Arnaldo Carvalho de Melo escreveu:

quoted

Em Fri, Oct 15, 2021 at 10:21:32AM -0700, Ian Rogers escreveu:

quoted

By allowing modifiers on metrics we can, for example, gather the
same metric for kernel and user mode. On a SkylakeX with
TopDownL1 this gives:

$ perf stat -M TopDownL1:u,TopDownL1:k -a sleep 2

 Performance counter stats for 'system wide':

Hi Ian, can you please take a look on this? this is on my perf/core
branch.

I processed the first version of this series, reviewed by Andi, can you
please submit the diff from one to the other?

The interdiff from the 21st patch on the first batch versus on the v2
batch is below, but it doesn't apply to my current perf/core branch,
lemme push it to tmp.perf/core...

It was there already, what I have locally is what is in tmp.perf/core.

Hi Arnaldo,

The last change I see in tmp.perf/core is:
https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/commit/?h=tmp.perf/core&id=5f43061b84d815c0f6477c1a8836bf0c6fec15a2

The difference between v2 and v1 which fixes this bug is:

+@@ -1500,7 +1568,10 @@ int metricgroup__copy_metric_events(struct
evlist *evlist, struct cgroup *cgrp,
+                               return -ENOMEM;
+
+                       new_expr->metric_expr = old_expr->metric_expr;
+-                      new_expr->metric_name = old_expr->metric_name;
++                      new_expr->metric_name = strdup(old_expr->metric_name);
++                      if (!new_expr->metric_name)
++                              return -ENOMEM;
++
+                       new_expr->metric_unit = old_expr->metric_unit;
+                       new_expr->runtime = old_expr->runtime;
+

I also cleaned up some checkpatch line length warnings in v2, which I
think is the reason for the other changes. Ideally I'd prefer the v2
patch set over the v1, but they are largely identical. Both were based
on acme/perf/core. Let me know how I can help.

Thanks,
Ian

quoted

- Arnaldo

⬢[acme@toolbox perf]$ interdiff ~/wb/old.patch ~/wb/new.patch
diff -u b/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c

--- b/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c

@@ -1308,8 +1308,7 @@
      int ret;

      *out_evlist = NULL;
-     ret = metricgroup__build_event_string(&events, ids, modifier,
-                                           has_constraint);
+     ret = metricgroup__build_event_string(&events, ids, has_constraint);
      if (ret)
              return ret;

@@ -1324,7 +1323,8 @@

              ids__insert(ids->ids, tmp);
      }
-     ret = metricgroup__build_event_string(&events, ids, has_constraint);
+     ret = metricgroup__build_event_string(&events, ids, modifier,
+                                           has_constraint);
      if (ret)
              return ret;

@@ -1568,7 +1568,10 @@
                              return -ENOMEM;

                      new_expr->metric_expr = old_expr->metric_expr;
-                     new_expr->metric_name = old_expr->metric_name;
+                     new_expr->metric_name = strdup(old_expr->metric_name);
+                     if (!new_expr->metric_name)
+                             return -ENOMEM;
+
                      new_expr->metric_unit = old_expr->metric_unit;
                      new_expr->runtime = old_expr->runtime;

⬢[acme@toolbox perf]$

--

- Arnaldo