[PATCH 1/2] arm64: Improve parking of stopped CPUs

[PATCH 1/2] arm64: Improve parking of stopped CPUs

[Aaro Koskinen]

From: Jayachandran C <jnair@caviumnetworks.com>

The current code puts the stopped cpus in an 'yield' instruction loop.
Using a busy loop here is unnecessary, we can use the cpu_park_loop()
function here to do a wfi/wfe.

Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---

	This is a rebased resend of the patch:

		https://patchwork.kernel.org/patch/9549145/

 arch/arm64/kernel/smp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index bb4b3f07761a..1a1b96a50245 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -854,9 +854,7 @@ static void ipi_cpu_stop(unsigned int cpu)
 
 	local_daif_mask();
 	sdei_mask_local_cpu();
-
-	while (1)
-		cpu_relax();
+	cpu_park_loop();
 }
 
 #ifdef CONFIG_KEXEC_CORE
-- 
2.17.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] arm64: Implement panic_smp_self_stop()

[Aaro Koskinen]

From: Aaro Koskinen <aaro.koskinen@nokia.com>

Currently arm64 uses the default implementation of panic_smp_self_stop()
that is simply a cpu_relax() loop. As a result, when two CPUs panic()
simultaneously we get "SMP: failed to stop secondary CPUs" warnings and
extra delays before a reset.

Provide an implementation of panic_smp_self_stop() that offlines the
CPU properly.

Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
---
 arch/arm64/kernel/smp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 1a1b96a50245..5e09e5032409 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -857,6 +857,11 @@ static void ipi_cpu_stop(unsigned int cpu)
 	cpu_park_loop();
 }
 
+void panic_smp_self_stop(void)
+{
+	ipi_cpu_stop(smp_processor_id());
+}
+
 #ifdef CONFIG_KEXEC_CORE
 static atomic_t waiting_for_crash_ipi = ATOMIC_INIT(0);
 #endif
-- 
2.17.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: Implement panic_smp_self_stop()

[James Morse]

Hi Aaro,

On 11/06/2019 19:10, Aaro Koskinen wrote:
> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> Currently arm64 uses the default implementation of panic_smp_self_stop()
> that is simply a cpu_relax() loop. As a result, when two CPUs panic()
> simultaneously we get "SMP: failed to stop secondary CPUs" warnings and
> extra delays before a reset.

> Provide an implementation of panic_smp_self_stop() that offlines the
> CPU properly.

This had me looking to the PSCI call that would take the CPU offline, but its just
conflicting terminology. Its the:
| set_cpu_online(cpu, false);
you're referring to here.

Would 'marks the CPU offline' be clearer?


Regardless,
Reviewed-by: James Morse <james.morse@arm.com>


Thanks,

James

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: Implement panic_smp_self_stop()

[Aaro Koskinen]

Hi,

On Wed, Jun 12, 2019 at 04:18:32PM +0100, James Morse wrote:
> On 11/06/2019 19:10, Aaro Koskinen wrote:
> > Currently arm64 uses the default implementation of panic_smp_self_stop()
> > that is simply a cpu_relax() loop. As a result, when two CPUs panic()
> > simultaneously we get "SMP: failed to stop secondary CPUs" warnings and
> > extra delays before a reset.
> 
> > Provide an implementation of panic_smp_self_stop() that offlines the
> > CPU properly.
> 
> This had me looking to the PSCI call that would take the CPU offline, but its just
> conflicting terminology. Its the:
> | set_cpu_online(cpu, false);
> you're referring to here.
> 
> Would 'marks the CPU offline' be clearer?

Yes, I will update the change log. I'll wait and see if there are other
comments as well, and send a new version next week.

> Regardless,
> Reviewed-by: James Morse <james.morse@arm.com>

Thanks,

A.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] arm64: Implement panic_smp_self_stop()

[Will Deacon]

On Tue, Jun 11, 2019 at 09:10:50PM +0300, Aaro Koskinen wrote:
> From: Aaro Koskinen <aaro.koskinen@nokia.com>
> 
> Currently arm64 uses the default implementation of panic_smp_self_stop()
> that is simply a cpu_relax() loop. As a result, when two CPUs panic()
> simultaneously we get "SMP: failed to stop secondary CPUs" warnings and
> extra delays before a reset.

Please can you elaborate a bit on this in the commit message (and preferably
a comment in panic_smp_self_stop() justifying the need for our own
implementation)? I initially thought it was something like two CPUs trying
to IPI each other, but it's much simpler than that.

> Provide an implementation of panic_smp_self_stop() that offlines the
> CPU properly.
> 
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> ---
>  arch/arm64/kernel/smp.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 1a1b96a50245..5e09e5032409 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -857,6 +857,11 @@ static void ipi_cpu_stop(unsigned int cpu)
>  	cpu_park_loop();
>  }
>  
> +void panic_smp_self_stop(void)
> +{
> +	ipi_cpu_stop(smp_processor_id());
> +}

ipi_cpu_stop should really be void, and just do smp_processor_id() itself.
It clearly only works for the local CPU. I'd be ok with you folding that
change in here, unless you fancy an extra patch. Maybe rename the function
too, now that it doesn't always run in interrupt context.

Cheers,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] arm64: Improve parking of stopped CPUs

[Will Deacon]

On Tue, Jun 11, 2019 at 09:10:49PM +0300, Aaro Koskinen wrote:
> From: Jayachandran C <jnair@caviumnetworks.com>
> 
> The current code puts the stopped cpus in an 'yield' instruction loop.
> Using a busy loop here is unnecessary, we can use the cpu_park_loop()
> function here to do a wfi/wfe.
> 
> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>
> Signed-off-by: Aaro Koskinen <aaro.koskinen@nokia.com>
> ---
> 
> 	This is a rebased resend of the patch:
> 
> 		https://patchwork.kernel.org/patch/9549145/
> 
>  arch/arm64/kernel/smp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index bb4b3f07761a..1a1b96a50245 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -854,9 +854,7 @@ static void ipi_cpu_stop(unsigned int cpu)
>  
>  	local_daif_mask();
>  	sdei_mask_local_cpu();
> -
> -	while (1)
> -		cpu_relax();
> +	cpu_park_loop();
>  }

Acked-by: Will Deacon <will.deacon@arm.com>

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel