Re: [PATCH v3] sparc64: Fix comparator problem with timer interrupts

[Andreas Larsson]

On 2026-08-31 20:07, Stian Halseth wrote:
> From: Tony Rodriguez <unixpro1970@xxxxxxxxx>
> 
> The tick/stick/hbtick add_compare() implementations program the
> comparator and then check whether the write took effect in time:
> 
> 	exp = read_cnt() + delta_ticks;
> 	write_cmp(exp);
> 	return (read_cnt() - exp) > 0;
> 
> A nonzero return value means the expiry time was already reached
> before the comparator write could take effect, so the interrupt may
> never fire, and the caller retries with a new expiry:
> 
> 	return tick.add_compare(delta_ticks) ? -ETIME : 0;
> 
> The check only fails the write when the counter has advanced past the
> expiry time, but not when it is equal to it. In the equal case it is
> unknown whether the comparator write took effect before or after the
> counter reached the expiry value, so the compare match - and with it
> the timer interrupt - may have been missed. add_compare() then
> reports success, the caller does not retry, and the CPU is left with
> no pending timer interrupt.
> 
> This results in stalled hrtimers and RCU stalls / hangs under load,
> observed on SPARC S7-2 and T7-1 systems:
> 
>     rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
>     rcu: rcu_sched kthread timer wakeup didn't happen for 5259 jiffies!
>     rcu:     Possible timer handling issue on cpu=100 timer-softirq=15
> 
> Treat counter == expiry as failure as well, so the caller retries
> with a new expiry time. After this change S7-2 and T7-1 systems no
> longer hang.
> 
> Diagnosed-by: Thomas Gleixner <tglx@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/87tssb6olo.ffs@tglx/
> Link: https://lore.kernel.org/all/871pfcznw0.ffs@tglx/
> Link: https://lore.kernel.org/all/20260519022421.5978-1-unixpro1970@xxxxxxxxx/
> Signed-off-by: Tony Rodriguez <unixpro1970@xxxxxxxxx>
> [stian: rewrote the changelog per review of v2, retested]
> Signed-off-by: Stian Halseth <stian@xxxxxx>
> ---
> v3:
>  - rewrite the changelog: describe the write/readback ordering and the
>    direction of the equal-compare case per Thomas Gleixner's review
>    https://lore.kernel.org/all/878q9fxywc.ffs@tglx/
>    (the code change is identical to v2)
>  - add the Link: tags to the original debugging discussion
>  - picked up with Tony's agreement after v2 stalled
>    https://lore.kernel.org/all/10382506-3f57-4b33-8356-34f23fb5f153@xxxxxxxxx/
> 
> Tested on an UltraSPARC T4-1 (sun4v, stick variant): 9.1M hrtimer
> reprograms in 90s across 32 threads with 10-500us expiries, no
> stalls, no lost wakeups (worst oversleep 657us).
> 
>  arch/sparc/kernel/time_64.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sparc/kernel/time_64.c b/arch/sparc/kernel/time_64.c
> --- a/arch/sparc/kernel/time_64.c
> +++ b/arch/sparc/kernel/time_64.c
> @@ -146,7 +146,7 @@
>  			     : "=r" (new_tick));
>  	new_tick &= ~TICKCMP_IRQ_BIT;
>  
> -	return ((long)(new_tick - (orig_tick+adj))) > 0L;
> +	return ((long)(new_tick - (orig_tick+adj))) >= 0L;
>  }
>  
>  static unsigned long tick_add_tick(unsigned long adj)
> @@ -277,7 +277,7 @@
>  			     : "=r" (new_tick));
>  	new_tick &= ~TICKCMP_IRQ_BIT;
>  
> -	return ((long)(new_tick - (orig_tick+adj))) > 0L;
> +	return ((long)(new_tick - (orig_tick+adj))) >= 0L;
>  }
>  
>  static unsigned long stick_get_frequency(void)
> @@ -411,7 +411,7 @@
>  
>  	val2 = __hbird_read_stick() & ~TICKCMP_IRQ_BIT;
>  
> -	return ((long)(val2 - val)) > 0L;
> +	return ((long)(val2 - val)) >= 0L;
>  }
>  
>  static unsigned long hbtick_get_frequency(void)
> --
> 2.53.0

Reviewed-by: Andreas Larsson <andreas@xxxxxxxxxxx>

Picking this up to my for-next.

Thanks,
Andreas