[PATCH] timers/migration: Fix livelock in tmigr_handle_remote_up()

[PATCH] timers/migration: Fix livelock in tmigr_handle_remote_up()

[Amit Matityahu]

tmigr_handle_remote_cpu() skips timer_expire_remote() when cpu ==
smp_processor_id(), assuming the local softirq path already handled
this CPU's timers.

This assumption breaks when jiffies advances between
run_timer_base(BASE_GLOBAL) and tmigr_handle_remote() in the same
softirq invocation - a timer expires after the wheel ran but before
the hierarchy snapshot is taken.

The stranded timer is never collected,
fetch_next_timer_interrupt_remote() keeps reporting it as expired,
and the event is re-queued with expires == now on each iteration.
The goto-again loop spins indefinitely.

Fix by calling timer_expire_remote() unconditionally.
__run_timer_base() already returns early when there is nothing to
expire, making this a no-op in the common case.

Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model")
Cc: stable@vger.kernel.org
Reported-by: Alon Kariv <alonka@amazon.com>
Cc: Jonathan Chocron <jonnyc@amazon.com>
Cc: Akram Baransi <abaransi@amazon.com>
Cc: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Amit Matityahu <amitmat@amazon.com>
---

Questions for maintainers:

1. What was the original rationale for the cpu != smp_processor_id()
   check? There is no code comment, commit message explanation or anything
   in the original patch's email discussion as to why
   timer_expire_remote() is skipped for the local CPU.

