[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240710090210.41856-4-kprateek.nayak@amd.com>
Date: Wed, 10 Jul 2024 09:02:10 +0000
From: K Prateek Nayak <kprateek.nayak@....com>
To: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot
<vincent.guittot@...aro.org>, <linux-kernel@...r.kernel.org>
CC: Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt
<rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman
<mgorman@...e.de>, Daniel Bristot de Oliveira <bristot@...hat.com>, "Valentin
Schneider" <vschneid@...hat.com>, "Paul E. McKenney" <paulmck@...nel.org>,
Imran Khan <imran.f.khan@...cle.com>, Leonardo Bras <leobras@...hat.com>,
"Guo Ren" <guoren@...nel.org>, Rik van Riel <riel@...riel.com>, Tejun Heo
<tj@...nel.org>, Cruz Zhao <CruzZhao@...ux.alibaba.com>, Lai Jiangshan
<jiangshanlai@...il.com>, Joel Fernandes <joel@...lfernandes.org>, Zqiang
<qiang.zhang1211@...il.com>, Julia Lawall <julia.lawall@...ia.fr>, "Gautham
R. Shenoy" <gautham.shenoy@....com>, K Prateek Nayak <kprateek.nayak@....com>
Subject: [RFC PATCH 3/3] softirq: Avoid waking up ksoftirqd from flush_smp_call_function_queue()
Since commit b2a02fc43a1f4 ("smp: Optimize
send_call_function_single_ipi()"), sending an actual interrupt to an
idle CPU in TIF_POLLING_NRFLAG mode can be avoided by queuing the SMP
call function on the call function queue of the CPU and setting the
TIF_NEED_RESCHED bit in idle task's thread info. The call function is
handled in the idle exit path when do_idle() calls
flush_smp_call_function_queue().
However, since flush_smp_call_function_queue() is executed in idle
thread's context, in_interrupt() check within a call function will
return false. raise_softirq() uses this check to decide whether to wake
ksoftirqd, since, a softirq raised from an interrupt context will be
handled at irq exit. In all other cases, raise_softirq() wakes up
ksoftirqd to handle the softirq on !PREEMPT_RT kernel.
Since flush_smp_call_function_queue() calls
do_softirq_post_smp_call_flush(), waking up ksoftirqd is not necessary
since the softirqs raised by the call functions will be handled soon
after the call function queue is flushed. Mark
__flush_smp_call_function_queue() within flush_smp_call_function_queue()
with "will_do_softirq_post_flush" and use "do_softirq_pending()" to
notify raise_softirq() an impending call to do_softirq() and avoid
waking up ksoftirqd.
Adding a trace_printk() in nohz_csd_func() at the spot of raising
SCHED_SOFTIRQ and enabling trace events for sched_switch, sched_wakeup,
and softirq_entry (for SCHED_SOFTIRQ vector alone) helps observing the
current behavior:
<idle>-0 [000] dN.1. nohz_csd_func: Raise SCHED_SOFTIRQ for idle balance
<idle>-0 [000] dN.4. sched_wakeup: comm=ksoftirqd/0 pid=16 prio=120 target_cpu=000
<idle>-0 [000] .Ns1. softirq_entry: vec=7 [action=SCHED]
<idle>-0 [000] d..2. sched_switch: prev_comm=swapper/0 ==> next_comm=ksoftirqd/0
ksoftirqd/0-16 [000] d..2. sched_switch: prev_comm=ksoftirqd/0 ==> next_comm=swapper/0
ksoftirqd is woken up before the idle thread calls
do_softirq_post_smp_call_flush() which can make the runqueue appear
busy and prevent the idle load balancer from pulling task from an
overloaded runqueue towards itself[1]. Following are the observations
with the changes when enabling the same set of events:
<idle>-0 [000] dN.1. 106.134226: nohz_csd_func: Raise SCHED_SOFTIRQ for idle balance
<idle>-0 [000] .Ns1. 106.134227: softirq_entry: vec=7 [action=SCHED]
...
No unnecessary ksoftirqd wakeups are seen from idle task's context to
service the softirq. When doing performance testing, it was noticed that
per-CPU "will_do_softirq_post_flush" variable needs to be defined as
cacheline aligned to minimize performance overheads of the writes in
flush_smp_call_function_queue(). Following is the IPI throughput
measured using a modified version of ipistorm that performs a fixed set
of IPIs between two CPUs on a dual socket 3rd Generation EPYC system
(2 x 64C/128T) (boost on, C2 disabled) by running ipistorm between CPU8
and CPU16:
cmdline: insmod ipistorm.ko numipi=100000 single=1 offset=8 cpulist=8 wait=1
==================================================================
Test : ipistorm (modified)
Units : Normalized runtime
Interpretation: Lower is better
Statistic : AMean
==================================================================
kernel: time [pct imp]
tip:sched/core 1.00 [baseline]
tip:sched/core + SM_IDLE 0.25 [75.11%]
tip:sched/core + SM_IDLE + unaligned var 0.47 [53.74%] *
tip:sched/core + SM_IDLE + aligned var 0.25 [75.04%]
* The version where "will_do_softirq_post_flush" was not cacheline
aligned takes twice as long as the cacheline aligned version to
perform a fixed set of IPIs.
Fixes: b2a02fc43a1f ("smp: Optimize send_call_function_single_ipi()")
Reported-by: Julia Lawall <julia.lawall@...ia.fr>
Closes: https://lore.kernel.org/lkml/fcf823f-195e-6c9a-eac3-25f870cb35ac@inria.fr/ [1]
Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
---
kernel/sched/smp.h | 2 ++
kernel/smp.c | 32 ++++++++++++++++++++++++++++++++
kernel/softirq.c | 10 +++++++++-
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/smp.h b/kernel/sched/smp.h
index 21ac44428bb0..3731e79fe19b 100644
--- a/kernel/sched/smp.h
+++ b/kernel/sched/smp.h
@@ -9,7 +9,9 @@ extern void sched_ttwu_pending(void *arg);
extern bool call_function_single_prep_ipi(int cpu);
#ifdef CONFIG_SMP
+extern bool do_softirq_pending(void);
extern void flush_smp_call_function_queue(void);
#else
+static inline bool do_softirq_pending(void) { return false; }
static inline void flush_smp_call_function_queue(void) { }
#endif
diff --git a/kernel/smp.c b/kernel/smp.c
index f085ebcdf9e7..2eab5e1d5cef 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -559,6 +559,36 @@ static void __flush_smp_call_function_queue(bool warn_cpu_offline)
}
}
+/* Indicate an impending call to do_softirq_post_smp_call_flush() */
+static DEFINE_PER_CPU_ALIGNED(bool, will_do_softirq_post_flush);
+
+static __always_inline void __set_will_do_softirq_post_flush(void)
+{
+ this_cpu_write(will_do_softirq_post_flush, true);
+}
+
+static __always_inline void __clr_will_do_softirq_post_flush(void)
+{
+ this_cpu_write(will_do_softirq_post_flush, false);
+}
+
+/**
+ * do_softirq_pending - Check if do_softirq_post_smp_call_flush() will
+ * be called after the invocation of
+ * __flush_smp_call_function_queue()
+ *
+ * When flush_smp_call_function_queue() executes in the context of idle,
+ * migration thread, a softirq raised from the smp-call-function ends up
+ * waking ksoftirqd despite an impending softirq processing via
+ * do_softirq_post_smp_call_flush().
+ *
+ * Indicate an impending do_softirq() to should_wake_ksoftirqd() despite
+ * not being in an interrupt context.
+ */
+__always_inline bool do_softirq_pending(void)
+{
+ return this_cpu_read(will_do_softirq_post_flush);
+}
/**
* flush_smp_call_function_queue - Flush pending smp-call-function callbacks
@@ -583,7 +613,9 @@ void flush_smp_call_function_queue(void)
local_irq_save(flags);
/* Get the already pending soft interrupts for RT enabled kernels */
was_pending = local_softirq_pending();
+ __set_will_do_softirq_post_flush();
__flush_smp_call_function_queue(true);
+ __clr_will_do_softirq_post_flush();
if (local_softirq_pending())
do_softirq_post_smp_call_flush(was_pending);
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 02582017759a..b39eeed03042 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -34,6 +34,8 @@
#define CREATE_TRACE_POINTS
#include <trace/events/irq.h>
+#include "sched/smp.h"
+
/*
- No shared variables, all the data are CPU local.
- If a softirq needs serialization, let it serialize itself
@@ -413,7 +415,13 @@ static inline void ksoftirqd_run_end(void)
static inline bool should_wake_ksoftirqd(void)
{
- return true;
+ /*
+ * Avoid waking up ksoftirqd when a softirq is raised from a
+ * call-function executed by flush_smp_call_function_queue()
+ * in idle, migration thread's context since it'll soon call
+ * do_softirq_post_smp_call_flush().
+ */
+ return !do_softirq_pending();
}
static inline void invoke_softirq(void)
--
2.34.1
Powered by blists - more mailing lists