Message ID | 20180929012607.10204-2-anton@ozlabs.org (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | [1/2] powerpc/time: Use clockevents_register_device(), fixing an issue with large decrementer | expand |
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(), &watchdog_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(&decrementers); 557 u64 *next_tb = this_cpu_ptr(&decrementers_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(), &watchdog_cpumask)) 581 set_dec(0x7fffffff); 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 infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, 29 Sep 2018 11:26:07 +1000 Anton Blanchard <anton@ozlabs.org> wrote: > If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to > 0x7fffffff. 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 <anton@samba.org> > --- 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
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
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 <linux/rtc.h> #include <linux/sched/cputime.h> #include <linux/processor.h> +#include <linux/nmi.h> #include <asm/trace.h> #include <asm/io.h> @@ -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(), &watchdog_cpumask)) set_dec(0x7fffffff); else set_dec(decrementer_max);
If CONFIG_PPC_WATCHDOG is enabled, we always cap the decrementer to 0x7fffffff. 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 <anton@samba.org> --- arch/powerpc/kernel/time.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)