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

Groups > linux.kernel > #1624102 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [patch 00/20] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 02/20] stop_machine: Provide stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 20/20] cpu/hotplug: Convert hotplug locking to percpu rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
      Re: [patch 20/20] cpu/hotplug: Convert hotplug locking to percpu  rwsem Peter Zijlstra <peterz@infradead.org> - 2017-04-17 09:00 +0200
    [patch 17/20] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
      Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Peter Zijlstra <peterz@infradead.org> - 2017-04-17 08:50 +0200
        Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-17 09:50 +0200
      Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Bjorn Helgaas <helgaas@kernel.org> - 2017-04-18 21:50 +0200
        Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of  get_online_cpus() Thomas Gleixner <tglx@linutronix.de> - 2017-04-18 22:00 +0200
    [patch 12/20] s390/kernel: Use stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 13/20] powerpc/powernv: Use stop_machine_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 03/20] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
      Re: [patch 03/20] padata: Make padata_alloc() static "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-04-16 08:30 +0200
        Re: [patch 03/20] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-04-17 11:20 +0200
    [patch 01/20] cpu/hotplug: Provide  cpuhp_setup/remove_state[_nocalls]_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 18/20] PCI: Replace the racy recursion prevention Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 04/20] padata: Avoid nested calls to get_online_cpus() in  pcrypt_init_padata() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 14/20] kernel/hotplug: Use stop_machine_locked() in  takedown_cpu() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 05/20] x86/mtrr: Remove get_online_cpus() from  mtrr_save_state() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 07/20] KVM/PPC/Book3S HV: Use  cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 16/20] perf/x86/intel: Drop get_online_cpus() in  intel_snb_check_microcode() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
      Re: [patch 16/20] perf/x86/intel: Drop get_online_cpus() in  intel_snb_check_microcode() Borislav Petkov <bp@alien8.de> - 2017-04-18 13:30 +0200
    [patch 09/20] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 10/20] perf/x86/intel/cqm: Use cpuhp_setup_state_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
      Re: [patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() "Rafael J. Wysocki" <rafael@kernel.org> - 2017-04-16 01:00 +0200
      Re: [patch 06/20] cpufreq: Use cpuhp_setup_state_nocalls_locked() Viresh Kumar <viresh.kumar@linaro.org> - 2017-04-17 06:20 +0200
    [patch 08/20] hwtracing/coresight-etm3x: Use the locked version of  cpuhp_setup_state_nocalls() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200
    [patch 11/20] ARM/hw_breakpoint: Use cpuhp_setup_state_locked() Thomas Gleixner <tglx@linutronix.de> - 2017-04-15 19:40 +0200

Page 1 of 2  [1] 2  Next page →

#1624102 — [patch 00/20] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem

get_online_cpus() is used in hot pathes in mainline and even more so in
RT. That can show up badly under certain conditions because every locker
contends on a global mutex. RT has it's own homebrewn mitigation which is
an (badly done) open coded implementation of percpu_rwsems with recursion
support.

The proper replacement for that are percpu_rwsems, but that requires to
remove recursion support.

The conversion unearthed real locking issues which were previously not
visible because the get_online_cpus() lockdep annotation was implemented
with recursion support which prevents lockdep from tracking full dependency
chains. These potential deadlocks are not related to recursive calls, they
trigger on the first invocation because lockdep now has the full dependency
chains available.

The following patch series addresses this by

 - Cleaning up places which call get_online_cpus() nested

 - Replacing a few instances with cpu_hotplug_disable() to prevent circular
   locking dependencies.

The series depends on

    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git sched/core
  plus
    Linus tree merged in to avoid conflicts

It's available in git from

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.hotplug

Thanks,

	tglx

-------
 arch/arm/kernel/hw_breakpoint.c               |    5 
 arch/powerpc/kvm/book3s_hv.c                  |    8 -
 arch/powerpc/platforms/powernv/subcore.c      |    2 
 arch/s390/kernel/time.c                       |    2 
 arch/x86/events/core.c                        |    1 
 arch/x86/events/intel/core.c                  |    4 
 arch/x86/events/intel/cqm.c                   |   12 +-
 arch/x86/kernel/cpu/mtrr/main.c               |    2 
 drivers/acpi/processor_driver.c               |    4 
 drivers/cpufreq/cpufreq.c                     |    9 -
 drivers/hwtracing/coresight/coresight-etm3x.c |   12 +-
 drivers/hwtracing/coresight/coresight-etm4x.c |   12 +-
 drivers/pci/pci-driver.c                      |   46 ++++----
 include/linux/cpuhotplug.h                    |   29 +++++
 include/linux/padata.h                        |    3 
 include/linux/pci.h                           |    1 
 include/linux/stop_machine.h                  |   26 ++++
 kernel/cpu.c                                  |  149 +++++++-------------------
 kernel/padata.c                               |   38 +++---
 kernel/stop_machine.c                         |    4 
 20 files changed, 177 insertions(+), 192 deletions(-)

[toc] | [next] | [standalone]

#1624103 — [patch 02/20] stop_machine: Provide stop_machine_locked()

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

Some call sites of stop_machine() are within a get_online_cpus() protected
region.

stop_machine() calls get_online_cpus() as well, which is possible in the
current implementation but prevents converting the hotplug locking to a
percpu rwsem.

Provide stop_machine_locked() to avoid nested calls to get_online_cpus().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/stop_machine.h |   26 +++++++++++++++++++++++---
 kernel/stop_machine.c        |    4 ++--
 2 files changed, 25 insertions(+), 5 deletions(-)

--- a/include/linux/stop_machine.h
+++ b/include/linux/stop_machine.h
@@ -116,15 +116,29 @@ static inline int try_stop_cpus(const st
  * @fn() runs.
  *
  * This can be thought of as a very heavy write lock, equivalent to
- * grabbing every spinlock in the kernel. */
+ * grabbing every spinlock in the kernel.
+ *
+ * Protects against CPU hotplug.
+ */
 int stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus);
 
+/**
+ * stop_machine_locked: freeze the machine on all CPUs and run this function
+ * @fn: the function to run
+ * @data: the data ptr for the @fn()
+ * @cpus: the cpus to run the @fn() on (NULL = any online cpu)
+ *
+ * Same as above. Must be called from with in a get_online_cpus() protected
+ * region. Avoids nested calls to get_online_cpus().
+ */
+int stop_machine_locked(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus);
+
 int stop_machine_from_inactive_cpu(cpu_stop_fn_t fn, void *data,
 				   const struct cpumask *cpus);
 #else	/* CONFIG_SMP || CONFIG_HOTPLUG_CPU */
 
-static inline int stop_machine(cpu_stop_fn_t fn, void *data,
-				 const struct cpumask *cpus)
+static inline int stop_machine_locked(cpu_stop_fn_t fn, void *data,
+				      const struct cpumask *cpus)
 {
 	unsigned long flags;
 	int ret;
@@ -134,6 +148,12 @@ static inline int stop_machine(cpu_stop_
 	return ret;
 }
 
+static inline int stop_machine(cpu_stop_fn_t fn, void *data,
+			       const struct cpumask *cpus)
+{
+	return stop_machine_locked(fn, data, cpus);
+}
+
 static inline int stop_machine_from_inactive_cpu(cpu_stop_fn_t fn, void *data,
 						 const struct cpumask *cpus)
 {
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -552,7 +552,7 @@ static int __init cpu_stop_init(void)
 }
 early_initcall(cpu_stop_init);
 
-static int __stop_machine(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
+int stop_machine_locked(cpu_stop_fn_t fn, void *data, const struct cpumask *cpus)
 {
 	struct multi_stop_data msdata = {
 		.fn = fn,
@@ -591,7 +591,7 @@ int stop_machine(cpu_stop_fn_t fn, void
 
 	/* No CPUs can come up or down during this. */
 	get_online_cpus();
-	ret = __stop_machine(fn, data, cpus);
+	ret = stop_machine_locked(fn, data, cpus);
 	put_online_cpus();
 	return ret;
 }

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

#1624104 — [patch 20/20] cpu/hotplug: Convert hotplug locking to percpu rwsem

There are no more (known) nested calls to get_online_cpus() so it's
possible to remove the nested call magic and convert the mutex to a
percpu-rwsem, which speeds up get/put_online_cpus() significantly for the
uncontended case.

The contended case (write locked for hotplug operations) is slow anyway, so
the slightly more expensive down_write of the percpu rwsem does not matter.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/cpu.c |  102 ++++-------------------------------------------------------
 1 file changed, 8 insertions(+), 94 deletions(-)

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -27,6 +27,7 @@
 #include <linux/smpboot.h>
 #include <linux/relay.h>
 #include <linux/slab.h>
+#include <linux/percpu-rwsem.h>
 
 #include <trace/events/power.h>
 #define CREATE_TRACE_POINTS
@@ -196,121 +197,36 @@ void cpu_maps_update_done(void)
 	mutex_unlock(&cpu_add_remove_lock);
 }
 
-/* If set, cpu_up and cpu_down will return -EBUSY and do nothing.
+/*
+ * If set, cpu_up and cpu_down will return -EBUSY and do nothing.
  * Should always be manipulated under cpu_add_remove_lock
  */
 static int cpu_hotplug_disabled;
 
 #ifdef CONFIG_HOTPLUG_CPU
 
-static struct {
-	struct task_struct *active_writer;
-	/* wait queue to wake up the active_writer */
-	wait_queue_head_t wq;
-	/* verifies that no writer will get active while readers are active */
-	struct mutex lock;
-	/*
-	 * Also blocks the new readers during
-	 * an ongoing cpu hotplug operation.
-	 */
-	atomic_t refcount;
-
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	struct lockdep_map dep_map;
-#endif
-} cpu_hotplug = {
-	.active_writer = NULL,
-	.wq = __WAIT_QUEUE_HEAD_INITIALIZER(cpu_hotplug.wq),
-	.lock = __MUTEX_INITIALIZER(cpu_hotplug.lock),
-#ifdef CONFIG_DEBUG_LOCK_ALLOC
-	.dep_map = STATIC_LOCKDEP_MAP_INIT("cpu_hotplug.dep_map", &cpu_hotplug.dep_map),
-#endif
-};
-
-/* Lockdep annotations for get/put_online_cpus() and cpu_hotplug_begin/end() */
-#define cpuhp_lock_acquire_read() lock_map_acquire_read(&cpu_hotplug.dep_map)
-#define cpuhp_lock_acquire_tryread() \
-				  lock_map_acquire_tryread(&cpu_hotplug.dep_map)
-#define cpuhp_lock_acquire()      lock_map_acquire(&cpu_hotplug.dep_map)
-#define cpuhp_lock_release()      lock_map_release(&cpu_hotplug.dep_map)
-
+DEFINE_STATIC_PERCPU_RWSEM(cpu_hotplug_lock);
 
 void get_online_cpus(void)
 {
-	might_sleep();
-	if (cpu_hotplug.active_writer == current)
-		return;
-	cpuhp_lock_acquire_read();
-	mutex_lock(&cpu_hotplug.lock);
-	atomic_inc(&cpu_hotplug.refcount);
-	mutex_unlock(&cpu_hotplug.lock);
+	percpu_down_read(&cpu_hotplug_lock);
 }
 EXPORT_SYMBOL_GPL(get_online_cpus);
 
 void put_online_cpus(void)
 {
-	int refcount;
-
-	if (cpu_hotplug.active_writer == current)
-		return;
-
-	refcount = atomic_dec_return(&cpu_hotplug.refcount);
-	if (WARN_ON(refcount < 0)) /* try to fix things up */
-		atomic_inc(&cpu_hotplug.refcount);
-
-	if (refcount <= 0 && waitqueue_active(&cpu_hotplug.wq))
-		wake_up(&cpu_hotplug.wq);
-
-	cpuhp_lock_release();
-
+	percpu_up_read(&cpu_hotplug_lock);
 }
 EXPORT_SYMBOL_GPL(put_online_cpus);
 
-/*
- * This ensures that the hotplug operation can begin only when the
- * refcount goes to zero.
- *
- * Note that during a cpu-hotplug operation, the new readers, if any,
- * will be blocked by the cpu_hotplug.lock
- *
- * Since cpu_hotplug_begin() is always called after invoking
- * cpu_maps_update_begin(), we can be sure that only one writer is active.
- *
- * Note that theoretically, there is a possibility of a livelock:
- * - Refcount goes to zero, last reader wakes up the sleeping
- *   writer.
- * - Last reader unlocks the cpu_hotplug.lock.
- * - A new reader arrives at this moment, bumps up the refcount.
- * - The writer acquires the cpu_hotplug.lock finds the refcount
- *   non zero and goes to sleep again.
- *
- * However, this is very difficult to achieve in practice since
- * get_online_cpus() not an api which is called all that often.
- *
- */
 void cpu_hotplug_begin(void)
 {
-	DEFINE_WAIT(wait);
-
-	cpu_hotplug.active_writer = current;
-	cpuhp_lock_acquire();
-
-	for (;;) {
-		mutex_lock(&cpu_hotplug.lock);
-		prepare_to_wait(&cpu_hotplug.wq, &wait, TASK_UNINTERRUPTIBLE);
-		if (likely(!atomic_read(&cpu_hotplug.refcount)))
-				break;
-		mutex_unlock(&cpu_hotplug.lock);
-		schedule();
-	}
-	finish_wait(&cpu_hotplug.wq, &wait);
+	percpu_down_write(&cpu_hotplug_lock);
 }
 
 void cpu_hotplug_done(void)
 {
-	cpu_hotplug.active_writer = NULL;
-	mutex_unlock(&cpu_hotplug.lock);
-	cpuhp_lock_release();
+	percpu_up_write(&cpu_hotplug_lock);
 }
 
 /*
@@ -344,8 +260,6 @@ void cpu_hotplug_enable(void)
 EXPORT_SYMBOL_GPL(cpu_hotplug_enable);
 #endif	/* CONFIG_HOTPLUG_CPU */
 
-/* Notifier wrappers for transitioning to state machine */
-
 static int bringup_wait_for_ap(unsigned int cpu)
 {
 	struct cpuhp_cpu_state *st = per_cpu_ptr(&cpuhp_state, cpu);

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

#1624579 — Re: [patch 20/20] cpu/hotplug: Convert hotplug locking to percpu rwsem

On Sat, Apr 15, 2017 at 07:01:27PM +0200, Thomas Gleixner wrote:
> There are no more (known) nested calls to get_online_cpus() so it's
> possible to remove the nested call magic and convert the mutex to a
> percpu-rwsem, which speeds up get/put_online_cpus() significantly for the
> uncontended case.
> 
> The contended case (write locked for hotplug operations) is slow anyway, so
> the slightly more expensive down_write of the percpu rwsem does not matter.

So the previous thing was reader biassed and thus prone to writer
starvation. So a slightly more expensive write path doesn't matter; esp.
as its now fair and provides a guarantee it will happen, unlike the
previous one.

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

#1624105 — [patch 17/20] 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 |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

--- 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_ATS
+	return 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 };
 
 	/*
@@ -349,13 +358,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]

#1624571 — Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

On Sat, Apr 15, 2017 at 07:01:24PM +0200, Thomas Gleixner wrote:
> +++ 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_ATS
> +	return dev->physfn->is_probed;
> +#else
> +	return false;
> +#endif
> +}
> +

Should be in the next patch perhaps?

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

#1624584 — Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

On Mon, 17 Apr 2017, Peter Zijlstra wrote:
> On Sat, Apr 15, 2017 at 07:01:24PM +0200, Thomas Gleixner wrote:
> > +++ 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_ATS
> > +	return dev->physfn->is_probed;
> > +#else
> > +	return false;
> > +#endif
> > +}
> > +
> 
> Should be in the next patch perhaps?

Indeed.

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

#1625577 — Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

On Sat, Apr 15, 2017 at 07:01:24PM +0200, Thomas Gleixner wrote:
> 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 |   15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> --- 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_ATS

I think this was intended to be CONFIG_PCI_ATS, not CONFIG_ATS.

But I think CONFIG_PCI_IOV would be more appropriate.  With that, and
squashing this into the next patch,

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

I expect you'll merge this along with the rest of the series.  Let me
know if you need anything else from me.

> +	return 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 };
>  
>  	/*
> @@ -349,13 +358,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]

#1625609 — Re: [patch 17/20] PCI: Use cpu_hotplug_disable() instead of get_online_cpus()

On Tue, 18 Apr 2017, Bjorn Helgaas wrote:
> > +static bool pci_physfn_is_probed(struct pci_dev *dev)
> > +{
> > +#ifdef CONFIG_ATS
> 
> I think this was intended to be CONFIG_PCI_ATS, not CONFIG_ATS.

yes.

> 
> But I think CONFIG_PCI_IOV would be more appropriate.  With that, and

The physfn member is under CONFIG_PCI_ATS so I took that one, but you are
right PCI_IOV is the proper one. Will fix.

> squashing this into the next patch,

Did so

Thanks,

	tglx

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

#1624106 — [patch 12/20] s390/kernel: Use stop_machine_locked()

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_locked() 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_locked(stp_sync_clock, &stp_sync, cpu_online_mask);
 	put_online_cpus();
 
 	if (!check_sync_clock())

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

#1624107 — [patch 13/20] powerpc/powernv: Use stop_machine_locked()

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_locked() 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 |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/powerpc/platforms/powernv/subcore.c
+++ b/arch/powerpc/platforms/powernv/subcore.c
@@ -356,7 +356,7 @@ 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_locked(cpu_update_split_mode, &new_mode, cpu_online_mask);
 
 	put_online_cpus();
 

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

#1624108 — [patch 03/20] padata: Make padata_alloc() static

No users outside of padata.c

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: linux-crypto@vger.kernel.org
---
 include/linux/padata.h |    3 ---
 kernel/padata.c        |   34 +++++++++++++++++-----------------
 2 files changed, 17 insertions(+), 20 deletions(-)

--- a/include/linux/padata.h
+++ b/include/linux/padata.h
@@ -166,9 +166,6 @@ struct padata_instance {
 
 extern struct padata_instance *padata_alloc_possible(
 					struct workqueue_struct *wq);
-extern struct padata_instance *padata_alloc(struct workqueue_struct *wq,
-					    const struct cpumask *pcpumask,
-					    const struct cpumask *cbcpumask);
 extern void padata_free(struct padata_instance *pinst);
 extern int padata_do_parallel(struct padata_instance *pinst,
 			      struct padata_priv *padata, int cb_cpu);
--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -913,7 +913,7 @@ static ssize_t padata_sysfs_show(struct
 }
 
 static ssize_t padata_sysfs_store(struct kobject *kobj, struct attribute *attr,
-				  const char *buf, size_t count)
+s				  const char *buf, size_t count)
 {
 	struct padata_instance *pinst;
 	struct padata_sysfs_entry *pentry;
@@ -939,19 +939,6 @@ static struct kobj_type padata_attr_type
 };
 
 /**
- * padata_alloc_possible - Allocate and initialize padata instance.
- *                         Use the cpu_possible_mask for serial and
- *                         parallel workers.
- *
- * @wq: workqueue to use for the allocated padata instance
- */
-struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
-{
-	return padata_alloc(wq, cpu_possible_mask, cpu_possible_mask);
-}
-EXPORT_SYMBOL(padata_alloc_possible);
-
-/**
  * padata_alloc - allocate and initialize a padata instance and specify
  *                cpumasks for serial and parallel workers.
  *
@@ -959,9 +946,9 @@ EXPORT_SYMBOL(padata_alloc_possible);
  * @pcpumask: cpumask that will be used for padata parallelization
  * @cbcpumask: cpumask that will be used for padata serialization
  */
-struct padata_instance *padata_alloc(struct workqueue_struct *wq,
-				     const struct cpumask *pcpumask,
-				     const struct cpumask *cbcpumask)
+static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
+					    const struct cpumask *pcpumask,
+					    const struct cpumask *cbcpumask)
 {
 	struct padata_instance *pinst;
 	struct parallel_data *pd = NULL;
@@ -1016,6 +1003,19 @@ struct padata_instance *padata_alloc(str
 }
 
 /**
+ * padata_alloc_possible - Allocate and initialize padata instance.
+ *                         Use the cpu_possible_mask for serial and
+ *                         parallel workers.
+ *
+ * @wq: workqueue to use for the allocated padata instance
+ */
+struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
+{
+	return padata_alloc(wq, cpu_possible_mask, cpu_possible_mask);
+}
+EXPORT_SYMBOL(padata_alloc_possible);
+
+/**
  * padata_free - free a padata instance
  *
  * @padata_inst: padata instance to free

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

#1624163 — Re: [patch 03/20] padata: Make padata_alloc() static

I rather like this option of padata, which, since it lives in
kernel/padata.c and linux/padata.h, should be generic and useful for
other components. Seems like the ability to allocate it for a
particular set of worker CPUs and callback CPUs could be useful down
the line. Would rather not see it become static.

Jason

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

#1624614 — Re: [patch 03/20] padata: Make padata_alloc() static

On Sun, 16 Apr 2017, Jason A. Donenfeld wrote:

> I rather like this option of padata, which, since it lives in
> kernel/padata.c and linux/padata.h, should be generic and useful for
> other components. Seems like the ability to allocate it for a
> particular set of worker CPUs and callback CPUs could be useful down
> the line. Would rather not see it become static.

It's simple enough to export it once there is an actual user. Just keeping
stuff global because it might be useful somewhere down the road is really
pointless.

Thanks,

	tglx

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

#1624109 — [patch 01/20] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_locked()

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

Some call sites of cpuhp_setup/remove_state[_nocalls]() are within a
get_online_cpus() protected region.

cpuhp_setup/remove_state[_nocalls]() call get_online_cpus() as well, which
is possible in the current implementation but prevetns converting the
hotplug locking to a percpu rwsem.

Provide locked versions of the interfaces to avoid nested calls to
get_online_cpus().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---
 include/linux/cpuhotplug.h |   29 +++++++++++++++++++++++++++++
 kernel/cpu.c               |   45 +++++++++++++++++++++++++++++++++------------
 2 files changed, 62 insertions(+), 12 deletions(-)

--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -151,6 +151,11 @@ int __cpuhp_setup_state(enum cpuhp_state
 			int (*startup)(unsigned int cpu),
 			int (*teardown)(unsigned int cpu), bool multi_instance);
 
+int __cpuhp_setup_state_locked(enum cpuhp_state state,	const char *name,
+			       bool invoke,
+			       int (*startup)(unsigned int cpu),
+			       int (*teardown)(unsigned int cpu),
+			       bool multi_instance);
 /**
  * cpuhp_setup_state - Setup hotplug state callbacks with calling the callbacks
  * @state:	The state for which the calls are installed
@@ -169,6 +174,15 @@ static inline int cpuhp_setup_state(enum
 	return __cpuhp_setup_state(state, name, true, startup, teardown, false);
 }
 
+static inline int cpuhp_setup_state_locked(enum cpuhp_state state,
+					   const char *name,
+					   int (*startup)(unsigned int cpu),
+					   int (*teardown)(unsigned int cpu))
+{
+	return __cpuhp_setup_state_locked(state, name, true, startup, teardown,
+					  false);
+}
+
 /**
  * cpuhp_setup_state_nocalls - Setup hotplug state callbacks without calling the
  *			       callbacks
@@ -189,6 +203,15 @@ static inline int cpuhp_setup_state_noca
 				   false);
 }
 
+static inline int cpuhp_setup_state_nocalls_locked(enum cpuhp_state state,
+						   const char *name,
+						   int (*startup)(unsigned int cpu),
+						   int (*teardown)(unsigned int cpu))
+{
+	return __cpuhp_setup_state_locked(state, name, false, startup, teardown,
+					  false);
+}
+
 /**
  * cpuhp_setup_state_multi - Add callbacks for multi state
  * @state:	The state for which the calls are installed
@@ -248,6 +271,7 @@ static inline int cpuhp_state_add_instan
 }
 
 void __cpuhp_remove_state(enum cpuhp_state state, bool invoke);
+void __cpuhp_remove_state_locked(enum cpuhp_state state, bool invoke);
 
 /**
  * cpuhp_remove_state - Remove hotplug state callbacks and invoke the teardown
@@ -271,6 +295,11 @@ static inline void cpuhp_remove_state_no
 	__cpuhp_remove_state(state, false);
 }
 
+static inline void cpuhp_remove_state_nocalls_locked(enum cpuhp_state state)
+{
+	__cpuhp_remove_state_locked(state, false);
+}
+
 /**
  * cpuhp_remove_multi_state - Remove hotplug multi state callback
  * @state:	The state for which the calls are removed
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1457,7 +1457,7 @@ int __cpuhp_state_add_instance(enum cpuh
 EXPORT_SYMBOL_GPL(__cpuhp_state_add_instance);
 
 /**
- * __cpuhp_setup_state - Setup the callbacks for an hotplug machine state
+ * __cpuhp_setup_state_locked - Setup the callbacks for an hotplug machine state
  * @state:		The state to setup
  * @invoke:		If true, the startup function is invoked for cpus where
  *			cpu state >= @state
@@ -1466,17 +1466,18 @@ EXPORT_SYMBOL_GPL(__cpuhp_state_add_inst
  * @multi_instance:	State is set up for multiple instances which get
  *			added afterwards.
  *
+ * The caller needs to hold get_online_cpus() while calling this function.
  * Returns:
  *   On success:
  *      Positive state number if @state is CPUHP_AP_ONLINE_DYN
  *      0 for all other states
  *   On failure: proper (negative) error code
  */
-int __cpuhp_setup_state(enum cpuhp_state state,
-			const char *name, bool invoke,
-			int (*startup)(unsigned int cpu),
-			int (*teardown)(unsigned int cpu),
-			bool multi_instance)
+int __cpuhp_setup_state_locked(enum cpuhp_state state,
+			       const char *name, bool invoke,
+			       int (*startup)(unsigned int cpu),
+			       int (*teardown)(unsigned int cpu),
+			       bool multi_instance)
 {
 	int cpu, ret = 0;
 	bool dynstate;
@@ -1484,7 +1485,6 @@ int __cpuhp_setup_state(enum cpuhp_state
 	if (cpuhp_cb_check(state) || !name)
 		return -EINVAL;
 
-	get_online_cpus();
 	mutex_lock(&cpuhp_state_mutex);
 
 	ret = cpuhp_store_callbacks(state, name, startup, teardown,
@@ -1520,7 +1520,6 @@ int __cpuhp_setup_state(enum cpuhp_state
 	}
 out:
 	mutex_unlock(&cpuhp_state_mutex);
-	put_online_cpus();
 	/*
 	 * If the requested state is CPUHP_AP_ONLINE_DYN, return the
 	 * dynamically allocated state in case of success.
@@ -1529,6 +1528,22 @@ int __cpuhp_setup_state(enum cpuhp_state
 		return state;
 	return ret;
 }
+EXPORT_SYMBOL(__cpuhp_setup_state_locked);
+
+int __cpuhp_setup_state(enum cpuhp_state state,
+			const char *name, bool invoke,
+			int (*startup)(unsigned int cpu),
+			int (*teardown)(unsigned int cpu),
+			bool multi_instance)
+{
+	int ret;
+
+	get_online_cpus();
+	ret = __cpuhp_setup_state_locked(state, name, invoke, startup, teardown,
+					 multi_instance);
+	put_online_cpus();
+	return ret;
+}
 EXPORT_SYMBOL(__cpuhp_setup_state);
 
 int __cpuhp_state_remove_instance(enum cpuhp_state state,
@@ -1570,23 +1585,22 @@ int __cpuhp_state_remove_instance(enum c
 EXPORT_SYMBOL_GPL(__cpuhp_state_remove_instance);
 
 /**
- * __cpuhp_remove_state - Remove the callbacks for an hotplug machine state
+ * __cpuhp_remove_state_locked - Remove the callbacks for an hotplug machine state
  * @state:	The state to remove
  * @invoke:	If true, the teardown function is invoked for cpus where
  *		cpu state >= @state
  *
+ * The caller needs to hold get_online_cpus() while calling this function.
  * The teardown callback is currently not allowed to fail. Think
  * about module removal!
  */
-void __cpuhp_remove_state(enum cpuhp_state state, bool invoke)
+void __cpuhp_remove_state_locked(enum cpuhp_state state, bool invoke)
 {
 	struct cpuhp_step *sp = cpuhp_get_step(state);
 	int cpu;
 
 	BUG_ON(cpuhp_cb_check(state));
 
-	get_online_cpus();
-
 	mutex_lock(&cpuhp_state_mutex);
 	if (sp->multi_instance) {
 		WARN(!hlist_empty(&sp->list),
@@ -1613,6 +1627,13 @@ void __cpuhp_remove_state(enum cpuhp_sta
 remove:
 	cpuhp_store_callbacks(state, NULL, NULL, NULL, false);
 	mutex_unlock(&cpuhp_state_mutex);
+}
+EXPORT_SYMBOL(__cpuhp_remove_state_locked);
+
+void __cpuhp_remove_state(enum cpuhp_state state, bool invoke)
+{
+	get_online_cpus();
+	__cpuhp_remove_state_locked(state, invoke);
 	put_online_cpus();
 }
 EXPORT_SYMBOL(__cpuhp_remove_state);

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

#1624110 — [patch 18/20] 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.

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.

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 |   35 ++++++++++++++---------------------
 include/linux/pci.h      |    1 +
 2 files changed, 15 insertions(+), 21 deletions(-)

--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -341,33 +341,26 @@ static int pci_call_probe(struct pci_dri
 	 * 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 (dev->is_virtfn && 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;
 }
 
--- 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]

#1624111 — [patch 04/20] padata: Avoid nested calls to get_online_cpus() in pcrypt_init_padata()

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

pcrypt_init_padata()
   get_online_cpus()
   padata_alloc_possible()
     padata_alloc()
       get_online_cpus()

The nested call to get_online_cpus() works with the current implementation,
but prevents the conversion to a percpu rwsem.

The other caller of padata_alloc_possible() is pcrypt_init_padata() which
calls from a get_online_cpus() protected region as well.

Remove the get_online_cpus() call in padata_alloc() and document the
calling convention.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: linux-crypto@vger.kernel.org

---
 kernel/padata.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -913,7 +913,7 @@ static ssize_t padata_sysfs_show(struct
 }
 
 static ssize_t padata_sysfs_store(struct kobject *kobj, struct attribute *attr,
-s				  const char *buf, size_t count)
+				  const char *buf, size_t count)
 {
 	struct padata_instance *pinst;
 	struct padata_sysfs_entry *pentry;
@@ -945,6 +945,8 @@ static struct kobj_type padata_attr_type
  * @wq: workqueue to use for the allocated padata instance
  * @pcpumask: cpumask that will be used for padata parallelization
  * @cbcpumask: cpumask that will be used for padata serialization
+ *
+ * Must be called from a get_online_cpus() protected region
  */
 static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
 					    const struct cpumask *pcpumask,
@@ -957,7 +959,6 @@ static struct padata_instance *padata_al
 	if (!pinst)
 		goto err;
 
-	get_online_cpus();
 	if (!alloc_cpumask_var(&pinst->cpumask.pcpu, GFP_KERNEL))
 		goto err_free_inst;
 	if (!alloc_cpumask_var(&pinst->cpumask.cbcpu, GFP_KERNEL)) {
@@ -997,7 +998,6 @@ static struct padata_instance *padata_al
 	free_cpumask_var(pinst->cpumask.cbcpu);
 err_free_inst:
 	kfree(pinst);
-	put_online_cpus();
 err:
 	return NULL;
 }
@@ -1008,6 +1008,8 @@ static struct padata_instance *padata_al
  *                         parallel workers.
  *
  * @wq: workqueue to use for the allocated padata instance
+ *
+ * Must be called from a get_online_cpus() protected region
  */
 struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
 {

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

#1624112 — [patch 14/20] kernel/hotplug: Use stop_machine_locked() in takedown_cpu()

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

takedown_cpu() is a cpu hotplug function invoking stop_machine(). The cpu
hotplug machinery holds the hotplug lock for write.

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_locked() to avoid the nested call.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

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

--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -701,7 +701,7 @@ static int takedown_cpu(unsigned int cpu
 	/*
 	 * So now all preempt/rcu users must observe !cpu_active().
 	 */
-	err = stop_machine(take_cpu_down, NULL, cpumask_of(cpu));
+	err = stop_machine_locked(take_cpu_down, NULL, cpumask_of(cpu));
 	if (err) {
 		/* CPU refused to die */
 		irq_unlock_sparse();

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

#1624113 — [patch 05/20] x86/mtrr: Remove get_online_cpus() from mtrr_save_state()

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

mtrr_save_state() is invoked from native_cpu_up() which is in the context
of a CPU hotplug operation and therefor calling get_online_cpus() is
pointless.

While this works in the current get_online_cpus() implementation it
prevents from converting the hotplug locking to percpu rwsems.

Remove it.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org

---
 arch/x86/kernel/cpu/mtrr/main.c |    2 --
 1 file changed, 2 deletions(-)

--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -807,10 +807,8 @@ void mtrr_save_state(void)
 	if (!mtrr_enabled())
 		return;
 
-	get_online_cpus();
 	first_cpu = cpumask_first(cpu_online_mask);
 	smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
-	put_online_cpus();
 }
 
 void set_mtrr_aps_delayed_init(void)

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

#1624114 — [patch 07/20] KVM/PPC/Book3S HV: Use cpuhp_setup_state_nocalls_locked()

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

kvmppc_alloc_host_rm_ops() 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_locked() to avoid the nested call.

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

---
 arch/powerpc/kvm/book3s_hv.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3336,10 +3336,10 @@ void kvmppc_alloc_host_rm_ops(void)
 		return;
 	}
 
-	cpuhp_setup_state_nocalls(CPUHP_KVM_PPC_BOOK3S_PREPARE,
-				  "ppc/kvm_book3s:prepare",
-				  kvmppc_set_host_core,
-				  kvmppc_clear_host_core);
+	cpuhp_setup_state_nocalls_locked(CPUHP_KVM_PPC_BOOK3S_PREPARE,
+					 "ppc/kvm_book3s:prepare",
+					 kvmppc_set_host_core,
+					 kvmppc_clear_host_core);
 	put_online_cpus();
 }
 

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web