diff mbox

[Bugme-new,Bug,18492] New: kernel softirq warning on boot

Message ID 20100915064655.GA21021@gondor.apana.org.au
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Herbert Xu Sept. 15, 2010, 6:46 a.m. UTC
On Tue, Sep 14, 2010 at 02:11:03PM -0700, David Miller wrote:
>
> I'll take a closer look at this if Herbert doesn't beat me to it.

I thought we were going to use this patch until Paul's BH warning
removal made it into mainline?

netpoll: Disable IRQ around RCU dereference in netpoll_rx

We cannot use rcu_dereference_bh safely in netpoll_rx as we may
be called with IRQs disabled.  We could however simply disable
IRQs as that too causes BH to be disabled and is safe in either
case.

Thanks to John Linville for discovering this bug and providing
a patch.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,

Comments

David Miller Sept. 16, 2010, 4:40 a.m. UTC | #1
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Wed, 15 Sep 2010 14:46:55 +0800

> On Tue, Sep 14, 2010 at 02:11:03PM -0700, David Miller wrote:
>>
>> I'll take a closer look at this if Herbert doesn't beat me to it.
> 
> I thought we were going to use this patch until Paul's BH warning
> removal made it into mainline?
> 
> netpoll: Disable IRQ around RCU dereference in netpoll_rx

I hadn't anticipated it taking this long for Paul's patch to
get upstream.

So I only queued this up for -stable, not Linus's tree.

Paul has started the work of moving his patch upstream, so I suspect
that if I queued this patch of our's up to my tree now it would land
in Linus's tree about the same time :-)

If something non-trivially stalls Paul's submission work, we can
queue this up.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Suchanek Sept. 17, 2010, 6 p.m. UTC | #2
On 15 September 2010 08:46, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Tue, Sep 14, 2010 at 02:11:03PM -0700, David Miller wrote:
>>
>> I'll take a closer look at this if Herbert doesn't beat me to it.
>
> I thought we were going to use this patch until Paul's BH warning
> removal made it into mainline?
>
> netpoll: Disable IRQ around RCU dereference in netpoll_rx
>
> We cannot use rcu_dereference_bh safely in netpoll_rx as we may
> be called with IRQs disabled.  We could however simply disable
> IRQs as that too causes BH to be disabled and is safe in either
> case.
>
> Thanks to John Linville for discovering this bug and providing
> a patch.
>
With the patch applied I no longet get the warning.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Sept. 17, 2010, 11:55 p.m. UTC | #3
From: Michal Suchanek <hramrach@centrum.cz>
Date: Fri, 17 Sep 2010 20:00:42 +0200

> On 15 September 2010 08:46, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Tue, Sep 14, 2010 at 02:11:03PM -0700, David Miller wrote:
>>>
>>> I'll take a closer look at this if Herbert doesn't beat me to it.
>>
>> I thought we were going to use this patch until Paul's BH warning
>> removal made it into mainline?
>>
>> netpoll: Disable IRQ around RCU dereference in netpoll_rx
>>
>> We cannot use rcu_dereference_bh safely in netpoll_rx as we may
>> be called with IRQs disabled.  We could however simply disable
>> IRQs as that too causes BH to be disabled and is safe in either
>> case.
>>
>> Thanks to John Linville for discovering this bug and providing
>> a patch.
>>
> With the patch applied I no longet get the warning.

Ok I'll toss this upstream since I have no indication that Paul's
RCU change will hit Linus's tree any time soon.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h
index 413742c..54f286b 100644
--- a/include/linux/netpoll.h
+++ b/include/linux/netpoll.h
@@ -63,20 +63,20 @@  static inline bool netpoll_rx(struct sk_buff *skb)
 	unsigned long flags;
 	bool ret = false;
 
-	rcu_read_lock_bh();
+	local_irq_save(flags);
 	npinfo = rcu_dereference_bh(skb->dev->npinfo);
 
 	if (!npinfo || (list_empty(&npinfo->rx_np) && !npinfo->rx_flags))
 		goto out;
 
-	spin_lock_irqsave(&npinfo->rx_lock, flags);
+	spin_lock(&npinfo->rx_lock);
 	/* check rx_flags again with the lock held */
 	if (npinfo->rx_flags && __netpoll_rx(skb))
 		ret = true;
-	spin_unlock_irqrestore(&npinfo->rx_lock, flags);
+	spin_unlock(&npinfo->rx_lock);
 
 out:
-	rcu_read_unlock_bh();
+	local_irq_restore(flags);
 	return ret;
 }