[PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[Wang Qing]

At present, whether the softirq is executed when the interrupt exits 
is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should 
be split and allowed to take effect separately.

At the same time, we should increase the priority of ksoftirqd when
forbidden to execute in interrupt exits. I refer to the implementation 
of PREEMPT_RT and think it is reasonable.

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 kernel/Kconfig.preempt | 10 ++++++++++
 kernel/irq/Kconfig     |  3 ++-
 kernel/softirq.c       | 21 ++++++++++++++++++++-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/kernel/Kconfig.preempt b/kernel/Kconfig.preempt
index 5876e30..42d60e7
--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -60,6 +60,7 @@ config PREEMPT_RT
 	bool "Fully Preemptible Kernel (Real-Time)"
 	depends on EXPERT && ARCH_SUPPORTS_RT
 	select PREEMPTION
+	select SOFTIRQ_FORCED_THREADING
 	help
 	  This option turns the kernel into a real-time kernel by replacing
 	  various locking primitives (spinlocks, rwlocks, etc.) with
@@ -118,4 +119,13 @@ config SCHED_CORE
 	  which is the likely usage by Linux distributions, there should
 	  be no measurable impact on performance.
 
+config SOFTIRQ_FORCED_THREADING
+	bool "Balance softirq execute"
+	help
+	 This option will force the softirq to be executed in ksoftirqd,
+	 cancel its execution timing when the interrupt exits, and change
+	 ksoftirqd to a real-time process.

+	 In this way, the execution of softirq can be executed more balanced,
+	 and the maximum scheduling delay caused by the execution of softirq
+	 in the RT process can be reduced.
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
index fbc54c2..ecd3236
--- a/kernel/irq/Kconfig
+++ b/kernel/irq/Kconfig
@@ -111,7 +111,8 @@ config GENERIC_IRQ_RESERVATION_MODE
 
 # Support forced irq threading
 config IRQ_FORCED_THREADING
-       bool
+    bool
+	select SOFTIRQ_FORCED_THREADING
 
 config SPARSE_IRQ
 	bool "Support sparse irq numbering" if MAY_HAVE_SPARSE_IRQ
diff --git a/kernel/softirq.c b/kernel/softirq.c
index f3a0121..f02f0d9
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -29,6 +29,7 @@
 #include <linux/wait_bit.h>
 
 #include <asm/softirq_stack.h>
+#include <uapi/linux/sched/types.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/irq.h>
@@ -417,12 +418,18 @@ static inline bool should_wake_ksoftirqd(void)
 	return true;
 }
 
