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

Groups > linux.kernel > #1625582 > unrolled thread

[patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem

Back to article view | Back to linux.kernel

Contents

  [patch V2 00/24] cpu/hotplug: Convert get_online_cpus() to a  percpu_rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 21:50 +0200
    [patch V2 03/24] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 21:50 +0200
      [tip:smp/hotplug] padata: Make padata_alloc() static tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 20/24] perf/core: Remove redundant get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] perf/core: Remove redundant get_online_cpus() tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 22/24] jump_label: Provide static_key_slow_inc_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
    [patch V2 15/24] x86/perf: Drop EXPORT of perf_check_microcode Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] x86/perf: Drop EXPORT of perf_check_microcode tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 09/24] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 23/24] perf: Avoid cpu_hotplug_lock r-r recursion Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
    [patch V2 16/24] perf/x86/intel: Drop get_online_cpus() in  intel_snb_check_microcode() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] perf/x86/intel: Drop get_online_cpus() in  intel_snb_check_microcode() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 24/24] cpu/hotplug: Convert hotplug locking to percpu rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] cpu/hotplug: Convert hotplug locking to percpu  rwsem tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 05/24] x86/mtrr: Remove get_online_cpus() from  mtrr_save_state() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] x86/mtrr: Remove get_online_cpus() from  mtrr_save_state() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 14/24] cpu/hotplug: Use stop_machine_cpuslocked() in  takedown_cpu() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] cpu/hotplug: Use stop_machine_cpuslocked() in  takedown_cpu() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 18/24] PCI: Replace the racy recursion prevention Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] PCI: Replace the racy recursion prevention tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 21/24] jump_label: Pull get_online_cpus() into generic code Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
    [patch V2 12/24] s390/kernel: Use stop_machine_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] s390/kernel: Use stop_machine_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 06/24] cpufreq: Use cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] cpufreq: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 19/24] ACPI/processor: Use cpu_hotplug_disable() instead of  get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] ACPI/processor: Use cpu_hotplug_disable() instead  of get_online_cpus() tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 08/24] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
      Re: [patch V2 08/24] hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked() Mathieu Poirier <mathieu.poirier@linaro.org> - 2017-04-20 17:20 +0200
      Re: [patch V2 08/24] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() Mathieu Poirier <mathieu.poirier@linaro.org> - 2017-04-20 17:40 +0200
    [patch V2 17/24] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-04-20 13:40 +0200
    [patch V2 13/24] powerpc/powernv: Use stop_machine_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] powerpc/powernv: Use stop_machine_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 11/24] ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      Re: [patch V2 11/24] ARM/hw_breakpoint: Use  cpuhp_setup_state_cpuslocked() Mark Rutland <mark.rutland@arm.com> - 2017-04-19 20:00 +0200
        Re: [patch V2 11/24] ARM/hw_breakpoint: Use  cpuhp_setup_state_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-19 20:30 +0200
      [tip:smp/hotplug] ARM/hw_breakpoint: Use  cpuhp_setup_state_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 07/24] KVM/PPC/Book3S HV: Use  cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] KVM/PPC/Book3S HV: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200
    [patch V2 10/24] perf/x86/intel/cqm: Use  cpuhp_setup_state_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
      [tip:smp/hotplug] perf/x86/intel/cqm: Use  cpuhp_setup_state_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-04-20 13:30 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1627386 — [tip:smp/hotplug] PCI: Replace the racy recursion prevention

Commit-ID:  81edd60135c58f893bcc07f8e633ea8efe72084d
Gitweb:     http://git.kernel.org/tip/81edd60135c58f893bcc07f8e633ea8efe72084d
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:05:00 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:56 +0200

PCI: Replace the racy recursion prevention

pci_call_probe() can called recursively when a physcial function is probed
and the probing creates virtual functions, which are populated via
pci_bus_add_device() which in turn can end up calling pci_call_probe()
again.

The code has an interesting way to prevent recursing into the workqueue
code.  That's accomplished by a check whether the current task runs already
on the numa node which is associated with the device.

While that works to prevent the recursion into the workqueue code, it's
racy versus normal execution as there is no guarantee that the node does
not vanish after the check.

There is another issue with this code. It dereferences cpumask_of_node()
unconditionally without checking whether the node is available.

Make the detection reliable by:

 - Mark a probed device as 'is_probed' in pci_call_probe()
 
 - Check in pci_call_probe for a virtual function. If it's a virtual
   function and the associated physical function device is marked
   'is_probed' then this is a recursive call, so the call can be invoked in
   the calling context.

 - Add a check whether the node is online before dereferencing it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: linux-pci@vger.kernel.org
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/20170418170553.885818800@linutronix.de

---
 drivers/pci/pci-driver.c | 47 +++++++++++++++++++++++++----------------------
 include/linux/pci.h      |  1 +
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f00e4d9..f84d2a8 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -320,10 +320,19 @@ static long local_pci_probe(void *_ddi)
 	return 0;
 }
 
