Re: [PATCH] softirq: Remove raise_softirq from tasklet_action_common()

From: Mukesh Ojha
Date: Mon Feb 21 2022 - 06:24:56 EST



On 2/9/2022 4:34 AM, Frederic Weisbecker wrote:
On Sat, Feb 05, 2022 at 06:43:25PM +0530, Mukesh Ojha wrote:
Think about a scenario when all other cores are in suspend
and one core is only running ksoftirqd and it is because
some client has invoked tasklet_hi_schedule() only once
during that phase.

tasklet_action_common() handles that softirq and marks the
same softirq as pending again. And due to that core keeps
running the softirq handler [1] forever and it is not able to
go to suspend.

We can get rid of raising softirq from tasklet handler.

[1]
<idle>-0 [003] 13058.769081: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769085: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13058.769087: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769091: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769094: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13058.769097: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769100: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769103: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13058.769106: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13058.769109: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058923: softirq_entry vec=0 action=HI_SOFTIRQ
...
..
..
..

<idle>-0 [003] 13059.058951: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058954: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.058957: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058960: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058963: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.058966: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058969: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058972: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.058975: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058978: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058981: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.058984: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058987: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058990: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.058993: softirq_exit vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.058996: softirq_entry vec=0 action=HI_SOFTIRQ
<idle>-0 [003] 13059.059000: softirq_raise: vec=0 [action=HI_SOFTIRQ]
<idle>-0 [003] 13059.059002: softirq_exit vec=0 action=HI_SOFTIRQ

Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
---
kernel/softirq.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 41f4709..d3e6fb9 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -795,7 +795,6 @@ static void tasklet_action_common(struct softirq_action *a,
t->next = NULL;
*tl_head->tail = t;
tl_head->tail = &t->next;
- __raise_softirq_irqoff(softirq_nr);
local_irq_enable();
That requeue happens when the tasklet is already executing on some other CPU
or when it has been disabled through tasklet_disable().

So you can't just remove that line or you'll break everything.

It would be nice to identify which tasklet keeps being requeued. Is it because
something called tasklet_disable() to it and never called back tasklet_enable() ?

Hi @Frederic,

Thanks for the reply.
Suppose a scenario where a tasklet is scheduled/queued from one client and before running of tasklet handler, same tasklet gets
disabled from some other cpu.
In this scenario, while the handlers runs it will be keep on marking the softirq pending even though tasklet is disabled.
Tasklet will be enabled but after coming out of low power mode.
Will it look to be valid case ?

-Mukesh



Thanks.