All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.