+static bool pci_physfn_is_probed(struct pci_dev *dev)
+{
+#ifdef CONFIG_PCI_IOV
+	return dev->is_virtfn && dev->physfn->is_probed;
+#else
+	return false;
+#endif
+}
+
 static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 			  const struct pci_device_id *id)
 {
-	int error, node;
+	int error, node, cpu;
 	struct drv_dev_and_id ddi = { drv, dev, id };
 
 	/*
@@ -332,33 +341,27 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 	 * on the right node.
 	 */
 	node = dev_to_node(&dev->dev);
+	dev->is_probed = 1;
+
+	cpu_hotplug_disable();
 
 	/*
-	 * On NUMA systems, we are likely to call a PF probe function using
-	 * work_on_cpu().  If that probe calls pci_enable_sriov() (which
-	 * adds the VF devices via pci_bus_add_device()), we may re-enter
-	 * this function to call the VF probe function.  Calling
-	 * work_on_cpu() again will cause a lockdep warning.  Since VFs are
-	 * always on the same node as the PF, we can work around this by
-	 * avoiding work_on_cpu() when we're already on the correct node.
-	 *
-	 * Preemption is enabled, so it's theoretically unsafe to use
-	 * numa_node_id(), but even if we run the probe function on the
-	 * wrong node, it should be functionally correct.
+	 * Prevent nesting work_on_cpu() for the case where a Virtual Function
+	 * device is probed from work_on_cpu() of the Physical device.
 	 */
-	if (node >= 0 && node != numa_node_id()) {
-		int cpu;
-
-		cpu_hotplug_disable();
+	if (node < 0 || node >= MAX_NUMNODES || !node_online(node) ||
+	    pci_physfn_is_probed(dev))
+		cpu = nr_cpu_ids;
+	else
 		cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
-		if (cpu < nr_cpu_ids)
-			error = work_on_cpu(cpu, local_pci_probe, &ddi);
-		else
-			error = local_pci_probe(&ddi);
-		cpu_hotplug_enable();
-	} else
+
+	if (cpu < nr_cpu_ids)
+		error = work_on_cpu(cpu, local_pci_probe, &ddi);
+	else
 		error = local_pci_probe(&ddi);
 
+	dev->is_probed = 0;
+	cpu_hotplug_enable();
 	return error;
 }
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index eb3da1a..3efe145 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -365,6 +365,7 @@ struct pci_dev {
 	unsigned int	irq_managed:1;
 	unsigned int	has_secondary_link:1;
 	unsigned int	non_compliant_bars:1;	/* broken BARs; ignore them */
+	unsigned int	is_probed:1;		/* device probing in progress */
 	pci_dev_flags_t dev_flags;
 	atomic_t	enable_cnt;	/* pci_enable_device has been called */
 

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

#1625596 — [patch V2 21/24] jump_label: Pull get_online_cpus() into generic code

This change does two things:

- it moves the get_online_cpus() call into generic code, with the aim of
  later providing some static_key ops that avoid it.

- as a side effect it inverts the lock order between cpu_hotplug_lock and
  jump_label_mutex.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: jbaron@akamai.com
Cc: bigeasy@linutronix.de
Cc: rostedt@goodmis.org
Link: http://lkml.kernel.org/r/20170418103422.590118425@infradead.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
---
 arch/mips/kernel/jump_label.c  |    2 --
 arch/sparc/kernel/jump_label.c |    2 --
 arch/tile/kernel/jump_label.c  |    2 --
 arch/x86/kernel/jump_label.c   |    2 --
 kernel/jump_label.c            |   14 ++++++++++++++
 5 files changed, 14 insertions(+), 8 deletions(-)

--- a/arch/mips/kernel/jump_label.c
+++ b/arch/mips/kernel/jump_label.c
@@ -58,7 +58,6 @@ void arch_jump_label_transform(struct ju
 		insn.word = 0; /* nop */
 	}
 
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 	if (IS_ENABLED(CONFIG_CPU_MICROMIPS)) {
 		insn_p->halfword[0] = insn.word >> 16;
@@ -70,7 +69,6 @@ void arch_jump_label_transform(struct ju
 			   (unsigned long)insn_p + sizeof(*insn_p));
 
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 #endif /* HAVE_JUMP_LABEL */
--- a/arch/sparc/kernel/jump_label.c
+++ b/arch/sparc/kernel/jump_label.c
@@ -41,12 +41,10 @@ void arch_jump_label_transform(struct ju
 		val = 0x01000000;
 	}
 
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 	*insn = val;
 	flushi(insn);
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 #endif
--- a/arch/tile/kernel/jump_label.c
+++ b/arch/tile/kernel/jump_label.c
@@ -45,14 +45,12 @@ static void __jump_label_transform(struc
 void arch_jump_label_transform(struct jump_entry *e,
 				enum jump_label_type type)
 {
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 
 	__jump_label_transform(e, type);
 	flush_icache_range(e->code, e->code + sizeof(tilegx_bundle_bits));
 
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 __init_or_module void arch_jump_label_transform_static(struct jump_entry *e,
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -105,11 +105,9 @@ static void __jump_label_transform(struc
 void arch_jump_label_transform(struct jump_entry *entry,
 			       enum jump_label_type type)
 {
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 	__jump_label_transform(entry, type, NULL, 0);
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 static enum {
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -15,6 +15,7 @@
 #include <linux/static_key.h>
 #include <linux/jump_label_ratelimit.h>
 #include <linux/bug.h>
+#include <linux/cpu.h>
 
 #ifdef HAVE_JUMP_LABEL
 
@@ -124,6 +125,12 @@ void static_key_slow_inc(struct static_k
 			return;
 	}
 
+	/*
+	 * A number of architectures need to synchronize I$ across
+	 * the all CPUs, for that to be serialized against CPU hot-plug
+	 * we need to avoid CPUs coming online.
+	 */
+	get_online_cpus();
 	jump_label_lock();
 	if (atomic_read(&key->enabled) == 0) {
 		atomic_set(&key->enabled, -1);
@@ -133,6 +140,7 @@ void static_key_slow_inc(struct static_k
 		atomic_inc(&key->enabled);
 	}
 	jump_label_unlock();
+	put_online_cpus();
 }
 EXPORT_SYMBOL_GPL(static_key_slow_inc);
 
@@ -146,6 +154,7 @@ static void __static_key_slow_dec(struct
 	 * returns is unbalanced, because all other static_key_slow_inc()
 	 * instances block while the update is in progress.
 	 */
+	get_online_cpus();
 	if (!atomic_dec_and_mutex_lock(&key->enabled, &jump_label_mutex)) {
 		WARN(atomic_read(&key->enabled) < 0,
 		     "jump label: negative count!\n");
@@ -159,6 +168,7 @@ static void __static_key_slow_dec(struct
 		jump_label_update(key);
 	}
 	jump_label_unlock();
+	put_online_cpus();
 }
 
 static void jump_label_update_timeout(struct work_struct *work)
@@ -592,6 +602,10 @@ jump_label_module_notify(struct notifier
 
 	switch (val) {
 	case MODULE_STATE_COMING:
+		/*
+		 * XXX do we need get_online_cpus() ?  the module isn't
+		 * executable yet, so nothing should be looking at our code.
+		 */
 		jump_label_lock();
 		ret = jump_label_add_module(mod);
 		if (ret) {

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

#1625597 — [patch V2 12/24] s390/kernel: Use stop_machine_cpuslocked()

stp_work_fn() holds get_online_cpus() while invoking stop_machine().

stop_machine() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use stop_machine_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: linux-s390@vger.kernel.org

---
 arch/s390/kernel/time.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -636,7 +636,7 @@ static void stp_work_fn(struct work_stru
 	memset(&stp_sync, 0, sizeof(stp_sync));
 	get_online_cpus();
 	atomic_set(&stp_sync.cpus, num_online_cpus() - 1);
-	stop_machine(stp_sync_clock, &stp_sync, cpu_online_mask);
+	stop_machine_cpuslocked(stp_sync_clock, &stp_sync, cpu_online_mask);
 	put_online_cpus();
 
 	if (!check_sync_clock())

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

#1627359 — [tip:smp/hotplug] s390/kernel: Use stop_machine_cpuslocked()

Commit-ID:  35cc1ac5c24cb497b3b15a9383ff098e4a8bf03c
Gitweb:     http://git.kernel.org/tip/35cc1ac5c24cb497b3b15a9383ff098e4a8bf03c
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:54 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:54 +0200

s390/kernel: Use stop_machine_cpuslocked()

stp_work_fn() holds get_online_cpus() while invoking stop_machine().

stop_machine() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use stop_machine_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-s390@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20170418170553.363283884@linutronix.de

---
 arch/s390/kernel/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index c31da46..3aa8acb 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -636,7 +636,7 @@ static void stp_work_fn(struct work_struct *work)
 	memset(&stp_sync, 0, sizeof(stp_sync));
 	get_online_cpus();
 	atomic_set(&stp_sync.cpus, num_online_cpus() - 1);
-	stop_machine(stp_sync_clock, &stp_sync, cpu_online_mask);
+	stop_machine_cpuslocked(stp_sync_clock, &stp_sync, cpu_online_mask);
 	put_online_cpus();
 
 	if (!check_sync_clock())

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

#1625598 — [patch V2 06/24] cpufreq: Use cpuhp_setup_state_nocalls_cpuslocked()

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

cpufreq holds get_online_cpus() while invoking cpuhp_setup_state_nocalls()
to make subsys_interface_register() and the registration of hotplug calls
atomic versus cpu hotplug.

cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
correct, but prevents the conversion of the hotplug locking to a percpu
rwsem.

Use cpuhp_setup/remove_state_nocalls_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org

---
 drivers/cpufreq/cpufreq.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2473,9 +2473,10 @@ int cpufreq_register_driver(struct cpufr
 		goto err_if_unreg;
 	}
 
-	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "cpufreq:online",
-					cpuhp_cpufreq_online,
-					cpuhp_cpufreq_offline);
+	ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+						   "cpufreq:online",
+						   cpuhp_cpufreq_online,
+						   cpuhp_cpufreq_offline);
 	if (ret < 0)
 		goto err_if_unreg;
 	hp_online = ret;
@@ -2519,7 +2520,7 @@ int cpufreq_unregister_driver(struct cpu
 	get_online_cpus();
 	subsys_interface_unregister(&cpufreq_interface);
 	remove_boost_sysfs_file();
-	cpuhp_remove_state_nocalls(hp_online);
+	cpuhp_remove_state_nocalls_cpuslocked(hp_online);
 
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
 

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

#1627366 — [tip:smp/hotplug] cpufreq: Use cpuhp_setup_state_nocalls_cpuslocked()

Commit-ID:  6d7d2f1f9e6e0c2d6fb9de82ad5fbc3aed115829
Gitweb:     http://git.kernel.org/tip/6d7d2f1f9e6e0c2d6fb9de82ad5fbc3aed115829
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:48 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:52 +0200

cpufreq: Use cpuhp_setup_state_nocalls_cpuslocked()

cpufreq holds get_online_cpus() while invoking cpuhp_setup_state_nocalls()
to make subsys_interface_register() and the registration of hotplug calls
atomic versus cpu hotplug.

cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
correct, but prevents the conversion of the hotplug locking to a percpu
rwsem.

Use cpuhp_setup/remove_state_nocalls_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/20170418170552.865966021@linutronix.de

---
 drivers/cpufreq/cpufreq.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 0e3f649..67ef7ce 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2473,9 +2473,10 @@ int cpufreq_register_driver(struct cpufreq_driver *driver_data)
 		goto err_if_unreg;
 	}
 
-	ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, "cpufreq:online",
-					cpuhp_cpufreq_online,
-					cpuhp_cpufreq_offline);
+	ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+						   "cpufreq:online",
+						   cpuhp_cpufreq_online,
+						   cpuhp_cpufreq_offline);
 	if (ret < 0)
 		goto err_if_unreg;
 	hp_online = ret;
@@ -2519,7 +2520,7 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
 	get_online_cpus();
 	subsys_interface_unregister(&cpufreq_interface);
 	remove_boost_sysfs_file();
-	cpuhp_remove_state_nocalls(hp_online);
+	cpuhp_remove_state_nocalls_cpuslocked(hp_online);
 
 	write_lock_irqsave(&cpufreq_driver_lock, flags);
 

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

#1625599 — [patch V2 19/24] ACPI/processor: Use cpu_hotplug_disable() instead of get_online_cpus()

Converting the hotplug locking, i.e. get_online_cpus(), to a percpu rwsem
unearthed a circular lock dependency which was hidden from lockdep due to
the lockdep annotation of get_online_cpus() which prevents lockdep from
creating full dependency chains.

CPU0                    CPU1
----                    ----
lock((&wfc.work));
                         lock(cpu_hotplug_lock.rw_sem);
                         lock((&wfc.work));
lock(cpu_hotplug_lock.rw_sem);

This dependency is established via acpi_processor_start() which calls into
the work queue code. And the work queue code establishes the reverse
dependency.

This is not a problem of get_online_cpus() recursion, it's a possible
deadlock undetected by lockdep so far.

The cure is to use cpu_hotplug_disable() instead of get_online_cpus() to
protect the probing from acpi_processor_start().

There is a side effect to this: cpu_hotplug_disable() makes a concurrent
cpu hotplug attempt via the sysfs interfaces fail with -EBUSY, but that
probing usually happens during the boot process where no interaction is
possible. Any later invocations are infrequent enough and concurrent
hotplug attempts are so unlikely that the danger of user space visible
regressions is very close to zero. Anyway, thats preferrable over a real
deadlock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org
---
 drivers/acpi/processor_driver.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -268,9 +268,9 @@ static int acpi_processor_start(struct d
 		return -ENODEV;
 
 	/* Protect against concurrent CPU hotplug operations */
-	get_online_cpus();
+	cpu_hotplug_disable();
 	ret = __acpi_processor_start(device);
-	put_online_cpus();
+	cpu_hotplug_enable();
 	return ret;
 }
 

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

#1627382 — [tip:smp/hotplug] ACPI/processor: Use cpu_hotplug_disable() instead of get_online_cpus()

Commit-ID:  16ab1d27c5040ae369c4652e6ce0a9a82bdbb6c5
Gitweb:     http://git.kernel.org/tip/16ab1d27c5040ae369c4652e6ce0a9a82bdbb6c5
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:05:01 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:56 +0200

ACPI/processor: Use cpu_hotplug_disable() instead of get_online_cpus()

Converting the hotplug locking, i.e. get_online_cpus(), to a percpu rwsem
unearthed a circular lock dependency which was hidden from lockdep due to
the lockdep annotation of get_online_cpus() which prevents lockdep from
creating full dependency chains.

CPU0                    CPU1
----                    ----
lock((&wfc.work));
                         lock(cpu_hotplug_lock.rw_sem);
                         lock((&wfc.work));
lock(cpu_hotplug_lock.rw_sem);

This dependency is established via acpi_processor_start() which calls into
the work queue code. And the work queue code establishes the reverse
dependency.

This is not a problem of get_online_cpus() recursion, it's a possible
deadlock undetected by lockdep so far.

The cure is to use cpu_hotplug_disable() instead of get_online_cpus() to
protect the probing from acpi_processor_start().

There is a side effect to this: cpu_hotplug_disable() makes a concurrent
cpu hotplug attempt via the sysfs interfaces fail with -EBUSY, but that
probing usually happens during the boot process where no interaction is
possible. Any later invocations are infrequent enough and concurrent
hotplug attempts are so unlikely that the danger of user space visible
regressions is very close to zero. Anyway, thats preferrable over a real
deadlock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-acpi@vger.kernel.org
Cc: Len Brown <lenb@kernel.org>
Link: http://lkml.kernel.org/r/20170418170553.964555153@linutronix.de
---
 drivers/acpi/processor_driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 8697a82..591d1dd 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -268,9 +268,9 @@ static int acpi_processor_start(struct device *dev)
 		return -ENODEV;
 
 	/* Protect against concurrent CPU hotplug operations */
-	get_online_cpus();
+	cpu_hotplug_disable();
 	ret = __acpi_processor_start(device);
-	put_online_cpus();
+	cpu_hotplug_enable();
 	return ret;
 }
 

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

#1625600 — [patch V2 08/24] hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked()

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

etm_probe() holds get_online_cpus() while invoking
cpuhp_setup_state_nocalls().

cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
correct, but prevents the conversion of the hotplug locking to a percpu
rwsem.

Use cpuhp_setup_state_nocalls_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: linux-arm-kernel@lists.infradead.org

---
 drivers/hwtracing/coresight/coresight-etm3x.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -803,12 +803,12 @@ static int etm_probe(struct amba_device
 		dev_err(dev, "ETM arch init failed\n");
 
 	if (!etm_count++) {
-		cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					  "arm/coresight:starting",
-					  etm_starting_cpu, etm_dying_cpu);
-		ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
-						"arm/coresight:online",
-						etm_online_cpu, NULL);
+		cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
+						     "arm/coresight:starting",
+						     etm_starting_cpu, etm_dying_cpu);
+		ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+							   "arm/coresight:online",
+							   etm_online_cpu, NULL);
 		if (ret < 0)
 			goto err_arch_supported;
 		hp_online = ret;

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

#1627370 — [tip:smp/hotplug] hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked()

Commit-ID:  c8fbca5abedcbf4212a34b7d3aef9c2adc44d36e
Gitweb:     http://git.kernel.org/tip/c8fbca5abedcbf4212a34b7d3aef9c2adc44d36e
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:50 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:52 +0200

hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked()

etm_probe() holds get_online_cpus() while invoking
cpuhp_setup_state_nocalls().

cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
correct, but prevents the conversion of the hotplug locking to a percpu
rwsem.

Use cpuhp_setup_state_nocalls_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170418170553.026838125@linutronix.de

---
 drivers/hwtracing/coresight/coresight-etm3x.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index a51b6b6..c9a0a75 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -803,12 +803,12 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
 		dev_err(dev, "ETM arch init failed\n");
 
 	if (!etm_count++) {
-		cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					  "arm/coresight:starting",
-					  etm_starting_cpu, etm_dying_cpu);
-		ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
-						"arm/coresight:online",
-						etm_online_cpu, NULL);
+		cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
+						     "arm/coresight:starting",
+						     etm_starting_cpu, etm_dying_cpu);
+		ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+							   "arm/coresight:online",
+							   etm_online_cpu, NULL);
 		if (ret < 0)
 			goto err_arch_supported;
 		hp_online = ret;

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

#1627580 — Re: [patch V2 08/24] hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked()

On 18 April 2017 at 11:04, Thomas Gleixner <tglx@linutronix.de> wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> etm_probe() holds get_online_cpus() while invoking
> cpuhp_setup_state_nocalls().
>
> cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
> correct, but prevents the conversion of the hotplug locking to a percpu
> rwsem.
>
> Use cpuhp_setup_state_nocalls_cpuslocked() to avoid the nested call.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -803,12 +803,12 @@ static int etm_probe(struct amba_device
>                 dev_err(dev, "ETM arch init failed\n");
>
>         if (!etm_count++) {
> -               cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
> -                                         "arm/coresight:starting",
> -                                         etm_starting_cpu, etm_dying_cpu);
> -               ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> -                                               "arm/coresight:online",
> -                                               etm_online_cpu, NULL);
> +               cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
> +                                                    "arm/coresight:starting",
> +                                                    etm_starting_cpu, etm_dying_cpu);
> +               ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
> +                                                          "arm/coresight:online",
> +                                                          etm_online_cpu, NULL);
>                 if (ret < 0)
>                         goto err_arch_supported;
>                 hp_online = ret;
>

Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>

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

#1627597 — Re: [patch V2 08/24] hwtracing/coresight-etm3x: Use cpuhp_setup_state_nocalls_cpuslocked()

On Tue, Apr 18, 2017 at 07:04:50PM +0200, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> etm_probe() holds get_online_cpus() while invoking
> cpuhp_setup_state_nocalls().
> 
> cpuhp_setup_state_nocalls() invokes get_online_cpus() as well. This is
> correct, but prevents the conversion of the hotplug locking to a percpu
> rwsem.
> 
> Use cpuhp_setup_state_nocalls_cpuslocked() to avoid the nested call.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: linux-arm-kernel@lists.infradead.org
> 
> ---
>  drivers/hwtracing/coresight/coresight-etm3x.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -803,12 +803,12 @@ static int etm_probe(struct amba_device
>  		dev_err(dev, "ETM arch init failed\n");
>  
>  	if (!etm_count++) {
> -		cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
> -					  "arm/coresight:starting",
> -					  etm_starting_cpu, etm_dying_cpu);
> -		ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> -						"arm/coresight:online",
> -						etm_online_cpu, NULL);
> +		cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
> +						     "arm/coresight:starting",
> +						     etm_starting_cpu, etm_dying_cpu);
> +		ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
> +							   "arm/coresight:online",
> +							   etm_online_cpu, NULL);

