From 35de589cb8793573ed56a915af9cb4b5f15ad7d7 Mon Sep 17 00:00:00 2001 From: Nicholas Piggin Date: Tue, 25 Jan 2022 00:39:30 +1000 Subject: [PATCH] powerpc/time: improve decrementer clockevent processing The stop/shutdown op should not use decrementer_set_next_event because that sets decrementers_next_tb to now + decrementer_max, which means a decrementer interrupt that occurs after that time will call the clockevent event handler unexpectedly. Set next_tb to ~0 here to prevent any clock event call. Init all clockevents to stopped. Then the decrementer clockevent device always has event_handler set and applicable because we know the clock event device was not stopped. So make this call unconditional to show that it is always called. next_tb need not be set to ~0 before the event handler is called because it will stop the clockevent device if there is no other timer. Finally, the timer broadcast interrupt should not modify next_tb because it is not involved with the local decrementer clockevent on this CPU. This doesn't fix a known bug, just tidies the code. Signed-off-by: Nicholas Piggin Signed-off-by: Michael Ellerman Link: https://lore.kernel.org/r/20220124143930.3923442-3-npiggin@gmail.com --- arch/powerpc/kernel/time.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index 0bb80f0bd0a6..f5cbfe5efd25 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -107,7 +107,12 @@ struct clock_event_device decrementer_clockevent = { }; EXPORT_SYMBOL(decrementer_clockevent); -DEFINE_PER_CPU(u64, decrementers_next_tb); +/* + * This always puts next_tb beyond now, so the clock event will never fire + * with the usual comparison, no need for a separate test for stopped. + */ +#define DEC_CLOCKEVENT_STOPPED ~0ULL +DEFINE_PER_CPU(u64, decrementers_next_tb) = DEC_CLOCKEVENT_STOPPED; EXPORT_SYMBOL_GPL(decrementers_next_tb); static DEFINE_PER_CPU(struct clock_event_device, decrementers); @@ -645,9 +650,7 @@ DEFINE_INTERRUPT_HANDLER_ASYNC(timer_interrupt) now = get_tb(); if (now >= *next_tb) { - *next_tb = ~(u64)0; - if (evt->event_handler) - evt->event_handler(evt); + evt->event_handler(evt); __this_cpu_inc(irq_stat.timer_irqs_event); } else { now = *next_tb - now; @@ -666,9 +669,6 @@ EXPORT_SYMBOL(timer_interrupt); #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST void timer_broadcast_interrupt(void) { - u64 *next_tb = this_cpu_ptr(&decrementers_next_tb); - - *next_tb = ~(u64)0; tick_receive_broadcast(); __this_cpu_inc(irq_stat.broadcast_irqs_event); } @@ -894,7 +894,9 @@ static int decrementer_set_next_event(unsigned long evt, static int decrementer_shutdown(struct clock_event_device *dev) { - decrementer_set_next_event(decrementer_max, dev); + __this_cpu_write(decrementers_next_tb, DEC_CLOCKEVENT_STOPPED); + set_dec_or_work(decrementer_max); + return 0; }