Groups > linux.kernel > #1649274 > unrolled thread

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

Started by	Thomas Gleixner <tglx@linutronix.de>
First post	2017-05-24 10:40 +0200
Last post	2017-05-26 09:10 +0200
Articles	20 on this page of 27 — 8 participants

Back to article view | Back to linux.kernel

  [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
      Re: [patch V3 12/32] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_cpuslocked() Mathieu Poirier <mathieu.poirier@linaro.org> - 2017-05-25 18:50 +0200
      [tip:smp/hotplug] hwtracing/coresight-etm4x: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 10:50 +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
      [tip:smp/hotplug] s390: Prevent hotplug rwsem recursion tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-26 10:50 +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
        Re: [patch V3 25/32] kprobes: Cure hotplug lock ordering issues Thomas Gleixner <tglx@linutronix.de> - 2017-05-26 09:50 +0200
      [tip:smp/hotplug] kprobes: Cure hotplug lock ordering issues tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-26 10:50 +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
      [tip:smp/hotplug] cpu/hotplug: Provide  cpuhp_setup/remove_state[_nocalls]_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 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
      [tip:smp/hotplug] s390/kernel: Use stop_machine_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 10:50 +0200
    [patch V3 06/32] padata: Make padata_alloc() static Thomas Gleixner <tglx@linutronix.de> - 2017-05-24 10:40 +0200
      [tip:smp/hotplug] padata: Make padata_alloc() static tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-26 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
      [tip:smp/hotplug] padata: Avoid nested calls to cpus_read_lock() in  pcrypt_init_padata() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 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
      [tip:smp/hotplug] perf/x86/intel/cqm: Use  cpuhp_setup_state_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 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 11/32] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() Mathieu Poirier <mathieu.poirier@linaro.org> - 2017-05-25 18:50 +0200
      [tip:smp/hotplug] hwtracing/coresight-etm3x: Use  cpuhp_setup_state_nocalls_cpuslocked() tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com> - 2017-05-26 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
    Re: [patch V3 00/32] cpu/hotplug: Convert get_online_cpus() to a  percpu_rwsem Ingo Molnar <mingo@kernel.org> - 2017-05-26 09:10 +0200

Page 1 of 2 [1] 2 Next page →

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

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 00/32] cpu/hotplug: Convert get_online_cpus() to a percpu_rwsem
Message-ID	<tKzQ6-8hN-3@gated-at.bofh.it>

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	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 12/32] hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()
Message-ID	<tKzZM-8mT-21@gated-at.bofh.it>
In reply to	#1649274

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]

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

From	Mathieu Poirier <mathieu.poirier@linaro.org>
Date	2017-05-25 18:50 +0200
Subject	Re: [patch V3 12/32] hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()
Message-ID	<tL47v-29T-7@gated-at.bofh.it>
In reply to	#1649276

On Wed, May 24, 2017 at 10:15:23AM +0200, Thomas Gleixner wrote:
> 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

Acked-by: Mathieu Poirier <mathieu.poirier@linaro.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]

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

From	tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com>
Date	2017-05-26 10:50 +0200
Subject	[tip:smp/hotplug] hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()
Message-ID	<tLj6z-3Cw-41@gated-at.bofh.it>
In reply to	#1649276

Commit-ID:  e9f5d63f84febb7e9dfe4e0dc696adf88053fbf2
Gitweb:     http://git.kernel.org/tip/e9f5d63f84febb7e9dfe4e0dc696adf88053fbf2
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:23 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:39 +0200

hwtracing/coresight-etm4x: Use cpuhp_setup_state_nocalls_cpuslocked()

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>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-arm-kernel@lists.infradead.org
Link: http://lkml.kernel.org/r/20170524081547.983493849@linutronix.de

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

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index d1340fb..532adc9 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -371,7 +371,7 @@ static void etm4_disable_sysfs(struct coresight_device *csdev)
 	 * 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 coresight_device *csdev)
 	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 *adev, const struct amba_id *id)
 
 	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 *adev, const struct amba_id *id)
 		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

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 28/32] s390: Prevent hotplug rwsem recursion
Message-ID	<tKzZM-8mT-23@gated-at.bofh.it>
In reply to	#1649274

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

From	Heiko Carstens <heiko.carstens@de.ibm.com>
Date	2017-05-24 13:00 +0200
Subject	Re: [patch V3 28/32] s390: Prevent hotplug rwsem recursion
Message-ID	<tKCbg-1hF-31@gated-at.bofh.it>
In reply to	#1649278

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]

#1651223 — [tip:smp/hotplug] s390: Prevent hotplug rwsem recursion

From	tip-bot for Thomas Gleixner <tipbot@zytor.com>
Date	2017-05-26 10:50 +0200
Subject	[tip:smp/hotplug] s390: Prevent hotplug rwsem recursion
Message-ID	<tLj6y-3Cw-23@gated-at.bofh.it>
In reply to	#1649278

