RE: [PATCH] softirq: wake up ktimer thread in softirq context

Chang, Junxiao posted 1 patch 2 years, 6 months ago
RE: [PATCH] softirq: wake up ktimer thread in softirq context
Posted by Chang, Junxiao 2 years, 6 months ago
Thank you for handling this, modified patch looks good to me.

Regards,
Junxiao

-----Original Message-----
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de> 
Sent: Saturday, February 18, 2023 1:30 AM
To: Chang, Junxiao <junxiao.chang@intel.com>
Cc: linux-kernel@vger.kernel.org; linux-rt-users@vger.kernel.org; tglx@linutronix.de; rostedt@goodmis.org; Peh, Hock Zhang <hock.zhang.peh@intel.com>
Subject: Re: [PATCH] softirq: wake up ktimer thread in softirq context

On 2022-12-08 15:56:04 [+0800], Junxiao Chang wrote:
> Occiaionally timer interrupt might be triggered in softirq context, 
> ktimer thread should be woken up with RT kernel, or else ktimer thread 
> might stay in sleep state although timer interrupt has been triggered.
> 
> This change fixes a latency issue that timer handler is delayed for 
> more than 4ms in network related test.

Sorry for keeping you waiting. Your observation and patch is correct. I'm going to apply a slightly modified version of the patch (see below) after I reworded the commit message on Monday.

diff --git a/kernel/softirq.c b/kernel/softirq.c index ab1fe34326bab..82f3e68fbe220 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -664,13 +664,12 @@ static inline void __irq_exit_rcu(void)  #endif
 	account_hardirq_exit(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	if (!in_interrupt()) {
-		if (local_softirq_pending())
-			invoke_softirq();
+	if (!in_interrupt() && local_softirq_pending())
+		invoke_softirq();
 
-		if (IS_ENABLED(CONFIG_PREEMPT_RT) && local_pending_timers())
-			wake_timersd();
-	}
+	if (IS_ENABLED(CONFIG_PREEMPT_RT) && local_pending_timers() &&
+	    !(in_nmi() | in_hardirq()))
+		wake_timersd();
 
 	tick_irq_exit();
 }

Sebastian
Re: RE: [PATCH] softirq: wake up ktimer thread in softirq context
Posted by Sebastian Andrzej Siewior 2 years, 6 months ago
------->8------

From: Junxiao Chang <junxiao.chang@intel.com>
Date: Mon, 20 Feb 2023 09:12:20 +0100
Subject: [PATCH] softirq: Wake ktimers thread also in softirq.

If the hrtimer is raised while a softirq is processed then it does not
wake the corresponding ktimers thread. This is due to the optimisation in the
irq-exit path which is also used to wake the ktimers thread. For the other
softirqs, this is okay because the additional softirq bits will be handled by
the currently running softirq handler.
The timer related softirq bits are added to a different variable and rely on
the ktimers thread.
As a consuequence the wake up of ktimersd is delayed until the next timer tick.

Always wake the ktimers thread if a timer related softirq is pending.

Reported-by: Peh, Hock Zhang <hock.zhang.peh@intel.com>
Signed-off-by: Junxiao Chang <junxiao.chang@intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/softirq.c |   11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -664,13 +664,12 @@ static inline void __irq_exit_rcu(void)
 #endif
 	account_hardirq_exit(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	if (!in_interrupt()) {
-		if (local_softirq_pending())
-			invoke_softirq();
+	if (!in_interrupt() && local_softirq_pending())
+		invoke_softirq();
 
-		if (IS_ENABLED(CONFIG_PREEMPT_RT) && local_pending_timers())
-			wake_timersd();
-	}
+	if (IS_ENABLED(CONFIG_PREEMPT_RT) && local_pending_timers() &&
+	    !(in_nmi() | in_hardirq()))
+		wake_timersd();
 
 	tick_irq_exit();
 }