mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: John Allen <jallen@linux.vnet.ibm.com>
To: tglx@linutronix.de, linux-kernel@vger.kernel.org
Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>,
	Michael Bringmann <mwb@linux.vnet.ibm.com>,
	John Allen <jallen@linux.vnet.ibm.com>
Subject: [RFC] cpu/hotplug: Modify lock status before making cpu hotplug callbacks
Date: Wed, 7 Jun 2017 16:13:15 -0500	[thread overview]
Message-ID: <fbfb6488-8d65-85eb-e764-d18c9589a0cc@linux.vnet.ibm.com> (raw)

A deadlock has been observed during a cpu hot add on powerpc machines.

The situation is as follows:
First, in _cpu_up, we take the cpu_hotplug lock and towards the end of _cpu_up,
we make the cpu hotplug callbacks, one of which is arch_update_cpu_topology.
For most other architectures, making this call while the parent thread has the
cpu_hotplug lock seems harmless as most implementations of
arch_update_cpu_topology appear to be quite simple. However, the powerpc
implementation is significantly more complex and requires us to make a call to
stop_machine which in turn attempts to take the cpu_hotplug lock in
get_online_cpus. We then deadlock as the parent thread is waiting on the child
to complete and the child is waiting on the parent to release the lock.

This solution attempts to resolve the issue by incrementing the cpu_hotplug
refcount and releasing the cpu_hotplug mutex in _cpu_up so that the subsequent
call to get_online_cpus can take the mutex while other invocations of _cpu_up
are still prevented from executing as the refcount is non-zero. From my testing,
this seems to resolve the deadlock, but I'm not sure if there is a reason that
get_online_cpus *should* be locked out at this point.

Previous RFC for an alternate solution from Michael Bringmann with additional
details about the bug can be seen here:
https://patchwork.ozlabs.org/patch/771293/

Signed-off-by: John Allen <jallen@linux.vnet.ibm.com>
---
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 9ae6fbe..fc19437 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -776,6 +776,10 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
 	cpu_hotplug_begin();

 	cpuhp_tasks_frozen = tasks_frozen;
+	
+	cpu_hotplug.active_writer = NULL;
+	atomic_inc(&cpu_hotplug.refcount);
+	mutex_unlock(&cpu_hotplug.lock);

 	prev_state = st->state;
 	st->target = target;
@@ -810,6 +814,9 @@ static int __ref _cpu_down(unsigned int cpu, int tasks_frozen,
 		cpuhp_kick_ap_work(cpu);
 	}

+	put_online_cpus();
+
+	return ret;
 out:
 	cpu_hotplug_done();
 	return ret;
@@ -939,7 +946,14 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target)
 	 * responsible for bringing it up to the target state.
 	 */
 	target = min((int)target, CPUHP_BRINGUP_CPU);
+
+	cpu_hotplug.active_writer = NULL;
+	atomic_inc(&cpu_hotplug.refcount);
+	mutex_unlock(&cpu_hotplug.lock);
 	ret = cpuhp_up_callbacks(cpu, st, target);
+	put_online_cpus();
+
+	return ret;
 out:
 	cpu_hotplug_done();
 	return ret;

             reply	other threads:[~2017-06-07 21:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-07 21:13 John Allen [this message]
2017-06-12 14:59 ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fbfb6488-8d65-85eb-e764-d18c9589a0cc@linux.vnet.ibm.com \
    --to=jallen@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mwb@linux.vnet.ibm.com \
    --cc=nfont@linux.vnet.ibm.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®