Commit-ID:  5d5dbc4ef27e72104dea6102e4d1a1bf5a8ed971
Gitweb:     http://git.kernel.org/tip/5d5dbc4ef27e72104dea6102e4d1a1bf5a8ed971
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:39 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:46 +0200

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>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20170524081549.354513406@linutronix.de

---
 arch/s390/kernel/jump_label.c | 2 +-
 arch/s390/kernel/kprobes.c    | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
index 6aa630a..262506c 100644
--- a/arch/s390/kernel/jump_label.c
+++ b/arch/s390/kernel/jump_label.c
@@ -93,7 +93,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	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,
diff --git a/arch/s390/kernel/kprobes.c b/arch/s390/kernel/kprobes.c
index 3d6a997..6842e45 100644
--- 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]

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

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 25/32] kprobes: Cure hotplug lock ordering issues
Message-ID	<tKzZN-8mT-39@gated-at.bofh.it>
In reply to	#1649274

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

From	Masami Hiramatsu <mhiramat@kernel.org>
Date	2017-05-24 18:00 +0200
Subject	Re: [patch V3 25/32] kprobes: Cure hotplug lock ordering issues
Message-ID	<tKGRz-4dh-5@gated-at.bofh.it>
In reply to	#1649284

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]

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

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-26 09:50 +0200
Subject	Re: [patch V3 25/32] kprobes: Cure hotplug lock ordering issues
Message-ID	<tLiat-31c-5@gated-at.bofh.it>
In reply to	#1649726

On Thu, 25 May 2017, Masami Hiramatsu wrote:
> 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 ;-)

Indeed.

> > 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)

This also changes the jump label / text mutex interaction with hotplug
locking if you look closely :)

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

#1651225 — [tip:smp/hotplug] kprobes: Cure hotplug lock ordering issues

From	tip-bot for Thomas Gleixner <tipbot@zytor.com>
Date	2017-05-26 10:50 +0200
Subject	[tip:smp/hotplug] kprobes: Cure hotplug lock ordering issues
Message-ID	<tLj6z-3Cw-27@gated-at.bofh.it>
In reply to	#1649284

Commit-ID:  2d1e38f56622b9bb5af85be63c1052c056f5c677
Gitweb:     http://git.kernel.org/tip/2d1e38f56622b9bb5af85be63c1052c056f5c677
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:36 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:45 +0200

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, module_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.

