Re: [PATCH 2/2] perf stat: Disable HW watchdog around a perf stat session

From: Arnaldo Carvalho de Melo
Date: Mon Feb 06 2017 - 20:45:31 EST


Em Tue, Feb 07, 2017 at 02:10:28AM +0100, Borislav Petkov escreveu:
> From: Borislav Petkov <bp@xxxxxxx>
>
> 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@xxxxxxx>
> ---
> 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.