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

Groups > linux.kernel > #1649274 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [patch V3 00/32] cpu/hotplug: Convert get_online_cpus() to a  percpu_rwsem Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
    [patch V3 12/32] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
    [patch V3 28/32] s390: Prevent hotplug rwsem recursion Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
      Re: [patch V3 28/32] s390: Prevent hotplug rwsem recursion Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-05-24 13:00 +0200
    [patch V3 25/32] kprobes: Cure hotplug lock ordering issues Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
      Re: [patch V3 25/32] kprobes: Cure hotplug lock ordering issues Masami Hiramatsu <mhiramat@kernel.org> - 2017-05-24 18:00 +0200
    [patch V3 03/32] cpu/hotplug: Provide  cpuhp_setup/remove_state[_nocalls]_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
    [patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
      Re: [patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked() Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-05-24 13:00 +0200
    [patch V3 06/32] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
    [patch V3 07/32] padata: Avoid nested calls to cpus_read_lock() in  pcrypt_init_padata() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
    [patch V3 13/32] perf/x86/intel/cqm: Use  cpuhp_setup_state_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:50 +0200
    [patch V3 11/32] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:50 +0200
    Re: [patch V3 00/32] cpu/hotplug: Convert get_online_cpus() to a  percpu_rwsem "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-05-24 18:30 +0200

#1649274 — [patch V3 00/32] 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
a (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 is on top of 4.12-rc2. It's available in git from

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

Changes since V2:

  - Reworked the approach vs. perf/ftrace/kprobes, which simplified the lot
  
  - Renamed get_online_cpus() to cpus_read_lock() to reflect the nature of
    the interface

  - Link the lockchains between hotplug control task and per cpu hotplug
    threads and fixed the fallout of that.

Thanks,

        tglx

---
 arch/arm/kernel/hw_breakpoint.c               |   11 -
 arch/arm/kernel/patch.c                       |    2 
 arch/arm/probes/kprobes/core.c                |    3 
 arch/arm64/include/asm/insn.h                 |    1 
 arch/arm64/kernel/insn.c                      |    5 
 arch/mips/kernel/jump_label.c                 |    2 
 arch/powerpc/kvm/book3s_hv.c                  |   14 -
 arch/powerpc/platforms/powernv/subcore.c      |    7 
 arch/s390/kernel/jump_label.c                 |    2 
 arch/s390/kernel/kprobes.c                    |    4 
 arch/s390/kernel/time.c                       |    6 
 arch/x86/events/core.c                        |    1 
 arch/x86/events/intel/cqm.c                   |   16 -
 arch/x86/kernel/cpu/mtrr/main.c               |    2 
 b/arch/sparc/kernel/jump_label.c              |    2 
 b/arch/tile/kernel/jump_label.c               |    2 
 b/arch/x86/events/intel/core.c                |   11 -
 b/arch/x86/kernel/jump_label.c                |    2 
 b/kernel/jump_label.c                         |   20 +-
 drivers/acpi/processor_driver.c               |    4 
 drivers/acpi/processor_throttling.c           |   16 -
 drivers/cpufreq/cpufreq.c                     |   21 +-
 drivers/hwtracing/coresight/coresight-etm3x.c |   20 +-
 drivers/hwtracing/coresight/coresight-etm4x.c |   20 +-
 drivers/pci/pci-driver.c                      |   47 +++--
 include/linux/cpu.h                           |   34 ++--
 include/linux/cpuhotplug.h                    |   38 ++++
 include/linux/padata.h                        |    3 
 include/linux/pci.h                           |    1 
 include/linux/perf_event.h                    |    2 
 include/linux/sched.h                         |   10 +
 include/linux/stop_machine.h                  |   26 ++-
 kernel/cpu.c                                  |  213 +++++++++++---------------
 kernel/events/core.c                          |  106 +++++++++---
 kernel/kprobes.c                              |   59 +++----
 kernel/padata.c                               |   43 ++---
 kernel/stop_machine.c                         |   11 -
 37 files changed, 444 insertions(+), 343 deletions(-)

[toc] | [next] | [standalone]

#1649276 — [patch V3 12/32] hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()

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

etm_probe4() 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. Convert *_online_cpus() to the new interfaces while at it.

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

---
 drivers/hwtracing/coresight/coresight-etm4x.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -371,7 +371,7 @@ static void etm4_disable_sysfs(struct co
 	 * after cpu online mask indicates the cpu is offline but before the
 	 * DYING hotplug callback is serviced by the ETM driver.
 	 */
-	get_online_cpus();
+	cpus_read_lock();
 	spin_lock(&drvdata->spinlock);
 
 	/*
@@ -381,7 +381,7 @@ static void etm4_disable_sysfs(struct co
 	smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
 
 	spin_unlock(&drvdata->spinlock);
-	put_online_cpus();
+	cpus_read_unlock();
 
 	dev_info(drvdata->dev, "ETM tracing disabled\n");
 }
@@ -982,7 +982,7 @@ static int etm4_probe(struct amba_device
 
 	drvdata->cpu = pdata ? pdata->cpu : 0;
 
-	get_online_cpus();
+	cpus_read_lock();
 	etmdrvdata[drvdata->cpu] = drvdata;
 
 	if (smp_call_function_single(drvdata->cpu,
@@ -990,18 +990,18 @@ static int etm4_probe(struct amba_device
 		dev_err(dev, "ETM arch init failed\n");
 
 	if (!etm4_count++) {
-		cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING,
-					  "arm/coresight4:starting",
-					  etm4_starting_cpu, etm4_dying_cpu);
-		ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
-						"arm/coresight4:online",
-						etm4_online_cpu, NULL);
+		cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ARM_CORESIGHT_STARTING,
+						     "arm/coresight4:starting",
+						     etm4_starting_cpu, etm4_dying_cpu);
+		ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN,
+							   "arm/coresight4:online",
+							   etm4_online_cpu, NULL);
 		if (ret < 0)
 			goto err_arch_supported;
 		hp_online = ret;
 	}
 
-	put_online_cpus();
+	cpus_read_unlock();
 
 	if (etm4_arch_supported(drvdata->arch) == false) {
 		ret = -EINVAL;

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

#1649278 — [patch V3 28/32] s390: Prevent hotplug rwsem recursion

The text patching functions which are invoked from jump_label and kprobes
code are protected against cpu hotplug at the call sites.

Use stop_machine_cpuslocked() to avoid recursion on the cpu hotplug
rwsem. stop_machine_cpuslocked() contains a lockdep assertion to catch any
unprotected callers.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
---
 arch/s390/kernel/jump_label.c |    2 +-
 arch/s390/kernel/kprobes.c    |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -93,7 +93,7 @@ void arch_jump_label_transform(struct ju
 	args.entry = entry;
 	args.type = type;
 
-	stop_machine(__sm_arch_jump_label_transform, &args, NULL);
+	stop_machine_cpuslocked(__sm_arch_jump_label_transform, &args, NULL);
 }
 
 void arch_jump_label_transform_static(struct jump_entry *entry,
--- a/arch/s390/kernel/kprobes.c
+++ b/arch/s390/kernel/kprobes.c
@@ -196,7 +196,7 @@ void arch_arm_kprobe(struct kprobe *p)
 {
 	struct swap_insn_args args = {.p = p, .arm_kprobe = 1};
 
-	stop_machine(swap_instruction, &args, NULL);
+	stop_machine_cpuslocked(swap_instruction, &args, NULL);
 }
 NOKPROBE_SYMBOL(arch_arm_kprobe);
 
@@ -204,7 +204,7 @@ void arch_disarm_kprobe(struct kprobe *p
 {
 	struct swap_insn_args args = {.p = p, .arm_kprobe = 0};
 
-	stop_machine(swap_instruction, &args, NULL);
+	stop_machine_cpuslocked(swap_instruction, &args, NULL);
 }
 NOKPROBE_SYMBOL(arch_disarm_kprobe);
 

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

#1649464 — Re: [patch V3 28/32] s390: Prevent hotplug rwsem recursion

On Wed, May 24, 2017 at 10:15:39AM +0200, Thomas Gleixner wrote:
> The text patching functions which are invoked from jump_label and kprobes
> code are protected against cpu hotplug at the call sites.
> 
> Use stop_machine_cpuslocked() to avoid recursion on the cpu hotplug
> rwsem. stop_machine_cpuslocked() contains a lockdep assertion to catch any
> unprotected callers.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/s390/kernel/jump_label.c |    2 +-
>  arch/s390/kernel/kprobes.c    |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>

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

#1649284 — [patch V3 25/32] kprobes: Cure hotplug lock ordering issues

Converting the cpu hotplug locking to a percpu rwsem unearthed hidden lock
ordering problems.

There is a wide range of locks involved in this: kprobe_mutex,
jump_label_mutex, ftrace_lock, text_mutex, event_mutex,
func_hash->regex_lock and a gazillion of lock order permutations with
nested get_online_cpus() calls.

Some of those permutations are potential deadlocks even with the current
nesting hotplug locking scheme, but they can't be discovered by lockdep.

The conversion of the hotplug locking to a percpu rwsem requires to prevent
nested locking, so it's required to take the hotplug rwsem early in the
call chain and establish a proper lock order.

After quite some analysis and going down the wrong road severa times the
following lock order has been chosen:

kprobe_mutex -> cpus_rwsem -> jump_label_mutex -> text_mutex

For kprobes which hook on an ftrace function trace point, it's required to
drop cpus_rwsem before calling into the ftrace code to avoid a deadlock on
the func_hash->regex_lock.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
[ Steven: Ftrace interaction fixes ]
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

Note: The above SOB chain is actually correct as Steven and me bounced the
patch series back and forth, but the result has to be a single patch.

 kernel/kprobes.c |   59 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

Index: b/kernel/kprobes.c
===================================================================
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -483,11 +483,6 @@ static DECLARE_DELAYED_WORK(optimizing_w
  */
 static void do_optimize_kprobes(void)
 {
-	/* Optimization never be done when disarmed */
-	if (kprobes_all_disarmed || !kprobes_allow_optimization ||
-	    list_empty(&optimizing_list))
-		return;
-
 	/*
 	 * The optimization/unoptimization refers online_cpus via
 	 * stop_machine() and cpu-hotplug modifies online_cpus.
@@ -495,14 +490,19 @@ static void do_optimize_kprobes(void)
 	 * This combination can cause a deadlock (cpu-hotplug try to lock
 	 * text_mutex but stop_machine can not be done because online_cpus
 	 * has been changed)
-	 * To avoid this deadlock, we need to call get_online_cpus()
+	 * To avoid this deadlock, caller must have locked cpu hotplug
 	 * for preventing cpu-hotplug outside of text_mutex locking.
 	 */
-	get_online_cpus();
+	lockdep_assert_cpus_held();
+
+	/* Optimization never be done when disarmed */
+	if (kprobes_all_disarmed || !kprobes_allow_optimization ||
+	    list_empty(&optimizing_list))
+		return;
+
 	mutex_lock(&text_mutex);
 	arch_optimize_kprobes(&optimizing_list);
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 /*
@@ -513,12 +513,13 @@ static void do_unoptimize_kprobes(void)
 {
 	struct optimized_kprobe *op, *tmp;
 
+	/* See comment in do_optimize_kprobes() */
+	lockdep_assert_cpus_held();
+
 	/* Unoptimization must be done anytime */
 	if (list_empty(&unoptimizing_list))
 		return;
 
-	/* Ditto to do_optimize_kprobes */
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 	arch_unoptimize_kprobes(&unoptimizing_list, &freeing_list);
 	/* Loop free_list for disarming */
@@ -537,7 +538,6 @@ static void do_unoptimize_kprobes(void)
 			list_del_init(&op->list);
 	}
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 }
 
 /* Reclaim all kprobes on the free_list */
@@ -562,6 +562,7 @@ static void kick_kprobe_optimizer(void)
 static void kprobe_optimizer(struct work_struct *work)
 {
 	mutex_lock(&kprobe_mutex);
+	cpus_read_lock();
 	/* Lock modules while optimizing kprobes */
 	mutex_lock(&module_mutex);
 
@@ -587,6 +588,7 @@ static void kprobe_optimizer(struct work
 	do_free_cleaned_kprobes();
 
 	mutex_unlock(&module_mutex);
+	cpus_read_unlock();
 	mutex_unlock(&kprobe_mutex);
 
 	/* Step 5: Kick optimizer again if needed */
@@ -650,9 +652,8 @@ static void optimize_kprobe(struct kprob
 /* Short cut to direct unoptimizing */
 static void force_unoptimize_kprobe(struct optimized_kprobe *op)
 {
-	get_online_cpus();
+	lockdep_assert_cpus_held();
 	arch_unoptimize_kprobe(op);
-	put_online_cpus();
 	if (kprobe_disabled(&op->kp))
 		arch_disarm_kprobe(&op->kp);
 }
@@ -791,6 +792,7 @@ static void try_to_optimize_kprobe(struc
 		return;
 
 	/* For preparing optimization, jump_label_text_reserved() is called */
+	cpus_read_lock();
 	jump_label_lock();
 	mutex_lock(&text_mutex);
 
@@ -812,6 +814,7 @@ static void try_to_optimize_kprobe(struc
 out:
 	mutex_unlock(&text_mutex);
 	jump_label_unlock();
+	cpus_read_unlock();
 }
 
 #ifdef CONFIG_SYSCTL
@@ -826,6 +829,7 @@ static void optimize_all_kprobes(void)
 	if (kprobes_allow_optimization)
 		goto out;
 
+	cpus_read_lock();
 	kprobes_allow_optimization = true;
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
@@ -833,6 +837,7 @@ static void optimize_all_kprobes(void)
 			if (!kprobe_disabled(p))
 				optimize_kprobe(p);
 	}
+	cpus_read_unlock();
 	printk(KERN_INFO "Kprobes globally optimized\n");
 out:
 	mutex_unlock(&kprobe_mutex);
@@ -851,6 +856,7 @@ static void unoptimize_all_kprobes(void)
 		return;
 	}
 
+	cpus_read_lock();
 	kprobes_allow_optimization = false;
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
@@ -859,6 +865,7 @@ static void unoptimize_all_kprobes(void)
 				unoptimize_kprobe(p, false);
 		}
 	}
+	cpus_read_unlock();
 	mutex_unlock(&kprobe_mutex);
 
 	/* Wait for unoptimizing completion */
@@ -1010,14 +1017,11 @@ static void arm_kprobe(struct kprobe *kp
 		arm_kprobe_ftrace(kp);
 		return;
 	}
-	/*
-	 * Here, since __arm_kprobe() doesn't use stop_machine(),
-	 * this doesn't cause deadlock on text_mutex. So, we don't
-	 * need get_online_cpus().
-	 */
+	cpus_read_lock();
 	mutex_lock(&text_mutex);
 	__arm_kprobe(kp);
 	mutex_unlock(&text_mutex);
+	cpus_read_unlock();
 }
 
 /* Disarm a kprobe with text_mutex */
@@ -1027,10 +1031,12 @@ static void disarm_kprobe(struct kprobe
 		disarm_kprobe_ftrace(kp);
 		return;
 	}
-	/* Ditto */
+
+	cpus_read_lock();
 	mutex_lock(&text_mutex);
 	__disarm_kprobe(kp, reopt);
 	mutex_unlock(&text_mutex);
+	cpus_read_unlock();
 }
 
 /*
@@ -1298,13 +1304,10 @@ static int register_aggr_kprobe(struct k
 	int ret = 0;
 	struct kprobe *ap = orig_p;
 
+	cpus_read_lock();
+
 	/* For preparing optimization, jump_label_text_reserved() is called */
 	jump_label_lock();
-	/*
-	 * Get online CPUs to avoid text_mutex deadlock.with stop machine,
-	 * which is invoked by unoptimize_kprobe() in add_new_kprobe()
-	 */
-	get_online_cpus();
 	mutex_lock(&text_mutex);
 
 	if (!kprobe_aggrprobe(orig_p)) {
@@ -1352,8 +1355,8 @@ static int register_aggr_kprobe(struct k
 
 out:
 	mutex_unlock(&text_mutex);
-	put_online_cpus();
 	jump_label_unlock();
+	cpus_read_unlock();
 
 	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
 		ap->flags &= ~KPROBE_FLAG_DISABLED;
@@ -1555,9 +1558,12 @@ int register_kprobe(struct kprobe *p)
 		goto out;
 	}
 
-	mutex_lock(&text_mutex);	/* Avoiding text modification */
+	cpus_read_lock();
+	/* Prevent text modification */
+	mutex_lock(&text_mutex);
 	ret = prepare_kprobe(p);
 	mutex_unlock(&text_mutex);
+	cpus_read_unlock();
 	if (ret)
 		goto out;
 
@@ -1570,7 +1576,6 @@ int register_kprobe(struct kprobe *p)
 
 	/* Try to optimize kprobe */
 	try_to_optimize_kprobe(p);
-
 out:
 	mutex_unlock(&kprobe_mutex);
 

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

#1649726 — Re: [patch V3 25/32] kprobes: Cure hotplug lock ordering issues

On Wed, 24 May 2017 10:15:36 +0200
Thomas Gleixner <tglx@linutronix.de> wrote:

> Converting the cpu hotplug locking to a percpu rwsem unearthed hidden lock
> ordering problems.
> 
> There is a wide range of locks involved in this: kprobe_mutex,
> jump_label_mutex, ftrace_lock, text_mutex, event_mutex,
> func_hash->regex_lock and a gazillion of lock order permutations with
> nested get_online_cpus() calls.

And module_mutex too ;-)

> 
> Some of those permutations are potential deadlocks even with the current
> nesting hotplug locking scheme, but they can't be discovered by lockdep.
> 
> The conversion of the hotplug locking to a percpu rwsem requires to prevent
> nested locking, so it's required to take the hotplug rwsem early in the
> call chain and establish a proper lock order.
> 
> After quite some analysis and going down the wrong road severa times the
> following lock order has been chosen:
> 
> kprobe_mutex -> cpus_rwsem -> jump_label_mutex -> text_mutex

This seems only change the locking order of module_mutex and
cpus_rwsem. Previously module_mutex -> cpus_rwsem, now
cpus_rwsem -> module_mutex. and it seems OK to me.
(checked in module.c and other use cases of module_mutex)

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> 
> For kprobes which hook on an ftrace function trace point, it's required to
> drop cpus_rwsem before calling into the ftrace code to avoid a deadlock on
> the func_hash->regex_lock.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> [ Steven: Ftrace interaction fixes ]
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> ---
> 
> Note: The above SOB chain is actually correct as Steven and me bounced the
> patch series back and forth, but the result has to be a single patch.
> 
>  kernel/kprobes.c |   59 +++++++++++++++++++++++++++++--------------------------
>  1 file changed, 32 insertions(+), 27 deletions(-)
> 
> Index: b/kernel/kprobes.c
> ===================================================================
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -483,11 +483,6 @@ static DECLARE_DELAYED_WORK(optimizing_w
>   */
>  static void do_optimize_kprobes(void)
>  {
> -	/* Optimization never be done when disarmed */
> -	if (kprobes_all_disarmed || !kprobes_allow_optimization ||
> -	    list_empty(&optimizing_list))
> -		return;
> -
>  	/*
>  	 * The optimization/unoptimization refers online_cpus via
>  	 * stop_machine() and cpu-hotplug modifies online_cpus.
> @@ -495,14 +490,19 @@ static void do_optimize_kprobes(void)
>  	 * This combination can cause a deadlock (cpu-hotplug try to lock
>  	 * text_mutex but stop_machine can not be done because online_cpus
>  	 * has been changed)
> -	 * To avoid this deadlock, we need to call get_online_cpus()
> +	 * To avoid this deadlock, caller must have locked cpu hotplug
>  	 * for preventing cpu-hotplug outside of text_mutex locking.
>  	 */
> -	get_online_cpus();
> +	lockdep_assert_cpus_held();
> +
> +	/* Optimization never be done when disarmed */
> +	if (kprobes_all_disarmed || !kprobes_allow_optimization ||
> +	    list_empty(&optimizing_list))
> +		return;
> +
>  	mutex_lock(&text_mutex);
>  	arch_optimize_kprobes(&optimizing_list);
>  	mutex_unlock(&text_mutex);
> -	put_online_cpus();
>  }
>  
>  /*
> @@ -513,12 +513,13 @@ static void do_unoptimize_kprobes(void)
>  {
>  	struct optimized_kprobe *op, *tmp;
>  
> +	/* See comment in do_optimize_kprobes() */
> +	lockdep_assert_cpus_held();
> +
>  	/* Unoptimization must be done anytime */
>  	if (list_empty(&unoptimizing_list))
>  		return;
>  
> -	/* Ditto to do_optimize_kprobes */
> -	get_online_cpus();
>  	mutex_lock(&text_mutex);
>  	arch_unoptimize_kprobes(&unoptimizing_list, &freeing_list);
>  	/* Loop free_list for disarming */
> @@ -537,7 +538,6 @@ static void do_unoptimize_kprobes(void)
>  			list_del_init(&op->list);
>  	}
>  	mutex_unlock(&text_mutex);
> -	put_online_cpus();
>  }
>  
>  /* Reclaim all kprobes on the free_list */
> @@ -562,6 +562,7 @@ static void kick_kprobe_optimizer(void)
>  static void kprobe_optimizer(struct work_struct *work)
>  {
>  	mutex_lock(&kprobe_mutex);
> +	cpus_read_lock();
>  	/* Lock modules while optimizing kprobes */
>  	mutex_lock(&module_mutex);
>  
> @@ -587,6 +588,7 @@ static void kprobe_optimizer(struct work
>  	do_free_cleaned_kprobes();
>  
>  	mutex_unlock(&module_mutex);
> +	cpus_read_unlock();
>  	mutex_unlock(&kprobe_mutex);
>  
>  	/* Step 5: Kick optimizer again if needed */
> @@ -650,9 +652,8 @@ static void optimize_kprobe(struct kprob
>  /* Short cut to direct unoptimizing */
>  static void force_unoptimize_kprobe(struct optimized_kprobe *op)
>  {
> -	get_online_cpus();
> +	lockdep_assert_cpus_held();
>  	arch_unoptimize_kprobe(op);
> -	put_online_cpus();
>  	if (kprobe_disabled(&op->kp))
>  		arch_disarm_kprobe(&op->kp);
>  }
> @@ -791,6 +792,7 @@ static void try_to_optimize_kprobe(struc
>  		return;
>  
>  	/* For preparing optimization, jump_label_text_reserved() is called */
> +	cpus_read_lock();
>  	jump_label_lock();
>  	mutex_lock(&text_mutex);
>  
> @@ -812,6 +814,7 @@ static void try_to_optimize_kprobe(struc
>  out:
>  	mutex_unlock(&text_mutex);
>  	jump_label_unlock();
> +	cpus_read_unlock();
>  }
>  
>  #ifdef CONFIG_SYSCTL
> @@ -826,6 +829,7 @@ static void optimize_all_kprobes(void)
>  	if (kprobes_allow_optimization)
>  		goto out;
>  
> +	cpus_read_lock();
>  	kprobes_allow_optimization = true;
>  	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>  		head = &kprobe_table[i];
> @@ -833,6 +837,7 @@ static void optimize_all_kprobes(void)
>  			if (!kprobe_disabled(p))
>  				optimize_kprobe(p);
>  	}
> +	cpus_read_unlock();
>  	printk(KERN_INFO "Kprobes globally optimized\n");
>  out:
>  	mutex_unlock(&kprobe_mutex);
> @@ -851,6 +856,7 @@ static void unoptimize_all_kprobes(void)
>  		return;
>  	}
>  
> +	cpus_read_lock();
>  	kprobes_allow_optimization = false;
>  	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>  		head = &kprobe_table[i];
> @@ -859,6 +865,7 @@ static void unoptimize_all_kprobes(void)
>  				unoptimize_kprobe(p, false);
>  		}
>  	}
> +	cpus_read_unlock();
>  	mutex_unlock(&kprobe_mutex);
>  
>  	/* Wait for unoptimizing completion */
> @@ -1010,14 +1017,11 @@ static void arm_kprobe(struct kprobe *kp
>  		arm_kprobe_ftrace(kp);
>  		return;
>  	}
> -	/*
> -	 * Here, since __arm_kprobe() doesn't use stop_machine(),
> -	 * this doesn't cause deadlock on text_mutex. So, we don't
> -	 * need get_online_cpus().
> -	 */
> +	cpus_read_lock();
>  	mutex_lock(&text_mutex);
>  	__arm_kprobe(kp);
>  	mutex_unlock(&text_mutex);
> +	cpus_read_unlock();
>  }
>  
>  /* Disarm a kprobe with text_mutex */
> @@ -1027,10 +1031,12 @@ static void disarm_kprobe(struct kprobe
>  		disarm_kprobe_ftrace(kp);
>  		return;
>  	}
> -	/* Ditto */
> +
> +	cpus_read_lock();
>  	mutex_lock(&text_mutex);
>  	__disarm_kprobe(kp, reopt);
>  	mutex_unlock(&text_mutex);
> +	cpus_read_unlock();
>  }
>  
>  /*
> @@ -1298,13 +1304,10 @@ static int register_aggr_kprobe(struct k
>  	int ret = 0;
>  	struct kprobe *ap = orig_p;
>  
> +	cpus_read_lock();
> +
>  	/* For preparing optimization, jump_label_text_reserved() is called */
>  	jump_label_lock();
> -	/*
> -	 * Get online CPUs to avoid text_mutex deadlock.with stop machine,
> -	 * which is invoked by unoptimize_kprobe() in add_new_kprobe()
> -	 */
> -	get_online_cpus();
>  	mutex_lock(&text_mutex);
>  
>  	if (!kprobe_aggrprobe(orig_p)) {
> @@ -1352,8 +1355,8 @@ static int register_aggr_kprobe(struct k
>  
>  out:
>  	mutex_unlock(&text_mutex);
> -	put_online_cpus();
>  	jump_label_unlock();
> +	cpus_read_unlock();
>  
>  	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
>  		ap->flags &= ~KPROBE_FLAG_DISABLED;
> @@ -1555,9 +1558,12 @@ int register_kprobe(struct kprobe *p)
>  		goto out;
>  	}
>  
> -	mutex_lock(&text_mutex);	/* Avoiding text modification */
> +	cpus_read_lock();
> +	/* Prevent text modification */
> +	mutex_lock(&text_mutex);
>  	ret = prepare_kprobe(p);
>  	mutex_unlock(&text_mutex);
> +	cpus_read_unlock();
>  	if (ret)
>  		goto out;
>  
> @@ -1570,7 +1576,6 @@ int register_kprobe(struct kprobe *p)
>  
>  	/* Try to optimize kprobe */
>  	try_to_optimize_kprobe(p);
> -
>  out:
>  	mutex_unlock(&kprobe_mutex);
>  
> 
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

#1649285 — [patch V3 03/32] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()

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

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

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

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

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>

---
 include/linux/cpuhotplug.h |   29 +++++++++++++++++++++++++++
 kernel/cpu.c               |   47 ++++++++++++++++++++++++++++++++++-----------
 2 files changed, 65 insertions(+), 11 deletions(-)

--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -153,6 +153,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_cpuslocked(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
@@ -171,6 +176,15 @@ static inline int cpuhp_setup_state(enum
 	return __cpuhp_setup_state(state, name, true, startup, teardown, false);
 }
 
+static inline int cpuhp_setup_state_cpuslocked(enum cpuhp_state state,
+					       const char *name,
+					       int (*startup)(unsigned int cpu),
+					       int (*teardown)(unsigned int cpu))
+{
+	return __cpuhp_setup_state_cpuslocked(state, name, true, startup,
+					      teardown, false);
+}
+
 /**
  * cpuhp_setup_state_nocalls - Setup hotplug state callbacks without calling the
  *			       callbacks
@@ -191,6 +205,15 @@ static inline int cpuhp_setup_state_noca
 				   false);
 }
 
+static inline int cpuhp_setup_state_nocalls_cpuslocked(enum cpuhp_state state,
+						     const char *name,
+						     int (*startup)(unsigned int cpu),
+						     int (*teardown)(unsigned int cpu))
+{
+	return __cpuhp_setup_state_cpuslocked(state, name, false, startup,
+					    teardown, false);
+}
+
 /**
  * cpuhp_setup_state_multi - Add callbacks for multi state
  * @state:	The state for which the calls are installed
@@ -250,6 +273,7 @@ static inline int cpuhp_state_add_instan
 }
 
 void __cpuhp_remove_state(enum cpuhp_state state, bool invoke);
+void __cpuhp_remove_state_cpuslocked(enum cpuhp_state state, bool invoke);
 
 /**
  * cpuhp_remove_state - Remove hotplug state callbacks and invoke the teardown
@@ -273,6 +297,11 @@ static inline void cpuhp_remove_state_no
 	__cpuhp_remove_state(state, false);
 }
 
+static inline void cpuhp_remove_state_nocalls_cpuslocked(enum cpuhp_state state)
+{
+	__cpuhp_remove_state_cpuslocked(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
@@ -1459,7 +1459,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_cpuslocked - 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
@@ -1468,25 +1468,27 @@ 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 cpus read locked 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_cpuslocked(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;
 
+	lockdep_assert_cpus_held();
+
 	if (cpuhp_cb_check(state) || !name)
 		return -EINVAL;
 
-	cpus_read_lock();
 	mutex_lock(&cpuhp_state_mutex);
 
 	ret = cpuhp_store_callbacks(state, name, startup, teardown,
@@ -1522,7 +1524,6 @@ int __cpuhp_setup_state(enum cpuhp_state
 	}
 out:
 	mutex_unlock(&cpuhp_state_mutex);
-	cpus_read_unlock();
 	/*
 	 * If the requested state is CPUHP_AP_ONLINE_DYN, return the
 	 * dynamically allocated state in case of success.
@@ -1531,6 +1532,22 @@ int __cpuhp_setup_state(enum cpuhp_state
 		return state;
 	return ret;
 }
+EXPORT_SYMBOL(__cpuhp_setup_state_cpuslocked);
+
+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;
+
+	cpus_read_lock();
+	ret = __cpuhp_setup_state_cpuslocked(state, name, invoke, startup,
+					     teardown, multi_instance);
+	cpus_read_unlock();
+	return ret;
+}
 EXPORT_SYMBOL(__cpuhp_setup_state);
 
 int __cpuhp_state_remove_instance(enum cpuhp_state state,
@@ -1572,22 +1589,23 @@ 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_cpuslocked - 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 cpus read locked 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_cpuslocked(enum cpuhp_state state, bool invoke)
 {
 	struct cpuhp_step *sp = cpuhp_get_step(state);
 	int cpu;
 
 	BUG_ON(cpuhp_cb_check(state));
 
-	cpus_read_lock();
+	lockdep_assert_cpus_held();
 
 	mutex_lock(&cpuhp_state_mutex);
 	if (sp->multi_instance) {
@@ -1615,6 +1633,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_cpuslocked);
+
+void __cpuhp_remove_state(enum cpuhp_state state, bool invoke)
+{
+	cpus_read_lock();
+	__cpuhp_remove_state_cpuslocked(state, invoke);
 	cpus_read_unlock();
 }
 EXPORT_SYMBOL(__cpuhp_remove_state);

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

#1649288 — [patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked()

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

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. Convert
*_online_cpus() to the new interfaces while at it.

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>

---
 arch/s390/kernel/time.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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

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

#1649465 — Re: [patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked()

On Wed, May 24, 2017 at 10:15:26AM +0200, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> 
> 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. Convert
> *_online_cpus() to the new interfaces while at it.
> 
> 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>
> 
> ---
>  arch/s390/kernel/time.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>

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

#1649290 — [patch V3 06/32] 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: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-crypto@vger.kernel.org

---
 include/linux/padata.h |    3 ---
 kernel/padata.c        |   32 ++++++++++++++++----------------
 2 files changed, 16 insertions(+), 19 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
@@ -934,19 +934,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.
  *
@@ -954,9 +941,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;
@@ -1011,6 +998,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]

#1649292 — [patch V3 07/32] padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()

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

pcrypt_init_padata()
   cpus_read_lock()
   padata_alloc_possible()
     padata_alloc()
       cpus_read_lock()

The nested call to cpus_read_lock() 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 cpus_read_lock() protected region as well.

Remove the cpus_read_lock() 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: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-crypto@vger.kernel.org

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

--- a/kernel/padata.c
+++ b/kernel/padata.c
@@ -940,6 +940,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 cpus_read_lock() protected region
  */
 static struct padata_instance *padata_alloc(struct workqueue_struct *wq,
 					    const struct cpumask *pcpumask,
@@ -952,7 +954,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)) {
@@ -976,14 +977,12 @@ static struct padata_instance *padata_al
 
 	pinst->flags = 0;
 
-	put_online_cpus();
-
 	BLOCKING_INIT_NOTIFIER_HEAD(&pinst->cpumask_change_notifier);
 	kobject_init(&pinst->kobj, &padata_attr_type);
 	mutex_init(&pinst->lock);
 
 #ifdef CONFIG_HOTPLUG_CPU
-	cpuhp_state_add_instance_nocalls(hp_online, &pinst->node);
+	cpuhp_state_add_instance_nocalls_cpuslocked(hp_online, &pinst->node);
 #endif
 	return pinst;
 
@@ -992,7 +991,6 @@ static struct padata_instance *padata_al
 	free_cpumask_var(pinst->cpumask.cbcpu);
 err_free_inst:
 	kfree(pinst);
-	put_online_cpus();
 err:
 	return NULL;
 }
@@ -1003,9 +1001,12 @@ static struct padata_instance *padata_al
  *                         parallel workers.
  *
  * @wq: workqueue to use for the allocated padata instance
+ *
+ * Must be called from a cpus_read_lock() protected region
  */
 struct padata_instance *padata_alloc_possible(struct workqueue_struct *wq)
 {
+	lockdep_assert_cpus_held();
 	return padata_alloc(wq, cpu_possible_mask, cpu_possible_mask);
 }
 EXPORT_SYMBOL(padata_alloc_possible);

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

#1649307 — [patch V3 13/32] perf/x86/intel/cqm: Use cpuhp_setup_state_cpuslocked()

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

intel_cqm_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. Convert
*_online_cpus() to the new interfaces while at it.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>

---
 arch/x86/events/intel/cqm.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

--- a/arch/x86/events/intel/cqm.c
+++ b/arch/x86/events/intel/cqm.c
@@ -1682,7 +1682,7 @@ static int __init intel_cqm_init(void)
 	 *
 	 * Also, check that the scales match on all cpus.
 	 */
-	get_online_cpus();
+	cpus_read_lock();
 	for_each_online_cpu(cpu) {
 		struct cpuinfo_x86 *c = &cpu_data(cpu);
 
@@ -1746,14 +1746,14 @@ static int __init intel_cqm_init(void)
 	 * Setup the hot cpu notifier once we are sure cqm
 	 * is enabled to avoid notifier leak.
 	 */
-	cpuhp_setup_state(CPUHP_AP_PERF_X86_CQM_STARTING,
-			  "perf/x86/cqm:starting",
-			  intel_cqm_cpu_starting, NULL);
-	cpuhp_setup_state(CPUHP_AP_PERF_X86_CQM_ONLINE, "perf/x86/cqm:online",
-			  NULL, intel_cqm_cpu_exit);
-
+	cpuhp_setup_state_cpuslocked(CPUHP_AP_PERF_X86_CQM_STARTING,
+				     "perf/x86/cqm:starting",
+				     intel_cqm_cpu_starting, NULL);
+	cpuhp_setup_state_cpuslocked(CPUHP_AP_PERF_X86_CQM_ONLINE,
+				     "perf/x86/cqm:online",
+				     NULL, intel_cqm_cpu_exit);
 out:
-	put_online_cpus();
+	cpus_read_unlock();
 
 	if (ret) {
 		kfree(str);

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

#1649314 — [patch V3 11/32] 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. Convert *_online_cpus() to the new interfaces while at it.

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

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

--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -587,7 +587,7 @@ static void etm_disable_sysfs(struct cor
 	 * after cpu online mask indicates the cpu is offline but before the
 	 * DYING hotplug callback is serviced by the ETM driver.
 	 */
-	get_online_cpus();
+	cpus_read_lock();
 	spin_lock(&drvdata->spinlock);
 
 	/*
@@ -597,7 +597,7 @@ static void etm_disable_sysfs(struct cor
 	smp_call_function_single(drvdata->cpu, etm_disable_hw, drvdata, 1);
 
 	spin_unlock(&drvdata->spinlock);
-	put_online_cpus();
+	cpus_read_unlock();
 
 	dev_info(drvdata->dev, "ETM tracing disabled\n");
 }
@@ -795,7 +795,7 @@ static int etm_probe(struct amba_device
 
 	drvdata->cpu = pdata ? pdata->cpu : 0;
 
-	get_online_cpus();
+	cpus_read_lock();
 	etmdrvdata[drvdata->cpu] = drvdata;
 
 	if (smp_call_function_single(drvdata->cpu,
@@ -803,17 +803,17 @@ 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;
 	}
-	put_online_cpus();
+	cpus_read_unlock();
 
 	if (etm_arch_supported(drvdata->arch) == false) {
 		ret = -EINVAL;

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

#1649750

On Wed, May 24, 2017 at 10:15:11AM +0200, Thomas Gleixner wrote:
> 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
> a (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 is on top of 4.12-rc2. It's available in git from
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.hotplug
> 
> Changes since V2:
> 
>   - Reworked the approach vs. perf/ftrace/kprobes, which simplified the lot
>   
>   - Renamed get_online_cpus() to cpus_read_lock() to reflect the nature of
>     the interface
> 
>   - Link the lockchains between hotplug control task and per cpu hotplug
>     threads and fixed the fallout of that.

Passed moderate rcutorture testing, so:

Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Thanks,
> 
>         tglx
> 
> ---
>  arch/arm/kernel/hw_breakpoint.c               |   11 -
>  arch/arm/kernel/patch.c                       |    2 
>  arch/arm/probes/kprobes/core.c                |    3 
>  arch/arm64/include/asm/insn.h                 |    1 
>  arch/arm64/kernel/insn.c                      |    5 
>  arch/mips/kernel/jump_label.c                 |    2 
>  arch/powerpc/kvm/book3s_hv.c                  |   14 -
>  arch/powerpc/platforms/powernv/subcore.c      |    7 
>  arch/s390/kernel/jump_label.c                 |    2 
>  arch/s390/kernel/kprobes.c                    |    4 
>  arch/s390/kernel/time.c                       |    6 
>  arch/x86/events/core.c                        |    1 
>  arch/x86/events/intel/cqm.c                   |   16 -
>  arch/x86/kernel/cpu/mtrr/main.c               |    2 
>  b/arch/sparc/kernel/jump_label.c              |    2 
>  b/arch/tile/kernel/jump_label.c               |    2 
>  b/arch/x86/events/intel/core.c                |   11 -
>  b/arch/x86/kernel/jump_label.c                |    2 
>  b/kernel/jump_label.c                         |   20 +-
>  drivers/acpi/processor_driver.c               |    4 
>  drivers/acpi/processor_throttling.c           |   16 -
>  drivers/cpufreq/cpufreq.c                     |   21 +-
>  drivers/hwtracing/coresight/coresight-etm3x.c |   20 +-
>  drivers/hwtracing/coresight/coresight-etm4x.c |   20 +-
>  drivers/pci/pci-driver.c                      |   47 +++--
>  include/linux/cpu.h                           |   34 ++--
>  include/linux/cpuhotplug.h                    |   38 ++++
>  include/linux/padata.h                        |    3 
>  include/linux/pci.h                           |    1 
>  include/linux/perf_event.h                    |    2 
>  include/linux/sched.h                         |   10 +
>  include/linux/stop_machine.h                  |   26 ++-
>  kernel/cpu.c                                  |  213 +++++++++++---------------
>  kernel/events/core.c                          |  106 +++++++++---
>  kernel/kprobes.c                              |   59 +++----
>  kernel/padata.c                               |   43 ++---
>  kernel/stop_machine.c                         |   11 -
>  37 files changed, 444 insertions(+), 343 deletions(-)
> 
> 
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web