From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932319AbcFJTDZ (ORCPT ); Fri, 10 Jun 2016 15:03:25 -0400 Received: from mail.kernel.org ([198.145.29.136]:53380 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932156AbcFJTDX (ORCPT ); Fri, 10 Jun 2016 15:03:23 -0400 Date: Fri, 10 Jun 2016 16:03:19 -0300 From: Arnaldo Carvalho de Melo To: Peter Zijlstra , x86@kernel.org, linux-kernel@vger.kernel.org, Jiri Olsa , Andi Kleen Cc: Andi Kleen Subject: Re: [PATCH 2/2] perf stat: Remove nmi watchdog check code again Message-ID: <20160610190319.GC3826@kernel.org> References: <1465478079-19993-1-git-send-email-andi@firstfloor.org> <1465478079-19993-2-git-send-email-andi@firstfloor.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1465478079-19993-2-git-send-email-andi@firstfloor.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Thu, Jun 09, 2016 at 06:14:39AM -0700, Andi Kleen escreveu: > From: Andi Kleen > > 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 > --- > 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 > -#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 > @@ -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