Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1514576 > unrolled thread

[PATCH 22/25] x86/mcheck: Do the init in one place

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-03 16:00 +0100
    Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-07 19:50 +0100
      Re: [PATCH 22/25] x86/mcheck: Do the init in one place "Luck, Tony" <tony.luck@intel.com> - 2016-11-07 20:00 +0100
        Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-07 21:20 +0100
          Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-08 10:30 +0100
            Re: [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-09 15:30 +0100
              Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-09 16:40 +0100
                Re: [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-09 17:30 +0100
                  Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-09 18:10 +0100
                    Re: [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-09 18:30 +0100
                RE: [PATCH 22/25] x86/mcheck: Do the init in one place "Luck, Tony" <tony.luck@intel.com> - 2016-11-09 19:40 +0100
                  Re: [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 10:10 +0100
                    Re: [PATCH 22/25] x86/mcheck: Do the init in one place Borislav Petkov <bp@alien8.de> - 2016-11-10 10:20 +0100
                      [PATCH 4/7] x86/mcheck: Split threshold_cpu_callback into two callbacks Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Split threshold_cpu_callback into two  callbacks tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 3/7] x86/mcheck: Be prepared for a rollback back to the ONLINE state Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Be prepared for a rollback back to  the ONLINE state tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 1/7] x86/mcheck: Move threshold_create_device() Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Move threshold_create_device() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks Borislav Petkov <bp@alien8.de> - 2016-11-11 20:00 +0100
                          Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-11 20:40 +0100
                            Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks Borislav Petkov <bp@alien8.de> - 2016-11-11 21:00 +0100
                              [PATCH 5/7 v2] x86/mcheck: reorganize the hotplug callbacks Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-14 11:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Reorganize the hotplug callbacks tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 2/7] x86/mcheck: Explicit cleanup on failure in mce_amd Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Explicit cleanup on failure in  mce_amd tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 6/7] x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        [tip:smp/hotplug] x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE  to hotplug state machine tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      [PATCH 7/7] x86/mcheck: Move CPU_DEAD to hotplug state machine Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                        Re: [PATCH 7/7] x86/mcheck: Move CPU_DEAD to hotplug state machine Borislav Petkov <bp@alien8.de> - 2016-11-11 21:20 +0100
                        [tip:smp/hotplug] x86/mcheck: Move CPU_DEAD to hotplug state  machine tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2016-11-16 09:50 +0100
                      x86/mcheck: convert to hotplug state engine, take #2 Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 18:50 +0100
                    Re: [PATCH 22/25] x86/mcheck: Do the init in one place Sebastian Andrzej Siewior <bigeasy@linutronix.de> - 2016-11-10 11:30 +0100
                    Re: [PATCH 22/25] x86/mcheck: Do the init in one place Thomas Gleixner <tglx@linutronix.de> - 2016-11-10 11:30 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1519967 — Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks

On Thu, Nov 10, 2016 at 06:44:45PM +0100, Sebastian Andrzej Siewior wrote:
> Initially I wanted to remove mcheck_cpu_init() from identify_cpu() and let it
> become an independent early hotplug callback. The main problem here was that
> the init on the boot CPU may happen too late
> (device_initcall_sync(mcheck_init_device)) and nobody wanted to risk receiving
> and MCE event at boot time leading to a shutdown (if the MCE feature is not yet
> enabled).
> 
> Here is attempt two: the timming stays as-is but the ordering of the functions

			   timing

> is changed:
> - mcheck_cpu_init() (which is run from identify_cpu()) will setup the timer
>   struct but won't fire the timer. This is moved to CPU_ONLINE since its
>   cleanup part is in CPU_DOWN_PREPARE. So if it is okay to stop the timer early
>   in the shutdown phase, it should be okay to start it late in the bring up phase.
> 
> - CPU_DOWN_PREPARE disables the MCE feature flags for !INTEL CPUs in

		     disables the MCE error reporting...

>   mce_disable_cpu(). If a failure occures it would be re-enabled on all vendor

				    occurs

>   CPUs (including Intel where it was not disabled during shutdown). To keep this
>   working I am moving it to CPU_ONLINE. smp_call_function_single() is dropped
>   beause the notifier runs nowdays on the target CPU.

"... because the notifier runs on the target CPU now."

Please run your commit messages text through a spellchecker.

> - CPU_ONLINE is invoking mce_device_create() + mce_threshold_create_device()
>   but its cleanup part is in CPU_DEAD (mce_threshold_remove_device() and
>   mce_device_remove()). In order to keep this symmetrical I am moving the clean
>   up from CPU_DEAD to CPU_DOWN_PREPARE.

   cleanup

