[PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[Matvejchikov Ilya]

When using nanosleep() in an userspace application we get a ratelimit warning:

	NOHZ: local_softirq_pending 08

According to 481a8199142c050b72bff8a1956a49fd0a75bbe0 the problem is caused by
netif_rx() function. This patch replaces netif_rx() with netif_rx_ni() which
has to be used from process/softirq context.

Signed-off-by: Matvejchikov Ilya <matvejchikov@gmail.com>
---
 drivers/net/slip.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/slip.c b/drivers/net/slip.c
index f11b3f3..4c61753 100644
--- a/drivers/net/slip.c
+++ b/drivers/net/slip.c
@@ -367,7 +367,7 @@ static void sl_bump(struct slip *sl)
 	memcpy(skb_put(skb, count), sl->rbuff, count);
 	skb_reset_mac_header(skb);
 	skb->protocol = htons(ETH_P_IP);
-	netif_rx(skb);
+	netif_rx_ni(skb);
 	dev->stats.rx_packets++;
 }

-- 
1.7.4.1

Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[David Miller]

From: Matvejchikov Ilya <matvejchikov@gmail.com>
Date: Fri, 5 Aug 2011 23:23:51 +0400

> When using nanosleep() in an userspace application we get a ratelimit warning:
> 
> 	NOHZ: local_softirq_pending 08
> 
> According to 481a8199142c050b72bff8a1956a49fd0a75bbe0 the problem is caused by
> netif_rx() function. This patch replaces netif_rx() with netif_rx_ni() which
> has to be used from process/softirq context.
> 
> Signed-off-by: Matvejchikov Ilya <matvejchikov@gmail.com>

Applied, thanks.

Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[Oliver Hartkopp]

Hello Ilya,

is the processing of characters in the tty input stream for line disciplines
*always* done in softirq context???

If so, i would send a patch for drivers/net/can/slcan.c too ...

Thanks,
Oliver

On 05.08.2011 21:23, Matvejchikov Ilya wrote:
> When using nanosleep() in an userspace application we get a ratelimit warning:
> 
> 	NOHZ: local_softirq_pending 08
> 
> According to 481a8199142c050b72bff8a1956a49fd0a75bbe0 the problem is caused by
> netif_rx() function. This patch replaces netif_rx() with netif_rx_ni() which
> has to be used from process/softirq context.
> 
> Signed-off-by: Matvejchikov Ilya <matvejchikov@gmail.com>
> ---
>  drivers/net/slip.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/slip.c b/drivers/net/slip.c
> index f11b3f3..4c61753 100644
> --- a/drivers/net/slip.c
> +++ b/drivers/net/slip.c
> @@ -367,7 +367,7 @@ static void sl_bump(struct slip *sl)
>  	memcpy(skb_put(skb, count), sl->rbuff, count);
>  	skb_reset_mac_header(skb);
>  	skb->protocol = htons(ETH_P_IP);
> -	netif_rx(skb);
> +	netif_rx_ni(skb);
>  	dev->stats.rx_packets++;
>  }
> 

Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[Matvejchikov Ilya]

Oliver,

As I see it, there is the following call chain of the netif_rx_ni() in
the slip (or slcan) driver:
[flush_to_ldisc] -> [disc->ops->receive_buf] -> [slip_unesc] ->
[sl_bump] -> [netif_rx_ni]

So flush_to_ldisc() may be called:
  1) From tty_flush_to_ldisc() with flush_work and as it mentioned
must not be called from IRQ context.
  2) From input_available_p() which can be called from n_tty_read() or
n_tty_poll(), so it's the user's context.
  2) From tty_flip_buffer_push() which schedules the flush_to_ldisc to
work or calls it directly if the tty->low_latency set.

So the only thing we must take into account that with tty->low_latency
set we can be in IRQ context when doing with netif_rx_ni(). But if we
are using netif_rx_ni() we can handle both IRQ and softirq contexts
well as if there are any softirq pending netif_rx_ni() calls
do_softirq()  with preemtion disabled. Then do_softirq() checks if
there is an IRQ context and does nothing if so.

So, I think it's correct to change netif_rx() with the netif_rx_ni().

2011/8/9 Oliver Hartkopp <socketcan@hartkopp.net>:
> Hello Ilya,
>
> is the processing of characters in the tty input stream for line disciplines
> *always* done in softirq context???
>
> If so, i would send a patch for drivers/net/can/slcan.c too ...
>
> Thanks,
> Oliver
>
> On 05.08.2011 21:23, Matvejchikov Ilya wrote:
>> When using nanosleep() in an userspace application we get a ratelimit warning:
>>
>>       NOHZ: local_softirq_pending 08
>>
>> According to 481a8199142c050b72bff8a1956a49fd0a75bbe0 the problem is caused by
>> netif_rx() function. This patch replaces netif_rx() with netif_rx_ni() which
>> has to be used from process/softirq context.
>>
>> Signed-off-by: Matvejchikov Ilya <matvejchikov@gmail.com>
>> ---
>>  drivers/net/slip.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/slip.c b/drivers/net/slip.c
>> index f11b3f3..4c61753 100644
>> --- a/drivers/net/slip.c
>> +++ b/drivers/net/slip.c
>> @@ -367,7 +367,7 @@ static void sl_bump(struct slip *sl)
>>       memcpy(skb_put(skb, count), sl->rbuff, count);
>>       skb_reset_mac_header(skb);
>>       skb->protocol = htons(ETH_P_IP);
>> -     netif_rx(skb);
>> +     netif_rx_ni(skb);
>>       dev->stats.rx_packets++;
>>  }
>>
>
>

Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[Alan Cox]

>   2) From tty_flip_buffer_push() which schedules the flush_to_ldisc to
> work or calls it directly if the tty->low_latency set.
> 
> So the only thing we must take into account that with tty->low_latency
> set we can be in IRQ context when doing with netif_rx_ni(). But if we

A driver is not permitted to directly call into flush_to_ldisc from an
interrupt so that case doesn't happen.

Alan

Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

[Oliver Hartkopp]

On 10.08.2011 11:28, Alan Cox wrote:
>>   2) From tty_flip_buffer_push() which schedules the flush_to_ldisc to
>> work or calls it directly if the tty->low_latency set.
>>
>> So the only thing we must take into account that with tty->low_latency
>> set we can be in IRQ context when doing with netif_rx_ni(). But if we
> 
> A driver is not permitted to directly call into flush_to_ldisc from an
> interrupt so that case doesn't happen.
> 

Thanks to both of you for the explanation!

I'll send a patch for slcan.c then.

Best regards,
Oliver