[PATCH 08/19] perf metric: Collect referenced metrics in struct metric_ref_node

2020-07-29 Thread Jiri Olsa
Collecting referenced metrics in struct metric_ref_node object,
so we can process them later on.

The change will parse nested metric names out of expression and
'resolve' them.

All referenced metrics are dissolved into one context, meaning all
nested metrics events and added to the parent context.

Signed-off-by: Jiri Olsa 
Cc: Alexander Shishkin 
Cc: Andi Kleen 
Cc: John Garry 
Cc: Michael Petlan 
Cc: Namhyung Kim 
Cc: Paul Clarke 
Cc: Peter Zijlstra 
Cc: Stephane Eranian 
Link: http://lore.kernel.org/lkml/20200719181320.785305-9-jo...@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/metricgroup.c | 170 +++---
 1 file changed, 156 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index ccd80538a6ae..0997df4e4f52 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -102,12 +102,25 @@ void metricgroup__rblist_exit(struct rblist 
*metric_events)
rblist__exit(metric_events);
 }
 
+/*
+ * A node in the list of referenced metrics. metric_expr
+ * is held as a convenience to avoid a search through the
+ * metric list.
+ */
+struct metric_ref_node {
+   const char *metric_name;
+   const char *metric_expr;
+   struct list_head list;
+};
+
 struct egroup {
struct list_head nd;
struct expr_parse_ctx pctx;
const char *metric_name;
const char *metric_expr;
const char *metric_unit;
+   struct list_head metric_refs;
+   int metric_refs_cnt;
int runtime;
bool has_constraint;
 };
