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

Groups > linux.kernel > #1547397

Re: [GIT pull] smp/hotplug: Removal of notifiers

Show all headers | View raw

On Mon, 26 Dec 2016, Thomas Gleixner wrote:
> On Mon, 26 Dec 2016, Borislav Petkov wrote:
> > On Mon, Dec 26, 2016 at 07:21:44PM +0100, Thomas Gleixner wrote:
> > > Is there anything interesting error message before the BUG hits? I'll try
> > > to reproduce on a AMD box tomorrow.
> > 
> > Hmm, so lemme see if I see it correctly:
> > 
> > threshold_create_bank() does kobject_create_and_add(name, &dev->kobj);
> > and that dev thing is
> > 
> > 	struct device *dev = per_cpu(mce_device, cpu);
> > 
> > BUT(!), those mce_device per-CPU things get initialized in
> > 
> > mce_cpu_online()
> > |-> mce_device_create(cpu);
> > 
> > With a CONFIG_HOTPLUG_CPU=n .config that doesn't happen, right?
> > 
> > Oh, and I see what could've changed that:
> > 
> >   8c0eeac819c8 ("x86/mcheck: Move CPU_ONLINE and CPU_DOWN_PREPARE to hotplug state machine")
> > 
> > And before that, we did call mce_device_create(cpu) in
> > mcheck_init_device() which is a device initcall and not dependent on CPU
> > hotplug.
> > 
> > And frankly, flipping back to the for_each_online_cpu(i) is yucky as
> > hell but I don't see any other/better solution besides pulling up
> > mce_device_create() into mcheck_init_device()...
> 
> The hotplug callbacks are invoked even with HOTPLUG=n. So that's not the
> problem. I can reproduce it. Will post info once I understand it.

So the issue is indeed in that commit. I'm a moron.

But the amd mce code should be made more solid, because exactly that issue
can happen when something goes wrong in mcheck_init_device(). If that
happens then the device pointer is NULL and this code crashes. Adding the
NULL pointer check makes the machine survive despite the wreckage in the
hotplug code.

Fix below.

Thanks,

	tglx

8<---------------------------

arch/x86/kernel/cpu/mcheck/mce_amd.c |    3 +++
 kernel/cpu.c                         |    9 ++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -1182,6 +1182,9 @@ static int threshold_create_bank(unsigne
 	const char *name = get_name(bank, NULL);
 	int err = 0;
 
+	if (!dev)
+		return -ENODEV;
+
 	if (is_shared_bank(bank)) {
 		nb = node_to_amd_nb(amd_get_nb_id(cpu));
 
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1471,6 +1471,7 @@ int __cpuhp_setup_state(enum cpuhp_state
 			bool multi_instance)
 {
 	int cpu, ret = 0;
+	bool dynstate;
 
 	if (cpuhp_cb_check(state) || !name)
 		return -EINVAL;
@@ -1480,6 +1481,12 @@ int __cpuhp_setup_state(enum cpuhp_state
 	ret = cpuhp_store_callbacks(state, name, startup, teardown,
 				    multi_instance);
 
+	dynstate = state == CPUHP_AP_ONLINE_DYN;
+	if (ret > 0 && dynstate) {
+		state = ret;
+		ret = 0;
+	}
+
 	if (ret || !invoke || !startup)
 		goto out;
 
@@ -1508,7 +1515,7 @@ int __cpuhp_setup_state(enum cpuhp_state
 	 * If the requested state is CPUHP_AP_ONLINE_DYN, return the
 	 * dynamically allocated state in case of success.
 	 */
-	if (!ret && state == CPUHP_AP_ONLINE_DYN)
+	if (!ret && dynstate)
 		return state;
 	return ret;
 }

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [GIT pull] smp/hotplug: Removal of notifiers Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-12-26 08:50 +0100
  Re: [GIT pull] smp/hotplug: Removal of notifiers Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-12-26 12:10 +0100
    Re: [GIT pull] smp/hotplug: Removal of notifiers Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-12-26 16:50 +0100
      Re: [GIT pull] smp/hotplug: Removal of notifiers Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-26 18:40 +0100
        Re: [GIT pull] smp/hotplug: Removal of notifiers Thomas Gleixner <tglx@linutronix.de> - 2016-12-26 19:30 +0100
          Re: [GIT pull] smp/hotplug: Removal of notifiers Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2016-12-26 22:00 +0100
          Re: [GIT pull] smp/hotplug: Removal of notifiers Borislav Petkov <bp@alien8.de> - 2016-12-26 22:10 +0100
            Re: [GIT pull] smp/hotplug: Removal of notifiers Thomas Gleixner <tglx@linutronix.de> - 2016-12-26 22:20 +0100
              Re: [GIT pull] smp/hotplug: Removal of notifiers Thomas Gleixner <tglx@linutronix.de> - 2016-12-26 22:50 +0100

csiph-web