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

Groups > linux.kernel > #1642760 > unrolled thread

[patch V2 00/17] init: Enable might_sleep() and smp_processor_id() debugging early

Back to article view | Back to linux.kernel

Contents

  [patch V2 00/17] init: Enable might_sleep() and smp_processor_id()  debugging early Thomas Gleixner <tglx@linutronix.de> - 2017-05-16 21:00 +0200
    [patch V2 06/17] powerpc: Adjust system_state check Thomas Gleixner <tglx@linutronix.de> - 2017-05-16 21:00 +0200
      [tip:sched/core] powerpc: Adjust system_state check tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-23 11:10 +0200
    [patch V2 03/17] arm64: Adjust system_state check Thomas Gleixner <tglx@linutronix.de> - 2017-05-16 21:00 +0200
      Re: [patch V2 03/17] arm64: Adjust system_state check Mark Rutland <mark.rutland@arm.com> - 2017-05-17 12:20 +0200
      Re: [patch V2 03/17] arm64: Adjust system_state check Catalin Marinas <catalin.marinas@arm.com> - 2017-05-17 16:30 +0200
      [tip:sched/core] arm64: Adjust system_state check tip-bot for Thomas Gleixner <tipbot@zytor.com> - 2017-05-23 11:00 +0200
    Re: [patch V2 00/17] init: Enable might_sleep() and  smp_processor_id() debugging early Mark Rutland <mark.rutland@arm.com> - 2017-05-17 13:00 +0200

#1642760 — [patch V2 00/17] init: Enable might_sleep() and smp_processor_id() debugging early

We recentlty discovered a call path which takes a mutex from the low level
secondary CPU bringup code and wondered why this was not caught by
might_sleep().

The reason is that both debug facilities depend on system_state ==
SYSTEM_RUNNING, which is set after init memory is freed.

That means that SMP bootup and builtin driver initialization are not
covered by these checks at all.

The patch series addresses this by adding an intermediate state which
enables both debug features right when scheduling starts, i.e. the boot CPU
idle task schedules the first time.

Changes since V1:
	- Use only one new state
	- Enable both debug facilities right before scheduling starts
	- Add more commentry about state ordering and placement of the
          state switch
	- CC ACPI folks on the relevant patch and amend changelog.
	- Collected acks/reviewed-by's

Thanks,

	tglx

[toc] | [next] | [standalone]

#1642761 — [patch V2 06/17] powerpc: Adjust system_state check

To enable smp_processor_id() and might_sleep() debug checks earlier, it's
required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.

