* [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() @ 2019-09-21 11:21 Yunfeng Ye 2019-10-09 4:45 ` Yunfeng Ye 2019-10-15 16:23 ` Will Deacon 0 siblings, 2 replies; 15+ messages in thread From: Yunfeng Ye @ 2019-09-21 11:21 UTC (permalink / raw) To: David Laight, catalin.marinas@arm.com, will@kernel.org, kstewart@linuxfoundation.org, gregkh@linuxfoundation.org, ard.biesheuvel@linaro.org, tglx@linutronix.de Cc: wuyun.wu@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org If psci_ops.affinity_info() fails, it will sleep 10ms, which will not take so long in the right case. Use usleep_range() instead of msleep(), reduce the waiting time, and give a chance to busy wait before sleep. Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> --- V1->V2: - use usleep_range() instead of udelay() after waiting for a while arch/arm64/kernel/psci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index c9f72b2..99b3122 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) static int cpu_psci_cpu_kill(unsigned int cpu) { int err, i; + unsigned long timeout; if (!psci_ops.affinity_info) return 0; @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) * while it is dying. So, try again a few times. */ - for (i = 0; i < 10; i++) { + i = 0; + timeout = jiffies + msecs_to_jiffies(100); + do { err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { pr_info("CPU%d killed.\n", cpu); return 0; } - msleep(10); - pr_info("Retrying again to check for CPU kill\n"); - } + /* busy-wait max 1ms */ + if (i++ < 100) { + cond_resched(); + udelay(10); + continue; + } + + usleep_range(100, 1000); + } while (time_before(jiffies, timeout)); pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n", cpu, err); -- 2.7.4.huawei.3 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-09-21 11:21 [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() Yunfeng Ye @ 2019-10-09 4:45 ` Yunfeng Ye 2019-10-16 15:32 ` Sudeep Holla 2019-10-15 16:23 ` Will Deacon 1 sibling, 1 reply; 15+ messages in thread From: Yunfeng Ye @ 2019-10-09 4:45 UTC (permalink / raw) To: David Laight, catalin.marinas@arm.com, will@kernel.org, kstewart@linuxfoundation.org, gregkh@linuxfoundation.org, ard.biesheuvel@linaro.org, tglx@linutronix.de Cc: wuyun.wu@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org If psci_ops.affinity_info() fails, it will sleep 10ms, which will not take so long in the right case. Use usleep_range() instead of msleep(), reduce the waiting time, and give a chance to busy wait before sleep. Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> --- V1->V2: - use usleep_range() instead of udelay() after waiting for a while arch/arm64/kernel/psci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index c9f72b2..99b3122 100644 --- a/arch/arm64/kernel/psci.c +++ b/arch/arm64/kernel/psci.c @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) static int cpu_psci_cpu_kill(unsigned int cpu) { int err, i; + unsigned long timeout; if (!psci_ops.affinity_info) return 0; @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) * while it is dying. So, try again a few times. */ - for (i = 0; i < 10; i++) { + i = 0; + timeout = jiffies + msecs_to_jiffies(100); + do { err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { pr_info("CPU%d killed.\n", cpu); return 0; } - msleep(10); - pr_info("Retrying again to check for CPU kill\n"); - } + /* busy-wait max 1ms */ + if (i++ < 100) { + cond_resched(); + udelay(10); + continue; + } + + usleep_range(100, 1000); + } while (time_before(jiffies, timeout)); pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n", cpu, err); -- 2.7.4.3 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-09 4:45 ` Yunfeng Ye @ 2019-10-16 15:32 ` Sudeep Holla 2019-10-17 13:26 ` Yunfeng Ye 0 siblings, 1 reply; 15+ messages in thread From: Sudeep Holla @ 2019-10-16 15:32 UTC (permalink / raw) To: Yunfeng Ye Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, catalin.marinas@arm.com, Sudeep Holla, linux-kernel@vger.kernel.org, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org On Wed, Oct 09, 2019 at 12:45:16PM +0800, Yunfeng Ye wrote: > If psci_ops.affinity_info() fails, it will sleep 10ms, which will not > take so long in the right case. Use usleep_range() instead of msleep(), > reduce the waiting time, and give a chance to busy wait before sleep. > > Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> > --- > V1->V2: > - use usleep_range() instead of udelay() after waiting for a while > > arch/arm64/kernel/psci.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c > index c9f72b2..99b3122 100644 > --- a/arch/arm64/kernel/psci.c > +++ b/arch/arm64/kernel/psci.c > @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) > static int cpu_psci_cpu_kill(unsigned int cpu) > { > int err, i; > + unsigned long timeout; > > if (!psci_ops.affinity_info) > return 0; > @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) > * while it is dying. So, try again a few times. > */ > > - for (i = 0; i < 10; i++) { > + i = 0; > + timeout = jiffies + msecs_to_jiffies(100); > + do { > err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); > if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { > pr_info("CPU%d killed.\n", cpu); > return 0; > } > > - msleep(10); > - pr_info("Retrying again to check for CPU kill\n"); You dropped this message, any particular reason ? > - } > + /* busy-wait max 1ms */ > + if (i++ < 100) { > + cond_resched(); > + udelay(10); > + continue; Why can't it be simple like loop of 100 * msleep(1) instead of loop of 10 * msleep(10). The above initial busy wait for 1 ms looks too much optimised for your setup where it takes 50-500us, what if it take just over 1 ms ? We need more generic solution. -- Regards, Sudeep _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-16 15:32 ` Sudeep Holla @ 2019-10-17 13:26 ` Yunfeng Ye 2019-10-17 13:54 ` Sudeep Holla 2019-10-17 14:00 ` David Laight 0 siblings, 2 replies; 15+ messages in thread From: Yunfeng Ye @ 2019-10-17 13:26 UTC (permalink / raw) To: Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org On 2019/10/16 23:32, Sudeep Holla wrote: > On Wed, Oct 09, 2019 at 12:45:16PM +0800, Yunfeng Ye wrote: >> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not >> take so long in the right case. Use usleep_range() instead of msleep(), >> reduce the waiting time, and give a chance to busy wait before sleep. >> >> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> >> --- >> V1->V2: >> - use usleep_range() instead of udelay() after waiting for a while >> >> arch/arm64/kernel/psci.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c >> index c9f72b2..99b3122 100644 >> --- a/arch/arm64/kernel/psci.c >> +++ b/arch/arm64/kernel/psci.c >> @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) >> static int cpu_psci_cpu_kill(unsigned int cpu) >> { >> int err, i; >> + unsigned long timeout; >> >> if (!psci_ops.affinity_info) >> return 0; >> @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) >> * while it is dying. So, try again a few times. >> */ >> >> - for (i = 0; i < 10; i++) { >> + i = 0; >> + timeout = jiffies + msecs_to_jiffies(100); >> + do { >> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); >> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { >> pr_info("CPU%d killed.\n", cpu); >> return 0; >> } >> >> - msleep(10); >> - pr_info("Retrying again to check for CPU kill\n"); > > You dropped this message, any particular reason ? > When reduce the time interval to 1ms, the print message maybe increase 10 times. on the other hand, cpu_psci_cpu_kill() will print message on success or failure, which this retry log is not very necessary. of cource, I think use pr_info_once() instead of pr_info() is better. thanks. >> - } >> + /* busy-wait max 1ms */ >> + if (i++ < 100) { >> + cond_resched(); >> + udelay(10); >> + continue; > > Why can't it be simple like loop of 100 * msleep(1) instead of loop of > 10 * msleep(10). The above initial busy wait for 1 ms looks too much > optimised for your setup where it takes 50-500us, what if it take just > over 1 ms ? > msleep() is implemented by jiffies. when HZ=100 or HZ=250, msleep(1) is not accurate. so I think usleep_range() is better. 1 ms looks simple and good, but how about 100us is better? I refer a function sunxi_mc_smp_cpu_kill(), it use usleep_range(50, 100). thanks. > We need more generic solution. > > -- > Regards, > Sudeep > > . > _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-17 13:26 ` Yunfeng Ye @ 2019-10-17 13:54 ` Sudeep Holla 2019-10-17 14:24 ` Yunfeng Ye 2019-10-17 14:00 ` David Laight 1 sibling, 1 reply; 15+ messages in thread From: Sudeep Holla @ 2019-10-17 13:54 UTC (permalink / raw) To: Yunfeng Ye Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, Sudeep Holla, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org On Thu, Oct 17, 2019 at 09:26:15PM +0800, Yunfeng Ye wrote: > > > On 2019/10/16 23:32, Sudeep Holla wrote: > > On Wed, Oct 09, 2019 at 12:45:16PM +0800, Yunfeng Ye wrote: > >> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not > >> take so long in the right case. Use usleep_range() instead of msleep(), > >> reduce the waiting time, and give a chance to busy wait before sleep. > >> > >> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> > >> --- > >> V1->V2: > >> - use usleep_range() instead of udelay() after waiting for a while > >> > >> arch/arm64/kernel/psci.c | 17 +++++++++++++---- > >> 1 file changed, 13 insertions(+), 4 deletions(-) > >> > >> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c > >> index c9f72b2..99b3122 100644 > >> --- a/arch/arm64/kernel/psci.c > >> +++ b/arch/arm64/kernel/psci.c > >> @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) > >> static int cpu_psci_cpu_kill(unsigned int cpu) > >> { > >> int err, i; > >> + unsigned long timeout; > >> > >> if (!psci_ops.affinity_info) > >> return 0; > >> @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) > >> * while it is dying. So, try again a few times. > >> */ > >> > >> - for (i = 0; i < 10; i++) { > >> + i = 0; > >> + timeout = jiffies + msecs_to_jiffies(100); > >> + do { > >> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); > >> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { > >> pr_info("CPU%d killed.\n", cpu); > >> return 0; > >> } > >> > >> - msleep(10); > >> - pr_info("Retrying again to check for CPU kill\n"); > > > > You dropped this message, any particular reason ? > > > When reduce the time interval to 1ms, the print message maybe increase 10 > times. on the other hand, cpu_psci_cpu_kill() will print message on success > or failure, which this retry log is not very necessary. of cource, I think > use pr_info_once() instead of pr_info() is better. > Yes changing it to pr_info_once is better than dropping it as it gives some indication to the firmware if there's scope for improvement. > >> - } > >> + /* busy-wait max 1ms */ > >> + if (i++ < 100) { > >> + cond_resched(); > >> + udelay(10); > >> + continue; > > > > Why can't it be simple like loop of 100 * msleep(1) instead of loop of > > 10 * msleep(10). The above initial busy wait for 1 ms looks too much > > optimised for your setup where it takes 50-500us, what if it take just > > over 1 ms ? > > > msleep() is implemented by jiffies. when HZ=100 or HZ=250, msleep(1) is not > accurate. so I think usleep_range() is better. 1 ms looks simple and good, but how > about 100us is better? I refer a function sunxi_mc_smp_cpu_kill(), it use > usleep_range(50, 100). > Again that's specific to sunxi platforms and may work well. While I agree msleep(1) may not be accurate, I am still inclined to have a max value of 1000(i.e. 1ms) for usleep_range. -- Regards, Sudeep _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-17 13:54 ` Sudeep Holla @ 2019-10-17 14:24 ` Yunfeng Ye 0 siblings, 0 replies; 15+ messages in thread From: Yunfeng Ye @ 2019-10-17 14:24 UTC (permalink / raw) To: Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org On 2019/10/17 21:54, Sudeep Holla wrote: > On Thu, Oct 17, 2019 at 09:26:15PM +0800, Yunfeng Ye wrote: >> >> >> On 2019/10/16 23:32, Sudeep Holla wrote: >>> On Wed, Oct 09, 2019 at 12:45:16PM +0800, Yunfeng Ye wrote: >>>> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not >>>> take so long in the right case. Use usleep_range() instead of msleep(), >>>> reduce the waiting time, and give a chance to busy wait before sleep. >>>> >>>> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> >>>> --- >>>> V1->V2: >>>> - use usleep_range() instead of udelay() after waiting for a while >>>> >>>> arch/arm64/kernel/psci.c | 17 +++++++++++++---- >>>> 1 file changed, 13 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c >>>> index c9f72b2..99b3122 100644 >>>> --- a/arch/arm64/kernel/psci.c >>>> +++ b/arch/arm64/kernel/psci.c >>>> @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) >>>> static int cpu_psci_cpu_kill(unsigned int cpu) >>>> { >>>> int err, i; >>>> + unsigned long timeout; >>>> >>>> if (!psci_ops.affinity_info) >>>> return 0; >>>> @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) >>>> * while it is dying. So, try again a few times. >>>> */ >>>> >>>> - for (i = 0; i < 10; i++) { >>>> + i = 0; >>>> + timeout = jiffies + msecs_to_jiffies(100); >>>> + do { >>>> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); >>>> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { >>>> pr_info("CPU%d killed.\n", cpu); >>>> return 0; >>>> } >>>> >>>> - msleep(10); >>>> - pr_info("Retrying again to check for CPU kill\n"); >>> >>> You dropped this message, any particular reason ? >>> >> When reduce the time interval to 1ms, the print message maybe increase 10 >> times. on the other hand, cpu_psci_cpu_kill() will print message on success >> or failure, which this retry log is not very necessary. of cource, I think >> use pr_info_once() instead of pr_info() is better. >> > > Yes changing it to pr_info_once is better than dropping it as it gives > some indication to the firmware if there's scope for improvement. > >>>> - } >>>> + /* busy-wait max 1ms */ >>>> + if (i++ < 100) { >>>> + cond_resched(); >>>> + udelay(10); >>>> + continue; >>> >>> Why can't it be simple like loop of 100 * msleep(1) instead of loop of >>> 10 * msleep(10). The above initial busy wait for 1 ms looks too much >>> optimised for your setup where it takes 50-500us, what if it take just >>> over 1 ms ? >>> >> msleep() is implemented by jiffies. when HZ=100 or HZ=250, msleep(1) is not >> accurate. so I think usleep_range() is better. 1 ms looks simple and good, but how >> about 100us is better? I refer a function sunxi_mc_smp_cpu_kill(), it use >> usleep_range(50, 100). >> > > Again that's specific to sunxi platforms and may work well. While I agree > msleep(1) may not be accurate, I am still inclined to have a max value > of 1000(i.e. 1ms) for usleep_range. > ok, I will send a new version patch that waiting max 1ms. thanks. > -- > Regards, > Sudeep > > . > _______________________________________________ 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] 15+ messages in thread
* RE: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-17 13:26 ` Yunfeng Ye 2019-10-17 13:54 ` Sudeep Holla @ 2019-10-17 14:00 ` David Laight 2019-10-17 14:19 ` Yunfeng Ye 1 sibling, 1 reply; 15+ messages in thread From: David Laight @ 2019-10-17 14:00 UTC (permalink / raw) To: 'Yunfeng Ye', Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan@huawei.com, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong@huawei.com, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org From: Yunfeng Ye > Sent: 17 October 2019 14:26 ... > >> - for (i = 0; i < 10; i++) { > >> + i = 0; > >> + timeout = jiffies + msecs_to_jiffies(100); > >> + do { > >> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); > >> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { > >> pr_info("CPU%d killed.\n", cpu); > >> return 0; > >> } > >> > >> - msleep(10); > >> - pr_info("Retrying again to check for CPU kill\n"); > > > > You dropped this message, any particular reason ? > > > When reduce the time interval to 1ms, the print message maybe increase 10 times. > on the other hand, cpu_psci_cpu_kill() will print message on success or failure, which > this retry log is not very necessary. of cource, I think use pr_info_once() instead of > pr_info() is better. Maybe you should print in on (say) the 10th time around the loop. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-17 14:00 ` David Laight @ 2019-10-17 14:19 ` Yunfeng Ye 2019-10-17 14:25 ` David Laight 0 siblings, 1 reply; 15+ messages in thread From: Yunfeng Ye @ 2019-10-17 14:19 UTC (permalink / raw) To: David Laight, Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan@huawei.com, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong@huawei.com, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org On 2019/10/17 22:00, David Laight wrote: > From: Yunfeng Ye >> Sent: 17 October 2019 14:26 > ... >>>> - for (i = 0; i < 10; i++) { >>>> + i = 0; >>>> + timeout = jiffies + msecs_to_jiffies(100); >>>> + do { >>>> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); >>>> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { >>>> pr_info("CPU%d killed.\n", cpu); >>>> return 0; >>>> } >>>> >>>> - msleep(10); >>>> - pr_info("Retrying again to check for CPU kill\n"); >>> >>> You dropped this message, any particular reason ? >>> >> When reduce the time interval to 1ms, the print message maybe increase 10 times. >> on the other hand, cpu_psci_cpu_kill() will print message on success or failure, which >> this retry log is not very necessary. of cource, I think use pr_info_once() instead of >> pr_info() is better. > > Maybe you should print in on (say) the 10th time around the loop. > Can it like this: pr_info("CPU%d killed with %d loops.\n", cpu, loops); If put the number of waiting times in the successful printing message, it is not necessary to print the "Retrying ..." message. thanks. > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) > _______________________________________________ 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] 15+ messages in thread
* RE: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-17 14:19 ` Yunfeng Ye @ 2019-10-17 14:25 ` David Laight 0 siblings, 0 replies; 15+ messages in thread From: David Laight @ 2019-10-17 14:25 UTC (permalink / raw) To: 'Yunfeng Ye', Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan@huawei.com, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong@huawei.com, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, will@kernel.org, linux-arm-kernel@lists.infradead.org From: Yunfeng Ye > Sent: 17 October 2019 15:20 > On 2019/10/17 22:00, David Laight wrote: > > From: Yunfeng Ye > >> Sent: 17 October 2019 14:26 > > ... > >>>> - for (i = 0; i < 10; i++) { > >>>> + i = 0; > >>>> + timeout = jiffies + msecs_to_jiffies(100); > >>>> + do { > >>>> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); > >>>> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { > >>>> pr_info("CPU%d killed.\n", cpu); > >>>> return 0; > >>>> } > >>>> > >>>> - msleep(10); > >>>> - pr_info("Retrying again to check for CPU kill\n"); > >>> > >>> You dropped this message, any particular reason ? > >>> > >> When reduce the time interval to 1ms, the print message maybe increase 10 times. > >> on the other hand, cpu_psci_cpu_kill() will print message on success or failure, which > >> this retry log is not very necessary. of cource, I think use pr_info_once() instead of > >> pr_info() is better. > > > > Maybe you should print in on (say) the 10th time around the loop. > > > Can it like this: > pr_info("CPU%d killed with %d loops.\n", cpu, loops); > > If put the number of waiting times in the successful printing message, it is > not necessary to print the "Retrying ..." message. That depends on whether you want to know how long it took or why the system is 'stuck'. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-09-21 11:21 [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() Yunfeng Ye 2019-10-09 4:45 ` Yunfeng Ye @ 2019-10-15 16:23 ` Will Deacon 2019-10-16 3:22 ` Yunfeng Ye 1 sibling, 1 reply; 15+ messages in thread From: Will Deacon @ 2019-10-15 16:23 UTC (permalink / raw) To: Yunfeng Ye, sudeep.holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, linux-arm-kernel@lists.infradead.org Hi, On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: > If psci_ops.affinity_info() fails, it will sleep 10ms, which will not > take so long in the right case. Use usleep_range() instead of msleep(), > reduce the waiting time, and give a chance to busy wait before sleep. Can you elaborate on "the right case" please? It's not clear to me exactly what problem you're solving here. I've also added Sudeep to the thread, since I'd like his ack on the change. Will > arch/arm64/kernel/psci.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c > index c9f72b2..99b3122 100644 > --- a/arch/arm64/kernel/psci.c > +++ b/arch/arm64/kernel/psci.c > @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) > static int cpu_psci_cpu_kill(unsigned int cpu) > { > int err, i; > + unsigned long timeout; > > if (!psci_ops.affinity_info) > return 0; > @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) > * while it is dying. So, try again a few times. > */ > > - for (i = 0; i < 10; i++) { > + i = 0; > + timeout = jiffies + msecs_to_jiffies(100); > + do { > err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); > if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { > pr_info("CPU%d killed.\n", cpu); > return 0; > } > > - msleep(10); > - pr_info("Retrying again to check for CPU kill\n"); > - } > + /* busy-wait max 1ms */ > + if (i++ < 100) { > + cond_resched(); > + udelay(10); > + continue; > + } > + > + usleep_range(100, 1000); > + } while (time_before(jiffies, timeout)); > > pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n", > cpu, err); > -- > 2.7.4.huawei.3 > _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-15 16:23 ` Will Deacon @ 2019-10-16 3:22 ` Yunfeng Ye 2019-10-16 10:25 ` Sudeep Holla 0 siblings, 1 reply; 15+ messages in thread From: Yunfeng Ye @ 2019-10-16 3:22 UTC (permalink / raw) To: Will Deacon, sudeep.holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, linux-arm-kernel@lists.infradead.org On 2019/10/16 0:23, Will Deacon wrote: > Hi, > > On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: >> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not >> take so long in the right case. Use usleep_range() instead of msleep(), >> reduce the waiting time, and give a chance to busy wait before sleep. > > Can you elaborate on "the right case" please? It's not clear to me > exactly what problem you're solving here. > The situation is that when the power is off, we have a battery to save some information, but the battery power is limited, so we reduce the power consumption by turning off the cores, and need fastly to complete the core shutdown. However, the time of cpu_psci_cpu_kill() will take 10ms. We have tested the time that it does not need 10ms, and most case is about 50us-500us. if we reduce the time of cpu_psci_cpu_kill(), we can reduce 10% - 30% of the total time. So change msleep (10) to usleep_range() to reduce the waiting time. In addition, we don't want to be scheduled during the sleeping time, some threads may take a long time and don't give up the CPU, which affects the time of core shutdown, Therefore, we add a chance to busy-wait max 1ms. thanks. > I've also added Sudeep to the thread, since I'd like his ack on the change. > > Will > >> arch/arm64/kernel/psci.c | 17 +++++++++++++---- >> 1 file changed, 13 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c >> index c9f72b2..99b3122 100644 >> --- a/arch/arm64/kernel/psci.c >> +++ b/arch/arm64/kernel/psci.c >> @@ -82,6 +82,7 @@ static void cpu_psci_cpu_die(unsigned int cpu) >> static int cpu_psci_cpu_kill(unsigned int cpu) >> { >> int err, i; >> + unsigned long timeout; >> >> if (!psci_ops.affinity_info) >> return 0; >> @@ -91,16 +92,24 @@ static int cpu_psci_cpu_kill(unsigned int cpu) >> * while it is dying. So, try again a few times. >> */ >> >> - for (i = 0; i < 10; i++) { >> + i = 0; >> + timeout = jiffies + msecs_to_jiffies(100); >> + do { >> err = psci_ops.affinity_info(cpu_logical_map(cpu), 0); >> if (err == PSCI_0_2_AFFINITY_LEVEL_OFF) { >> pr_info("CPU%d killed.\n", cpu); >> return 0; >> } >> >> - msleep(10); >> - pr_info("Retrying again to check for CPU kill\n"); >> - } >> + /* busy-wait max 1ms */ >> + if (i++ < 100) { >> + cond_resched(); >> + udelay(10); >> + continue; >> + } >> + >> + usleep_range(100, 1000); >> + } while (time_before(jiffies, timeout)); >> >> pr_warn("CPU%d may not have shut down cleanly (AFFINITY_INFO reports %d)\n", >> cpu, err); >> -- >> 2.7.4.huawei.3 >> > > . > _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-16 3:22 ` Yunfeng Ye @ 2019-10-16 10:25 ` Sudeep Holla 2019-10-16 11:29 ` Yunfeng Ye 0 siblings, 1 reply; 15+ messages in thread From: Sudeep Holla @ 2019-10-16 10:25 UTC (permalink / raw) To: Yunfeng Ye Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, Will Deacon, linux-arm-kernel@lists.infradead.org On Wed, Oct 16, 2019 at 11:22:23AM +0800, Yunfeng Ye wrote: > > > On 2019/10/16 0:23, Will Deacon wrote: > > Hi, > > > > On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: > >> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not > >> take so long in the right case. Use usleep_range() instead of msleep(), > >> reduce the waiting time, and give a chance to busy wait before sleep. > > > > Can you elaborate on "the right case" please? It's not clear to me > > exactly what problem you're solving here. > > > The situation is that when the power is off, we have a battery to save some > information, but the battery power is limited, so we reduce the power consumption > by turning off the cores, and need fastly to complete the core shutdown. However, the > time of cpu_psci_cpu_kill() will take 10ms. We have tested the time that it does not > need 10ms, and most case is about 50us-500us. if we reduce the time of cpu_psci_cpu_kill(), > we can reduce 10% - 30% of the total time. > Have you checked why PSCI AFFINITY_INFO not returning LEVEL_OFF quickly then ? We wait for upto 5s in cpu_wait_death(worst case) before cpu_kill is called from __cpu_die. Moreover I don't understand the argument here. The cpu being killed will be OFF, as soon as it can and firmware controls that and this change is not related to CPU_OFF. And this CPU calling cpu_kill can sleep and 10ms is good to enter idle states if it's idle saving power, so I fail to map the power saving you mention above. > So change msleep (10) to usleep_range() to reduce the waiting time. In addition, > we don't want to be scheduled during the sleeping time, some threads may take a > long time and don't give up the CPU, which affects the time of core shutdown, > Therefore, we add a chance to busy-wait max 1ms. > On the other hand, usleep_range reduces the timer interval and hence increases the chance of the callee CPU not to enter deeper idle states. What am I missing here ? What's the use case or power off situation you are talking about above ? > > > I've also added Sudeep to the thread, since I'd like his ack on the change. > > Thanks Will. -- Regards, Sudeep _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-16 10:25 ` Sudeep Holla @ 2019-10-16 11:29 ` Yunfeng Ye 2019-10-16 15:05 ` Sudeep Holla 0 siblings, 1 reply; 15+ messages in thread From: Yunfeng Ye @ 2019-10-16 11:29 UTC (permalink / raw) To: Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, Will Deacon, linux-arm-kernel@lists.infradead.org On 2019/10/16 18:25, Sudeep Holla wrote: > On Wed, Oct 16, 2019 at 11:22:23AM +0800, Yunfeng Ye wrote: >> >> >> On 2019/10/16 0:23, Will Deacon wrote: >>> Hi, >>> >>> On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: >>>> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not >>>> take so long in the right case. Use usleep_range() instead of msleep(), >>>> reduce the waiting time, and give a chance to busy wait before sleep. >>> >>> Can you elaborate on "the right case" please? It's not clear to me >>> exactly what problem you're solving here. >>> >> The situation is that when the power is off, we have a battery to save some >> information, but the battery power is limited, so we reduce the power consumption >> by turning off the cores, and need fastly to complete the core shutdown. However, the >> time of cpu_psci_cpu_kill() will take 10ms. We have tested the time that it does not >> need 10ms, and most case is about 50us-500us. if we reduce the time of cpu_psci_cpu_kill(), >> we can reduce 10% - 30% of the total time. >> > > Have you checked why PSCI AFFINITY_INFO not returning LEVEL_OFF quickly > then ? We wait for upto 5s in cpu_wait_death(worst case) before cpu_kill > is called from __cpu_die. > When cpu_wait_death() is done, it means that the cpu core's hardware prepare to die. I think not returning LEVEL_OFF quickly is that hardware need time to handle. I don't know how much time it need is reasonable, but I test that it need about 50us - 500us. In addition I have not meat the worst case that cpu_wait_death() need upto 5s, and we only take normal case into account. thanks. > Moreover I don't understand the argument here. The cpu being killed > will be OFF, as soon as it can and firmware controls that and this > change is not related to CPU_OFF. And this CPU calling cpu_kill can > sleep and 10ms is good to enter idle states if it's idle saving power, > so I fail to map the power saving you mention above. > We have hundreds of CPU cores that need to be shut down. For example, a CPU has 200 cores, and the thread to shut down the core is in CPU 0. and the thread need to shut down from core 1 to core 200. However, the implementation of the kernel can only shut down cpu cores one by one, so we need to wait for cpu_kill() to finish before shutting down the next CPU core. If it wait for 10ms each time in cpu_kill, it will takes up about 2 seconds in cpu_kill() total. It is not to save power through msleep to idle state, but to quickly turn off other CPU core's hardware to reduce power consumption. thanks. >> So change msleep (10) to usleep_range() to reduce the waiting time. In addition, >> we don't want to be scheduled during the sleeping time, some threads may take a >> long time and don't give up the CPU, which affects the time of core shutdown, >> Therefore, we add a chance to busy-wait max 1ms. >> > > On the other hand, usleep_range reduces the timer interval and hence > increases the chance of the callee CPU not to enter deeper idle states. > > What am I missing here ? What's the use case or power off situation > you are talking about above ? > As mentioned above, we are not to save power through msleep to idle state, but to quickly turn off other CPU core's hardware to reduce power consumption. >> >>> I've also added Sudeep to the thread, since I'd like his ack on the change. >>> > > Thanks Will. > > -- > Regards, > Sudeep > > . > _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-16 11:29 ` Yunfeng Ye @ 2019-10-16 15:05 ` Sudeep Holla 2019-10-17 14:08 ` Yunfeng Ye 0 siblings, 1 reply; 15+ messages in thread From: Sudeep Holla @ 2019-10-16 15:05 UTC (permalink / raw) To: Yunfeng Ye Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, Sudeep Holla, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, Will Deacon, linux-arm-kernel@lists.infradead.org On Wed, Oct 16, 2019 at 07:29:59PM +0800, Yunfeng Ye wrote: > > > On 2019/10/16 18:25, Sudeep Holla wrote: > > On Wed, Oct 16, 2019 at 11:22:23AM +0800, Yunfeng Ye wrote: > >> > >> > >> On 2019/10/16 0:23, Will Deacon wrote: > >>> Hi, > >>> > >>> On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: > >>>> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not > >>>> take so long in the right case. Use usleep_range() instead of msleep(), > >>>> reduce the waiting time, and give a chance to busy wait before sleep. > >>> > >>> Can you elaborate on "the right case" please? It's not clear to me > >>> exactly what problem you're solving here. > >>> > >> The situation is that when the power is off, we have a battery to save some > >> information, but the battery power is limited, so we reduce the power consumption > >> by turning off the cores, and need fastly to complete the core shutdown. However, the > >> time of cpu_psci_cpu_kill() will take 10ms. We have tested the time that it does not > >> need 10ms, and most case is about 50us-500us. if we reduce the time of cpu_psci_cpu_kill(), > >> we can reduce 10% - 30% of the total time. > >> > > > > Have you checked why PSCI AFFINITY_INFO not returning LEVEL_OFF quickly > > then ? We wait for upto 5s in cpu_wait_death(worst case) before cpu_kill > > is called from __cpu_die. > > > When cpu_wait_death() is done, it means that the cpu core's hardware prepare to > die. I think not returning LEVEL_OFF quickly is that hardware need time to handle. > I don't know how much time it need is reasonable, but I test that it need about > 50us - 500us. > Fair enough. > In addition I have not meat the worst case that cpu_wait_death() need upto > 5s, and we only take normal case into account. > Good > > > Moreover I don't understand the argument here. The cpu being killed > > will be OFF, as soon as it can and firmware controls that and this > > change is not related to CPU_OFF. And this CPU calling cpu_kill can > > sleep and 10ms is good to enter idle states if it's idle saving power, > > so I fail to map the power saving you mention above. > > > We have hundreds of CPU cores that need to be shut down. For example, > a CPU has 200 cores, and the thread to shut down the core is in CPU 0. > and the thread need to shut down from core 1 to core 200. However, the > implementation of the kernel can only shut down cpu cores one by one, so we > need to wait for cpu_kill() to finish before shutting down the next > CPU core. If it wait for 10ms each time in cpu_kill, it will takes up > about 2 seconds in cpu_kill() total. > OK, thanks for the illustrative example. This make sense to me now. But you comparing with battery powered devices confused me and I assumed it as some hack to optimise mobile workload. > >> So change msleep (10) to usleep_range() to reduce the waiting time. In addition, > >> we don't want to be scheduled during the sleeping time, some threads may take a > >> long time and don't give up the CPU, which affects the time of core shutdown, > >> Therefore, we add a chance to busy-wait max 1ms. > >> > > > > On the other hand, usleep_range reduces the timer interval and hence > > increases the chance of the callee CPU not to enter deeper idle states. > > > > What am I missing here ? What's the use case or power off situation > > you are talking about above ? > > > As mentioned above, we are not to save power through msleep to idle state, > but to quickly turn off other CPU core's hardware to reduce power consumption. You still don't provide your use-case in which this is required. I know this will be useful for suspend-to-ram. Do you have any other use-case that you need to power-off large number of CPUs like this ? Also you mentioned battery powered, and I don't think any battery powered device has 200 thread like in your example :) You need to mention few things clearly in the commit log: 1. How the CPU hotplug operation is serialised in some use-case like suspend-to-ram 2. How that may impact systems with large number of CPUs 3. How your change helps to improve that It may it easy for anyone to understand the motivation for this change. The commit message you have doesn't give any clue on all the above and hence we have lot of questions. I will respond to the original patch separately. -- Regards, Sudeep _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() 2019-10-16 15:05 ` Sudeep Holla @ 2019-10-17 14:08 ` Yunfeng Ye 0 siblings, 0 replies; 15+ messages in thread From: Yunfeng Ye @ 2019-10-17 14:08 UTC (permalink / raw) To: Sudeep Holla Cc: kstewart@linuxfoundation.org, ard.biesheuvel@linaro.org, hushiyuan, catalin.marinas@arm.com, linux-kernel@vger.kernel.org, linfeilong, David Laight, gregkh@linuxfoundation.org, tglx@linutronix.de, wuyun.wu@huawei.com, Will Deacon, linux-arm-kernel@lists.infradead.org On 2019/10/16 23:05, Sudeep Holla wrote: > On Wed, Oct 16, 2019 at 07:29:59PM +0800, Yunfeng Ye wrote: >> >> >> On 2019/10/16 18:25, Sudeep Holla wrote: >>> On Wed, Oct 16, 2019 at 11:22:23AM +0800, Yunfeng Ye wrote: >>>> >>>> >>>> On 2019/10/16 0:23, Will Deacon wrote: >>>>> Hi, >>>>> >>>>> On Sat, Sep 21, 2019 at 07:21:17PM +0800, Yunfeng Ye wrote: >>>>>> If psci_ops.affinity_info() fails, it will sleep 10ms, which will not >>>>>> take so long in the right case. Use usleep_range() instead of msleep(), >>>>>> reduce the waiting time, and give a chance to busy wait before sleep. >>>>> >>>>> Can you elaborate on "the right case" please? It's not clear to me >>>>> exactly what problem you're solving here. >>>>> >>>> The situation is that when the power is off, we have a battery to save some >>>> information, but the battery power is limited, so we reduce the power consumption >>>> by turning off the cores, and need fastly to complete the core shutdown. However, the >>>> time of cpu_psci_cpu_kill() will take 10ms. We have tested the time that it does not >>>> need 10ms, and most case is about 50us-500us. if we reduce the time of cpu_psci_cpu_kill(), >>>> we can reduce 10% - 30% of the total time. >>>> >>> >>> Have you checked why PSCI AFFINITY_INFO not returning LEVEL_OFF quickly >>> then ? We wait for upto 5s in cpu_wait_death(worst case) before cpu_kill >>> is called from __cpu_die. >>> >> When cpu_wait_death() is done, it means that the cpu core's hardware prepare to >> die. I think not returning LEVEL_OFF quickly is that hardware need time to handle. >> I don't know how much time it need is reasonable, but I test that it need about >> 50us - 500us. >> > > Fair enough. > >> In addition I have not meat the worst case that cpu_wait_death() need upto >> 5s, and we only take normal case into account. >> > > Good > >> >>> Moreover I don't understand the argument here. The cpu being killed >>> will be OFF, as soon as it can and firmware controls that and this >>> change is not related to CPU_OFF. And this CPU calling cpu_kill can >>> sleep and 10ms is good to enter idle states if it's idle saving power, >>> so I fail to map the power saving you mention above. >>> >> We have hundreds of CPU cores that need to be shut down. For example, >> a CPU has 200 cores, and the thread to shut down the core is in CPU 0. >> and the thread need to shut down from core 1 to core 200. However, the >> implementation of the kernel can only shut down cpu cores one by one, so we >> need to wait for cpu_kill() to finish before shutting down the next >> CPU core. If it wait for 10ms each time in cpu_kill, it will takes up >> about 2 seconds in cpu_kill() total. >> > > OK, thanks for the illustrative example. This make sense to me now. But > you comparing with battery powered devices confused me and I assumed > it as some hack to optimise mobile workload. > It is not mobile workload, but a arm64 server with hundreds of cpu cores. Battery powered is a backup battery for reliability and to prevent accidental power failure. >>>> So change msleep (10) to usleep_range() to reduce the waiting time. In addition, >>>> we don't want to be scheduled during the sleeping time, some threads may take a >>>> long time and don't give up the CPU, which affects the time of core shutdown, >>>> Therefore, we add a chance to busy-wait max 1ms. >>>> >>> >>> On the other hand, usleep_range reduces the timer interval and hence >>> increases the chance of the callee CPU not to enter deeper idle states. >>> >>> What am I missing here ? What's the use case or power off situation >>> you are talking about above ? >>> >> As mentioned above, we are not to save power through msleep to idle state, >> but to quickly turn off other CPU core's hardware to reduce power consumption. > > You still don't provide your use-case in which this is required. I know > this will be useful for suspend-to-ram. Do you have any other use-case > that you need to power-off large number of CPUs like this ? Also you > mentioned battery powered, and I don't think any battery powered device > has 200 thread like in your example :) > The use-case is like suspend-to-disk, but a little different: In the abnormal power failure of server equipment, in order to increase reliability, there is a backup battery for power supply. Before the battery runs out, we need to save the key datas to the disk. In order to maintain the battery power supply, a series of power reduction processing is needed, include which all the cores need to be shut down quickly, we have max near 200 cores need to shutdown. Also this modify will be useful for suspend-to-ram too. thanks. > You need to mention few things clearly in the commit log: > 1. How the CPU hotplug operation is serialised in some use-case like > suspend-to-ram > 2. How that may impact systems with large number of CPUs > 3. How your change helps to improve that > > It may it easy for anyone to understand the motivation for this change. > The commit message you have doesn't give any clue on all the above and > hence we have lot of questions. > ok, thanks. > I will respond to the original patch separately. > > -- > Regards, > Sudeep > > . > _______________________________________________ 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] 15+ messages in thread
end of thread, other threads:[~2019-10-17 14:25 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-09-21 11:21 [PATCH V2] arm64: psci: Reduce waiting time of cpu_psci_cpu_kill() Yunfeng Ye 2019-10-09 4:45 ` Yunfeng Ye 2019-10-16 15:32 ` Sudeep Holla 2019-10-17 13:26 ` Yunfeng Ye 2019-10-17 13:54 ` Sudeep Holla 2019-10-17 14:24 ` Yunfeng Ye 2019-10-17 14:00 ` David Laight 2019-10-17 14:19 ` Yunfeng Ye 2019-10-17 14:25 ` David Laight 2019-10-15 16:23 ` Will Deacon 2019-10-16 3:22 ` Yunfeng Ye 2019-10-16 10:25 ` Sudeep Holla 2019-10-16 11:29 ` Yunfeng Ye 2019-10-16 15:05 ` Sudeep Holla 2019-10-17 14:08 ` Yunfeng Ye
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).