netdev - Re: [PATCH] net: 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]

Message-ID: <4AC45556.9080303@hartkopp.net>
Date:	Thu, 01 Oct 2009 09:08:06 +0200
From:	Oliver Hartkopp <oliver@...tkopp.net>
To:	David Miller <davem@...emloft.net>
CC:	johannes@...solutions.net, mb@...sch.de, kalle.valo@....fi,
	linville@...driver.com, linux-wireless@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH] net: fix NOHZ: local_softirq_pending 08

David Miller wrote:
> From: Oliver Hartkopp <oliver@...tkopp.net>
> Date: Wed, 30 Sep 2009 20:18:25 +0200
> 
>> Socket buffers that are generated and received inside softirqs or from process
>> context must not use netif_rx() that's intended to be used from irq context only.
>>
>> This patch introduces a new helper function netif_rx_ti(skb) that tests for
>> in_interrupt() before invoking netif_rx() or netif_rx_ni().
>>
>> It fixes the ratelimited kernel warning
>>
>>         NOHZ: local_softirq_pending 08
>>
>> in the mac80211 and can subsystems.
>>
>> Signed-off-by: Oliver Hartkopp <oliver@...tkopp.net>
> 
> I bet all of these code paths can use netif_receive_skb() or
> don't need this conditional blob at all.
> 
> Looking at some specific cases in this patch:
> 
> 1) VCAN:  This RX routine is only invoked from the drivers
>    ->ndo_start_xmit() handler, and therefore like the loopback
>    driver we know that BH's are already disabled and therefore
>    it can always use netif_rx() safely.
> 
>    Why did you convert this case?
> 
>    And if this needs to be converted, why doesn't loopback need
>    to be?
> 
> 2) af_can.c:  In what situation will netif_rx_ni() not be appropriate
>    here?  It should always execute in softirq or user context, now
>    hardirq context.
> 
> And the list goes on and on, I don't really like this new conditional
> testing of interrupt state.

Hello Dave,

i'm confused about in_interrupt(), in_softirq() and in_irq() as pointed out by
Johannes here:

http://marc.info/?l=linux-wireless&m=125432410405562&w=2

Indeed in the two cases for the CAN stuff (in vcan.c and af_can.c) the skb's
are received in process-context and softirq-context only.

Therefore i used netif_rx_ni() in my last change of this code.

Now i was reading from Johannes that in_interrupt() is used for
hardirq-context /and/ softirq-context, so i was just *unsure* and used the
newly introduced netif_rx_ti() for that, which tests for in_interrupt().

Indeed i'm not really happy with that, as it is always better to check each
receive case in which context it can be used and used exactly the right
function for that.

So when netif_rx_ni() or netif_receive_skb() is the best i can use when in
process-context or in softirq-context, i'll do it with pleasure.

And if it is like this the problematic netif_rx() calls in mac80211 need to be
sorted out in detail also ...

Regards,
Oliver

--
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