Messages in this thread | | | Date | Fri, 21 Feb 2020 15:48:03 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH 4/5] perf metricgroup: Support metric constraint |
| |
On Fri, Feb 21, 2020 at 09:30:15AM -0500, Liang, Kan wrote: > > > On 2/21/2020 8:09 AM, Jiri Olsa wrote: > > On Thu, Feb 20, 2020 at 11:14:09AM -0500, Liang, Kan wrote: > > > > > > > > > On 2/20/2020 6:35 AM, Jiri Olsa wrote: > > > > On Wed, Feb 19, 2020 at 11:08:39AM -0800, kan.liang@linux.intel.com 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. > > > > we use the 'print once' static flags in functions, > > so plz keep it inside like WARN_ONCE, or use it directly > > > > If using WARN_ONCE, the warning is always printed for the first violation. > 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. > > > The output of current patch is as below. > #perf stat -M Page_Walks_Utilization,Page_Walks_Utilization > Splitting metric group Page_Walks_Utilization into standalone metrics. > 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 > > > Personally, I think the output of current patch looks better. > But there is nothing wrong with the output of WARN_ONCE. > > Should I use WARN_ONCE in next V2?
I just wanted you to keep that static flag inside the function, so we don't have another static variable used across the code
if the WARN_ONCE does not fit, just use your own flag inside the function
jirka
| |