Checkpatch complains about lines being over 80 characters long.  It is better to
keep the above or do as follow and make checkpatch happy?

                cpuhp_setup_state_nocalls_cpuslocked(
                                        PUHP_AP_ARM_CORESIGHT_STARTING,
                                        "arm/coresight:starting",
                                        etm_starting_cpu, etm_dying_cpu);
                ret = cpuhp_setup_state_nocalls_cpuslocked(
                                        CPUHP_AP_ONLINE_DYN,
                                        "arm/coresight:online",
                                        etm_online_cpu, NULL);

Same for coresight-etm4x.c

Thanks,
Mathieu

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

#1625601 — [patch V2 17/24] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

Converting the hotplug locking, i.e. get_online_cpus(), to a percpu rwsem
unearthed a circular lock dependency which was hidden from lockdep due to
the lockdep annotation of get_online_cpus() which prevents lockdep from
creating full dependency chains. There are several variants of this. And
example is:

Chain exists of:

cpu_hotplug_lock.rw_sem --> drm_global_mutex --> &item->mutex

CPU0                    CPU1
----                    ----
lock(&item->mutex);
                        lock(drm_global_mutex);
                        lock(&item->mutex);
lock(cpu_hotplug_lock.rw_sem);

because there are dependencies through workqueues. The call chain is:

	get_online_cpus
	apply_workqueue_attrs
	__alloc_workqueue_key
	ttm_mem_global_init
	ast_ttm_mem_global_init
	drm_global_item_ref
	ast_mm_init
	ast_driver_load
	drm_dev_register
	drm_get_pci_dev
	ast_pci_probe
	local_pci_probe
	work_for_cpu_fn
	process_one_work
	worker_thread