> 
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: linux-edac@vger.kernel.org
> Cc: x86@kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  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() ?

...

> @@ -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();

> @@ -2529,12 +2529,11 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
>  			cmci_rediscover();
>  		break;
>  	case CPU_DOWN_PREPARE:
> -		smp_call_function_single(cpu, mce_disable_cpu, &action, 1);
> +		mce_disable_cpu();
>  		del_timer_sync(t);
> -		break;
> -	case CPU_DOWN_FAILED:
> -		smp_call_function_single(cpu, mce_reenable_cpu, &action, 1);
> -		mce_start_timer(cpu, t);
> +
> +		mce_threshold_remove_device(cpu);
> +		mce_device_remove(cpu);
>  		break;
>  	}
>  
> -- 
> 2.10.2
> 
> 

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1519990 — Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks

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

[toc] | [prev] | [next] | [standalone]

#1519999 — Re: [PATCH 5/7] x86/mcheck: reorganize the hotplug callbacks

On Fri, Nov 11, 2016 at 08:36:31PM +0100, Sebastian Andrzej Siewior wrote:
> __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.

Then please do it right:

static void __mcheck_cap_setup_timer(void)
{
        struct timer_list *t = this_cpu_ptr(&mce_timer);
        unsigned int cpu = smp_processor_id();

        setup_pinned_timer(t, mce_timer_fn, cpu);
}

and call that function then.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1521524 — [PATCH 5/7 v2] x86/mcheck: reorganize the hotplug callbacks

Initially I wanted to remove mcheck_cpu_init() from identify_cpu() and let it
become an independent early hotplug callback. The main problem here was that
the init on the boot CPU may happen too late
(device_initcall_sync(mcheck_init_device)) and nobody wanted to risk receiving
and MCE event at boot time leading to a shutdown (if the MCE feature is not yet
enabled).

Here is attempt two: the timing stays as-is but the ordering of the functions
is changed:
- mcheck_cpu_init() (which is run from identify_cpu()) will setup the timer
  struct but won't fire the timer. This is moved to CPU_ONLINE since its
  cleanup part is in CPU_DOWN_PREPARE. So if it is okay to stop the timer early
  in the shutdown phase, it should be okay to start it late in the bring up phase.

- CPU_DOWN_PREPARE disables the MCE feature flags for !INTEL CPUs in
  mce_disable_cpu(). If a failure occurs it would be re-enabled on all vendor
  CPUs (including Intel where it was not disabled during shutdown). To keep this
  working I am moving it to CPU_ONLINE. smp_call_function_single() is dropped
  because the notifier runs on the target CPU (since core code rework).

- CPU_ONLINE is invoking mce_device_create() + mce_threshold_create_device()
  but its cleanup part is in CPU_DEAD (mce_threshold_remove_device() and
  mce_device_remove()). In order to keep this symmetrical I am moving the clean
  up from CPU_DEAD to CPU_DOWN_PREPARE.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
On 2016-11-11 20:57:37 [+0100], Borislav Petkov wrote:
> Then please do it right:
> 
> static void __mcheck_cap_setup_timer(void)
> {
>         struct timer_list *t = this_cpu_ptr(&mce_timer);
>         unsigned int cpu = smp_processor_id();
> 
>         setup_pinned_timer(t, mce_timer_fn, cpu);
> }
> 
> and call that function then.

I can't believe that you ask for this but here it is.

 arch/x86/kernel/cpu/mcheck/mce.c |   36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1745,6 +1745,14 @@ static void mce_start_timer(unsigned int
 	add_timer_on(t, cpu);
 }
 
+static void __mcheck_cap_setup_timer(void)
+{
+	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	unsigned int cpu = smp_processor_id();
+
+	setup_pinned_timer(t, mce_timer_fn, cpu);
+}
+
 static void __mcheck_cpu_init_timer(void)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
@@ -1796,7 +1804,7 @@ void mcheck_cpu_init(struct cpuinfo_x86
 	__mcheck_cpu_init_generic();
 	__mcheck_cpu_init_vendor(c);
 	__mcheck_cpu_init_clear_banks();
-	__mcheck_cpu_init_timer();
+	__mcheck_cap_setup_timer();
 }
 
 /*
@@ -2470,28 +2478,25 @@ static void mce_device_remove(unsigned i
 }
 
 /* Make sure there are no machine checks on offlined CPUs. */
-static void mce_disable_cpu(void *h)
+static void mce_disable_cpu(void)
 {
-	unsigned long action = *(unsigned long *)h;
-
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!cpuhp_tasks_frozen)
 		cmci_clear();
 
 	vendor_disable_error_reporting();
 }
 
