Messages in this thread | | | Date | Mon, 6 Feb 2017 22:45:20 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 2/2] perf stat: Disable HW watchdog around a perf stat session |
| |
Em Tue, Feb 07, 2017 at 02:10:28AM +0100, Borislav Petkov escreveu: > From: Borislav Petkov <bp@suse.de> > > When using perf stat on an AMD F15h system with the default hw events > attributes, some of the events don't get counted: > > Performance counter stats for 'sleep 1': > > 0.749208 task-clock (msec) # 0.001 CPUs utilized > 1 context-switches # 0.001 M/sec > 0 cpu-migrations # 0.000 K/sec > 54 page-faults # 0.072 M/sec > 1,122,815 cycles # 1.499 GHz > 286,740 stalled-cycles-frontend # 25.54% frontend cycles idle > <not counted> stalled-cycles-backend (0.00%) > ^^^^^^^^^^^^ > <not counted> instructions (0.00%) > ^^^^^^^^^^^^ > <not counted> branches (0.00%) > <not counted> branch-misses (0.00%) > > 1.001550070 seconds time elapsed > > The reason is that we have the HW watchdog consume one PMU counter > and when perf tries to schedule 6 events on 6 counters and some of > those counters are constrained to only a specific subset of PMCs by the > hardware, the event scheduling fails. > > So let's disable the HW watchdog around a perf stat session running as > root and restore it after it to its previous state. This frees up the > one counter and the scheduling of the default events succeeds: > > Performance counter stats for 'sleep 1': > > 0.806902 task-clock (msec) # 0.001 CPUs utilized > 1 context-switches # 0.001 M/sec > 0 cpu-migrations # 0.000 K/sec > 55 page-faults # 0.068 M/sec > 1,200,677 cycles # 1.488 GHz > 308,044 stalled-cycles-frontend # 25.66% frontend cycles idle > 424,292 stalled-cycles-backend # 35.34% backend cycles idle > 672,694 instructions # 0.56 insn per cycle > # 0.63 stalled cycles per insn > 132,965 branches # 164.785 M/sec > 7,300 branch-misses # 5.49% of all branches > > 1.001689739 seconds time elapsed > > There's a --dont-disable-hwdt option which preserves the old behavior. > > Signed-off-by: Borislav Petkov <bp@suse.de> > --- > tools/perf/builtin-stat.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index a02f2e965628..b2aa2ed3161c 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -146,6 +146,8 @@ static aggr_get_id_t aggr_get_id; > static bool append_file; > static const char *output_name; > static int output_fd; > +static bool keep_hwdt; > +static int prev_hwdt; /* previous HW watchdog state */ > > struct perf_stat { > bool record; > @@ -1539,6 +1541,39 @@ static void print_counters(struct timespec *ts, int argc, const char **argv) > fflush(stat_config.output); > } > > +static void perf_stat_toggle_hwdt(int on) > +{ > + static const char *p = "sys/kernel/nmi_watchdog"; static const char *p = "kernel/nmi_watchdog"; > + int val; > + > + if (keep_hwdt) > + return; > + > + if (geteuid()) > + return; > + > + if (procfs__read_int(p, &val) < 0) sysctl__read_int(p, &val) < 0) > + return; > + > + /* Reenable only when it was enabled before. */ > + if (on) { > + if (prev_hwdt) > + goto write; > + /* Disable HWDT only when it is enabled. */ > + } else { > + prev_hwdt = val; > + > + if (val) > + goto write; > + } > + > + return; > + > +write: > + if (procfs__write_int(p, on) < 0) > + return; > +} > + > static volatile int signr = -1; > > static void skip_signal(int signo) > @@ -1575,6 +1610,8 @@ static void sig_atexit(void) > > sigprocmask(SIG_SETMASK, &oset, NULL); > > + perf_stat_toggle_hwdt(1); > + > if (signr == -1) > return; > > @@ -1659,6 +1696,8 @@ static const struct option stat_options[] = { > "Only print computed metrics. No raw values", enable_metric_only), > OPT_BOOLEAN(0, "topdown", &topdown_run, > "measure topdown level 1 statistics"), > + OPT_BOOLEAN(0, "dont-disable-hwdt", &keep_hwdt, > + "Do not disable HW NMI watchdog during the current session"), > OPT_END() > }; > > @@ -2523,6 +2562,8 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) > if (perf_stat_init_aggr_mode()) > goto out; > > + perf_stat_toggle_hwdt(0); > + > /* > * We dont want to block the signals - that would cause > * child tasks to inherit that and Ctrl-C would not work. > -- > 2.11.0 > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply.
| |