Adjust the system_state check in smp_generic_cpu_bootable() to handle the
extra states.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/smp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -97,7 +97,7 @@ int smp_generic_cpu_bootable(unsigned in
 	/* Special case - we inhibit secondary thread startup
 	 * during boot if the user requests it.
 	 */
-	if (system_state == SYSTEM_BOOTING && cpu_has_feature(CPU_FTR_SMT)) {
+	if (system_state < SYSTEM_RUNNING && cpu_has_feature(CPU_FTR_SMT)) {
 		if (!smt_enabled_at_boot && cpu_thread_in_core(nr) != 0)
 			return 0;
 		if (smt_enabled_at_boot

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

#1647860 — [tip:sched/core] powerpc: Adjust system_state check

Commit-ID:  a8fcfc1917681ba1ccc23a429543a67aad8bfd00
Gitweb:     http://git.kernel.org/tip/a8fcfc1917681ba1ccc23a429543a67aad8bfd00
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 16 May 2017 20:42:37 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 23 May 2017 10:01:35 +0200

powerpc: Adjust system_state check

To enable smp_processor_id() and might_sleep() debug checks earlier, it's
required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.

Adjust the system_state check in smp_generic_cpu_bootable() to handle the
extra states.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/20170516184735.359536998@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/powerpc/kernel/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index df2a416..1069f74 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -97,7 +97,7 @@ int smp_generic_cpu_bootable(unsigned int nr)
 	/* Special case - we inhibit secondary thread startup
 	 * during boot if the user requests it.
 	 */
-	if (system_state == SYSTEM_BOOTING && cpu_has_feature(CPU_FTR_SMT)) {
+	if (system_state < SYSTEM_RUNNING && cpu_has_feature(CPU_FTR_SMT)) {
 		if (!smt_enabled_at_boot && cpu_thread_in_core(nr) != 0)
 			return 0;
 		if (smt_enabled_at_boot

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

#1642763 — [patch V2 03/17] arm64: Adjust system_state check

To enable smp_processor_id() and might_sleep() debug checks earlier, it's
required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.

Adjust the system_state check in smp_send_stop() to handle the extra states.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
---
 arch/arm64/kernel/smp.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -961,8 +961,7 @@ void smp_send_stop(void)
 		cpumask_copy(&mask, cpu_online_mask);
 		cpumask_clear_cpu(smp_processor_id(), &mask);
 
-		if (system_state == SYSTEM_BOOTING ||
-		    system_state == SYSTEM_RUNNING)
+		if (system_state <= SYSTEM_RUNNING)
 			pr_crit("SMP: stopping secondary CPUs\n");
 		smp_cross_call(&mask, IPI_CPU_STOP);
 	}

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

#1643218 — Re: [patch V2 03/17] arm64: Adjust system_state check

On Tue, May 16, 2017 at 08:42:34PM +0200, Thomas Gleixner wrote:
> To enable smp_processor_id() and might_sleep() debug checks earlier, it's
> required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.
> 
> Adjust the system_state check in smp_send_stop() to handle the extra states.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>  arch/arm64/kernel/smp.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

FWIW:

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

Mark.

> 
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -961,8 +961,7 @@ void smp_send_stop(void)
>  		cpumask_copy(&mask, cpu_online_mask);
>  		cpumask_clear_cpu(smp_processor_id(), &mask);
>  
> -		if (system_state == SYSTEM_BOOTING ||
> -		    system_state == SYSTEM_RUNNING)
> +		if (system_state <= SYSTEM_RUNNING)
>  			pr_crit("SMP: stopping secondary CPUs\n");
>  		smp_cross_call(&mask, IPI_CPU_STOP);
>  	}
> 
> 

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

#1643403 — Re: [patch V2 03/17] arm64: Adjust system_state check

On Tue, May 16, 2017 at 08:42:34PM +0200, Thomas Gleixner wrote:
> To enable smp_processor_id() and might_sleep() debug checks earlier, it's
> required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.
> 
> Adjust the system_state check in smp_send_stop() to handle the extra states.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

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

#1647831 — [tip:sched/core] arm64: Adjust system_state check

Commit-ID:  ef284f5ca5f102bf855e599305c0c16d6e844635
Gitweb:     http://git.kernel.org/tip/ef284f5ca5f102bf855e599305c0c16d6e844635
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Tue, 16 May 2017 20:42:34 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 23 May 2017 10:01:35 +0200

arm64: Adjust system_state check

To enable smp_processor_id() and might_sleep() debug checks earlier, it's
required to add system states between SYSTEM_BOOTING and SYSTEM_RUNNING.

Adjust the system_state check in smp_send_stop() to handle the extra states.

Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Deacon <will.deacon@arm.com>
Link: http://lkml.kernel.org/r/20170516184735.112589728@linutronix.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm64/kernel/smp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 6e0e16a..3211198 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -961,8 +961,7 @@ void smp_send_stop(void)
 		cpumask_copy(&mask, cpu_online_mask);
 		cpumask_clear_cpu(smp_processor_id(), &mask);
 
-		if (system_state == SYSTEM_BOOTING ||
-		    system_state == SYSTEM_RUNNING)
+		if (system_state <= SYSTEM_RUNNING)
 			pr_crit("SMP: stopping secondary CPUs\n");
 		smp_cross_call(&mask, IPI_CPU_STOP);
 	}

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

#1643263 — Re: [patch V2 00/17] init: Enable might_sleep() and smp_processor_id() debugging early

Hi,

On Tue, May 16, 2017 at 08:42:31PM +0200, Thomas Gleixner wrote:
> We recentlty discovered a call path which takes a mutex from the low level
> secondary CPU bringup code and wondered why this was not caught by
> might_sleep().
> 
> The reason is that both debug facilities depend on system_state ==
> SYSTEM_RUNNING, which is set after init memory is freed.
> 
> That means that SMP bootup and builtin driver initialization are not
> covered by these checks at all.
> 
> The patch series addresses this by adding an intermediate state which
> enables both debug features right when scheduling starts, i.e. the boot CPU
> idle task schedules the first time.

Thanks again for attacking this.

I gave this a spin atop of v4.12-rc1 on an ARM Juno platform. It picks
up the mutex issue, and I see no other new warnings. With a fix [1] for
the mutex issue appplied, I see no warnings.

Feel free to add my Tested-by for the arm64 and common bits.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-May/506558.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web