-static void mce_reenable_cpu(void *h)
+static void mce_reenable_cpu(void)
 {
-	unsigned long action = *(unsigned long *)h;
 	int i;
 
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!cpuhp_tasks_frozen)
 		cmci_reenable();
 	for (i = 0; i < mca_cfg.banks; i++) {
 		struct mce_bank *b = &mce_banks[i];
@@ -2510,6 +2515,7 @@ mce_cpu_callback(struct notifier_block *
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_ONLINE:
+	case CPU_DOWN_FAILED:
 
 		mce_device_create(cpu);
 
@@ -2517,11 +2523,10 @@ mce_cpu_callback(struct notifier_block *
 			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 */
@@ -2529,12 +2534,11 @@ mce_cpu_callback(struct notifier_block *
 			cmci_rediscover();
 		break;
 	case CPU_DOWN_PREPARE:
-		smp_call_function_single(cpu, mce_disable_cpu, &action, 1);
+		mce_disable_cpu();
 		del_timer_sync(t);
-		break;
-	case CPU_DOWN_FAILED:
-		smp_call_function_single(cpu, mce_reenable_cpu, &action, 1);
-		mce_start_timer(cpu, t);
+
+		mce_threshold_remove_device(cpu);
+		mce_device_remove(cpu);
 		break;
 	}
 

[toc] | [prev] | [next] | [standalone]

#1523302 — [tip:smp/hotplug] x86/mcheck: Reorganize the hotplug callbacks

Commit-ID:  39f152ffbfedb42b57b6e0c896eeae51dbe83b7a
Gitweb:     http://git.kernel.org/tip/39f152ffbfedb42b57b6e0c896eeae51dbe83b7a
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Thu, 10 Nov 2016 18:44:45 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 16 Nov 2016 09:34:18 +0100

x86/mcheck: Reorganize the hotplug callbacks

Initially I wanted to remove mcheck_cpu_init() from identify_cpu() and let it
become an independent early hotplug callback. The main problem here was that
the init on the boot CPU may happen too late
(device_initcall_sync(mcheck_init_device)) and nobody wanted to risk receiving
and MCE event at boot time leading to a shutdown (if the MCE feature is not yet
enabled).

Here is attempt two: the timming stays as-is but the ordering of the functions
is changed:
- mcheck_cpu_init() (which is run from identify_cpu()) will setup the timer
  struct but won't fire the timer. This is moved to CPU_ONLINE since its
  cleanup part is in CPU_DOWN_PREPARE. So if it is okay to stop the timer early
  in the shutdown phase, it should be okay to start it late in the bring up phase.

- CPU_DOWN_PREPARE disables the MCE feature flags for !INTEL CPUs in
  mce_disable_cpu(). If a failure occures it would be re-enabled on all vendor
  CPUs (including Intel where it was not disabled during shutdown). To keep this
  working I am moving it to CPU_ONLINE. smp_call_function_single() is dropped
  beause the notifier runs nowdays on the target CPU.

- CPU_ONLINE is invoking mce_device_create() + mce_threshold_create_device()
  but its cleanup part is in CPU_DEAD (mce_threshold_remove_device() and
  mce_device_remove()). In order to keep this symmetrical I am moving the clean
  up from CPU_DEAD to CPU_DOWN_PREPARE.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: rt@linutronix.de
Cc: linux-edac@vger.kernel.org
Link: http://lkml.kernel.org/r/20161110174447.11848-6-bigeasy@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/mcheck/mce.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 052b5e0..a524faa 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -1745,6 +1745,14 @@ static void mce_start_timer(unsigned int cpu, struct timer_list *t)
 	add_timer_on(t, cpu);
 }
 
+static void __mcheck_cpu_setup_timer(void)
+{
+	struct timer_list *t = this_cpu_ptr(&mce_timer);
+	unsigned int cpu = smp_processor_id();
+
+	setup_pinned_timer(t, mce_timer_fn, cpu);
+}
+
 static void __mcheck_cpu_init_timer(void)
 {
 	struct timer_list *t = this_cpu_ptr(&mce_timer);
@@ -1796,7 +1804,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();
+	__mcheck_cpu_setup_timer();
 }
 
 /*
@@ -2470,28 +2478,25 @@ static void mce_device_remove(unsigned int cpu)
 }
 
 /* Make sure there are no machine checks on offlined CPUs. */
-static void mce_disable_cpu(void *h)
+static void mce_disable_cpu(void)
 {
-	unsigned long action = *(unsigned long *)h;
-
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!cpuhp_tasks_frozen)
 		cmci_clear();
 
 	vendor_disable_error_reporting();
 }
 
-static void mce_reenable_cpu(void *h)
+static void mce_reenable_cpu(void)
 {
-	unsigned long action = *(unsigned long *)h;
 	int i;
 
 	if (!mce_available(raw_cpu_ptr(&cpu_info)))
 		return;
 
-	if (!(action & CPU_TASKS_FROZEN))
+	if (!cpuhp_tasks_frozen)
 		cmci_reenable();
 	for (i = 0; i < mca_cfg.banks; i++) {
 		struct mce_bank *b = &mce_banks[i];
@@ -2510,6 +2515,7 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 
 	switch (action & ~CPU_TASKS_FROZEN) {
 	case CPU_ONLINE:
+	case CPU_DOWN_FAILED:
 
 		mce_device_create(cpu);
 
@@ -2517,11 +2523,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 */
@@ -2529,12 +2534,11 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 			cmci_rediscover();
 		break;
 	case CPU_DOWN_PREPARE:
-		smp_call_function_single(cpu, mce_disable_cpu, &action, 1);
+		mce_disable_cpu();
 		del_timer_sync(t);
-		break;
-	case CPU_DOWN_FAILED:
-		smp_call_function_single(cpu, mce_reenable_cpu, &action, 1);
-		mce_start_timer(cpu, t);
+
+		mce_threshold_remove_device(cpu);
+		mce_device_remove(cpu);
 		break;
 	}
 

[toc] | [prev] | [next] | [standalone]

#1519238 — [PATCH 2/7] x86/mcheck: Explicit cleanup on failure in mce_amd

If the ONLINE callback fails, the driver does not any clean up right
away instead it waits to get to the DEAD stage to do it. Yes, it waits.
Since we don't pass the error code back to the caller, no one knows.

Do the clean up right away so it does not look like a leak.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 75a3e3eab81e..55cd018bc1ae 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -1087,6 +1087,7 @@ static void threshold_remove_device(unsigned int cpu)
 		threshold_remove_bank(cpu, bank);
 	}
 	kfree(per_cpu(threshold_banks, cpu));
+	per_cpu(threshold_banks, cpu) = NULL;
 }
 
 /* create dir/files for all valid threshold banks */
@@ -1108,9 +1109,11 @@ static int threshold_create_device(unsigned int cpu)
 			continue;
 		err = threshold_create_bank(cpu, bank);
 		if (err)
-			return err;
+			goto err;
 	}
