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 <dzic...@redhat.com>
Just re-pinging on this. I haven't seen it picked up by anyone.
P.
>
>
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> Cc: Borislav Petkov <b...@suse.de>
>> Cc: Tejun Heo <t...@kernel.org>
>> Cc: Don Zickus <dzic...@redhat.com>
>> Cc: Hidehiro Kawai <hidehiro.kawai...@hitachi.com>
>> Cc: Thomas Gleixner <t...@linutronix.de>
>> Cc: Andi Kleen <a...@linux.intel.com>
>> Cc: Joshua Hunt <joh...@akamai.com>
>> Cc: Ingo Molnar <mi...@kernel.org>
>> Cc: Babu Moger <babu.mo...@oracle.com>
>> ---
>> 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
>>