2. There seems to be a design tension where a CPU can have timers
   visible in the migration hierarchy while simultaneously running its
   own local softirq. Is the expectation that run_timer_base() always
   drains everything before tmigr_handle_remote() sees it, or should
   the remote path handle local-CPU timers as a fallback?

 kernel/time/timer_migration.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 1d0d3a4058d5..298c34c942ae 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -978,8 +978,7 @@ static void tmigr_handle_remote_cpu(unsigned int cpu, u64 now,
 	/* Drop the lock to allow the remote CPU to exit idle */
 	raw_spin_unlock_irq(&tmc->lock);
 
-	if (cpu != smp_processor_id())
-		timer_expire_remote(cpu);
+	timer_expire_remote(cpu);
 
 	/*
 	 * Lock ordering needs to be preserved - timer_base locks before tmigr

base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
-- 
2.47.3

Re: [PATCH] timers/migration: Fix livelock in tmigr_handle_remote_up()

[Frederic Weisbecker]

Le Wed, Jun 03, 2026 at 05:01:39PM +0000, Amit Matityahu a écrit :
> tmigr_handle_remote_cpu() skips timer_expire_remote() when cpu ==
> smp_processor_id(), assuming the local softirq path already handled
> this CPU's timers.
> 
> This assumption breaks when jiffies advances between
> run_timer_base(BASE_GLOBAL) and tmigr_handle_remote() in the same
> softirq invocation - a timer expires after the wheel ran but before
> the hierarchy snapshot is taken.
> 
> The stranded timer is never collected,
> fetch_next_timer_interrupt_remote() keeps reporting it as expired,
> and the event is re-queued with expires == now on each iteration.
> The goto-again loop spins indefinitely.
> 
> Fix by calling timer_expire_remote() unconditionally.
> __run_timer_base() already returns early when there is nothing to
> expire, making this a no-op in the common case.
> 
> Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model")
> Cc: stable@vger.kernel.org
> Reported-by: Alon Kariv <alonka@amazon.com>
> Cc: Jonathan Chocron <jonnyc@amazon.com>
> Cc: Akram Baransi <abaransi@amazon.com>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Signed-off-by: Amit Matityahu <amitmat@amazon.com>

That's quite serious indeed!

> ---
> 
> Questions for maintainers:
> 
> 1. What was the original rationale for the cpu != smp_processor_id()
>    check? There is no code comment, commit message explanation or anything
>    in the original patch's email discussion as to why
>    timer_expire_remote() is skipped for the local CPU.

The rationale was about assuming that such an expired timerqueue actually
reflected a timer that was handled locally already and so it could be safely
discarded. So we could spare some locking.

> 
> 2. There seems to be a design tension where a CPU can have timers
>    visible in the migration hierarchy while simultaneously running its
>    own local softirq. Is the expectation that run_timer_base() always
>    drains everything before tmigr_handle_remote() sees it, or should
>    the remote path handle local-CPU timers as a fallback?

That's not easy to defer all global timers handling to remote expiration
because the current CPU may or may not be the migrator.

> 
>  kernel/time/timer_migration.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
> index 1d0d3a4058d5..298c34c942ae 100644
> --- a/kernel/time/timer_migration.c
> +++ b/kernel/time/timer_migration.c
> @@ -978,8 +978,7 @@ static void tmigr_handle_remote_cpu(unsigned int cpu, u64 now,
>  	/* Drop the lock to allow the remote CPU to exit idle */
>  	raw_spin_unlock_irq(&tmc->lock);
>  
> -	if (cpu != smp_processor_id())
> -		timer_expire_remote(cpu);
> +	timer_expire_remote(cpu);

Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

Thanks!

>  
>  	/*
>  	 * Lock ordering needs to be preserved - timer_base locks before tmigr
> 
> base-commit: e43ffb69e0438cddd72aaa30898b4dc446f664f8
> -- 
> 2.47.3
> 

-- 
Frederic Weisbecker
SUSE Labs

Re: [PATCH] timers/migration: Fix livelock in tmigr_handle_remote_up()

[Thomas Gleixner]

On Thu, Jun 04 2026 at 11:45, Frederic Weisbecker wrote:
> Le Wed, Jun 03, 2026 at 05:01:39PM +0000, Amit Matityahu a écrit :
>> Questions for maintainers:
>> 
>> 1. What was the original rationale for the cpu != smp_processor_id()
>>    check? There is no code comment, commit message explanation or anything
>>    in the original patch's email discussion as to why
>>    timer_expire_remote() is skipped for the local CPU.
>
> The rationale was about assuming that such an expired timerqueue actually
> reflected a timer that was handled locally already and so it could be safely
> discarded. So we could spare some locking.

Right, but the assumption would only be valid _if_ the jiffies value
which was used in run_timers(GLOBAL) is propagated into the remote
handling.

>> -	if (cpu != smp_processor_id())
>> -		timer_expire_remote(cpu);
>> +	timer_expire_remote(cpu);
>
> Reviewed-by: Frederic Weisbecker <frederic@kernel.org>

I'll add a comment to that for posterity.

[tip: timers/urgent] timers/migration: Fix livelock in tmigr_handle_remote_up()

[tip-bot2 for Amit Matityahu]

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     d486b4934a8e504376b85cdb3766f306d57aff5b
Gitweb:        https://git.kernel.org/tip/d486b4934a8e504376b85cdb3766f306d57aff5b
Author:        Amit Matityahu <amitmat@amazon.com>
AuthorDate:    Wed, 03 Jun 2026 17:01:39 
Committer:     Thomas Gleixner <tglx@kernel.org>
CommitterDate: Thu, 04 Jun 2026 14:35:33 +02:00

timers/migration: Fix livelock in tmigr_handle_remote_up()

tmigr_handle_remote_cpu() skips timer_expire_remote() when cpu ==
smp_processor_id(), assuming the local softirq path already handled this
CPU's timers.

This assumption is wrong because jiffies can advance after the handling of
the CPU's global timers in run_timer_base(BASE_GLOBAL) and before
tmigr_handle_remote() evaluates the expiry times.

As a consequence a timer which expires after the CPU local timer wheel
advanced and becomes expired in the remote handling is ignored and the
callback is never invoked and removed from the timer wheel.

What's worse is that fetch_next_timer_interrupt_remote() keeps reporting it
as expired, and the event is re-queued with expires == now on each
iteration.  The goto-again loop spins indefinitely.

Fix this by calling timer_expire_remote() unconditionally. That's minimal
overhead for the common case as __run_timer_base() returns immediately if
there is nothing to expire in the local wheel.

[ tglx: Amend change log and add a comment ]

Fixes: 7ee988770326 ("timers: Implement the hierarchical pull model")
Reported-by: Alon Kariv <alonka@amazon.com>
Signed-off-by: Amit Matityahu <amitmat@amazon.com>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260603170139.33628-1-amitmat@amazon.com
---
 kernel/time/timer_migration.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/time/timer_migration.c b/kernel/time/timer_migration.c
index 1d0d3a4..52c15af 100644
--- a/kernel/time/timer_migration.c
+++ b/kernel/time/timer_migration.c
@@ -978,8 +978,12 @@ static void tmigr_handle_remote_cpu(unsigned int cpu, u64 now,
 	/* Drop the lock to allow the remote CPU to exit idle */
 	raw_spin_unlock_irq(&tmc->lock);
 
-	if (cpu != smp_processor_id())
-		timer_expire_remote(cpu);
+	/*
+	 * This can't exclude the local CPU because jiffies might have advanced
+	 * after the timer softirq invoked run_timer_base(BASE_GLOBAL) and the
+	 * point where the jiffies snapshot @jif was taken in tmigr_handle_remote().
+	 */
+	timer_expire_remote(cpu);
 
 	/*
 	 * Lock ordering needs to be preserved - timer_base locks before tmigr