[2/2] powerpc/time: Only cap decrementer when watchdog is enabled

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(-)

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