Re: [PATCHv3 00/19] perf metric: Add support to reuse metric

Jiri Olsa
2020-07-20T19:16:42Z

On Mon, Jul 20, 2020 at 02:32:40PM +0530, kajoljain wrote:
>
>
> On 7/20/20 1:49 PM, Jiri Olsa wrote:
> > On Mon, Jul 20, 2020 at 01:39:24PM +0530, kajoljain wrote:
> >
> > SNIP
> >
> >> This is with your perf/metric branch:
> >> command# ./perf stat -M PowerBUS_Frequency -C 0 -I 1000
> >> assertion failed at util/metricgroup.c:709
> >> # time counts unit events
> >> 1.000054545 7,807,505 hv_24x7/pm_pb_cyc,chip=0/ # 2.0 GHz PowerBUS_Frequency_0
> >> 1.000054545 7,807,485 hv_24x7/pm_pb_cyc,chip=1/
> >> 2.000232761 7,807,500 hv_24x7/pm_pb_cyc,chip=0/ # 2.0 GHz PowerBUS_Frequency_0
> >> 2.000232761 7,807,478 hv_24x7/pm_pb_cyc,chip=1/
> >> 3.000363762 7,799,665 hv_24x7/pm_pb_cyc,chip=0/ # 1.9 GHz PowerBUS_Frequency_0
> >> 3.000363762 7,807,502 hv_24x7/pm_pb_cyc,chip=1/
> >> ^C 3.259418599 2,022,150 hv_24x7/pm_pb_cyc,chip=0/ # 0.5 GHz PowerBUS_Frequency_0
> >> 3.259418599 2,022,164 hv_24x7/pm_pb_cyc,chip=1/
> >>
> >> Performance counter stats for 'CPU(s) 0':
> >>
> >> 25,436,820 hv_24x7/pm_pb_cyc,chip=0/ # 6.4 GHz PowerBUS_Frequency_0
> >> 25,444,629 hv_24x7/pm_pb_cyc,chip=1/
> >>
> >> 3.259505529 seconds time elapsed
> >
> > I found the bug, we are not adding runtime metrics as standalone ones,
> > but as referenced metrics.. will fix and try to add test for that
> >
> > as for testing.. do I need some special ppc server to have support for this?
>
> Hi jiri,
> We need power9 lpar machine and need to make sure `CONFIG_HV_PERF_CTRS` is
> enabled.

could you please try with following patch on top?

thanks,
jirka

---
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6f179b9903a0..03aa4bd4a38b 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -820,11 +820,11 @@ static int add_metric(struct list_head *metric_list,
struct expr_id *parent,
struct expr_ids *ids);

-static int resolve_metric(struct metric *m,
- bool metric_no_group,
- struct list_head *metric_list,
- struct pmu_events_map *map,
- struct expr_ids *ids)
+static int __resolve_metric(struct metric *m,
+ bool metric_no_group,
+ struct list_head *metric_list,
+ struct pmu_events_map *map,
+ struct expr_ids *ids)
{
struct hashmap_entry *cur;
size_t bkt;
@@ -869,6 +869,23 @@ static int resolve_metric(struct metric *m,
return 0;
}

+static int resolve_metric(bool metric_no_group,
+ struct list_head *metric_list,
+ struct pmu_events_map *map,
+ struct expr_ids *ids)
+{
+ struct metric *m;
+ int err;
+
+ list_for_each_entry(m, metric_list, nd) {
+ err = __resolve_metric(m, metric_no_group, metric_list, map, ids);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static int add_metric(struct list_head *metric_list,
struct pmu_event *pe,
bool metric_no_group,
@@ -876,6 +893,7 @@ static int add_metric(struct list_head *metric_list,
struct expr_id *parent,
struct expr_ids *ids)
{
+ struct metric *orig = *m;
int ret = 0;

pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
@@ -892,7 +910,7 @@ static int add_metric(struct list_head *metric_list,
* those events to metric_list.
*/

- for (j = 0; j < count && !ret; j++) {
+ for (j = 0; j < count && !ret; j++, *m = orig) {
ret = __add_metric(metric_list, pe, metric_no_group, j, m, parent, ids);
}
}
@@ -907,8 +925,8 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,

{
struct expr_ids ids = { 0 };
+ struct metric *m = NULL;
struct pmu_event *pe;
- struct metric *m;
LIST_HEAD(list);
int i, ret;
bool has_match = false;
@@ -925,7 +943,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
* Process any possible referenced metrics
* included in the expression.
*/
- ret = resolve_metric(m, metric_no_group,
+ ret = resolve_metric(metric_no_group,
&list, map, &ids);
if (ret)
return ret;