[PATCH v2 1/1] sparc64: Fix comparator problem with timer interrupts

[PATCH v2 1/1] sparc64: Fix comparator problem with timer interrupts

[Tony Rodriguez]

sparc64: Fix comparator problem with timer interrupts

This patch fixes SPARC64 tick/stick comparator logic that can cause missed
timer interrupts

Full dmesg and crash dumps:
https://github.com/unixpro1970/Sparc64-Kernel-Debugging-Dumps/blob/main/s7-2-05122026-dump.tar.gz

Debugging discussion:
https://lore.kernel.org/all/87tssb6olo.ffs@tglx/
https://lore.kernel.org/all/871pfcznw0.ffs@tglx/

Short dmesg excerpt:
[   48.632215] rcu: INFO: rcu_sched detected stalls on CPUs/tasks:
[   48.676018] rcu: rcu_sched kthread timer wakeup didn't happen for 5259 jiffies!
[   48.743621] rcu:     Possible timer handling issue on cpu=100 timer-softirq=15

Test summary:
After applying this patch, S7-2 and T7-1 systems no longer hang on v7 and v7.1 kernels.

Tony Rodriguez (1):
  sparc64: Fix comparator problem with timer interrupts

 arch/sparc/kernel/time_64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

--
2.53.0

[PATCH v2 1/1] sparc64: Fix comparator problem with timer interrupts

[Tony Rodriguez]

On SPARC64 the check:

    return ((long)(new_tick - (orig_tick + adj))) > 0L;

Is safe only if retries make forward progress. The comparator can
take effect with a latency, so the moment when counter == comparator
may be missed, which can cause delays or hangs on some SPARC64 systems.

For clarity:
    exp = orig_tick + adj   /* expected comparator value */

The current check requires new_tick to be strictly greater than exp;
equality (new_tick == exp) is treated as not yet passed and the caller
will retry.

By contrast, using:

    return ((long)(new_tick - (orig_tick + adj))) >= 0L;

causes the caller to stop retrying and assume the timer is scheduled;
both equality and greater-than are accepted (new_tick == exp or
new_tick > exp).

Signed-off-by: Tony Rodriguez <unixpro1970@gmail.com>
---
 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
index 87b267043ccd..783b60e547c4 100644
--- a/arch/sparc/kernel/time_64.c
+++ b/arch/sparc/kernel/time_64.c
@@ -146,7 +146,7 @@ static int tick_add_compare(unsigned long adj)
 			     : "=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 @@ static int stick_add_compare(unsigned long adj)
 			     : "=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 @@ static int hbtick_add_compare(unsigned long adj)

 	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

Re: [PATCH v2 1/1] sparc64: Fix comparator problem with timer interrupts

[Thomas Gleixner]

On Mon, May 18 2026 at 19:24, Tony Rodriguez wrote:
> On SPARC64 the check:
>
>     return ((long)(new_tick - (orig_tick + adj))) > 0L;
>
> Is safe only if retries make forward progress. The comparator can
> take effect with a latency, so the moment when counter == comparator
> may be missed, which can cause delays or hangs on some SPARC64 systems.
>
> For clarity:
>     exp = orig_tick + adj   /* expected comparator value */
>
> The current check requires new_tick to be strictly greater than exp;
> equality (new_tick == exp) is treated as not yet passed and the caller
> will retry.

That's confusing at best. You really want to explain how the ordering is
similar to what I described in the analysis:

        exp = read_cnt() + delta_ticks;
        write_cmp(exp);
        return (read_cnt() - exp) > 0;

If the counter advanced past the expected expiry time, after writing it,
then the caller will retry, as the calling code does:

     return tick.add_compare(delta_ticks) ? -ETIME : 0;

But it won't do so when the counter is equal, which is causing the
problem.

> By contrast, using:
>
>     return ((long)(new_tick - (orig_tick + adj))) >= 0L;
>
> causes the caller to stop retrying and assume the timer is scheduled;
> both equality and greater-than are accepted (new_tick == exp or
> new_tick > exp).

It's the other way round. When counter >= expiry time, then the write is
considered failed. If the counter has not yet reached expiry time,
i.e. it is smaller, then it assumes the timer is scheduled.

> Signed-off-by: Tony Rodriguez <unixpro1970@gmail.com>

It would be nice to have a link to the original thread in the change log
itself as that gives people quick access when they are wondering about
this a year down the road.

Thanks,

        tglx

Re: [PATCH v2 1/1] sparc64: Fix comparator problem with timer interrupts

[Tony Rodriguez]

Hi Thomas,

Thanks again for the careful analysis of the comparator ordering. After 
applying the changes in the patch I confirmed S7‑2 and T7‑1 systems no 
longer hang; timer scheduling now behaves as expected.

I don’t have bandwidth right now to finish the remaining tidy steps and 
I’d rather avoid delaying this fix. Since we worked on this together and 
you’re familiar with the issue, would you be willing to take it over so 
upstream has everything required for approval? I’m happy to answer any 
questions; if you’re busy I can ask the maintainer to reassign.

