* [PATCH] softirq: Remove raise_softirq from tasklet_action_common()
@ 2022-02-05 13:13 Mukesh Ojha
2022-02-08 13:40 ` Mukesh Ojha
2022-02-08 23:04 ` Frederic Weisbecker
0 siblings, 2 replies; 5+ messages in thread
From: Mukesh Ojha @ 2022-02-05 13:13 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, tglx, paulmck, will, dave, frederic, Mukesh Ojha
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@quicinc.com>
---
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();
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Remove raise_softirq from tasklet_action_common()
2022-02-05 13:13 [PATCH] softirq: Remove raise_softirq from tasklet_action_common() Mukesh Ojha
@ 2022-02-08 13:40 ` Mukesh Ojha
2022-02-08 23:04 ` Frederic Weisbecker
1 sibling, 0 replies; 5+ messages in thread
From: Mukesh Ojha @ 2022-02-08 13:40 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, tglx, paulmck, will, dave, frederic
Hi All,
Sorry for the reminder so quickly as we are hitting this issue more
frequently.
Please suggest if whether below patch will have any side-effect.
-Mukesh
On 2/5/2022 6:43 PM, 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@quicinc.com>
> ---
> 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();
> }
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Remove raise_softirq from tasklet_action_common()
2022-02-05 13:13 [PATCH] softirq: Remove raise_softirq from tasklet_action_common() Mukesh Ojha
2022-02-08 13:40 ` Mukesh Ojha
@ 2022-02-08 23:04 ` Frederic Weisbecker
2022-02-21 11:18 ` Mukesh Ojha
1 sibling, 1 reply; 5+ messages in thread
From: Frederic Weisbecker @ 2022-02-08 23:04 UTC (permalink / raw)
To: Mukesh Ojha; +Cc: linux-kernel, peterz, tglx, paulmck, will, dave
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@quicinc.com>
> ---
> 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() ?
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Remove raise_softirq from tasklet_action_common()
2022-02-08 23:04 ` Frederic Weisbecker
@ 2022-02-21 11:18 ` Mukesh Ojha
2022-02-23 15:50 ` Mukesh Ojha
0 siblings, 1 reply; 5+ messages in thread
From: Mukesh Ojha @ 2022-02-21 11:18 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: linux-kernel, peterz, tglx, paulmck, will, dave
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@quicinc.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] softirq: Remove raise_softirq from tasklet_action_common()
2022-02-21 11:18 ` Mukesh Ojha
@ 2022-02-23 15:50 ` Mukesh Ojha
0 siblings, 0 replies; 5+ messages in thread
From: Mukesh Ojha @ 2022-02-23 15:50 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: linux-kernel, peterz, tglx, paulmck, will, dave
On 2/21/2022 4:48 PM, Mukesh Ojha wrote:
>
> 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@quicinc.com>
>>> ---
>>> 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 ?
Never mind, we should call tasklet_kill() followed by tasklet_disable().
I suspect, the issue is in client code, some race is setting schedule
bit and marking the softirq pending even after doing tasklet_kill =>
tasklet_disable in cleanup path.
-Mukesh
>
> -Mukesh
>
>
>>
>> Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-23 15:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-05 13:13 [PATCH] softirq: Remove raise_softirq from tasklet_action_common() Mukesh Ojha
2022-02-08 13:40 ` Mukesh Ojha
2022-02-08 23:04 ` Frederic Weisbecker
2022-02-21 11:18 ` Mukesh Ojha
2022-02-23 15:50 ` Mukesh Ojha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox