[PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Florian Bezdeka]

From: K Prateek Nayak <kprateek.nayak@amd.com>

commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.

do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
Since do_softirq_post_smp_call_flush() is called with preempt disabled,
raising a SOFTIRQ during flush_smp_call_function_queue() can lead to
longer preempt disabled sections.

Since commit b2a02fc43a1f ("smp: Optimize
send_call_function_single_ipi()") IPIs to an idle CPU in
TIF_POLLING_NRFLAG mode can be optimized out by instead setting
TIF_NEED_RESCHED bit in idle task's thread_info and relying on the
flush_smp_call_function_queue() in the idle-exit path to run the
SMP-call-function.

To trigger an idle load balancing, the scheduler queues
nohz_csd_function() responsible for triggering an idle load balancing on
a target nohz idle CPU and sends an IPI. Only now, this IPI is optimized
out and the SMP-call-function is executed from
flush_smp_call_function_queue() in do_idle() which can raise a
SCHED_SOFTIRQ to trigger the balancing.

So far, this went undetected since, the need_resched() check in
nohz_csd_function() would make it bail out of idle load balancing early
as the idle thread does not clear TIF_POLLING_NRFLAG before calling
flush_smp_call_function_queue(). The need_resched() check was added with
the intent to catch a new task wakeup, however, it has recently
discovered to be unnecessary and will be removed in the subsequent
commit after which nohz_csd_function() can raise a SCHED_SOFTIRQ from
flush_smp_call_function_queue() to trigger an idle load balance on an
idle target in TIF_POLLING_NRFLAG mode.

nohz_csd_function() bails out early if "idle_cpu()" check for the
target CPU, and does not lock the target CPU's rq until the very end,
once it has found tasks to run on the CPU and will not inhibit the
wakeup of, or running of a newly woken up higher priority task. Account
for this and prevent a WARN_ON_ONCE() when SCHED_SOFTIRQ is raised from
flush_smp_call_function_queue().

Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---

Newer stable branches (6.12, 6.6) got this already, 5.10 and lower are
not affected.

The warning triggered for SCHED_SOFTIRQ under high network load while
testing.

 kernel/softirq.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index a47396161843..6665f5cd60cb 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -294,17 +294,24 @@ static inline void invoke_softirq(void)
 		wakeup_softirqd();
 }
 
+#define SCHED_SOFTIRQ_MASK	BIT(SCHED_SOFTIRQ)
+
 /*
  * flush_smp_call_function_queue() can raise a soft interrupt in a function
- * call. On RT kernels this is undesired and the only known functionality
- * in the block layer which does this is disabled on RT. If soft interrupts
- * get raised which haven't been raised before the flush, warn so it can be
+ * call. On RT kernels this is undesired and the only known functionalities
+ * are in the block layer which is disabled on RT, and in the scheduler for
+ * idle load balancing. If soft interrupts get raised which haven't been
+ * raised before the flush, warn if it is not a SCHED_SOFTIRQ so it can be
  * investigated.
  */
 void do_softirq_post_smp_call_flush(unsigned int was_pending)
 {
-	if (WARN_ON_ONCE(was_pending != local_softirq_pending()))
+	unsigned int is_pending = local_softirq_pending();
+
+	if (unlikely(was_pending != is_pending)) {
+		WARN_ON_ONCE(was_pending != (is_pending & ~SCHED_SOFTIRQ_MASK));
 		invoke_softirq();
+	}
 }
 
 #else /* CONFIG_PREEMPT_RT */
-- 
2.39.5

Re: [PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Greg KH]