This is not a problem of get_online_cpus() recursion, it's a possible
deadlock undetected by lockdep so far.

The cure is to use cpu_hotplug_disable() instead of get_online_cpus() to
protect the PCI probing.

There is a side effect to this: cpu_hotplug_disable() makes a concurrent
cpu hotplug attempt via the sysfs interfaces fail with -EBUSY, but PCI
probing usually happens during the boot process where no interaction is
possible. Any later invocations are infrequent enough and concurrent
hotplug attempts are so unlikely that the danger of user space visible
regressions is very close to zero. Anyway, thats preferrable over a real
deadlock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/pci/pci-driver.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -349,13 +349,13 @@ static int pci_call_probe(struct pci_dri
 	if (node >= 0 && node != numa_node_id()) {
 		int cpu;
 
-		get_online_cpus();
+		cpu_hotplug_disable();
 		cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
 		if (cpu < nr_cpu_ids)
 			error = work_on_cpu(cpu, local_pci_probe, &ddi);
 		else
 			error = local_pci_probe(&ddi);
-		put_online_cpus();
+		cpu_hotplug_enable();
 	} else
 		error = local_pci_probe(&ddi);
 

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

#1627385 — [tip:smp/hotplug] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

Commit-ID:  b4d1673371196dd9aebdd2f61d946165c777b931
Gitweb:     http://git.kernel.org/tip/b4d1673371196dd9aebdd2f61d946165c777b931
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:59 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:55 +0200

PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

Converting the hotplug locking, i.e. get_online_cpus(), to a percpu rwsem
unearthed a circular lock dependency which was hidden from lockdep due to
the lockdep annotation of get_online_cpus() which prevents lockdep from
creating full dependency chains. There are several variants of this. And
example is:

Chain exists of:

cpu_hotplug_lock.rw_sem --> drm_global_mutex --> &item->mutex

CPU0                    CPU1
----                    ----
lock(&item->mutex);
                        lock(drm_global_mutex);
                        lock(&item->mutex);
lock(cpu_hotplug_lock.rw_sem);

because there are dependencies through workqueues. The call chain is:

	get_online_cpus
	apply_workqueue_attrs
	__alloc_workqueue_key
	ttm_mem_global_init
	ast_ttm_mem_global_init
	drm_global_item_ref
	ast_mm_init
	ast_driver_load
	drm_dev_register
	drm_get_pci_dev
	ast_pci_probe
	local_pci_probe
	work_for_cpu_fn
	process_one_work
	worker_thread

This is not a problem of get_online_cpus() recursion, it's a possible
deadlock undetected by lockdep so far.

The cure is to use cpu_hotplug_disable() instead of get_online_cpus() to
protect the PCI probing.

There is a side effect to this: cpu_hotplug_disable() makes a concurrent
cpu hotplug attempt via the sysfs interfaces fail with -EBUSY, but PCI
probing usually happens during the boot process where no interaction is
possible. Any later invocations are infrequent enough and concurrent
hotplug attempts are so unlikely that the danger of user space visible
regressions is very close to zero. Anyway, thats preferrable over a real
deadlock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-pci@vger.kernel.org
Link: http://lkml.kernel.org/r/20170418170553.806707929@linutronix.de
---
 drivers/pci/pci-driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index afa7271..f00e4d9 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -349,13 +349,13 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 	if (node >= 0 && node != numa_node_id()) {
 		int cpu;
 
-		get_online_cpus();
+		cpu_hotplug_disable();
 		cpu = cpumask_any_and(cpumask_of_node(node), cpu_online_mask);
 		if (cpu < nr_cpu_ids)
 			error = work_on_cpu(cpu, local_pci_probe, &ddi);
 		else
 			error = local_pci_probe(&ddi);
-		put_online_cpus();
+		cpu_hotplug_enable();
 	} else
 		error = local_pci_probe(&ddi);
 

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

#1625602 — [patch V2 13/24] powerpc/powernv: Use stop_machine_cpuslocked()

set_subcores_per_core() holds get_online_cpus() while invoking stop_machine().

stop_machine() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use stop_machine_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org

---
 arch/powerpc/platforms/powernv/subcore.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/arch/powerpc/platforms/powernv/subcore.c
+++ b/arch/powerpc/platforms/powernv/subcore.c
@@ -356,7 +356,8 @@ static int set_subcores_per_core(int new
 	/* Ensure state is consistent before we call the other cpus */
 	mb();
 
-	stop_machine(cpu_update_split_mode, &new_mode, cpu_online_mask);
+	stop_machine_cpuslocked(cpu_update_split_mode, &new_mode,
+				cpu_online_mask);
 
 	put_online_cpus();
 

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

#1627361 — [tip:smp/hotplug] powerpc/powernv: Use stop_machine_cpuslocked()

Commit-ID:  6e9e61778f976b52fbdeb315850ee6eb4695a1e5
Gitweb:     http://git.kernel.org/tip/6e9e61778f976b52fbdeb315850ee6eb4695a1e5
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:55 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:54 +0200

powerpc/powernv: Use stop_machine_cpuslocked()

set_subcores_per_core() holds get_online_cpus() while invoking stop_machine().

stop_machine() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use stop_machine_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/20170418170553.447314339@linutronix.de

---
 arch/powerpc/platforms/powernv/subcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/subcore.c b/arch/powerpc/platforms/powernv/subcore.c
index 0babef1..67cbf45 100644
--- a/arch/powerpc/platforms/powernv/subcore.c
+++ b/arch/powerpc/platforms/powernv/subcore.c
@@ -356,7 +356,8 @@ static int set_subcores_per_core(int new_mode)
 	/* Ensure state is consistent before we call the other cpus */
 	mb();
 
-	stop_machine(cpu_update_split_mode, &new_mode, cpu_online_mask);
+	stop_machine_cpuslocked(cpu_update_split_mode, &new_mode,
+				cpu_online_mask);
 
 	put_online_cpus();
 

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

#1625603 — [patch V2 11/24] ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked()

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

arch_hw_breakpoint_init() holds get_online_cpus() while registerring the
hotplug callbacks.

cpuhp_setup_state() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use cpuhp_setup_state_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org

---
 arch/arm/kernel/hw_breakpoint.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -1098,8 +1098,9 @@ static int __init arch_hw_breakpoint_ini
 	 * assume that a halting debugger will leave the world in a nice state
 	 * for us.
 	 */
-	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
-				dbg_reset_online, NULL);
+	ret = cpuhp_setup_state_cpuslocked(CPUHP_AP_ONLINE_DYN,
+					   "arm/hw_breakpoint:online",
+					   dbg_reset_online, NULL);
 	unregister_undef_hook(&debug_reg_hook);
 	if (WARN_ON(ret < 0) || !cpumask_empty(&debug_err_mask)) {
 		core_num_brps = 0;

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

#1626659 — Re: [patch V2 11/24] ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked()

Hi,

On Tue, Apr 18, 2017 at 07:04:53PM +0200, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> arch_hw_breakpoint_init() holds get_online_cpus() while registerring the
> hotplug callbacks.
> 
> cpuhp_setup_state() invokes get_online_cpus() as well. This is correct, but
> prevents the conversion of the hotplug locking to a percpu rwsem.
> 
> Use cpuhp_setup_state_cpuslocked() to avoid the nested call.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> 
> ---
>  arch/arm/kernel/hw_breakpoint.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --- a/arch/arm/kernel/hw_breakpoint.c
> +++ b/arch/arm/kernel/hw_breakpoint.c
> @@ -1098,8 +1098,9 @@ static int __init arch_hw_breakpoint_ini
>  	 * assume that a halting debugger will leave the world in a nice state
>  	 * for us.
>  	 */
> -	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
> -				dbg_reset_online, NULL);
> +	ret = cpuhp_setup_state_cpuslocked(CPUHP_AP_ONLINE_DYN,
> +					   "arm/hw_breakpoint:online",
> +					   dbg_reset_online, NULL);

Given the callsite, this particular change looks ok to me. So FWIW:

Acked-by: Mark Rutland <mark.rutland@arm.com>

However, as a more general note, the changes make the API feel odd. per
their current names, {get,put}_online_cpus() sound/feel like refcounting
ops, which should be able to nest.

Is there any chance these could get a better name, e.g.
{lock,unlock}_online_cpus(), so as to align with _cpuslocked?

Thanks,
Mark.

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

#1626682 — Re: [patch V2 11/24] ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked()

On Wed, 19 Apr 2017, Mark Rutland wrote:

> Hi,
> 
> On Tue, Apr 18, 2017 at 07:04:53PM +0200, Thomas Gleixner wrote:
> > From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > 
> > arch_hw_breakpoint_init() holds get_online_cpus() while registerring the
> > hotplug callbacks.
> > 
> > cpuhp_setup_state() invokes get_online_cpus() as well. This is correct, but
> > prevents the conversion of the hotplug locking to a percpu rwsem.
> > 
> > Use cpuhp_setup_state_cpuslocked() to avoid the nested call.
> > 
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: linux-arm-kernel@lists.infradead.org
> > 
> > ---
> >  arch/arm/kernel/hw_breakpoint.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > --- a/arch/arm/kernel/hw_breakpoint.c
> > +++ b/arch/arm/kernel/hw_breakpoint.c
> > @@ -1098,8 +1098,9 @@ static int __init arch_hw_breakpoint_ini
> >  	 * assume that a halting debugger will leave the world in a nice state
> >  	 * for us.
> >  	 */
> > -	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
> > -				dbg_reset_online, NULL);
> > +	ret = cpuhp_setup_state_cpuslocked(CPUHP_AP_ONLINE_DYN,
> > +					   "arm/hw_breakpoint:online",
> > +					   dbg_reset_online, NULL);
> 
> Given the callsite, this particular change looks ok to me. So FWIW:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>
> 
> However, as a more general note, the changes make the API feel odd. per
> their current names, {get,put}_online_cpus() sound/feel like refcounting
> ops, which should be able to nest.
> 
> Is there any chance these could get a better name, e.g.
> {lock,unlock}_online_cpus(), so as to align with _cpuslocked?