+#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
+static inline void invoke_softirq(void)
+{
+	wakeup_softirqd();
+}
+#else
 static inline void invoke_softirq(void)
 {
 	if (ksoftirqd_running(local_softirq_pending()))
 		return;
 
-	if (!force_irqthreads || !__this_cpu_read(ksoftirqd)) {
+	if (!__this_cpu_read(ksoftirqd)) {
 #ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
 		/*
 		 * We can safely execute softirq on the current stack if
@@ -442,6 +449,7 @@ static inline void invoke_softirq(void)
 		wakeup_softirqd();
 	}
 }
+#endif
 
 asmlinkage __visible void do_softirq(void)
 {
@@ -909,6 +917,14 @@ static int ksoftirqd_should_run(unsigned int cpu)
 	return local_softirq_pending();
 }
 
+#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
+static void ksoftirqd_set_sched_params(unsigned int cpu)
+{
+	struct sched_param param = { .sched_priority = 1 };
+	sched_setscheduler(current, SCHED_FIFO, &param);
+}
+#endif
+
 static void run_ksoftirqd(unsigned int cpu)
 {
 	ksoftirqd_run_begin();
@@ -957,6 +973,9 @@ static int takeover_tasklets(unsigned int cpu)
 
 static struct smp_hotplug_thread softirq_threads = {
 	.store			= &ksoftirqd,
+#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
+	.setup			= ksoftirqd_set_sched_params,
+#endif
 	.thread_should_run	= ksoftirqd_should_run,
 	.thread_fn		= run_ksoftirqd,
 	.thread_comm		= "ksoftirqd/%u",
-- 
2.7.4

Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[Mike Galbraith]

On Mon, 2021-08-23 at 11:33 +0800, Wang Qing wrote:
> At present, whether the softirq is executed when the interrupt exits
> is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should
> be split and allowed to take effect separately.

Decades long practice suddenly became "unreasonable"?  I think not.  

Trying to carve out bits and pieces of RT to merge immediately isn't
likely to make the ongoing merge effort go anyfaster or smoother.

	Just my $.02,

	-Mike

Re:Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[王擎]

>On Mon, 2021-08-23 at 11:33 +0800, Wang Qing wrote:
>> At present, whether the softirq is executed when the interrupt exits
>> is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should
>> be split and allowed to take effect separately.
>
>Decades long practice suddenly became "unreasonable"?  I think not.  

"unreasonable" may be my misnomer, but it is really necessary to separate
softirq from IRQ_FORCED_THREADING, which can be effective separately.

>
>Trying to carve out bits and pieces of RT to merge immediately isn't
>likely to make the ongoing merge effort go anyfaster or smoother.

I am not trying to carve out bits and pieces of RT, but I encountered actual
problems in my project. For example, in Android, we will not enable 
IRQ_FORCED_THREADING, Android is not a high real-time requirements, 
but in some scenariossome, RT processes cannot be scheduled in time
and the frame is dropped due to the execution time of softirq is too long,
also some softirq cannot be executed in time in ksoftirqs, and delays occur, 
such as IO.

Therefore, why not give the user a choice to balance the execution of softirq
while not enable IRQ_FORCED_THREADING, so as to meet the inconsistent 
scenes and needs

Thanks.
Qing
>
>	Just my $.02,
>
>	-Mike
>
>

Re: Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[Mike Galbraith]

On Mon, 2021-08-23 at 14:33 +0800, 王擎 wrote:
>
> > On Mon, 2021-08-23 at 11:33 +0800, Wang Qing wrote:
> > > At present, whether the softirq is executed when the interrupt exits
> > > is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should
> > > be split and allowed to take effect separately.
> >
> > Decades long practice suddenly became "unreasonable"?  I think not.  
>
> "unreasonable" may be my misnomer, but it is really necessary to separate
> softirq from IRQ_FORCED_THREADING, which can be effective separately.

Well, no, it's not necessary, but would be damn convenient to you,
which is of course a perfectly fine motivation to post a patch :)

> >
> > Trying to carve out bits and pieces of RT to merge immediately isn't
> > likely to make the ongoing merge effort go anyfaster or smoother.
>
> I am not trying to carve out bits and pieces of RT, but I encountered actual
> problems in my project. For example, in Android, we will not enable
> IRQ_FORCED_THREADING, Android is not a high real-time requirements,
> but in some scenariossome, RT processes cannot be scheduled in time
> and the frame is dropped due to the execution time of softirq is too long,
> also some softirq cannot be executed in time in ksoftirqs, and delays occur,
> such as IO.

That didn't parse well here.  What you seem to be saying is that you
have a hard constraint that can't be meet, yet are unwilling to use an
existing facility to resolve that issue because the kernel you're using
is not _intended_ to support hard realtime... therefore you want this
other facility to enable it to support your hard realtime requirement.

That can't be right, but it doesn't really matter, because...

> Therefore, why not give the user a choice to balance the execution of softirq
> while not enable IRQ_FORCED_THREADING, so as to meet the inconsistent
> scenes and needs

...it's not my call, I just found the language rather odd.  It still
looks to me like you're carving out a slice of RT.

	-Mike

Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[Thomas Gleixner]

Wang,

On Mon, Aug 23 2021 at 11:33, Wang Qing wrote:

> At present, whether the softirq is executed when the interrupt exits 
> is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should 
> be split and allowed to take effect separately.

There is nothing unreasonable about it. When interrupt force threading
is in effect then it obviously requires that soft interrupt processing
goes into threaded mode as well. But the threaded execution still takes
place when the force threaded interrupt handler completes. Only softirqs
which are raised from hard interrupt context (e.g. timer interrupt) are
forced off to ksoftirqd.

What you are proposing here is completly different as you enforce
softirq execution in context of ksoftirqd only.

> At the same time, we should increase the priority of ksoftirqd when
> forbidden to execute in interrupt exits. I refer to the implementation 
> of PREEMPT_RT and think it is reasonable.

What are you referring to? PREEMPT_RT does not modify the priority of
ksoftirqd. If system designers want to do that, then they can do so from
user space. 

And doing so can be problematic depending on the workload as this
effectively breaks the softirq overload mitigation mechanism which
depends on deferring to ksoftirqd so that e.g. the consumers of received
network packets can be scheduled and the system can make progress.

Just because it does not break on your system with your particular
workload and configuration does not make it suitable for general
consumption.

> +#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
> +static inline void invoke_softirq(void)
> +{
> +	wakeup_softirqd();

Depending on the configuration and timing this breaks any early boot
mechanism which depends on softirqs being handled before ksoftirqd is
available. This was clearly never tested with full RCU debugging
enabled. 

Aside of that the changelog lacks any form of technical analysis and
justification for this. Just claiming that things are [un]reasonable and
making uninformed statements about PREEMPT_RT does not qualify. Quite
the contrary it's definitely unreasonable.

Thanks,

        tglx

Re:Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[王擎]

>Wang,
>
>On Mon, Aug 23 2021 at 11:33, Wang Qing wrote:
>
>> At present, whether the softirq is executed when the interrupt exits 
>> is controlled by IRQ_FORCED_THREADING. This is unreasonable. It should 
>> be split and allowed to take effect separately.
>
>There is nothing unreasonable about it. When interrupt force threading
>is in effect then it obviously requires that soft interrupt processing
>goes into threaded mode as well. But the threaded execution still takes
>place when the force threaded interrupt handler completes. Only softirqs
>which are raised from hard interrupt context (e.g. timer interrupt) are
>forced off to ksoftirqd.
>
>What you are proposing here is completly different as you enforce
>softirq execution in context of ksoftirqd only.

Thank you for reply and explanation, I just provide a choice to balance
the execution of softirq according to their own business scenarios.

>
>> At the same time, we should increase the priority of ksoftirqd when
>> forbidden to execute in interrupt exits. I refer to the implementation 
>> of PREEMPT_RT and think it is reasonable.
>
>What are you referring to? PREEMPT_RT does not modify the priority of
>ksoftirqd. If system designers want to do that, then they can do so from
>user space. 

I refer to the kernel-3.14 RT Patches. I used it at that time and achieved 
very good results.
I remember where I saw that softirqd was split into the original process 
and the RT process. This can partially solve my problem.

>
>And doing so can be problematic depending on the workload as this
>effectively breaks the softirq overload mitigation mechanism which
>depends on deferring to ksoftirqd so that e.g. the consumers of received
>network packets can be scheduled and the system can make progress.
>
>Just because it does not break on your system with your particular
>workload and configuration does not make it suitable for general
>consumption.
>
>> +#ifdef CONFIG_SOFTIRQ_FORCED_THREADING
>> +static inline void invoke_softirq(void)
>> +{
>> +	wakeup_softirqd();
>
>Depending on the configuration and timing this breaks any early boot
>mechanism which depends on softirqs being handled before ksoftirqd is
>available. This was clearly never tested with full RCU debugging
>enabled. 
>
>Aside of that the changelog lacks any form of technical analysis and
>justification for this. Just claiming that things are [un]reasonable and
>making uninformed statements about PREEMPT_RT does not qualify. Quite
>the contrary it's definitely unreasonable.
>
>Thanks,
>
>        tglx
>
>

Thank you for your patient guidance, if necessary, I will add it in the next version.

Thanks,
Qing

Re:Re: [PATCH,RESEND] softirq: Introduce SOFTIRQ_FORCED_THREADING

[Thomas Gleixner]

Qing,

On Sat, Aug 28 2021 at 10:18, 王擎 wrote:
>> On Mon, Aug 23 2021 at 11:33, Wang Qing wrote:
>> What you are proposing here is completly different as you enforce
>> softirq execution in context of ksoftirqd only.
>
> Thank you for reply and explanation, I just provide a choice to balance
> the execution of softirq according to their own business scenarios.

That's not a choice. Forced interrupt threading is a boot-time option
and not a compile time boolean. So with your change you even changed the
behaviour of the kernel when your magic config switch is not selected by
the user.

>> What are you referring to? PREEMPT_RT does not modify the priority of
>> ksoftirqd. If system designers want to do that, then they can do so from
>> user space. 
>
> I refer to the kernel-3.14 RT Patches. I used it at that time and achieved 
> very good results.

There is a reason why RT does not use this anymore and switched to a
different model. As I said before. Just because it works for you, it's
not necessarily a solution which should be exposed for general
consumption.

> I remember where I saw that softirqd was split into the original process 
> and the RT process. This can partially solve my problem.

Your patch has absolutely nothing to do with that. You just picked some
random part out of those 7+ years old patches and then claim that it's
something RT does, which is just not true.

Thanks,

        tglx