From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758535AbdACVT7 (ORCPT ); Tue, 3 Jan 2017 16:19:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35288 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751257AbdACVTv (ORCPT ); Tue, 3 Jan 2017 16:19:51 -0500 Message-ID: <586C1576.5050307@redhat.com> Date: Tue, 03 Jan 2017 16:19:50 -0500 From: Prarit Bhargava User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Don Zickus CC: linux-kernel@vger.kernel.org, Borislav Petkov , Tejun Heo , Hidehiro Kawai , Thomas Gleixner , Andi Kleen , Joshua Hunt , Ingo Molnar , Babu Moger Subject: Re: [PATCH] kernel/watchdog.c: Do not hardcode CPU 0 as the initial thread References: <1480425321-32296-1-git-send-email-prarit@redhat.com> <20161201200613.GG35881@redhat.com> In-Reply-To: <20161201200613.GG35881@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 03 Jan 2017 21:19:51 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12/01/2016 03:06 PM, Don Zickus wrote: > On Tue, Nov 29, 2016 at 08:15:21AM -0500, Prarit Bhargava wrote: >> When CONFIG_BOOTPARAM_HOTPLUG_CPU0 is enabled, the socket containing the >> boot cpu can be replaced. During the hot add event, the message >> >> NMI watchdog: enabled on all CPUs, permanently consumes one hw-PMU counter. >> >> is output implying that the NMI watchdog was disabled at some point. This >> is not the case and the message has caused confusion for users of systems >> that support the removal of the boot cpu socket. >> >> The watchdog code is coded to assume that cpu 0 is always the first cpu to >> initialize the watchdog, and the last to stop its watchdog thread. That >> is not the case for initializing if cpu 0 has been removed and added. The >> removal case has never been correct because the smpboot code will remove >> the watchdog threads starting with the lowest cpu number. >> >> This patch adds watchdog_cpus to track the number of cpus with active NMI >> watchdog threads so that the first and last thread can be used to set and >> clear the value of firstcpu_err. firstcpu_err is set when the first >> watchdog thread is enabled, and cleared when the last watchdog thread is >> disabled. >> >> This patch is based on top of linux-next akpm-base. > > It passed my tests. Thanks! > > Acked-by: Don Zickus Just re-pinging on this. I haven't seen it picked up by anyone. P. > > >> >> Signed-off-by: Prarit Bhargava >> Cc: Borislav Petkov >> Cc: Tejun Heo >> Cc: Don Zickus >> Cc: Hidehiro Kawai >> Cc: Thomas Gleixner >> Cc: Andi Kleen >> Cc: Joshua Hunt >> Cc: Ingo Molnar >> Cc: Babu Moger >> --- >> kernel/watchdog_hld.c | 25 +++++++++++++++---------- >> 1 file changed, 15 insertions(+), 10 deletions(-) >> >> diff --git a/kernel/watchdog_hld.c b/kernel/watchdog_hld.c >> index 84016c8aee6b..30761f7504ef 100644 >> --- a/kernel/watchdog_hld.c >> +++ b/kernel/watchdog_hld.c >> @@ -134,12 +134,14 @@ static void watchdog_overflow_callback(struct perf_event *event, >> * Reduce the watchdog noise by only printing messages >> * that are different from what cpu0 displayed. >> */ >> -static unsigned long cpu0_err; >> +static unsigned long firstcpu_err; >> +static atomic_t watchdog_cpus; >> >> int watchdog_nmi_enable(unsigned int cpu) >> { >> struct perf_event_attr *wd_attr; >> struct perf_event *event = per_cpu(watchdog_ev, cpu); >> + int firstcpu = 0; >> >> /* nothing to do if the hard lockup detector is disabled */ >> if (!(watchdog_enabled & NMI_WATCHDOG_ENABLED)) >> @@ -153,19 +155,22 @@ int watchdog_nmi_enable(unsigned int cpu) >> if (event != NULL) >> goto out_enable; >> >> + if (atomic_inc_return(&watchdog_cpus) == 1) >> + firstcpu = 1; >> + >> wd_attr = &wd_hw_attr; >> wd_attr->sample_period = hw_nmi_get_sample_period(watchdog_thresh); >> >> /* Try to register using hardware perf events */ >> event = perf_event_create_kernel_counter(wd_attr, cpu, NULL, watchdog_overflow_callback, NULL); >> >> - /* save cpu0 error for future comparision */ >> - if (cpu == 0 && IS_ERR(event)) >> - cpu0_err = PTR_ERR(event); >> + /* save the first cpu's error for future comparision */ >> + if (firstcpu && IS_ERR(event)) >> + firstcpu_err = PTR_ERR(event); >> >> if (!IS_ERR(event)) { >> - /* only print for cpu0 or different than cpu0 */ >> - if (cpu == 0 || cpu0_err) >> + /* only print for the first cpu initialized */ >> + if (firstcpu || firstcpu_err) >> pr_info("enabled on all CPUs, permanently consumes one hw-PMU counter.\n"); >> goto out_save; >> } >> @@ -183,7 +188,7 @@ int watchdog_nmi_enable(unsigned int cpu) >> smp_mb__after_atomic(); >> >> /* skip displaying the same error again */ >> - if (cpu > 0 && (PTR_ERR(event) == cpu0_err)) >> + if (!firstcpu && (PTR_ERR(event) == firstcpu_err)) >> return PTR_ERR(event); >> >> /* vary the KERN level based on the returned errno */ >> @@ -219,9 +224,9 @@ void watchdog_nmi_disable(unsigned int cpu) >> >> /* should be in cleanup, but blocks oprofile */ >> perf_event_release_kernel(event); >> - } >> - if (cpu == 0) { >> + >> /* watchdog_nmi_enable() expects this to be zero initially. */ >> - cpu0_err = 0; >> + if (atomic_dec_and_test(&watchdog_cpus)) >> + firstcpu_err = 0; >> } >> } >> -- >> 1.7.9.3 >>