[PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

[PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

[Andrew Bresticker]

When starting the local sync timer to synchronize the state of a remote
CPU it should be added on the CPU to be synchronized, not the initiating
CPU. This results in interrupt delivery being delayed until the timer
eventually runs (due to another mask/unmask/migrate operation) on the
target CPU.

Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>
---
 drivers/irqchip/irq-riscv-imsic-state.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
index bdf5cd2037f2..62f76950a113 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.c
+++ b/drivers/irqchip/irq-riscv-imsic-state.c
@@ -208,17 +208,17 @@ static bool __imsic_local_sync(struct imsic_local_priv *lpriv)
 }
 
 #ifdef CONFIG_SMP
-static void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
+static void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
 {
 	lockdep_assert_held(&lpriv->lock);
 
 	if (!timer_pending(&lpriv->timer)) {
 		lpriv->timer.expires = jiffies + 1;
-		add_timer_on(&lpriv->timer, smp_processor_id());
+		add_timer_on(&lpriv->timer, cpu);
 	}
 }
 #else
-static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
+static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
 {
 }
 #endif
@@ -233,7 +233,7 @@ void imsic_local_sync_all(bool force_all)
 	if (force_all)
 		bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
 	if (!__imsic_local_sync(lpriv))
-		__imsic_local_timer_start(lpriv);
+		__imsic_local_timer_start(lpriv, smp_processor_id());
 
 	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
 }
@@ -278,7 +278,7 @@ static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu
 				return;
 		}
 
-		__imsic_local_timer_start(lpriv);
+		__imsic_local_timer_start(lpriv, cpu);
 	}
 }
 #else
-- 
2.43.0

Re: [PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

[Anup Patel]

On Wed, May 14, 2025 at 10:43 PM Andrew Bresticker
<abrestic@rivosinc.com> wrote:
>
> When starting the local sync timer to synchronize the state of a remote
> CPU it should be added on the CPU to be synchronized, not the initiating
> CPU. This results in interrupt delivery being delayed until the timer
> eventually runs (due to another mask/unmask/migrate operation) on the
> target CPU.
>
> Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
> Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>

My bad. Thanks for catching the issue.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  drivers/irqchip/irq-riscv-imsic-state.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
> index bdf5cd2037f2..62f76950a113 100644
> --- a/drivers/irqchip/irq-riscv-imsic-state.c
> +++ b/drivers/irqchip/irq-riscv-imsic-state.c
> @@ -208,17 +208,17 @@ static bool __imsic_local_sync(struct imsic_local_priv *lpriv)
>  }
>
>  #ifdef CONFIG_SMP
> -static void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
> +static void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
>  {
>         lockdep_assert_held(&lpriv->lock);
>
>         if (!timer_pending(&lpriv->timer)) {
>                 lpriv->timer.expires = jiffies + 1;
> -               add_timer_on(&lpriv->timer, smp_processor_id());
> +               add_timer_on(&lpriv->timer, cpu);
>         }
>  }
>  #else
> -static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
> +static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
>  {
>  }
>  #endif
> @@ -233,7 +233,7 @@ void imsic_local_sync_all(bool force_all)
>         if (force_all)
>                 bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
>         if (!__imsic_local_sync(lpriv))
> -               __imsic_local_timer_start(lpriv);
> +               __imsic_local_timer_start(lpriv, smp_processor_id());
>
>         raw_spin_unlock_irqrestore(&lpriv->lock, flags);
>  }
> @@ -278,7 +278,7 @@ static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu
>                                 return;
>                 }
>
> -               __imsic_local_timer_start(lpriv);
> +               __imsic_local_timer_start(lpriv, cpu);
>         }
>  }
>  #else
> --
> 2.43.0
>

[tip: irq/urgent] irqchip/riscv-imsic: Start local sync timer on correct CPU

[tip-bot2 for Andrew Bresticker]

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

Commit-ID:     08fb624802d8786253994d8ebdbbcdaa186f04f5
Gitweb:        https://git.kernel.org/tip/08fb624802d8786253994d8ebdbbcdaa186f04f5
Author:        Andrew Bresticker <abrestic@rivosinc.com>
AuthorDate:    Wed, 14 May 2025 10:13:20 -07:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Thu, 15 May 2025 16:01:50 +02:00