[ Steven: Ftrace interaction fixes ]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sebastian Siewior <bigeasy@linutronix.de>
Link: http://lkml.kernel.org/r/20170524081549.104864779@linutronix.de

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

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2d2d3a5..9f60567 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -483,11 +483,6 @@ static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
  */
 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_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 kprobe *p)
 /* 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(struct kprobe *p)
 		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(struct kprobe *p)
 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 *kp, bool reopt)
 		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 kprobe *orig_p, struct kprobe *p)
 	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 kprobe *orig_p, struct kprobe *p)
 
 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]

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

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 03/32] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()
Message-ID	<tKzZN-8mT-33@gated-at.bofh.it>
In reply to	#1649274

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]

#1651203 — [tip:smp/hotplug] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()

From	tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com>
Date	2017-05-26 10:40 +0200
Subject	[tip:smp/hotplug] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()
Message-ID	<tLiWS-3x5-31@gated-at.bofh.it>
In reply to	#1649285

Commit-ID:  71def423fe3da0d40ad3427a4cd5f9edc53bff67
Gitweb:     http://git.kernel.org/tip/71def423fe3da0d40ad3427a4cd5f9edc53bff67
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:14 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:35 +0200

cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()

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>
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/20170524081547.239600868@linutronix.de

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

diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 0f2a803..4fac564 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -153,6 +153,11 @@ 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);
 /**
  * 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 cpuhp_state state,
 	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_nocalls(enum cpuhp_state state,
 				   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_instance_nocalls(enum cpuhp_state state,
 }
 
 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_nocalls(enum cpuhp_state state)
 	__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
diff --git a/kernel/cpu.c b/kernel/cpu.c
index d3221ae5..dc27c5a 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1459,7 +1459,7 @@ unlock:
 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_instance);
  * @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 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 @@ out:
 		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 @@ remove:
 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_state state, bool invoke)
 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	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked()
Message-ID	<tKzZN-8mT-45@gated-at.bofh.it>
In reply to	#1649274

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()

From	Heiko Carstens <heiko.carstens@de.ibm.com>
Date	2017-05-24 13:00 +0200
Subject	Re: [patch V3 15/32] s390/kernel: Use stop_machine_cpuslocked()
Message-ID	<tKCbh-1hF-35@gated-at.bofh.it>
In reply to	#1649288

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]

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

From	tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com>
Date	2017-05-26 10:50 +0200
Subject	[tip:smp/hotplug] s390/kernel: Use stop_machine_cpuslocked()
Message-ID	<tLj6y-3Cw-13@gated-at.bofh.it>
In reply to	#1649288

Commit-ID:  2337e879e8805a630b418f3e73a98084d4724b83
Gitweb:     http://git.kernel.org/tip/2337e879e8805a630b418f3e73a98084d4724b83
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:26 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:41 +0200

s390/kernel: Use stop_machine_cpuslocked()

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

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

Use stop_machine_cpuslocked() to avoid the nested call. 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>
Acked-by: Ingo Molnar <mingo@kernel.org>
Acked-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-s390@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Link: http://lkml.kernel.org/r/20170524081548.250203087@linutronix.de

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

diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index c3a52f9..192efdf 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -636,10 +636,10 @@ static void stp_work_fn(struct work_struct *work)
 		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]

#1649290 — [patch V3 06/32] padata: Make padata_alloc() static

From	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 06/32] padata: Make padata_alloc() static
Message-ID	<tKzZN-8mT-53@gated-at.bofh.it>
In reply to	#1649274

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]

#1651201 — [tip:smp/hotplug] padata: Make padata_alloc() static

From	tip-bot for Thomas Gleixner <tipbot@zytor.com>
Date	2017-05-26 10:40 +0200
Subject	[tip:smp/hotplug] padata: Make padata_alloc() static
Message-ID	<tLiWS-3x5-25@gated-at.bofh.it>
In reply to	#1649290

Commit-ID:  9596695ee1e7eedd743c43811fe68299eb005b5c
Gitweb:     http://git.kernel.org/tip/9596695ee1e7eedd743c43811fe68299eb005b5c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:17 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:37 +0200

padata: Make padata_alloc() static

No users outside of padata.c

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
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
Link: http://lkml.kernel.org/r/20170524081547.491457256@linutronix.de

---
 include/linux/padata.h |  3 ---
 kernel/padata.c        | 32 ++++++++++++++++----------------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/include/linux/padata.h b/include/linux/padata.h
index 0f9e567..2f9c1f9 100644
--- 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);
diff --git a/kernel/padata.c b/kernel/padata.c
index ac8f1e5..0c708f6 100644
--- 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 @@ err:
 }
 
 /**
+ * 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	Thomas Gleixner <tglx@linutronix.de>
Date	2017-05-24 10:40 +0200
Subject	[patch V3 07/32] padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()
Message-ID	<tKzZN-8mT-47@gated-at.bofh.it>
In reply to	#1649274

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]

#1651195 — [tip:smp/hotplug] padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()

From	tip-bot for Sebastian Andrzej Siewior <tipbot@zytor.com>
Date	2017-05-26 10:40 +0200
Subject	[tip:smp/hotplug] padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()
Message-ID	<tLiWR-3x5-11@gated-at.bofh.it>
In reply to	#1649292

Commit-ID:  c5a81c8ff816d89941fe86961b286765d6ca2f5f
Gitweb:     http://git.kernel.org/tip/c5a81c8ff816d89941fe86961b286765d6ca2f5f
Author:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
AuthorDate: Wed, 24 May 2017 10:15:18 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Fri, 26 May 2017 10:10:37 +0200

padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()

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>
Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
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
Link: http://lkml.kernel.org/r/20170524081547.571278910@linutronix.de

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

diff --git a/kernel/padata.c b/kernel/padata.c
index 0c708f6..868f947 100644
--- 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_alloc(struct workqueue_struct *wq,
 	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_alloc(struct workqueue_struct *wq,
 
 	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 @@ err_free_masks:
 	free_cpumask_var(pinst->cpumask.cbcpu);
 err_free_inst:
 	kfree(pinst);
-	put_online_cpus();
 err:
 	return NULL;
 }
@@ -1003,9 +1001,12 @@ err:
  *                         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]

Page 1 of 2 [1] 2 Next page →

csiph-web

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

Contents

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

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

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

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

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

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

#1651223 — [tip:smp/hotplug] s390: Prevent hotplug rwsem recursion

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

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

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

#1651225 — [tip:smp/hotplug] kprobes: Cure hotplug lock ordering issues

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

#1651203 — [tip:smp/hotplug] cpu/hotplug: Provide cpuhp_setup/remove_state[_nocalls]_cpuslocked()

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

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

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

#1649290 — [patch V3 06/32] padata: Make padata_alloc() static

#1651201 — [tip:smp/hotplug] padata: Make padata_alloc() static

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

#1651195 — [tip:smp/hotplug] padata: Avoid nested calls to cpus_read_lock() in pcrypt_init_padata()