-
+	return err;
+err:
+	threshold_remove_device(cpu);
 	return err;
 }
 
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1523301 — [tip:smp/hotplug] x86/mcheck: Explicit cleanup on failure in mce_amd

Commit-ID:  ec553abb318d75b1eafd275c71125569fa13b33b
Gitweb:     http://git.kernel.org/tip/ec553abb318d75b1eafd275c71125569fa13b33b
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Thu, 10 Nov 2016 18:44:42 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 16 Nov 2016 09:34:17 +0100

x86/mcheck: Explicit cleanup on failure in mce_amd

If the ONLINE callback fails, the driver does not any clean up right
away instead it waits to get to the DEAD stage to do it. Yes, it waits.
Since we don't pass the error code back to the caller, no one knows.

Do the clean up right away so it does not look like a leak.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: rt@linutronix.de
Cc: linux-edac@vger.kernel.org
Link: http://lkml.kernel.org/r/20161110174447.11848-3-bigeasy@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/mcheck/mce_amd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 75a3e3e..55cd018 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -1087,6 +1087,7 @@ static void threshold_remove_device(unsigned int cpu)
 		threshold_remove_bank(cpu, bank);
 	}
 	kfree(per_cpu(threshold_banks, cpu));
+	per_cpu(threshold_banks, cpu) = NULL;
 }
 
 /* create dir/files for all valid threshold banks */
@@ -1108,9 +1109,11 @@ static int threshold_create_device(unsigned int cpu)
 			continue;
 		err = threshold_create_bank(cpu, bank);
 		if (err)
-			return err;
+			goto err;
 	}
-
+	return err;
+err:
+	threshold_remove_device(cpu);
 	return err;
 }
 

[toc] | [prev] | [next] | [standalone]

#1519240 — [PATCH 6/7] x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE to hotplug state machine

The CPU_ONLINE and CPU_DOWN_PREPARE look fully symmetrical and could be move
to the hotplug state machine.
On a failure during registration we have the tear down callback invoked
(mce_cpu_pre_down()) so there should be no timer around and so no need to need
keep notifier installed (this was the reason according to the comment why the
notifier was registered despite of errors).

Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 78 +++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 3da6fd94fa2e..444177cafb65 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2506,21 +2506,8 @@ static int
 mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
