From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752404AbcELJbf (ORCPT ); Thu, 12 May 2016 05:31:35 -0400 Received: from merlin.infradead.org ([205.233.59.134]:60649 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751105AbcELJbe (ORCPT ); Thu, 12 May 2016 05:31:34 -0400 Date: Thu, 12 May 2016 11:31:27 +0200 From: Peter Zijlstra To: kan.liang@intel.com Cc: mingo@redhat.com, acme@kernel.org, alexander.shishkin@linux.intel.com, ak@linux.intel.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] perf/core: fix implicitly enable dynamic interrupt throttle Message-ID: <20160512093127.GI3190@twins.programming.kicks-ass.net> References: <1462260366-3160-1-git-send-email-kan.liang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462260366-3160-1-git-send-email-kan.liang@intel.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 03, 2016 at 12:26:06AM -0700, kan.liang@intel.com wrote: > From: Kan Liang > > This patch fixes an issue which was introduced from 'commit 91a612eea9a3 > ("perf/core: Fix dynamic interrupt throttle")' > The old patch unconditionally sets the perf_sample_allowed_ns value to > !0. But that could trigger an issue in the following corner case. > The user can disable the dynamic interrupt throttle mechanism by setting > perf_cpu_time_max_percent to 0. Then they changes > perf_event_max_sample_rate. > For this case, the mechanism will be enabled implicitly, because > perf_sample_allowed_ns becomes !0. > > This patch only updates the perf_sample_allowed_ns when the dynamic > interrupt throttle mechanism is enabled. > > Signed-off-by: Kan Liang > --- > kernel/events/core.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index 4e2ebf6..4042a3d 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -375,6 +375,14 @@ static void update_perf_cpu_limits(void) > { > u64 tmp = perf_sample_period_ns; > > + /* > + * Don't update the perf_sample_allowed_ns, > + * if the dynamic interrupt throttle mechanism is disabled. > + */ > + if (sysctl_perf_cpu_time_max_percent == 100 || > + sysctl_perf_cpu_time_max_percent == 0) > + return; > + Hmm, would it not be nicer to simply reject the write instead of silently ignoring it? --- kernel/events/core.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/kernel/events/core.c b/kernel/events/core.c index 050a290c72c7..0a51a568d4eb 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -396,6 +396,13 @@ int perf_proc_update_handler(struct ctl_table *table, int write, if (ret || !write) return ret; + /* + * If the throttling is disabled; don't allow the write. + */ + if (sysctl_perf_cpu_time_max_percent == 100 || + sysctl_perf_cpu_time_max_percent == 0) + return -EINVAL; + max_samples_per_tick = DIV_ROUND_UP(sysctl_perf_event_sample_rate, HZ); perf_sample_period_ns = NSEC_PER_SEC / sysctl_perf_event_sample_rate; update_perf_cpu_limits();