* [PATCH] arm64: Improve parking of stopped CPUs
@ 2017-02-01 9:48 Jayachandran C
2017-02-01 14:16 ` Will Deacon
2019-05-16 18:44 ` Aaro Koskinen
0 siblings, 2 replies; 10+ messages in thread
From: Jayachandran C @ 2017-02-01 9:48 UTC (permalink / raw)
To: linux-arm-kernel
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>
---
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 cbaab44..0691d2f 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
local_irq_disable();
- while (1)
- cpu_relax();
+ cpu_park_loop();
}
/*
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] arm64: Improve parking of stopped CPUs
2017-02-01 9:48 [PATCH] arm64: Improve parking of stopped CPUs Jayachandran C
@ 2017-02-01 14:16 ` Will Deacon
2017-02-01 14:31 ` Suzuki K Poulose
2019-05-16 18:44 ` Aaro Koskinen
1 sibling, 1 reply; 10+ messages in thread
From: Will Deacon @ 2017-02-01 14:16 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> 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>
> ---
> 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 cbaab44..0691d2f 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
>
> local_irq_disable();
>
> - while (1)
> - cpu_relax();
> + cpu_park_loop();
> }
Hmm, so we actually added the yield for QEMU's benefit iirc, where QEMU
will trap the yield and schedule a different vCPU. Should we be adding
a yield to cpu_park_loop instead?
Will
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] arm64: Improve parking of stopped CPUs
2017-02-01 14:16 ` Will Deacon
@ 2017-02-01 14:31 ` Suzuki K Poulose
2017-02-01 15:07 ` Will Deacon
0 siblings, 1 reply; 10+ messages in thread
From: Suzuki K Poulose @ 2017-02-01 14:31 UTC (permalink / raw)
To: linux-arm-kernel
On 01/02/17 14:16, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
>> 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>
>> ---
>> 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 cbaab44..0691d2f 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
>>
>> local_irq_disable();
>>
>> - while (1)
>> - cpu_relax();
>> + cpu_park_loop();
>> }
>
> Hmm, so we actually added the yield for QEMU's benefit iirc, where QEMU
> will trap the yield and schedule a different vCPU. Should we be adding
> a yield to cpu_park_loop instead?
Wouldn't wfi/wfe trigger the same ? I don't know how yield affects a physical
CPU. The cpu_park_loop is also used by CPUs which cannot run due to the missing
capabilities on the system. As long as yield() doesn't affect the PCPUs, we
could do that. Going another step further, we could also include
local_irq_disable() in cpu_park_loop().
Suzuki
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] arm64: Improve parking of stopped CPUs
2017-02-01 14:31 ` Suzuki K Poulose
@ 2017-02-01 15:07 ` Will Deacon
2017-02-02 7:42 ` Jayachandran C
0 siblings, 1 reply; 10+ messages in thread
From: Will Deacon @ 2017-02-01 15:07 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 01, 2017 at 02:31:38PM +0000, Suzuki K Poulose wrote:
> On 01/02/17 14:16, Will Deacon wrote:
> >On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> >>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>
> >>---
> >> 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 cbaab44..0691d2f 100644
> >>--- a/arch/arm64/kernel/smp.c
> >>+++ b/arch/arm64/kernel/smp.c
> >>@@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
> >>
> >> local_irq_disable();
> >>
> >>- while (1)
> >>- cpu_relax();
> >>+ cpu_park_loop();
> >> }
> >
> >Hmm, so we actually added the yield for QEMU's benefit iirc, where QEMU
> >will trap the yield and schedule a different vCPU. Should we be adding
> >a yield to cpu_park_loop instead?
>
> Wouldn't wfi/wfe trigger the same ? I don't know how yield affects a physical
> CPU. The cpu_park_loop is also used by CPUs which cannot run due to the missing
> capabilities on the system. As long as yield() doesn't affect the PCPUs, we
> could do that.
Yes, good point, it looks like WFE should do the same thing.
> Going another step further, we could also include local_irq_disable() in
> cpu_park_loop().
Why?
Will
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] arm64: Improve parking of stopped CPUs
2017-02-01 15:07 ` Will Deacon
@ 2017-02-02 7:42 ` Jayachandran C
0 siblings, 0 replies; 10+ messages in thread
From: Jayachandran C @ 2017-02-02 7:42 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 01, 2017 at 03:07:56PM +0000, Will Deacon wrote:
> On Wed, Feb 01, 2017 at 02:31:38PM +0000, Suzuki K Poulose wrote:
> > On 01/02/17 14:16, Will Deacon wrote:
> > >On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> > >>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>
> > >>---
> > >> 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 cbaab44..0691d2f 100644
> > >>--- a/arch/arm64/kernel/smp.c
> > >>+++ b/arch/arm64/kernel/smp.c
> > >>@@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
> > >>
> > >> local_irq_disable();
> > >>
> > >>- while (1)
> > >>- cpu_relax();
> > >>+ cpu_park_loop();
> > >> }
> > >
> > >Hmm, so we actually added the yield for QEMU's benefit iirc, where QEMU
> > >will trap the yield and schedule a different vCPU. Should we be adding
> > >a yield to cpu_park_loop instead?
> >
> > Wouldn't wfi/wfe trigger the same ? I don't know how yield affects a physical
> > CPU. The cpu_park_loop is also used by CPUs which cannot run due to the missing
> > capabilities on the system. As long as yield() doesn't affect the PCPUs, we
> > could do that.
>
> Yes, good point, it looks like WFE should do the same thing.
Agree, I am not sure if there is any case where 'yield' is a better option.
There are a few cases in kernel/process.c which ends in 'while (1);' where
the point about QEMU may be valid. These can be fixed up if needed.
> > Going another step further, we could also include local_irq_disable() in
> > cpu_park_loop().
There is a cpu_panic_kernel() which calls cpu_park_loop() without the irq
disable. It maybe OK to local_irq_disable for that too, but probably not
in the scope of this patch.
JC.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: Improve parking of stopped CPUs
2017-02-01 9:48 [PATCH] arm64: Improve parking of stopped CPUs Jayachandran C
2017-02-01 14:16 ` Will Deacon
@ 2019-05-16 18:44 ` Aaro Koskinen
2019-05-18 22:19 ` Jayachandran Chandrasekharan Nair
2019-05-23 10:05 ` Will Deacon
1 sibling, 2 replies; 10+ messages in thread
From: Aaro Koskinen @ 2019-05-16 18:44 UTC (permalink / raw)
To: Jayachandran C, Catalin Marinas, Will Deacon, jnair
Cc: linux-arm-kernel, Suzuki K Poulose
Hi,
On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> 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>
Looks like this patch has been forgotten?
I have a system where CPUs need to be put in wfi/wfe for the warm reset
to work, and using cpu_park_loop() would solve this.
A.
> 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 cbaab44..0691d2f 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
>
> local_irq_disable();
>
> - while (1)
> - cpu_relax();
> + cpu_park_loop();
> }
>
> /*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: Improve parking of stopped CPUs
2019-05-16 18:44 ` Aaro Koskinen
@ 2019-05-18 22:19 ` Jayachandran Chandrasekharan Nair
2019-05-18 23:12 ` Aaro Koskinen
2019-05-23 10:05 ` Will Deacon
1 sibling, 1 reply; 10+ messages in thread
From: Jayachandran Chandrasekharan Nair @ 2019-05-18 22:19 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Catalin Marinas, Will Deacon, Jayachandran C, linux-arm-kernel,
Suzuki K Poulose
On Thu, May 16, 2019 at 09:44:53PM +0300, Aaro Koskinen wrote:
> External Email
>
> ----------------------------------------------------------------------
> Hi,
>
> On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> > 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>
>
> Looks like this patch has been forgotten?
>
> I have a system where CPUs need to be put in wfi/wfe for the warm reset
> to work, and using cpu_park_loop() would solve this.
If I remember correctly (it has been a while), in my testing I saw that
the wfi does not block after a while since interrupts are pending.
Most likely that timer interrupts will still be enabled.
We might need some code to reset/disable the GIC interface for the CPU
before parking.
> > 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 cbaab44..0691d2f 100644
> > --- a/arch/arm64/kernel/smp.c
> > +++ b/arch/arm64/kernel/smp.c
> > @@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
> >
> > local_irq_disable();
> >
> > - while (1)
> > - cpu_relax();
> > + cpu_park_loop();
> > }
> >
> > /*
JC
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: Improve parking of stopped CPUs
2019-05-18 22:19 ` Jayachandran Chandrasekharan Nair
@ 2019-05-18 23:12 ` Aaro Koskinen
2019-05-24 19:24 ` [EXT] " Jayachandran Chandrasekharan Nair
0 siblings, 1 reply; 10+ messages in thread
From: Aaro Koskinen @ 2019-05-18 23:12 UTC (permalink / raw)
To: Jayachandran Chandrasekharan Nair
Cc: Catalin Marinas, Will Deacon, Jayachandran C, linux-arm-kernel,
Suzuki K Poulose
Hi,
On Sat, May 18, 2019 at 10:19:39PM +0000, Jayachandran Chandrasekharan Nair wrote:
> On Thu, May 16, 2019 at 09:44:53PM +0300, Aaro Koskinen wrote:
> > On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> > > 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>
> >
> > Looks like this patch has been forgotten?
> >
> > I have a system where CPUs need to be put in wfi/wfe for the warm reset
> > to work, and using cpu_park_loop() would solve this.
>
> If I remember correctly (it has been a while), in my testing I saw that
> the wfi does not block after a while since interrupts are pending.
> Most likely that timer interrupts will still be enabled.
Right, this is correct; I also observed the same in my testing.
> We might need some code to reset/disable the GIC interface for the CPU
> before parking.
I solved this by doing the global GIC disable in the firmware PSCI reset
function code. So from Linux side cpu_park_loop() is enough, and already
this patch is an improvement.
A.
> > > 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 cbaab44..0691d2f 100644
> > > --- a/arch/arm64/kernel/smp.c
> > > +++ b/arch/arm64/kernel/smp.c
> > > @@ -829,8 +829,7 @@ static void ipi_cpu_stop(unsigned int cpu)
> > >
> > > local_irq_disable();
> > >
> > > - while (1)
> > > - cpu_relax();
> > > + cpu_park_loop();
> > > }
> > >
> > > /*
>
> JC
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] arm64: Improve parking of stopped CPUs
2019-05-16 18:44 ` Aaro Koskinen
2019-05-18 22:19 ` Jayachandran Chandrasekharan Nair
@ 2019-05-23 10:05 ` Will Deacon
1 sibling, 0 replies; 10+ messages in thread
From: Will Deacon @ 2019-05-23 10:05 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Catalin Marinas, jnair, Jayachandran C, linux-arm-kernel,
Suzuki K Poulose
On Thu, May 16, 2019 at 09:44:53PM +0300, Aaro Koskinen wrote:
> On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> > 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>
>
> Looks like this patch has been forgotten?
>
> I have a system where CPUs need to be put in wfi/wfe for the warm reset
> to work, and using cpu_park_loop() would solve this.
Yikes, that's over two years old. Maybe resend it?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [EXT] Re: [PATCH] arm64: Improve parking of stopped CPUs
2019-05-18 23:12 ` Aaro Koskinen
@ 2019-05-24 19:24 ` Jayachandran Chandrasekharan Nair
0 siblings, 0 replies; 10+ messages in thread
From: Jayachandran Chandrasekharan Nair @ 2019-05-24 19:24 UTC (permalink / raw)
To: Aaro Koskinen
Cc: Catalin Marinas, Will Deacon, Jayachandran C, linux-arm-kernel,
Suzuki K Poulose
On Sun, May 19, 2019 at 02:12:44AM +0300, Aaro Koskinen wrote:
> Hi,
>
> On Sat, May 18, 2019 at 10:19:39PM +0000, Jayachandran Chandrasekharan Nair wrote:
> > On Thu, May 16, 2019 at 09:44:53PM +0300, Aaro Koskinen wrote:
> > > On Wed, Feb 01, 2017 at 09:48:52AM +0000, Jayachandran C wrote:
> > > > 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>
> > >
> > > Looks like this patch has been forgotten?
> > >
> > > I have a system where CPUs need to be put in wfi/wfe for the warm reset
> > > to work, and using cpu_park_loop() would solve this.
> >
> > If I remember correctly (it has been a while), in my testing I saw that
> > the wfi does not block after a while since interrupts are pending.
> > Most likely that timer interrupts will still be enabled.
>
> Right, this is correct; I also observed the same in my testing.
>
> > We might need some code to reset/disable the GIC interface for the CPU
> > before parking.
>
> I solved this by doing the global GIC disable in the firmware PSCI reset
> function code. So from Linux side cpu_park_loop() is enough, and already
> this patch is an improvement.
Ideally, I think the GIC driver has to be updated to turn off the interrupts
to the CPU before we get to this point in the code. There are some power
management and hotplug notifiers already in the GIC driver, I wonder if
it is possible to take care of this too.
Since Linux turns on the interrupts, it would be better if Linux turns
them off too, rather than relying on the platform to do it.
JC
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-05-24 19:25 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-01 9:48 [PATCH] arm64: Improve parking of stopped CPUs Jayachandran C
2017-02-01 14:16 ` Will Deacon
2017-02-01 14:31 ` Suzuki K Poulose
2017-02-01 15:07 ` Will Deacon
2017-02-02 7:42 ` Jayachandran C
2019-05-16 18:44 ` Aaro Koskinen
2019-05-18 22:19 ` Jayachandran Chandrasekharan Nair
2019-05-18 23:12 ` Aaro Koskinen
2019-05-24 19:24 ` [EXT] " Jayachandran Chandrasekharan Nair
2019-05-23 10:05 ` Will Deacon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).