[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160610190319.GC3826@kernel.org>
Date: Fri, 10 Jun 2016 16:03:19 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>, x86@...nel.org,
linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Andi Kleen <ak@...ux.intel.com>
Cc: Andi Kleen <andi@...stfloor.org>
Subject: Re: [PATCH 2/2] perf stat: Remove nmi watchdog check code again
Em Thu, Jun 09, 2016 at 06:14:39AM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@...ux.intel.com>
>
> Now that the NMI watchdog runs with reference cycles, and does not
> conflict with TopDown anymore, we don't need to check that the
> NMI watchdog is off in perf stat --topdown.
>
> Remove the code that does this and always use a group unconditionally.
PeterZ, will you pick this one? I'm ok with it, Andi is just removing
some code he introduced recently.
- Arnaldo
> Signed-off-by: Andi Kleen <ak@...ux.intel.com>
> ---
> tools/perf/Documentation/perf-stat.txt | 6 ------
> tools/perf/arch/x86/util/Build | 1 -
> tools/perf/arch/x86/util/group.c | 27 ---------------------------
> tools/perf/builtin-stat.c | 18 +-----------------
> tools/perf/util/group.h | 7 -------
> 5 files changed, 1 insertion(+), 58 deletions(-)
> delete mode 100644 tools/perf/arch/x86/util/group.c
> delete mode 100644 tools/perf/util/group.h
>
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index d96ccd4844df..6cf4028ddc2c 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -225,12 +225,6 @@ CPU thread. Per core mode is automatically enabled
> and -a (global monitoring) is needed, requiring root rights or
> perf.perf_event_paranoid=-1.
>
> -Topdown uses the full Performance Monitoring Unit, and needs
> -disabling of the NMI watchdog (as root):
> -echo 0 > /proc/sys/kernel/nmi_watchdog
> -for best results. Otherwise the bottlenecks may be inconsistent
> -on workload with changing phases.
> -
> This enables --metric-only, unless overriden with --no-metric-only.
>
> To interpret the results it is usually needed to know on which
> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
> index f95e6f46ef0d..bc24b75add88 100644
> --- a/tools/perf/arch/x86/util/Build
> +++ b/tools/perf/arch/x86/util/Build
> @@ -3,7 +3,6 @@ libperf-y += tsc.o
> libperf-y += pmu.o
> libperf-y += kvm-stat.o
> libperf-y += perf_regs.o
> -libperf-y += group.o
>
> libperf-$(CONFIG_DWARF) += dwarf-regs.o
> libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
> diff --git a/tools/perf/arch/x86/util/group.c b/tools/perf/arch/x86/util/group.c
> deleted file mode 100644
> index 37f92aa39a5d..000000000000
> --- a/tools/perf/arch/x86/util/group.c
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -#include <stdio.h>
> -#include "api/fs/fs.h"
> -#include "util/group.h"
> -
> -/*
> - * Check whether we can use a group for top down.
> - * Without a group may get bad results due to multiplexing.
> - */
> -bool arch_topdown_check_group(bool *warn)
> -{
> - int n;
> -
> - if (sysctl__read_int("kernel/nmi_watchdog", &n) < 0)
> - return false;
> - if (n > 0) {
> - *warn = true;
> - return false;
> - }
> - return true;
> -}
> -
> -void arch_topdown_group_warn(void)
> -{
> - fprintf(stderr,
> - "nmi_watchdog enabled with topdown. May give wrong results.\n"
> - "Disable with echo 0 > /proc/sys/kernel/nmi_watchdog\n");
> -}
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index dff63733dfb7..a599a78b2f22 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -59,10 +59,8 @@
> #include "util/thread.h"
> #include "util/thread_map.h"
> #include "util/counts.h"
> -#include "util/group.h"
> #include "util/session.h"
> #include "util/tool.h"
> -#include "util/group.h"
> #include "asm/bug.h"
>
> #include <api/fs/fs.h>
> @@ -1861,16 +1859,6 @@ static int topdown_filter_events(const char **attr, char **str, bool use_group)
> return 0;
> }
>
> -__weak bool arch_topdown_check_group(bool *warn)
> -{
> - *warn = false;
> - return false;
> -}
> -
> -__weak void arch_topdown_group_warn(void)
> -{
> -}
> -
> /*
> * Add default attributes, if there were no attributes specified or
> * if -d/--detailed, -d -d or -d -d -d is used:
> @@ -2010,7 +1998,6 @@ static int add_default_attributes(void)
>
> if (topdown_run) {
> char *str = NULL;
> - bool warn = false;
>
> if (stat_config.aggr_mode != AGGR_GLOBAL &&
> stat_config.aggr_mode != AGGR_CORE) {
> @@ -2025,14 +2012,11 @@ static int add_default_attributes(void)
>
> if (!force_metric_only)
> metric_only = true;
> - if (topdown_filter_events(topdown_attrs, &str,
> - arch_topdown_check_group(&warn)) < 0) {
> + if (topdown_filter_events(topdown_attrs, &str, true) < 0) {
> pr_err("Out of memory\n");
> return -1;
> }
> if (topdown_attrs[0] && str) {
> - if (warn)
> - arch_topdown_group_warn();
> err = parse_events(evsel_list, str, NULL);
> if (err) {
> fprintf(stderr,
> diff --git a/tools/perf/util/group.h b/tools/perf/util/group.h
> deleted file mode 100644
> index 116debe7a995..000000000000
> --- a/tools/perf/util/group.h
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -#ifndef GROUP_H
> -#define GROUP_H 1
> -
> -bool arch_topdown_check_group(bool *warn);
> -void arch_topdown_group_warn(void);
> -
> -#endif
> --
> 2.5.5
Powered by blists - more mailing lists