Re: [PATCH 4/5] perf metricgroup: Support metric constraint

[Liang, Kan]

On 2/20/2020 6:35 AM, Jiri Olsa wrote:
> On Wed, Feb 19, 2020 at 11:08:39AM -0800, kan.liang@xxxxxxxxxxxxxxx wrote:
>
> SNIP
>
> > +static bool violate_nmi_constraint;
> > +
> > +static bool metricgroup__has_constraint(struct pmu_event *pe)
> > +{
> > +	if (!pe->metric_constraint)
> > +		return false;
> > +
> > +	if (!strcmp(pe->metric_constraint, "NO_NMI_WATCHDOG") &&
> > +	    sysctl__nmi_watchdog_enabled()) {
> > +		pr_warning("Splitting metric group %s into standalone metrics.\n",
> > +			   pe->metric_name);
> > +		violate_nmi_constraint = true;
>
> no static flags plz.. can't you just print that rest of the warning in here?

Because we only want to print the NMI watchdog warning once.
If there are more than one metric groups with constraint, the warning 
may be printed several times. For example,
  $ perf stat -M Page_Walks_Utilization,Page_Walks_Utilization
  Splitting metric group Page_Walks_Utilization into standalone metrics.
  Try disabling the NMI watchdog to comply NO_NMI_WATCHDOG metric 
constraint:
      echo 0 > /proc/sys/kernel/nmi_watchdog
      perf stat ...
      echo 1 > /proc/sys/kernel/nmi_watchdog
  Splitting metric group Page_Walks_Utilization into standalone metrics.
  Try disabling the NMI watchdog to comply NO_NMI_WATCHDOG metric 
constraint:
      echo 0 > /proc/sys/kernel/nmi_watchdog
      perf stat ...
      echo 1 > /proc/sys/kernel/nmi_watchdog
Is it OK?

If it's OK, I think we can remove the flag.

Thanks,
Kan

> jirka
>
> > +		return true;
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >   static int metricgroup__add_metric(const char *metric, struct strbuf *events,
> >   				   struct list_head *group_list)
> >   {
> > @@ -460,7 +490,10 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
> >   			if (events->len > 0)
> >   				strbuf_addf(events, ",");
> >   
> > -			metricgroup__add_metric_weak_group(events, ids, idnum);
> > +			if (metricgroup__has_constraint(pe))
> > +				metricgroup__add_metric_non_group(events, ids, idnum);
> > +			else
> > +				metricgroup__add_metric_weak_group(events, ids, idnum);
> >   
> >   			eg = malloc(sizeof(struct egroup));
> >   			if (!eg) {
> > @@ -544,6 +577,13 @@ int metricgroup__parse_groups(const struct option *opt,
> >   	strbuf_release(&extra_events);
> >   	ret = metricgroup__setup_events(&group_list, perf_evlist,
> >   					metric_events);
> > +
> > +	if (violate_nmi_constraint) {
> > +		pr_warning("Try disabling the NMI watchdog to comply NO_NMI_WATCHDOG metric constraint:\n"
> > +			   "    echo 0 > /proc/sys/kernel/nmi_watchdog\n"
> > +			   "    perf stat ...\n"
> > +			   "    echo 1 > /proc/sys/kernel/nmi_watchdog\n");
> > +	}
> >   out:
> >   	metricgroup__free_egroups(&group_list);
> >   	return ret;
> > --
> > 2.7.4