* [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@lists.infradead.org, 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@lists.infradead.org, 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: [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@lists.infradead.org, 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
* 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
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).