-	struct timer_list *t = &per_cpu(mce_timer, cpu);
 
 	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_ONLINE:
-	case CPU_DOWN_FAILED:
-
-		mce_device_create(cpu);
-
-		if (mce_threshold_create_device(cpu)) {
-			mce_device_remove(cpu);
-			return NOTIFY_BAD;
-		}
-		mce_reenable_cpu();
-		mce_start_timer(cpu, t);
-		break;
 	case CPU_DEAD:
 		mce_intel_hcpu_update(cpu);
 
@@ -2529,17 +2516,41 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 			cmci_rediscover();
 		break;
 	case CPU_DOWN_PREPARE:
-		mce_disable_cpu();
-		del_timer_sync(t);
 
-		mce_threshold_remove_device(cpu);
-		mce_device_remove(cpu);
 		break;
 	}
 
 	return NOTIFY_OK;
 }
 
+static int mce_cpu_online(unsigned int cpu)
+{
+	struct timer_list *t = &per_cpu(mce_timer, cpu);
+	int ret;
+
+	mce_device_create(cpu);
+
+	ret = mce_threshold_create_device(cpu);
+	if (ret) {
+		mce_device_remove(cpu);
+		return ret;
+	}
+	mce_reenable_cpu();
+	mce_start_timer(cpu, t);
+	return 0;
+}
+
+static int mce_cpu_pre_down(unsigned int cpu)
+{
+	struct timer_list *t = &per_cpu(mce_timer, cpu);
+
+	mce_disable_cpu();
+	del_timer_sync(t);
+	mce_threshold_remove_device(cpu);
+	mce_device_remove(cpu);
+	return 0;
+}
+
 static struct notifier_block mce_cpu_notifier = {
 	.notifier_call = mce_cpu_callback,
 };
@@ -2564,8 +2575,8 @@ static __init void mce_init_banks(void)
 
 static __init int mcheck_init_device(void)
 {
+	enum cpuhp_state hp_online;
 	int err;
-	int i = 0;
 
 	if (!mce_available(&boot_cpu_data)) {
 		err = -EIO;
@@ -2583,21 +2594,13 @@ static __init int mcheck_init_device(void)
 	if (err)
 		goto err_out_mem;
 
-	cpu_notifier_register_begin();
-	for_each_online_cpu(i) {
-		err = mce_device_create(i);
-		if (err) {
-			/*
-			 * Register notifier anyway (and do not unreg it) so
-			 * that we don't leave undeleted timers, see notifier
-			 * callback above.
-			 */
-			__register_hotcpu_notifier(&mce_cpu_notifier);
-			cpu_notifier_register_done();
-			goto err_device_create;
-		}
-	}
+	err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/mce:online",
+				mce_cpu_online, mce_cpu_pre_down);
+	if (err < 0)
+		goto err_out_mem;
+	hp_online = err;
 
+	cpu_notifier_register_begin();
 	__register_hotcpu_notifier(&mce_cpu_notifier);
 	cpu_notifier_register_done();
 
@@ -2612,16 +2615,7 @@ static __init int mcheck_init_device(void)
 
 err_register:
 	unregister_syscore_ops(&mce_syscore_ops);
-
-err_device_create:
-	/*
-	 * We didn't keep track of which devices were created above, but
-	 * even if we had, the set of online cpus might have changed.
-	 * Play safe and remove for every possible cpu, since
-	 * mce_device_remove() will do the right thing.
-	 */
-	for_each_possible_cpu(i)
-		mce_device_remove(i);
+	cpuhp_remove_state(hp_online);
 
 err_out_mem:
 	free_cpumask_var(mce_device_initialized);
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1523310 — [tip:smp/hotplug] x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE to hotplug state machine

Commit-ID:  8c0eeac819c85e4c1143f7a874d87b4594739208
Gitweb:     http://git.kernel.org/tip/8c0eeac819c85e4c1143f7a874d87b4594739208
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Thu, 10 Nov 2016 18:44:46 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 16 Nov 2016 09:34:18 +0100

x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE to hotplug state machine

The CPU_ONLINE and CPU_DOWN_PREPARE look fully symmetrical and could be move
to the hotplug state machine.
On a failure during registration we have the tear down callback invoked
(mce_cpu_pre_down()) so there should be no timer around and so no need to need
keep notifier installed (this was the reason according to the comment why the
notifier was registered despite of errors).

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: rt@linutronix.de
Cc: linux-edac@vger.kernel.org
Link: http://lkml.kernel.org/r/20161110174447.11848-7-bigeasy@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/mcheck/mce.c | 78 +++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index a524faa..78955f5 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2511,21 +2511,8 @@ static int
 mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 {
 	unsigned int cpu = (unsigned long)hcpu;
-	struct timer_list *t = &per_cpu(mce_timer, cpu);
 
 	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_ONLINE:
-	case CPU_DOWN_FAILED:
-
-		mce_device_create(cpu);
-
-		if (mce_threshold_create_device(cpu)) {
-			mce_device_remove(cpu);
-			return NOTIFY_BAD;
-		}
-		mce_reenable_cpu();
-		mce_start_timer(cpu, t);
-		break;
 	case CPU_DEAD:
 		mce_intel_hcpu_update(cpu);
 
@@ -2534,17 +2521,41 @@ mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
 			cmci_rediscover();
 		break;
 	case CPU_DOWN_PREPARE:
-		mce_disable_cpu();
-		del_timer_sync(t);
 
-		mce_threshold_remove_device(cpu);
-		mce_device_remove(cpu);
 		break;
 	}
 
 	return NOTIFY_OK;
 }
 