irqchip/riscv-imsic: Start local sync timer on correct CPU

When starting the local sync timer to synchronize the state of a remote
CPU it should be added on the CPU to be synchronized, not the initiating
CPU. This results in interrupt delivery being delayed until the timer
eventually runs (due to another mask/unmask/migrate operation) on the
target CPU.

Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/all/20250514171320.3494917-1-abrestic@rivosinc.com

---
 drivers/irqchip/irq-riscv-imsic-state.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-imsic-state.c b/drivers/irqchip/irq-riscv-imsic-state.c
index bdf5cd2..62f7695 100644
--- a/drivers/irqchip/irq-riscv-imsic-state.c
+++ b/drivers/irqchip/irq-riscv-imsic-state.c
@@ -208,17 +208,17 @@ skip:
 }
 
 #ifdef CONFIG_SMP
-static void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
+static void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
 {
 	lockdep_assert_held(&lpriv->lock);
 
 	if (!timer_pending(&lpriv->timer)) {
 		lpriv->timer.expires = jiffies + 1;
-		add_timer_on(&lpriv->timer, smp_processor_id());
+		add_timer_on(&lpriv->timer, cpu);
 	}
 }
 #else
-static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv)
+static inline void __imsic_local_timer_start(struct imsic_local_priv *lpriv, unsigned int cpu)
 {
 }
 #endif
@@ -233,7 +233,7 @@ void imsic_local_sync_all(bool force_all)
 	if (force_all)
 		bitmap_fill(lpriv->dirty_bitmap, imsic->global.nr_ids + 1);
 	if (!__imsic_local_sync(lpriv))
-		__imsic_local_timer_start(lpriv);
+		__imsic_local_timer_start(lpriv, smp_processor_id());
 
 	raw_spin_unlock_irqrestore(&lpriv->lock, flags);
 }
@@ -278,7 +278,7 @@ static void __imsic_remote_sync(struct imsic_local_priv *lpriv, unsigned int cpu
 				return;
 		}
 
-		__imsic_local_timer_start(lpriv);
+		__imsic_local_timer_start(lpriv, cpu);
 	}
 }
 #else

Re: [PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

[patchwork-bot+linux-riscv]

Hello:

This patch was applied to riscv/linux.git (fixes)
by Thomas Gleixner <tglx@linutronix.de>:

On Wed, 14 May 2025 10:13:20 -0700 you wrote:
> When starting the local sync timer to synchronize the state of a remote
> CPU it should be added on the CPU to be synchronized, not the initiating
> CPU. This results in interrupt delivery being delayed until the timer
> eventually runs (due to another mask/unmask/migrate operation) on the
> target CPU.
> 
> Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
> Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>
> 
> [...]

Here is the summary with links:
  - irqchip/riscv-imsic: Start local sync timer on correct CPU
    https://git.kernel.org/riscv/c/08fb624802d8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] irqchip/riscv-imsic: Start local sync timer on correct CPU

[Thomas Gleixner]

On Tue, Jun 10 2025 at 16:28, patchwork-bot wrote:
> Hello:
>
> This patch was applied to riscv/linux.git (fixes)
> by Thomas Gleixner <tglx@linutronix.de>:

No. I never apply patches to the riscv tree :)

> On Wed, 14 May 2025 10:13:20 -0700 you wrote:
>> When starting the local sync timer to synchronize the state of a remote
>> CPU it should be added on the CPU to be synchronized, not the initiating
>> CPU. This results in interrupt delivery being delayed until the timer
>> eventually runs (due to another mask/unmask/migrate operation) on the
>> target CPU.
>> 
>> Fixes: 0f67911e821c ("irqchip/riscv-imsic: Separate next and previous pointers in IMSIC vector")
>> Signed-off-by: Andrew Bresticker <abrestic@rivosinc.com>
>> 
>> [...]
>
> Here is the summary with links:
>   - irqchip/riscv-imsic: Start local sync timer on correct CPU
>     https://git.kernel.org/riscv/c/08fb624802d8
>
> You are awesome, thank you!

That patch has been committed to the tip tree and is already upstream.

Can you fix your bot scripts please?