Thanks again for the review and the clear explanation.

Tony

On 5/19/26 7:22 AM, Thomas Gleixner wrote:
>> On SPARC64 the check:
>>
>>      return ((long)(new_tick - (orig_tick + adj))) > 0L;
>>
>> Is safe only if retries make forward progress. The comparator can
>> take effect with a latency, so the moment when counter == comparator
>> may be missed, which can cause delays or hangs on some SPARC64 systems.
>>
>> For clarity:
>>      exp = orig_tick + adj   /* expected comparator value */
>>
>> The current check requires new_tick to be strictly greater than exp;
>> equality (new_tick == exp) is treated as not yet passed and the caller
>> will retry.
> That's confusing at best. You really want to explain how the ordering is
> similar to what I described in the analysis:
>
>          exp = read_cnt() + delta_ticks;
>          write_cmp(exp);
>          return (read_cnt() - exp) > 0;
>
> If the counter advanced past the expected expiry time, after writing it,
> then the caller will retry, as the calling code does:
>
>       return tick.add_compare(delta_ticks) ? -ETIME : 0;
>
> But it won't do so when the counter is equal, which is causing the
> problem.
>
>> By contrast, using:
>>
>>      return ((long)(new_tick - (orig_tick + adj))) >= 0L;
>>
>> causes the caller to stop retrying and assume the timer is scheduled;
>> both equality and greater-than are accepted (new_tick == exp or
>> new_tick > exp).
> It's the other way round. When counter >= expiry time, then the write is
> considered failed. If the counter has not yet reached expiry time,
> i.e. it is smaller, then it assumes the timer is scheduled.
>
>> Signed-off-by: Tony Rodriguez<unixpro1970@gmail.com>
> It would be nice to have a link to the original thread in the change log
> itself as that gives people quick access when they are wondering about
> this a year down the road.
>
> Thanks,
>
>          tglx

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

[Stian Halseth]

From: Tony Rodriguez <unixpro1970@gmail.com>

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@kernel.org>
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@gmail.com/
Signed-off-by: Tony Rodriguez <unixpro1970@gmail.com>
[stian: rewrote the changelog per review of v2, retested]
Signed-off-by: Stian Halseth <stian@itx.no>
---
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@gmail.com/

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

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

[Stian Halseth]

[-- Attachment #1: Type: text/plain, Size: 5216 bytes --]

Hi,

I just got a SPARC T7-1 and have independent reproduction and
confirmation of the fix.

Without the patch the system is not really usable. I can just bring
down a network interface and that alone triggers a hang. 
Hung on 3 out of 3 boots unpatched:

  task:ip              state:D
   [<0000000000cfebc4>] wait_for_completion+0x64/0x180
   [<0000000000509c30>] synchronize_rcu_normal+0x150/0x280
   [<0000000010079060>] ixgbe_down+0x1a0/0x460 [ixgbe]
   [<000000001007a4c8>] ixgbe_close+0xe8/0x100 [ixgbe]
   [<0000000000af1cf8>] netif_change_flags+0x18/0x80
   [<0000000000b08618>] rtnl_newlink+0x758/0xaa0

A second caller hit the same stall under kernel build load, from
network namespace teardown rather than any driver path:

  task:kworker/u1024:0 state:D
   Workqueue: netns cleanup_net
   [<0000000000cfebc4>] wait_for_completion+0x64/0x180
   [<0000000000504ab4>] rcu_barrier+0x214/0x480
   [<0000000000af3f88>] netdev_run_todo+0x48/0x600
   [<0000000000ad99c8>] cleanup_net+0x1c8/0x360

Both are plain RCU waits:

  rcu: rcu_sched kthread timer wakeup didn't happen for 6002 jiffies!
       g193 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402
  rcu:     Possible timer handling issue on cpu=20 timer-softirq=107
  rcu: rcu_sched kthread starved for 6004 jiffies! ->cpu=20


With the patch applied: 3 boots with a clean first ifdown on each, plus
6 consecutive interface down/up cycles within one boot, and no stall or
hung-task messages in any of them.

Tested-by: Stian Halseth <stian@itx.no>


On Mon, 2026-08-31 at 20:07 +0200, Stian Halseth wrote:
> From: Tony Rodriguez <unixpro1970@gmail.com>
> 
> 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@kernel.org>
> 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@gmail.com/
> Signed-off-by: Tony Rodriguez <unixpro1970@gmail.com>
> [stian: rewrote the changelog per review of v2, retested]
> Signed-off-by: Stian Halseth <stian@itx.no>
> ---
> 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@gmail.com/
> 
> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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@gmail.com>
> 
> 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@kernel.org>
> 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@gmail.com/
> Signed-off-by: Tony Rodriguez <unixpro1970@gmail.com>
> [stian: rewrote the changelog per review of v2, retested]
> Signed-off-by: Stian Halseth <stian@itx.no>
> ---
> 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@gmail.com/
> 
> 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@gaisler.com>

Picking this up to my for-next.

Thanks,
Andreas