netdev - Re: [PATCH] slip: fix NOHZ local_softirq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Aug 2011 11:47:56 +0400
From:	Matvejchikov Ilya <matvejchikov@...il.com>
To:	Oliver Hartkopp <socketcan@...tkopp.net>
Cc:	netdev@...r.kernel.org, Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH] slip: fix NOHZ local_softirq_pending 08 warning

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@...tkopp.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@...il.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++;
>>  }
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html