+static int mce_cpu_online(unsigned int cpu)
+{
+	struct timer_list *t = &per_cpu(mce_timer, cpu);
+	int ret;
+
+	mce_device_create(cpu);
+
+	ret = mce_threshold_create_device(cpu);
+	if (ret) {
+		mce_device_remove(cpu);
+		return ret;
+	}
+	mce_reenable_cpu();
+	mce_start_timer(cpu, t);
+	return 0;
+}
+
+static int mce_cpu_pre_down(unsigned int cpu)
+{
+	struct timer_list *t = &per_cpu(mce_timer, cpu);
+
+	mce_disable_cpu();
+	del_timer_sync(t);
+	mce_threshold_remove_device(cpu);
+	mce_device_remove(cpu);
+	return 0;
+}
+
 static struct notifier_block mce_cpu_notifier = {
 	.notifier_call = mce_cpu_callback,
 };
@@ -2569,8 +2580,8 @@ static __init void mce_init_banks(void)
 
 static __init int mcheck_init_device(void)
 {
+	enum cpuhp_state hp_online;
 	int err;
-	int i = 0;
 
 	if (!mce_available(&boot_cpu_data)) {
 		err = -EIO;
@@ -2588,21 +2599,13 @@ static __init int mcheck_init_device(void)
 	if (err)
 		goto err_out_mem;
 
-	cpu_notifier_register_begin();
-	for_each_online_cpu(i) {
-		err = mce_device_create(i);
-		if (err) {
-			/*
-			 * Register notifier anyway (and do not unreg it) so
-			 * that we don't leave undeleted timers, see notifier
-			 * callback above.
-			 */
-			__register_hotcpu_notifier(&mce_cpu_notifier);
-			cpu_notifier_register_done();
-			goto err_device_create;
-		}
-	}
+	err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/mce:online",
+				mce_cpu_online, mce_cpu_pre_down);
+	if (err < 0)
+		goto err_out_mem;
+	hp_online = err;
 
+	cpu_notifier_register_begin();
 	__register_hotcpu_notifier(&mce_cpu_notifier);
 	cpu_notifier_register_done();
 
@@ -2617,16 +2620,7 @@ static __init int mcheck_init_device(void)
 
 err_register:
 	unregister_syscore_ops(&mce_syscore_ops);
-
-err_device_create:
-	/*
-	 * We didn't keep track of which devices were created above, but
-	 * even if we had, the set of online cpus might have changed.
-	 * Play safe and remove for every possible cpu, since
-	 * mce_device_remove() will do the right thing.
-	 */
-	for_each_possible_cpu(i)
-		mce_device_remove(i);
+	cpuhp_remove_state(hp_online);
 
 err_out_mem:
 	free_cpumask_var(mce_device_initialized);

[toc] | [prev] | [next] | [standalone]

#1519241 — [PATCH 7/7] x86/mcheck: Move CPU_DEAD to hotplug state machine

This moves the last piece of the old hotplug notifier code in MCE to the
new hotplug state machine.

Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 42 ++++++++++++++--------------------------
 include/linux/cpuhotplug.h       |  1 +
 2 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 444177cafb65..d31e3a7d8866 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2501,26 +2501,14 @@ static void mce_reenable_cpu(void)
 	}
 }
 