Yes, that's a follow up cleanup patch treewide once this hit Linus tree.

Thanks,

	tglx

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

#1627355 — [tip:smp/hotplug] ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked()

Commit-ID:  f29f3fd01356bd8ac1f95ab098e9139ae6c945bb
Gitweb:     http://git.kernel.org/tip/f29f3fd01356bd8ac1f95ab098e9139ae6c945bb
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Tue, 18 Apr 2017 19:04:53 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 20 Apr 2017 13:08:53 +0200

ARM/hw_breakpoint: Use cpuhp_setup_state_cpuslocked()

arch_hw_breakpoint_init() holds get_online_cpus() while registerring the
hotplug callbacks.

cpuhp_setup_state() invokes get_online_cpus() as well. This is correct, but
prevents the conversion of the hotplug locking to a percpu rwsem.

Use cpuhp_setup_state_cpuslocked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170418170553.274229815@linutronix.de

---
 arch/arm/kernel/hw_breakpoint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c
index be3b3fb..2320780 100644
--- a/arch/arm/kernel/hw_breakpoint.c
+++ b/arch/arm/kernel/hw_breakpoint.c
@@ -1098,8 +1098,9 @@ static int __init arch_hw_breakpoint_init(void)
 	 * assume that a halting debugger will leave the world in a nice state
 	 * for us.
 	 */
-	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "arm/hw_breakpoint:online",
-				dbg_reset_online, NULL);
+	ret = cpuhp_setup_state_cpuslocked(CPUHP_AP_ONLINE_DYN,
+					   "arm/hw_breakpoint:online",
+					   dbg_reset_online, NULL);
 	unregister_undef_hook(&debug_reg_hook);
 	if (WARN_ON(ret < 0) || !cpumask_empty(&debug_err_mask)) {
 		core_num_brps = 0;

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

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web