From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964883AbcKKTgw (ORCPT ); Fri, 11 Nov 2016 14:36:52 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:36490 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932697AbcKKTgv (ORCPT ); Fri, 11 Nov 2016 14:36:51 -0500 Date: Fri, 11 Nov 2016 20:36:31 +0100 From: Sebastian Andrzej Siewior To: Borislav Petkov Cc: linux-kernel@vger.kernel.org, tglx@linutronix.de, rt@linutronix.de, Tony Luck , linux-edac@vger.kernel.org, x86@kernel.org Subject: Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks Message-ID: <20161111193631.srv7sxqzykvymgbf@linutronix.de> References: <20161110091809.vxyf3yiuxtjy3vqv@pd.tnic> <20161110174447.11848-1-bigeasy@linutronix.de> <20161110174447.11848-6-bigeasy@linutronix.de> <20161111184453.ax5getrj5y3i4fuc@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161111184453.ax5getrj5y3i4fuc@pd.tnic> User-Agent: NeoMutt/20161014 (1.7.1) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-11-11 19:44:53 [+0100], Borislav Petkov wrote: > > --- > > arch/x86/kernel/cpu/mcheck/mce.c | 31 +++++++++++++++---------------- > > 1 file changed, 15 insertions(+), 16 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c > > index 052b5e05c3c4..3da6fd94fa2e 100644 > > --- a/arch/x86/kernel/cpu/mcheck/mce.c > > +++ b/arch/x86/kernel/cpu/mcheck/mce.c > > @@ -1771,6 +1771,9 @@ void (*machine_check_vector)(struct pt_regs *, long error_code) = > > */ > > void mcheck_cpu_init(struct cpuinfo_x86 *c) > > { > > + struct timer_list *t = this_cpu_ptr(&mce_timer); > > + unsigned int cpu = smp_processor_id(); > > + > > if (mca_cfg.disabled) > > return; > > > > @@ -1796,7 +1799,7 @@ void mcheck_cpu_init(struct cpuinfo_x86 *c) > > __mcheck_cpu_init_generic(); > > __mcheck_cpu_init_vendor(c); > > __mcheck_cpu_init_clear_banks(); > > - __mcheck_cpu_init_timer(); > > + setup_pinned_timer(t, mce_timer_fn, cpu); > > Why not leave all that setup stuff in __mcheck_cpu_init_timer() ? > __mcheck_cpu_init_timer() does not only prepare the timer but also fires it. And __mcheck_cpu_init_timer() is invoked from other places and the caller probably expects the timer to be fired. > > @@ -2517,11 +2518,10 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu) > > mce_device_remove(cpu); > > return NOTIFY_BAD; > > } > > - > > + mce_reenable_cpu(); > > + mce_start_timer(cpu, t); > > break; > > case CPU_DEAD: > > - mce_threshold_remove_device(cpu); > > - mce_device_remove(cpu); > > mce_intel_hcpu_update(cpu); > > > > /* intentionally ignoring frozen here */ > > There's another place for cpuhp_tasks_frozen replacement here: > > /* intentionally ignoring frozen here */ > if (!(action & CPU_TASKS_FROZEN)) > cmci_rediscover(); > > into > > if (!cpuhp_tasks_frozen) > cmci_rediscover(); Correct. This is done later since it still works. For functions like mce_disable_cpu() which were converted here I replaced the check so I keep them void and don't care about the argument. Sebastian