On Wed, Jan 29, 2025 at 04:32:26PM +0100, Florian Bezdeka wrote:
> From: K Prateek Nayak <kprateek.nayak@amd.com>
> 
> commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.
> 
> do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
> WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
> Since do_softirq_post_smp_call_flush() is called with preempt disabled,
> raising a SOFTIRQ during flush_smp_call_function_queue() can lead to
> longer preempt disabled sections.
> 
> Since commit b2a02fc43a1f ("smp: Optimize
> send_call_function_single_ipi()") IPIs to an idle CPU in
> TIF_POLLING_NRFLAG mode can be optimized out by instead setting
> TIF_NEED_RESCHED bit in idle task's thread_info and relying on the
> flush_smp_call_function_queue() in the idle-exit path to run the
> SMP-call-function.
> 
> To trigger an idle load balancing, the scheduler queues
> nohz_csd_function() responsible for triggering an idle load balancing on
> a target nohz idle CPU and sends an IPI. Only now, this IPI is optimized
> out and the SMP-call-function is executed from
> flush_smp_call_function_queue() in do_idle() which can raise a
> SCHED_SOFTIRQ to trigger the balancing.
> 
> So far, this went undetected since, the need_resched() check in
> nohz_csd_function() would make it bail out of idle load balancing early
> as the idle thread does not clear TIF_POLLING_NRFLAG before calling
> flush_smp_call_function_queue(). The need_resched() check was added with
> the intent to catch a new task wakeup, however, it has recently
> discovered to be unnecessary and will be removed in the subsequent
> commit after which nohz_csd_function() can raise a SCHED_SOFTIRQ from
> flush_smp_call_function_queue() to trigger an idle load balance on an
> idle target in TIF_POLLING_NRFLAG mode.
> 
> nohz_csd_function() bails out early if "idle_cpu()" check for the
> target CPU, and does not lock the target CPU's rq until the very end,
> once it has found tasks to run on the CPU and will not inhibit the
> wakeup of, or running of a newly woken up higher priority task. Account
> for this and prevent a WARN_ON_ONCE() when SCHED_SOFTIRQ is raised from
> flush_smp_call_function_queue().
> 
> Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
> Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
> 
> Newer stable branches (6.12, 6.6) got this already, 5.10 and lower are
> not affected.
> 
> The warning triggered for SCHED_SOFTIRQ under high network load while
> testing.

But RT is not in the 6.1.y tree, right?  Or is it?  Why was it only
backported to 6.6.y and 6.12.y?

thanks,

greg k-h

Re: [PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Greg KH]

On Wed, Jan 29, 2025 at 04:47:59PM +0100, Greg KH wrote:
> On Wed, Jan 29, 2025 at 04:32:26PM +0100, Florian Bezdeka wrote:
> > From: K Prateek Nayak <kprateek.nayak@amd.com>
> > 
> > commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.
> > 
> > do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
> > WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
> > Since do_softirq_post_smp_call_flush() is called with preempt disabled,
> > raising a SOFTIRQ during flush_smp_call_function_queue() can lead to
> > longer preempt disabled sections.
> > 
> > Since commit b2a02fc43a1f ("smp: Optimize
> > send_call_function_single_ipi()") IPIs to an idle CPU in
> > TIF_POLLING_NRFLAG mode can be optimized out by instead setting
> > TIF_NEED_RESCHED bit in idle task's thread_info and relying on the
> > flush_smp_call_function_queue() in the idle-exit path to run the
> > SMP-call-function.
> > 
> > To trigger an idle load balancing, the scheduler queues
> > nohz_csd_function() responsible for triggering an idle load balancing on
> > a target nohz idle CPU and sends an IPI. Only now, this IPI is optimized
> > out and the SMP-call-function is executed from
> > flush_smp_call_function_queue() in do_idle() which can raise a
> > SCHED_SOFTIRQ to trigger the balancing.
> > 
> > So far, this went undetected since, the need_resched() check in
> > nohz_csd_function() would make it bail out of idle load balancing early
> > as the idle thread does not clear TIF_POLLING_NRFLAG before calling
> > flush_smp_call_function_queue(). The need_resched() check was added with
> > the intent to catch a new task wakeup, however, it has recently
> > discovered to be unnecessary and will be removed in the subsequent
> > commit after which nohz_csd_function() can raise a SCHED_SOFTIRQ from
> > flush_smp_call_function_queue() to trigger an idle load balance on an
> > idle target in TIF_POLLING_NRFLAG mode.
> > 
> > nohz_csd_function() bails out early if "idle_cpu()" check for the
> > target CPU, and does not lock the target CPU's rq until the very end,
> > once it has found tasks to run on the CPU and will not inhibit the
> > wakeup of, or running of a newly woken up higher priority task. Account
> > for this and prevent a WARN_ON_ONCE() when SCHED_SOFTIRQ is raised from
> > flush_smp_call_function_queue().
> > 
> > Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
> > Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > ---
> > 
> > Newer stable branches (6.12, 6.6) got this already, 5.10 and lower are
> > not affected.
> > 
> > The warning triggered for SCHED_SOFTIRQ under high network load while
> > testing.
> 
> But RT is not in the 6.1.y tree, right?  Or is it?  Why was it only
> backported to 6.6.y and 6.12.y?

And see:
	https://lore.kernel.org/r/d21a8129-e982-463f-af8b-07a14b6a674a@amd.com
for why we added it to 6.12.y in the first place (I don't know why Sasha
added it to 6.6.y...)

thanks,

greg k-h

Re: [PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Florian Bezdeka]

On Wed, 2025-01-29 at 16:58 +0100, Greg KH wrote:
> On Wed, Jan 29, 2025 at 04:47:59PM +0100, Greg KH wrote:
> > On Wed, Jan 29, 2025 at 04:32:26PM +0100, Florian Bezdeka wrote:
> > > From: K Prateek Nayak <kprateek.nayak@amd.com>
> > > 
> > > commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.
> > > 
> > > do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
> > > WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
> > > Since do_softirq_post_smp_call_flush() is called with preempt disabled,
> > > raising a SOFTIRQ during flush_smp_call_function_queue() can lead to
> > > longer preempt disabled sections.
> > > 
> > > Since commit b2a02fc43a1f ("smp: Optimize
> > > send_call_function_single_ipi()") IPIs to an idle CPU in
> > > TIF_POLLING_NRFLAG mode can be optimized out by instead setting
> > > TIF_NEED_RESCHED bit in idle task's thread_info and relying on the
> > > flush_smp_call_function_queue() in the idle-exit path to run the
> > > SMP-call-function.
> > > 
> > > To trigger an idle load balancing, the scheduler queues
> > > nohz_csd_function() responsible for triggering an idle load balancing on
> > > a target nohz idle CPU and sends an IPI. Only now, this IPI is optimized
> > > out and the SMP-call-function is executed from
> > > flush_smp_call_function_queue() in do_idle() which can raise a
> > > SCHED_SOFTIRQ to trigger the balancing.
> > > 
> > > So far, this went undetected since, the need_resched() check in
> > > nohz_csd_function() would make it bail out of idle load balancing early
> > > as the idle thread does not clear TIF_POLLING_NRFLAG before calling
> > > flush_smp_call_function_queue(). The need_resched() check was added with
> > > the intent to catch a new task wakeup, however, it has recently
> > > discovered to be unnecessary and will be removed in the subsequent
> > > commit after which nohz_csd_function() can raise a SCHED_SOFTIRQ from
> > > flush_smp_call_function_queue() to trigger an idle load balance on an
> > > idle target in TIF_POLLING_NRFLAG mode.
> > > 
> > > nohz_csd_function() bails out early if "idle_cpu()" check for the
> > > target CPU, and does not lock the target CPU's rq until the very end,
> > > once it has found tasks to run on the CPU and will not inhibit the
> > > wakeup of, or running of a newly woken up higher priority task. Account
> > > for this and prevent a WARN_ON_ONCE() when SCHED_SOFTIRQ is raised from
> > > flush_smp_call_function_queue().
> > > 
> > > Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
> > > Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > > ---
> > > 
> > > Newer stable branches (6.12, 6.6) got this already, 5.10 and lower are
> > > not affected.
> > > 
> > > The warning triggered for SCHED_SOFTIRQ under high network load while
> > > testing.
> > 
> > But RT is not in the 6.1.y tree, right?  Or is it?  Why was it only
> > backported to 6.6.y and 6.12.y?
> 
> And see:
> 	https://lore.kernel.org/r/d21a8129-e982-463f-af8b-07a14b6a674a@amd.com
> for why we added it to 6.12.y in the first place (I don't know why Sasha
> added it to 6.6.y...)

All versions down to 6.1 have the same problem. The same splat seen by
Prateek on 6.12 can happen in 6.6 (already fixed by Sasha) and 6.1 as
well. I think it was simply overlooked.

We did intensive testing on 6.1 these days. We can confirm that it
fixes the problem on 6.1 as well.

Does that help?

Best regards,
Florian

Re: [PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Greg KH]

On Wed, Jan 29, 2025 at 05:09:56PM +0100, Florian Bezdeka wrote:
> On Wed, 2025-01-29 at 16:58 +0100, Greg KH wrote:
> > On Wed, Jan 29, 2025 at 04:47:59PM +0100, Greg KH wrote:
> > > On Wed, Jan 29, 2025 at 04:32:26PM +0100, Florian Bezdeka wrote:
> > > > From: K Prateek Nayak <kprateek.nayak@amd.com>
> > > > 
> > > > commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.
> > > > 
> > > > do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
> > > > WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
> > > > Since do_softirq_post_smp_call_flush() is called with preempt disabled,
> > > > raising a SOFTIRQ during flush_smp_call_function_queue() can lead to
> > > > longer preempt disabled sections.
> > > > 
> > > > Since commit b2a02fc43a1f ("smp: Optimize
> > > > send_call_function_single_ipi()") IPIs to an idle CPU in
> > > > TIF_POLLING_NRFLAG mode can be optimized out by instead setting
> > > > TIF_NEED_RESCHED bit in idle task's thread_info and relying on the
> > > > flush_smp_call_function_queue() in the idle-exit path to run the
> > > > SMP-call-function.
> > > > 
> > > > To trigger an idle load balancing, the scheduler queues
> > > > nohz_csd_function() responsible for triggering an idle load balancing on
> > > > a target nohz idle CPU and sends an IPI. Only now, this IPI is optimized
> > > > out and the SMP-call-function is executed from
> > > > flush_smp_call_function_queue() in do_idle() which can raise a
> > > > SCHED_SOFTIRQ to trigger the balancing.
> > > > 
> > > > So far, this went undetected since, the need_resched() check in
> > > > nohz_csd_function() would make it bail out of idle load balancing early
> > > > as the idle thread does not clear TIF_POLLING_NRFLAG before calling
> > > > flush_smp_call_function_queue(). The need_resched() check was added with
> > > > the intent to catch a new task wakeup, however, it has recently
> > > > discovered to be unnecessary and will be removed in the subsequent
> > > > commit after which nohz_csd_function() can raise a SCHED_SOFTIRQ from
> > > > flush_smp_call_function_queue() to trigger an idle load balance on an
> > > > idle target in TIF_POLLING_NRFLAG mode.
> > > > 
> > > > nohz_csd_function() bails out early if "idle_cpu()" check for the
> > > > target CPU, and does not lock the target CPU's rq until the very end,
> > > > once it has found tasks to run on the CPU and will not inhibit the
> > > > wakeup of, or running of a newly woken up higher priority task. Account
> > > > for this and prevent a WARN_ON_ONCE() when SCHED_SOFTIRQ is raised from
> > > > flush_smp_call_function_queue().
> > > > 
> > > > Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
> > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > > > Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
> > > > Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > > Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> > > > ---
> > > > 
> > > > Newer stable branches (6.12, 6.6) got this already, 5.10 and lower are
> > > > not affected.
> > > > 
> > > > The warning triggered for SCHED_SOFTIRQ under high network load while
> > > > testing.
> > > 
> > > But RT is not in the 6.1.y tree, right?  Or is it?  Why was it only
> > > backported to 6.6.y and 6.12.y?
> > 
> > And see:
> > 	https://lore.kernel.org/r/d21a8129-e982-463f-af8b-07a14b6a674a@amd.com
> > for why we added it to 6.12.y in the first place (I don't know why Sasha
> > added it to 6.6.y...)
> 
> All versions down to 6.1 have the same problem. The same splat seen by
> Prateek on 6.12 can happen in 6.6 (already fixed by Sasha) and 6.1 as
> well. I think it was simply overlooked.
> 
> We did intensive testing on 6.1 these days. We can confirm that it
> fixes the problem on 6.1 as well.
> 
> Does that helpa

Yes, that helps, thanks!  I'll go queue it up now.

greg k-h

Re: [PATCH 6.1] softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel

[Sasha Levin]

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 6675ce20046d149e1e1ffe7e9577947dee17aad5

WARNING: Author mismatch between patch and upstream commit:
Backport author: Florian Bezdeka<florian.bezdeka@siemens.com>
Commit author: K Prateek Nayak<kprateek.nayak@amd.com>


Status in newer kernel trees:
6.13.y | Branch not found
6.12.y | Present (different SHA1: 6aeef0214de7)
6.6.y | Present (different SHA1: 3dd65ffa2df6)
6.1.y | Not found

Note: The patch differs from the upstream commit:
---
1:  6675ce20046d1 ! 1:  c2bdd4c44f80e softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel
    @@ Metadata
      ## Commit message ##
         softirq: Allow raising SCHED_SOFTIRQ from SMP-call-function on RT kernel
     
    +    commit 6675ce20046d149e1e1ffe7e9577947dee17aad5 upstream.
    +
         do_softirq_post_smp_call_flush() on PREEMPT_RT kernels carries a
         WARN_ON_ONCE() for any SOFTIRQ being raised from an SMP-call-function.
         Since do_softirq_post_smp_call_flush() is called with preempt disabled,
    @@ Commit message
         Signed-off-by: K Prateek Nayak <kprateek.nayak@amd.com>
         Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
         Link: https://lore.kernel.org/r/20241119054432.6405-2-kprateek.nayak@amd.com
    +    Tested-by: Felix Moessbauer <felix.moessbauer@siemens.com>
    +    Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
     
      ## kernel/softirq.c ##
     @@ kernel/softirq.c: static inline void invoke_softirq(void)
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-6.1.y        |  Success    |  Success   |