[PATCH] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[PATCH] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[Sebastian Andrzej Siewior]

The softirq implementation on PREEMPT_RT does not provide do_softirq().
The other user of do_softirq() is replaced with a local_bh_disable()
+ enable() around the possible raise-softirq invocation. This can not be
done here because migration_cpu_stop() is invoked with disabled
preemption.

Wake the softirq thread on PREEMPT_RT if there are any pending softirqs.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 kernel/smp.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
---
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -690,10 +690,21 @@ void flush_smp_call_function_from_idle(v
 
 	cfd_seq_store(this_cpu_ptr(&cfd_seq_local)->idle, CFD_SEQ_NOCPU,
 		      smp_processor_id(), CFD_SEQ_IDLE);
+
 	local_irq_save(flags);
 	flush_smp_call_function_queue(true);
-	if (local_softirq_pending())
-		do_softirq();
+
+	if (local_softirq_pending()) {
+
+		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
+			do_softirq();
+		} else {
+			struct task_struct *ksoftirqd = this_cpu_ksoftirqd();
+
+			if (ksoftirqd && !task_is_running(ksoftirqd))
+				wake_up_process(ksoftirqd);
+		}
+	}
 
 	local_irq_restore(flags);
 }

Re: [PATCH] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[Christoph Hellwig]

On Fri, Sep 24, 2021 at 11:47:55AM +0200, Sebastian Andrzej Siewior wrote:
> +	if (local_softirq_pending()) {
> +
> +		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
> +			do_softirq();
> +		} else {
> +			struct task_struct *ksoftirqd = this_cpu_ksoftirqd();
> +
> +			if (ksoftirqd && !task_is_running(ksoftirqd))
> +				wake_up_process(ksoftirqd);
> +		}
> +	}

At a cosmetic level this looks pretty weird.  Why the empty line inside
the indented block?  Why the pointless negation instead of the obvious
more straightforward order?

Re: [PATCH] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[Thomas Gleixner]

On Sat, Sep 25 2021 at 11:31, Christoph Hellwig wrote:

> On Fri, Sep 24, 2021 at 11:47:55AM +0200, Sebastian Andrzej Siewior wrote:
>> +	if (local_softirq_pending()) {
>> +
>> +		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
>> +			do_softirq();
>> +		} else {
>> +			struct task_struct *ksoftirqd = this_cpu_ksoftirqd();
>> +
>> +			if (ksoftirqd && !task_is_running(ksoftirqd))
>> +				wake_up_process(ksoftirqd);
>> +		}
>> +	}
>
> At a cosmetic level this looks pretty weird.  Why the empty line inside
> the indented block?  Why the pointless negation instead of the obvious
> more straightforward order?

Yeah, the empty line is stray.

The negation is because quite some people complained in the past about
doing it the other way round as they want to see the !RT case first.

De gustibus non est disputandum :)

Thanks,

        tglx

[PATCH v2] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[Sebastian Andrzej Siewior]

The softirq implementation on PREEMPT_RT does not provide do_softirq().
The other user of do_softirq() is replaced with a local_bh_disable()
+ enable() around the possible raise-softirq invocation. This can not be
done here because migration_cpu_stop() is invoked with disabled
preemption.

Wake the softirq thread on PREEMPT_RT if there are any pending softirqs.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2: Drop an empty line.

 kernel/smp.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
---
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -690,10 +690,20 @@ void flush_smp_call_function_from_idle(v
 
 	cfd_seq_store(this_cpu_ptr(&cfd_seq_local)->idle, CFD_SEQ_NOCPU,
 		      smp_processor_id(), CFD_SEQ_IDLE);
+
 	local_irq_save(flags);
 	flush_smp_call_function_queue(true);
-	if (local_softirq_pending())
-		do_softirq();
+
+	if (local_softirq_pending()) {
+		if (!IS_ENABLED(CONFIG_PREEMPT_RT)) {
+			do_softirq();
+		} else {
+			struct task_struct *ksoftirqd = this_cpu_ksoftirqd();
+
+			if (ksoftirqd && !task_is_running(ksoftirqd))
+				wake_up_process(ksoftirqd);
+		}
+	}
 
 	local_irq_restore(flags);
 }

Re: [PATCH v2] smp: Wake ksoftirqd on PREEMPT_RT instead do_softirq().

[Sebastian Andrzej Siewior]

On 2021-09-27 09:38:16 [+0200], To Thomas Gleixner wrote:
> The softirq implementation on PREEMPT_RT does not provide do_softirq().
> The other user of do_softirq() is replaced with a local_bh_disable()
> + enable() around the possible raise-softirq invocation. This can not be
> done here because migration_cpu_stop() is invoked with disabled
> preemption.
> 
> Wake the softirq thread on PREEMPT_RT if there are any pending softirqs.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> v1…v2: Drop an empty line.

ping.

Sebastian