-/* Get notified when a cpu comes on/off. Be hotplug friendly. */
-static int
-mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+static int mce_cpu_dead(unsigned int cpu)
 {
-	unsigned int cpu = (unsigned long)hcpu;
+	mce_intel_hcpu_update(cpu);
 
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_DEAD:
-		mce_intel_hcpu_update(cpu);
-
-		/* intentionally ignoring frozen here */
-		if (!(action & CPU_TASKS_FROZEN))
-			cmci_rediscover();
-		break;
-	case CPU_DOWN_PREPARE:
-
-		break;
-	}
-
-	return NOTIFY_OK;
+	/* intentionally ignoring frozen here */
+	if (!cpuhp_tasks_frozen)
+		cmci_rediscover();
+	return 0;
 }
 
 static int mce_cpu_online(unsigned int cpu)
@@ -2551,10 +2539,6 @@ static int mce_cpu_pre_down(unsigned int cpu)
 	return 0;
 }
 
-static struct notifier_block mce_cpu_notifier = {
-	.notifier_call = mce_cpu_callback,
-};
-
 static __init void mce_init_banks(void)
 {
 	int i;
@@ -2594,16 +2578,17 @@ static __init int mcheck_init_device(void)
 	if (err)
 		goto err_out_mem;
 
+	err = cpuhp_setup_state(CPUHP_X86_MCE_DEAD, "x86/mce:dead", NULL,
+				mce_cpu_dead);
+	if (err)
+		goto err_out_mem;
+
 	err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/mce:online",
 				mce_cpu_online, mce_cpu_pre_down);
 	if (err < 0)
-		goto err_out_mem;
+		goto err_out_online;
 	hp_online = err;
 
-	cpu_notifier_register_begin();
-	__register_hotcpu_notifier(&mce_cpu_notifier);
-	cpu_notifier_register_done();
-
 	register_syscore_ops(&mce_syscore_ops);
 
 	/* register character device /dev/mcelog */
@@ -2617,6 +2602,9 @@ static __init int mcheck_init_device(void)
 	unregister_syscore_ops(&mce_syscore_ops);
 	cpuhp_remove_state(hp_online);
 
+err_out_online:
+	cpuhp_remove_state(CPUHP_X86_MCE_DEAD);
+
 err_out_mem:
 	free_cpumask_var(mce_device_initialized);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 3410d83cc2e2..79b96f647d64 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -16,6 +16,7 @@ enum cpuhp_state {
 	CPUHP_PERF_SUPERH,
 	CPUHP_X86_HPET_DEAD,
 	CPUHP_X86_APB_DEAD,
+	CPUHP_X86_MCE_DEAD,
 	CPUHP_VIRT_NET_DEAD,
 	CPUHP_SLUB_DEAD,
 	CPUHP_MM_WRITEBACK_DEAD,
-- 
2.10.2

[toc] | [prev] | [next] | [standalone]

#1520005 — Re: [PATCH 7/7] x86/mcheck: Move CPU_DEAD to hotplug state machine

On Thu, Nov 10, 2016 at 06:44:47PM +0100, Sebastian Andrzej Siewior wrote:
> This moves the last piece of the old hotplug notifier code in MCE to the
> new hotplug state machine.
> 
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: linux-edac@vger.kernel.org
> Cc: x86@kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/x86/kernel/cpu/mcheck/mce.c | 42 ++++++++++++++--------------------------
>  include/linux/cpuhotplug.h       |  1 +
>  2 files changed, 16 insertions(+), 27 deletions(-)

Rest looks ok.

Acked-by: Borislav Petkov <bp@suse.de>

Thanks!

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

[toc] | [prev] | [next] | [standalone]

#1523306 — [tip:smp/hotplug] x86/mcheck: Move CPU_DEAD to hotplug state machine

Commit-ID:  0e285d36bd2bfee0b95433ccc9065c878164f5b2
Gitweb:     http://git.kernel.org/tip/0e285d36bd2bfee0b95433ccc9065c878164f5b2
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Thu, 10 Nov 2016 18:44:47 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 16 Nov 2016 09:34:18 +0100

x86/mcheck: Move CPU_DEAD to hotplug state machine

This moves the last piece of the old hotplug notifier code in MCE to the
new hotplug state machine.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Borislav Petkov <bp@alien8.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: rt@linutronix.de
Cc: linux-edac@vger.kernel.org
Link: http://lkml.kernel.org/r/20161110174447.11848-8-bigeasy@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 arch/x86/kernel/cpu/mcheck/mce.c | 42 ++++++++++++++--------------------------
 include/linux/cpuhotplug.h       |  1 +
 2 files changed, 16 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 78955f5..b888e2f 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2506,26 +2506,14 @@ static void mce_reenable_cpu(void)
 	}
 }
 