@@ -574,27 +587,72 @@ int __weak arch_get_runtimeparam(void)
 static int __add_metric(struct list_head *group_list,
struct pmu_event *pe,
bool metric_no_group,
-   int runtime)
+   int runtime,
+   struct egroup **egp)
 {
+   struct metric_ref_node *ref;
struct egroup *eg;
 
-   eg = malloc(sizeof(*eg));
-   if (!eg)
-   return -ENOMEM;
+   if (*egp == NULL) {
+   /*
+* We got in here for the parent group,
+* allocate it and put it on the list.
+*/
+   eg = malloc(sizeof(*eg));
+   if (!eg)
+   return -ENOMEM;
+
+   expr__ctx_init(>pctx);
+   eg->metric_name = pe->metric_name;
+   eg->metric_expr = pe->metric_expr;
+   eg->metric_unit = pe->unit;
+   eg->runtime = runtime;
+   eg->has_constraint = metric_no_group || 
metricgroup__has_constraint(pe);
+   INIT_LIST_HEAD(>metric_refs);
+   eg->metric_refs_cnt = 0;
+   *egp = eg;
+   } else {
+   /*
+* We got here for the referenced metric, via the
+* recursive metricgroup__add_metric call, add
+* it to the parent group.
+*/
+   eg = *egp;
+
+   ref = malloc(sizeof(*ref));
+   if (!ref)
+   return -ENOMEM;
+
+   /*
+* Intentionally passing just const char pointers,
+* from 'pe' object, so they never go away. We don't
+* need to change them, so there's no need to create
+* our own copy.
+*/
+   ref->metric_name = pe->metric_name;
+   ref->metric_expr = pe->metric_expr;
 
-   expr__ctx_init(>pctx);
-   eg->metric_name = pe->metric_name;
-   eg->metric_expr = pe->metric_expr;
-   eg->metric_unit = pe->unit;
-   eg->runtime = runtime;
-   eg->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
+   list_add(>list, >metric_refs);
+   eg->metric_refs_cnt++;
+   }
 
+   /*
+* For both the parent and referenced metrics, we parse
+* all the metric's IDs and add it to the parent context.
+*/
if (expr__find_other(pe->metric_expr, NULL, >pctx, runtime) < 0) {
expr__ctx_clear(>pctx);
free(eg);
return -EINVAL;
}
 
+   /*
+* We add new group only in the 'parent' call,
+* so bail out for referenced metric case.
+*/
+   if (eg->metric_refs_cnt)
+   return 0;
+
if (list_empty(group_list))
list_add(>nd, group_list);
else {
@@ -625,16 +683,78 @@ static int __add_metric(struct list_head *group_list,
(match_metric(__pe->metric_group, __metric) ||  \
 match_metric(__pe->metric_name, __metric)))
 
+static struct pmu_event *find_metric(const char *metric, struct pmu_events_map 
*map)
+{
+   struct pmu_event *pe;
+   int i;
+
+   map_for_each_event(pe, i, map) {
+   if (match_metric(pe->metric_name, metric))
+ 

Re: [PATCH 08/19] perf metric: Collect referenced metrics in struct metric_ref_node

2020-07-28 Thread Arnaldo Carvalho de Melo
Em Sun, Jul 19, 2020 at 03:18:58PM -0700, Ian Rogers escreveu:
> On Sun, Jul 19, 2020 at 11:13 AM Jiri Olsa  wrote:
> >
> > Collecting referenced metrics in struct metric_ref_node object,
> > so we can process them later on.
> >
> > The change will parse nested metric names out of expression and
> > 'resolve' them.
> >
> > All referenced metrics are dissolved into one context, meaning all
> > nested metrics events and added to the parent context.
> >
> > Signed-off-by: Jiri Olsa 
> 
> Acked-by: Ian Rogers 

Thanks, applied.

- Arnaldo
 
> Thanks,
> Ian
> 
> > ---
> >  tools/perf/util/metricgroup.c | 170 +++---
> >  1 file changed, 156 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index ccd80538a6ae..d1b2c1aa436f 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -102,12 +102,25 @@ void metricgroup__rblist_exit(struct rblist 
> > *metric_events)
> > rblist__exit(metric_events);
> >  }
> >
> > +/*
> > + * A node in the list of referenced metrics. metric_expr
> > + * is held as a convenience to avoid a search through the
> > + * metric list.
> > + */
> > +struct metric_ref_node {
> > +   const char *metric_name;
> > +   const char *metric_expr;
> > +   struct list_head list;
> > +};
> > +
> >  struct egroup {
> > struct list_head nd;
> > struct expr_parse_ctx pctx;
> > const char *metric_name;
> > const char *metric_expr;
> > const char *metric_unit;
> > +   struct list_head metric_refs;
> > +   int metric_refs_cnt;
> > int runtime;
> > bool has_constraint;
> >  };
> > @@ -574,27 +587,72 @@ int __weak arch_get_runtimeparam(void)
> >  static int __add_metric(struct list_head *group_list,
> > struct pmu_event *pe,
> > bool metric_no_group,
> > -   int runtime)
> > +   int runtime,
> > +   struct egroup **egp)
> >  {
> > +   struct metric_ref_node *ref;
> > struct egroup *eg;
> >
> > -   eg = malloc(sizeof(*eg));
> > -   if (!eg)
> > -   return -ENOMEM;
> > +   if (*egp == NULL) {
> > +   /*
> > +* We got in here for the parent group,
> > +* allocate it and put it on the list.
> > +*/
> > +   eg = malloc(sizeof(*eg));
> > +   if (!eg)
> > +   return -ENOMEM;
> > +
> > +   expr__ctx_init(>pctx);
> > +   eg->metric_name = pe->metric_name;
> > +   eg->metric_expr = pe->metric_expr;
> > +   eg->metric_unit = pe->unit;
> > +   eg->runtime = runtime;
> > +   eg->has_constraint = metric_no_group || 
> > metricgroup__has_constraint(pe);
> > +   INIT_LIST_HEAD(>metric_refs);
> > +   eg->metric_refs_cnt = 0;
> > +   *egp = eg;
> > +   } else {
> > +   /*
> > +* We got here for the referenced metric, via the
> > +* recursive metricgroup__add_metric call, add
> > +* it to the parent group.
> > +*/
> > +   eg = *egp;
> > +
> > +   ref = malloc(sizeof(*ref));
> > +   if (!ref)
> > +   return -ENOMEM;
> > +
> > +   /*
> > +* Intentionally passing just const char pointers,
> > +* from 'pe' object, so they never go away. We don't
> > +* need to change them, so there's no need to create
> > +* our own copy.
> > +*/
> > +   ref->metric_name = pe->metric_name;
> > +   ref->metric_expr = pe->metric_expr;
> >
> > -   expr__ctx_init(>pctx);
> > -   eg->metric_name = pe->metric_name;
> > -   eg->metric_expr = pe->metric_expr;
> > -   eg->metric_unit = pe->unit;
> > -   eg->runtime = runtime;
> > -   eg->has_constraint = metric_no_group || 
> > metricgroup__has_constraint(pe);
> > +   list_add(>list, >metric_refs);
> > +   eg->metric_refs_cnt++;
> > +   }
> >
> > +   /*
> > +* For both the parent and referenced metrics, we parse
> > +* all the metric's IDs and add it to the parent context.
> > +*/
> > if (expr__find_other(pe->metric_expr, NULL, >pctx, runtime) < 
> > 0) {
> > expr__ctx_clear(>pctx);
> > free(eg);
> > return -EINVAL;
> > }
> >
> > +   /*
> > +* We add new group only in the 'parent' call,
> > +* so bail out for referenced metric case.
> > +*/
> > +   if (eg->metric_refs_cnt)
> > +   return 0;
> > +
> > if (list_empty(group_list))
> > list_add(>nd, group_list);
> > else {
> > @@ -625,16 

Re: [PATCH 08/19] perf metric: Collect referenced metrics in struct metric_ref_node

2020-07-26 Thread kajoljain



On 7/20/20 3:48 AM, Ian Rogers wrote:
> On Sun, Jul 19, 2020 at 11:13 AM Jiri Olsa  wrote:
>>
>> Collecting referenced metrics in struct metric_ref_node object,
>> so we can process them later on.
>>
>> The change will parse nested metric names out of expression and
>> 'resolve' them.
>>
>> All referenced metrics are dissolved into one context, meaning all
>> nested metrics events and added to the parent context.
>>
>> Signed-off-by: Jiri Olsa 
> 
> Acked-by: Ian Rogers 
> 
> Thanks,
> Ian

Reviewed-By : Kajol Jain

Thanks,
Kajol Jain
> 
>> ---
>>  tools/perf/util/metricgroup.c | 170 +++---
>>  1 file changed, 156 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index ccd80538a6ae..d1b2c1aa436f 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -102,12 +102,25 @@ void metricgroup__rblist_exit(struct rblist 
>> *metric_events)
>> rblist__exit(metric_events);
>>  }
>>
>> +/*
>> + * A node in the list of referenced metrics. metric_expr
>> + * is held as a convenience to avoid a search through the
>> + * metric list.
>> + */
>> +struct metric_ref_node {
>> +   const char *metric_name;
>> +   const char *metric_expr;
>> +   struct list_head list;
>> +};
>> +
>>  struct egroup {
>> struct list_head nd;
>> struct expr_parse_ctx pctx;
>> const char *metric_name;
>> const char *metric_expr;
>> const char *metric_unit;
>> +   struct list_head metric_refs;
>> +   int metric_refs_cnt;
>> int runtime;
>> bool has_constraint;
>>  };
>> @@ -574,27 +587,72 @@ int __weak arch_get_runtimeparam(void)
>>  static int __add_metric(struct list_head *group_list,
>> struct pmu_event *pe,
>> bool metric_no_group,
>> -   int runtime)
>> +   int runtime,
>> +   struct egroup **egp)
>>  {
>> +   struct metric_ref_node *ref;
>> struct egroup *eg;
>>
>> -   eg = malloc(sizeof(*eg));
>> -   if (!eg)
>> -   return -ENOMEM;
>> +   if (*egp == NULL) {
>> +   /*
>> +* We got in here for the parent group,
>> +* allocate it and put it on the list.
>> +*/
>> +   eg = malloc(sizeof(*eg));
>> +   if (!eg)
>> +   return -ENOMEM;
>> +
>> +   expr__ctx_init(>pctx);
>> +   eg->metric_name = pe->metric_name;
>> +   eg->metric_expr = pe->metric_expr;
>> +   eg->metric_unit = pe->unit;
>> +   eg->runtime = runtime;
>> +   eg->has_constraint = metric_no_group || 
>> metricgroup__has_constraint(pe);
>> +   INIT_LIST_HEAD(>metric_refs);
>> +   eg->metric_refs_cnt = 0;
>> +   *egp = eg;
>> +   } else {
>> +   /*
>> +* We got here for the referenced metric, via the
>> +* recursive metricgroup__add_metric call, add
>> +* it to the parent group.
>> +*/
>> +   eg = *egp;
>> +
>> +   ref = malloc(sizeof(*ref));
>> +   if (!ref)
>> +   return -ENOMEM;
>> +
>> +   /*
>> +* Intentionally passing just const char pointers,
>> +* from 'pe' object, so they never go away. We don't
>> +* need to change them, so there's no need to create
>> +* our own copy.
>> +*/
>> +   ref->metric_name = pe->metric_name;
>> +   ref->metric_expr = pe->metric_expr;
>>
>> -   expr__ctx_init(>pctx);
>> -   eg->metric_name = pe->metric_name;
>> -   eg->metric_expr = pe->metric_expr;
>> -   eg->metric_unit = pe->unit;
>> -   eg->runtime = runtime;
>> -   eg->has_constraint = metric_no_group || 
>> metricgroup__has_constraint(pe);
>> +   list_add(>list, >metric_refs);
>> +   eg->metric_refs_cnt++;
>> +   }
>>
>> +   /*
>> +* For both the parent and referenced metrics, we parse
>> +* all the metric's IDs and add it to the parent context.
>> +*/
>> if (expr__find_other(pe->metric_expr, NULL, >pctx, runtime) < 0) 
>> {
>> expr__ctx_clear(>pctx);
>> free(eg);
>> return -EINVAL;
>> }
>>
>> +   /*
>> +* We add new group only in the 'parent' call,
>> +* so bail out for referenced metric case.
>> +*/
>> +   if (eg->metric_refs_cnt)
>> +   return 0;
>> +
>> if (list_empty(group_list))
>> list_add(>nd, group_list);
>> else {
>> @@ -625,16 +683,78 @@ static int __add_metric(struct list_head *group_list,
>> (match_metric(__pe->metric_group, __metric) ||

Re: [PATCH 08/19] perf metric: Collect referenced metrics in struct metric_ref_node

2020-07-19 Thread Ian Rogers
On Sun, Jul 19, 2020 at 11:13 AM Jiri Olsa  wrote:
>
> Collecting referenced metrics in struct metric_ref_node object,
> so we can process them later on.
>
> The change will parse nested metric names out of expression and
> 'resolve' them.
>
> All referenced metrics are dissolved into one context, meaning all
> nested metrics events and added to the parent context.
>
> Signed-off-by: Jiri Olsa 

Acked-by: Ian Rogers 

Thanks,
Ian

> ---
>  tools/perf/util/metricgroup.c | 170 +++---
>  1 file changed, 156 insertions(+), 14 deletions(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index ccd80538a6ae..d1b2c1aa436f 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -102,12 +102,25 @@ void metricgroup__rblist_exit(struct rblist 
> *metric_events)
> rblist__exit(metric_events);
>  }
>
> +/*
> + * A node in the list of referenced metrics. metric_expr
> + * is held as a convenience to avoid a search through the
> + * metric list.
> + */
> +struct metric_ref_node {
> +   const char *metric_name;
> +   const char *metric_expr;
> +   struct list_head list;
> +};
> +
>  struct egroup {
> struct list_head nd;
> struct expr_parse_ctx pctx;
> const char *metric_name;
> const char *metric_expr;
> const char *metric_unit;
> +   struct list_head metric_refs;
> +   int metric_refs_cnt;
> int runtime;
> bool has_constraint;
>  };
> @@ -574,27 +587,72 @@ int __weak arch_get_runtimeparam(void)
>  static int __add_metric(struct list_head *group_list,
> struct pmu_event *pe,
> bool metric_no_group,
> -   int runtime)
> +   int runtime,
> +   struct egroup **egp)
>  {
> +   struct metric_ref_node *ref;
> struct egroup *eg;
>
> -   eg = malloc(sizeof(*eg));
> -   if (!eg)
> -   return -ENOMEM;
> +   if (*egp == NULL) {
> +   /*
> +* We got in here for the parent group,
> +* allocate it and put it on the list.
> +*/
> +   eg = malloc(sizeof(*eg));
> +   if (!eg)
> +   return -ENOMEM;
> +
> +   expr__ctx_init(>pctx);
> +   eg->metric_name = pe->metric_name;
> +   eg->metric_expr = pe->metric_expr;
> +   eg->metric_unit = pe->unit;
> +   eg->runtime = runtime;
> +   eg->has_constraint = metric_no_group || 
> metricgroup__has_constraint(pe);
> +   INIT_LIST_HEAD(>metric_refs);
> +   eg->metric_refs_cnt = 0;
> +   *egp = eg;
> +   } else {
> +   /*
> +* We got here for the referenced metric, via the
> +* recursive metricgroup__add_metric call, add
> +* it to the parent group.
> +*/
> +   eg = *egp;
> +
> +   ref = malloc(sizeof(*ref));
> +   if (!ref)
> +   return -ENOMEM;
> +
> +   /*
> +* Intentionally passing just const char pointers,
> +* from 'pe' object, so they never go away. We don't
> +* need to change them, so there's no need to create
> +* our own copy.
> +*/
> +   ref->metric_name = pe->metric_name;
> +   ref->metric_expr = pe->metric_expr;
>
> -   expr__ctx_init(>pctx);
> -   eg->metric_name = pe->metric_name;
> -   eg->metric_expr = pe->metric_expr;
> -   eg->metric_unit = pe->unit;
> -   eg->runtime = runtime;
> -   eg->has_constraint = metric_no_group || 
> metricgroup__has_constraint(pe);
> +   list_add(>list, >metric_refs);
> +   eg->metric_refs_cnt++;
> +   }
>
> +   /*
> +* For both the parent and referenced metrics, we parse
> +* all the metric's IDs and add it to the parent context.
> +*/
> if (expr__find_other(pe->metric_expr, NULL, >pctx, runtime) < 0) {
> expr__ctx_clear(>pctx);
> free(eg);
> return -EINVAL;
> }
>
> +   /*
> +* We add new group only in the 'parent' call,
> +* so bail out for referenced metric case.
> +*/
> +   if (eg->metric_refs_cnt)
> +   return 0;
> +
> if (list_empty(group_list))
> list_add(>nd, group_list);
> else {
> @@ -625,16 +683,78 @@ static int __add_metric(struct list_head *group_list,
> (match_metric(__pe->metric_group, __metric) ||  \
>  match_metric(__pe->metric_name, __metric)))
>
> +static struct pmu_event *find_metric(const char *metric, struct 
> pmu_events_map *map)
> +{
> +   struct pmu_event *pe;
> +   int i;
> +
> +   

[PATCH 08/19] perf metric: Collect referenced metrics in struct metric_ref_node

2020-07-19 Thread Jiri Olsa
Collecting referenced metrics in struct metric_ref_node object,
so we can process them later on.

The change will parse nested metric names out of expression and
'resolve' them.

All referenced metrics are dissolved into one context, meaning all
nested metrics events and added to the parent context.

Signed-off-by: Jiri Olsa 
---
 tools/perf/util/metricgroup.c | 170 +++---
 1 file changed, 156 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index ccd80538a6ae..d1b2c1aa436f 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -102,12 +102,25 @@ void metricgroup__rblist_exit(struct rblist 
*metric_events)
rblist__exit(metric_events);
 }
 
+/*
+ * A node in the list of referenced metrics. metric_expr
+ * is held as a convenience to avoid a search through the
+ * metric list.
+ */
+struct metric_ref_node {
+   const char *metric_name;
+   const char *metric_expr;
+   struct list_head list;
+};
+
 struct egroup {
struct list_head nd;
struct expr_parse_ctx pctx;
const char *metric_name;
const char *metric_expr;
const char *metric_unit;
+   struct list_head metric_refs;
+   int metric_refs_cnt;
int runtime;
bool has_constraint;
 };
@@ -574,27 +587,72 @@ int __weak arch_get_runtimeparam(void)
 static int __add_metric(struct list_head *group_list,
struct pmu_event *pe,
bool metric_no_group,
-   int runtime)
+   int runtime,
+   struct egroup **egp)
 {
+   struct metric_ref_node *ref;
struct egroup *eg;
 
-   eg = malloc(sizeof(*eg));
-   if (!eg)
-   return -ENOMEM;
+   if (*egp == NULL) {
+   /*
+* We got in here for the parent group,
+* allocate it and put it on the list.
+*/
+   eg = malloc(sizeof(*eg));
+   if (!eg)
+   return -ENOMEM;
+
+   expr__ctx_init(>pctx);
+   eg->metric_name = pe->metric_name;
+   eg->metric_expr = pe->metric_expr;
+   eg->metric_unit = pe->unit;
+   eg->runtime = runtime;
+   eg->has_constraint = metric_no_group || 
metricgroup__has_constraint(pe);
+   INIT_LIST_HEAD(>metric_refs);
+   eg->metric_refs_cnt = 0;
+   *egp = eg;
+   } else {
+   /*
+* We got here for the referenced metric, via the
+* recursive metricgroup__add_metric call, add
+* it to the parent group.
+*/
+   eg = *egp;
+
+   ref = malloc(sizeof(*ref));
+   if (!ref)
+   return -ENOMEM;
+
+   /*
+* Intentionally passing just const char pointers,
+* from 'pe' object, so they never go away. We don't
+* need to change them, so there's no need to create
+* our own copy.
+*/
+   ref->metric_name = pe->metric_name;
+   ref->metric_expr = pe->metric_expr;
 
-   expr__ctx_init(>pctx);
-   eg->metric_name = pe->metric_name;
-   eg->metric_expr = pe->metric_expr;
-   eg->metric_unit = pe->unit;
-   eg->runtime = runtime;
-   eg->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
+   list_add(>list, >metric_refs);
+   eg->metric_refs_cnt++;
+   }
 
+   /*
+* For both the parent and referenced metrics, we parse
+* all the metric's IDs and add it to the parent context.
+*/
if (expr__find_other(pe->metric_expr, NULL, >pctx, runtime) < 0) {
expr__ctx_clear(>pctx);
free(eg);
return -EINVAL;
}
 
+   /*
+* We add new group only in the 'parent' call,
+* so bail out for referenced metric case.
+*/
+   if (eg->metric_refs_cnt)
+   return 0;
+
if (list_empty(group_list))
list_add(>nd, group_list);
else {
@@ -625,16 +683,78 @@ static int __add_metric(struct list_head *group_list,
(match_metric(__pe->metric_group, __metric) ||  \
 match_metric(__pe->metric_name, __metric)))
 
+static struct pmu_event *find_metric(const char *metric, struct pmu_events_map 
*map)
+{
+   struct pmu_event *pe;
+   int i;
+
+   map_for_each_event(pe, i, map) {
+   if (match_metric(pe->metric_name, metric))
+   return pe;
+   }
+
+   return NULL;
+}
+
+static int add_metric(struct list_head *group_list,
+ struct pmu_event *pe,
+ bool metric_no_group,
+ struct egroup **egp);
+
+static