Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
Hi Nick,
> Thanks for tracking this down. It's a fix for my breakage
>
> a7cba02deced ("powerpc: allow soft-NMI watchdog to cover timer
> interrupts with large decrementers")
>
> Taking another look... what I had expected here is the timer subsystem
> would have stopped the decrementer device after it processed the timer
> and found nothing left. And we should have set DEC to max at that
> time.
>
> The above patch was really intended to only cover the timer interrupt
> itself locking up. I wonder if we need to add
>
> .set_state_oneshot_stopped = decrementer_shutdown
>
> In our decremementer clockevent device?
Thanks Nick, that looks much nicer, and passes my tests.
Anton
Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
On Sat, 29 Sep 2018 11:26:07 +1000
Anton Blanchard wrote:
> If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to
> 0x7fff. As suggested by Nick, add a run time check of the watchdog
> cpumask, so if it is disabled we use the large decrementer.
>
> Signed-off-by: Anton Blanchard
> ---
Thanks for tracking this down. It's a fix for my breakage
a7cba02deced ("powerpc: allow soft-NMI watchdog to cover timer
interrupts with large decrementers")
Taking another look... what I had expected here is the timer subsystem
would have stopped the decrementer device after it processed the timer
and found nothing left. And we should have set DEC to max at that time.
The above patch was really intended to only cover the timer interrupt
itself locking up. I wonder if we need to add
.set_state_oneshot_stopped = decrementer_shutdown
In our decremementer clockevent device?
Thanks,
Nick
Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
Hi Anton,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Anton-Blanchard/powerpc-time-Use-clockevents_register_device-fixing-an-issue-with-large-decrementer/20180929-093322
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
arch/powerpc/kernel/time.c: In function 'timer_interrupt':
>> arch/powerpc/kernel/time.c:580:44: error: 'watchdog_cpumask' undeclared
>> (first use in this function); did you mean 'proc_watchdog_cpumask'?
cpumask_test_cpu(smp_processor_id(), _cpumask))
^~~~
proc_watchdog_cpumask
arch/powerpc/kernel/time.c:580:44: note: each undeclared identifier is
reported only once for each function it appears in
vim +580 arch/powerpc/kernel/time.c
549
550 /*
551 * timer_interrupt - gets called when the decrementer overflows,
552 * with interrupts disabled.
553 */
554 void timer_interrupt(struct pt_regs *regs)
555 {
556 struct clock_event_device *evt = this_cpu_ptr();
557 u64 *next_tb = this_cpu_ptr(_next_tb);
558 struct pt_regs *old_regs;
559 u64 now;
560
561 /* Some implementations of hotplug will get timer interrupts
while
562 * offline, just ignore these and we also need to set
563 * decrementers_next_tb as MAX to make sure __check_irq_replay
564 * don't replay timer interrupt when return, otherwise we'll
trap
565 * here infinitely :(
566 */
567 if (unlikely(!cpu_online(smp_processor_id( {
568 *next_tb = ~(u64)0;
569 set_dec(decrementer_max);
570 return;
571 }
572
573 /* Ensure a positive value is written to the decrementer, or
else
574 * some CPUs will continue to take decrementer exceptions. When
the
575 * PPC_WATCHDOG (decrementer based) is configured, keep this at
most
576 * 31 bits, which is about 4 seconds on most systems, which
gives
577 * the watchdog a chance of catching timer interrupt hard
lockups.
578 */
579 if (IS_ENABLED(CONFIG_PPC_WATCHDOG) &&
> 580 cpumask_test_cpu(smp_processor_id(), _cpumask))
581 set_dec(0x7fff);
582 else
583 set_dec(decrementer_max);
584
585 /* Conditionally hard-enable interrupts now that the DEC has
been
586 * bumped to its maximum value
587 */
588 may_hard_irq_enable();
589
590
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
[PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to 0x7fff. As suggested by Nick, add a run time check of the watchdog cpumask, so if it is disabled we use the large decrementer. Signed-off-by: Anton Blanchard --- arch/powerpc/kernel/time.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 6a1f0a084ca3..3372019f52bd 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -60,6 +60,7 @@ #include #include #include +#include #include #include @@ -575,7 +576,8 @@ void timer_interrupt(struct pt_regs *regs) * 31 bits, which is about 4 seconds on most systems, which gives * the watchdog a chance of catching timer interrupt hard lockups. */ - if (IS_ENABLED(CONFIG_PPC_WATCHDOG)) + if (IS_ENABLED(CONFIG_PPC_WATCHDOG) && + cpumask_test_cpu(smp_processor_id(), _cpumask)) set_dec(0x7fff); else set_dec(decrementer_max); -- 2.17.1