-/* Get notified when a cpu comes on/off. Be hotplug friendly. */
-static int
-mce_cpu_callback(struct notifier_block *nfb, unsigned long action, void *hcpu)
+static int mce_cpu_dead(unsigned int cpu)
 {
-	unsigned int cpu = (unsigned long)hcpu;
+	mce_intel_hcpu_update(cpu);
 
-	switch (action & ~CPU_TASKS_FROZEN) {
-	case CPU_DEAD:
-		mce_intel_hcpu_update(cpu);
-
-		/* intentionally ignoring frozen here */
-		if (!(action & CPU_TASKS_FROZEN))
-			cmci_rediscover();
-		break;
-	case CPU_DOWN_PREPARE:
-
-		break;
-	}
-
-	return NOTIFY_OK;
+	/* intentionally ignoring frozen here */
+	if (!cpuhp_tasks_frozen)
+		cmci_rediscover();
+	return 0;
 }
 
 static int mce_cpu_online(unsigned int cpu)
@@ -2556,10 +2544,6 @@ static int mce_cpu_pre_down(unsigned int cpu)
 	return 0;
 }
 
-static struct notifier_block mce_cpu_notifier = {
-	.notifier_call = mce_cpu_callback,
-};
-
 static __init void mce_init_banks(void)
 {
 	int i;
@@ -2599,16 +2583,17 @@ static __init int mcheck_init_device(void)
 	if (err)
 		goto err_out_mem;
 
+	err = cpuhp_setup_state(CPUHP_X86_MCE_DEAD, "x86/mce:dead", NULL,
+				mce_cpu_dead);
+	if (err)
+		goto err_out_mem;
+
 	err = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/mce:online",
 				mce_cpu_online, mce_cpu_pre_down);
 	if (err < 0)
-		goto err_out_mem;
+		goto err_out_online;
 	hp_online = err;
 
-	cpu_notifier_register_begin();
-	__register_hotcpu_notifier(&mce_cpu_notifier);
-	cpu_notifier_register_done();
-
 	register_syscore_ops(&mce_syscore_ops);
 
 	/* register character device /dev/mcelog */
@@ -2622,6 +2607,9 @@ err_register:
 	unregister_syscore_ops(&mce_syscore_ops);
 	cpuhp_remove_state(hp_online);
 
+err_out_online:
+	cpuhp_remove_state(CPUHP_X86_MCE_DEAD);
+
 err_out_mem:
 	free_cpumask_var(mce_device_initialized);
 
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 3410d83..79b96f6 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -16,6 +16,7 @@ enum cpuhp_state {
 	CPUHP_PERF_SUPERH,
 	CPUHP_X86_HPET_DEAD,
 	CPUHP_X86_APB_DEAD,
+	CPUHP_X86_MCE_DEAD,
 	CPUHP_VIRT_NET_DEAD,
 	CPUHP_SLUB_DEAD,
 	CPUHP_MM_WRITEBACK_DEAD,

[toc] | [prev] | [next] | [standalone]

#1519249 — x86/mcheck: convert to hotplug state engine, take #2

The fixed up version of patch 1-4 was already posted and are here just for
completeness.
Patches 5-7 are new and address Boris's concern not to init the MCE feature too
late on the boot CPU during the boot process.

Sebastian

[toc] | [prev] | [next] | [standalone]

#1518834

On 2016-11-10 11:22:22 [+0100], Thomas Gleixner wrote:
> 
> Why on earth do we enable MCE stuff in identify_cpu() ?
so it happens asap.

Boris' mail in this thread:
|Uh, hm, I'm not sure about this: so the issue I see with this is that
|the more we're delaying the enabling or MCE reporting - and especially
|setting CR4[MCE] - the more we're increasing the window where a MCE
|during early boot will cause a shutdown. (This is what happens if
|CR4[MCE]=0b).

> Thanks,
> 
> 	tglx

Sebastian

[toc] | [prev] | [next] | [standalone]

#1518835

On Thu, 10 Nov 2016, Sebastian Andrzej Siewior wrote:

> On 2016-11-09 18:37:23 [+0000], Luck, Tony wrote:
> > > That's why the hotplug callback mce_disable_cpu() doesn't fiddle with
> > > CR4 - it only clears the bits in MCi_CTL. And I think we should remain
> > > that way.
> > 
> > N.B. See vendor_disable_error_reporting() ... on Intel we don't clear MCi_CTL.
> 
> okay. So I had a discussion with Boris and we agreed that we don't do
> the init part in one place only the timer stuff will be moved.
> If I understand Tony correctly then we undo something in
> vendor_disable_error_reporting() (on !INTEL CPUs) that we enable in
> identify_cpu().

Why on earth do we enable MCE stuff in identify_cpu() ?

Thanks,

	tglx

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web