public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
@ 2013-10-22  1:04 Alexei Starovoitov
  2013-10-22 11:52 ` Nicolas Dichtel
  2013-10-23  3:53 ` Cong Wang
  0 siblings, 2 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2013-10-22  1:04 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: netdev

Hi Nicolas,

after commit 991fb3f74c "dev: always advertise rx_flags changes via netlink"
I'm seeing 'sleeping in atomic' bug.

Steps to reproduce:
ip tuntap add dev tap1 mode tap
ifconfig tap1 up
tcpdump -nei tap1
and in different terminal:
ip tuntap del dev tap1 mode tap

[  271.627994] device tap1 left promiscuous mode
[  271.639897] BUG: sleeping function called from invalid context at
mm/slub.c:940
[  271.664491] in_atomic(): 1, irqs_disabled(): 0, pid: 3394, name: ip
[  271.677525] INFO: lockdep is turned off.
[  271.690503] CPU: 0 PID: 3394 Comm: ip Tainted: G        W    3.12.0-rc3+ #73
[  271.703996] Hardware name: System manufacturer System Product
Name/P8Z77 WS, BIOS 3007 07/26/2012
[  271.731254]  ffffffff81a58506 ffff8807f0d57a58 ffffffff817544e5
ffff88082fa0f428
[  271.760261]  ffff8808071f5f40 ffff8807f0d57a88 ffffffff8108bad1
ffffffff81110ff8
[  271.790683]  0000000000000010 00000000000000d0 00000000000000d0
ffff8807f0d57af8
[  271.822332] Call Trace:
[  271.838234]  [<ffffffff817544e5>] dump_stack+0x55/0x76
[  271.854446]  [<ffffffff8108bad1>] __might_sleep+0x181/0x240
[  271.870836]  [<ffffffff81110ff8>] ? rcu_irq_exit+0x68/0xb0
[  271.887076]  [<ffffffff811a80be>] kmem_cache_alloc_node+0x4e/0x2a0
[  271.903368]  [<ffffffff810b4ddc>] ? vprintk_emit+0x1dc/0x5a0
[  271.919716]  [<ffffffff81614d67>] ? __alloc_skb+0x57/0x2a0
[  271.936088]  [<ffffffff810b4de0>] ? vprintk_emit+0x1e0/0x5a0
[  271.952504]  [<ffffffff81614d67>] __alloc_skb+0x57/0x2a0
[  271.968902]  [<ffffffff8163a0b2>] rtmsg_ifinfo+0x52/0x100
[  271.985302]  [<ffffffff8162ac6d>] __dev_notify_flags+0xad/0xc0
[  272.001642]  [<ffffffff8162ad0c>] __dev_set_promiscuity+0x8c/0x1c0
[  272.017917]  [<ffffffff81731ea5>] ? packet_notifier+0x5/0x380
[  272.033961]  [<ffffffff8162b109>] dev_set_promiscuity+0x29/0x50
[  272.049855]  [<ffffffff8172e937>] packet_dev_mc+0x87/0xc0
[  272.065494]  [<ffffffff81732052>] packet_notifier+0x1b2/0x380
[  272.080915]  [<ffffffff81731ea5>] ? packet_notifier+0x5/0x380
[  272.096009]  [<ffffffff81761c66>] notifier_call_chain+0x66/0x150
[  272.110803]  [<ffffffff8108503e>] __raw_notifier_call_chain+0xe/0x10
[  272.125468]  [<ffffffff81085056>] raw_notifier_call_chain+0x16/0x20
[  272.139984]  [<ffffffff81620190>] call_netdevice_notifiers_info+0x40/0x70
[  272.154523]  [<ffffffff816201d6>] call_netdevice_notifiers+0x16/0x20
[  272.168552]  [<ffffffff816224c5>] rollback_registered_many+0x145/0x240
[  272.182263]  [<ffffffff81622641>] rollback_registered+0x31/0x40
[  272.195369]  [<ffffffff816229c8>] unregister_netdevice_queue+0x58/0x90
[  272.208230]  [<ffffffff81547ca0>] __tun_detach+0x140/0x340
[  272.220686]  [<ffffffff81547ed6>] tun_chr_close+0x36/0x60

packet_notifier() does rcu_read_lock() before calling into packet_dev_mc() .

Not sure how to fix it cleanly, other than disabling a notify here.
Any suggestion?

Thanks
Alexei

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
  2013-10-22  1:04 BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags Alexei Starovoitov
@ 2013-10-22 11:52 ` Nicolas Dichtel
  2013-10-23  3:53 ` Cong Wang
  1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Dichtel @ 2013-10-22 11:52 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: netdev

Le 22/10/2013 03:04, Alexei Starovoitov a écrit :
> Hi Nicolas,
>
> after commit 991fb3f74c "dev: always advertise rx_flags changes via netlink"
> I'm seeing 'sleeping in atomic' bug.
>
> Steps to reproduce:
> ip tuntap add dev tap1 mode tap
> ifconfig tap1 up
> tcpdump -nei tap1
> and in different terminal:
> ip tuntap del dev tap1 mode tap
>
> [  271.627994] device tap1 left promiscuous mode
> [  271.639897] BUG: sleeping function called from invalid context at
> mm/slub.c:940
> [  271.664491] in_atomic(): 1, irqs_disabled(): 0, pid: 3394, name: ip
> [  271.677525] INFO: lockdep is turned off.
> [  271.690503] CPU: 0 PID: 3394 Comm: ip Tainted: G        W    3.12.0-rc3+ #73
> [  271.703996] Hardware name: System manufacturer System Product
> Name/P8Z77 WS, BIOS 3007 07/26/2012
> [  271.731254]  ffffffff81a58506 ffff8807f0d57a58 ffffffff817544e5
> ffff88082fa0f428
> [  271.760261]  ffff8808071f5f40 ffff8807f0d57a88 ffffffff8108bad1
> ffffffff81110ff8
> [  271.790683]  0000000000000010 00000000000000d0 00000000000000d0
> ffff8807f0d57af8
> [  271.822332] Call Trace:
> [  271.838234]  [<ffffffff817544e5>] dump_stack+0x55/0x76
> [  271.854446]  [<ffffffff8108bad1>] __might_sleep+0x181/0x240
> [  271.870836]  [<ffffffff81110ff8>] ? rcu_irq_exit+0x68/0xb0
> [  271.887076]  [<ffffffff811a80be>] kmem_cache_alloc_node+0x4e/0x2a0
> [  271.903368]  [<ffffffff810b4ddc>] ? vprintk_emit+0x1dc/0x5a0
> [  271.919716]  [<ffffffff81614d67>] ? __alloc_skb+0x57/0x2a0
> [  271.936088]  [<ffffffff810b4de0>] ? vprintk_emit+0x1e0/0x5a0
> [  271.952504]  [<ffffffff81614d67>] __alloc_skb+0x57/0x2a0
> [  271.968902]  [<ffffffff8163a0b2>] rtmsg_ifinfo+0x52/0x100
> [  271.985302]  [<ffffffff8162ac6d>] __dev_notify_flags+0xad/0xc0
> [  272.001642]  [<ffffffff8162ad0c>] __dev_set_promiscuity+0x8c/0x1c0
> [  272.017917]  [<ffffffff81731ea5>] ? packet_notifier+0x5/0x380
> [  272.033961]  [<ffffffff8162b109>] dev_set_promiscuity+0x29/0x50
> [  272.049855]  [<ffffffff8172e937>] packet_dev_mc+0x87/0xc0
> [  272.065494]  [<ffffffff81732052>] packet_notifier+0x1b2/0x380
> [  272.080915]  [<ffffffff81731ea5>] ? packet_notifier+0x5/0x380
> [  272.096009]  [<ffffffff81761c66>] notifier_call_chain+0x66/0x150
> [  272.110803]  [<ffffffff8108503e>] __raw_notifier_call_chain+0xe/0x10
> [  272.125468]  [<ffffffff81085056>] raw_notifier_call_chain+0x16/0x20
> [  272.139984]  [<ffffffff81620190>] call_netdevice_notifiers_info+0x40/0x70
> [  272.154523]  [<ffffffff816201d6>] call_netdevice_notifiers+0x16/0x20
> [  272.168552]  [<ffffffff816224c5>] rollback_registered_many+0x145/0x240
> [  272.182263]  [<ffffffff81622641>] rollback_registered+0x31/0x40
> [  272.195369]  [<ffffffff816229c8>] unregister_netdevice_queue+0x58/0x90
> [  272.208230]  [<ffffffff81547ca0>] __tun_detach+0x140/0x340
> [  272.220686]  [<ffffffff81547ed6>] tun_chr_close+0x36/0x60
>
> packet_notifier() does rcu_read_lock() before calling into packet_dev_mc() .
>
> Not sure how to fix it cleanly, other than disabling a notify here.
> Any suggestion?
I don't reproduce it. Can you send me your .config?
I will look more deeply at the code.


Regards,
Nicolas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
  2013-10-22  1:04 BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags Alexei Starovoitov
  2013-10-22 11:52 ` Nicolas Dichtel
@ 2013-10-23  3:53 ` Cong Wang
  2013-10-23  5:34   ` Alexei Starovoitov
  1 sibling, 1 reply; 6+ messages in thread
From: Cong Wang @ 2013-10-23  3:53 UTC (permalink / raw)
  To: netdev

On Tue, 22 Oct 2013 at 01:04 GMT, Alexei Starovoitov <ast@plumgrid.com> wrote:
>
> packet_notifier() does rcu_read_lock() before calling into packet_dev_mc() .
>
> Not sure how to fix it cleanly, other than disabling a notify here.
> Any suggestion?
>

Passing a gfp flag to rtmsg_ifinfo() seems a right fix for me, but I don't
know if there is other better way to fix it.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
  2013-10-23  3:53 ` Cong Wang
@ 2013-10-23  5:34   ` Alexei Starovoitov
  2013-10-23  7:48     ` Nicolas Dichtel
  0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2013-10-23  5:34 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, nicolas.dichtel

On Tue, Oct 22, 2013 at 8:53 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, 22 Oct 2013 at 01:04 GMT, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>
>> packet_notifier() does rcu_read_lock() before calling into packet_dev_mc() .
>>
>> Not sure how to fix it cleanly, other than disabling a notify here.
>> Any suggestion?
>>
>
> Passing a gfp flag to rtmsg_ifinfo() seems a right fix for me, but I don't
> know if there is other better way to fix it.

Indeed. rtnl_notify() already accepts gfp_t.

The following diff fixes it for me:
---
 include/linux/rtnetlink.h |    3 ++-
 net/core/dev.c            |    2 +-
 net/core/rtnetlink.c      |   12 +++++++++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index f28544b..0180523 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -15,7 +15,8 @@ extern int rtnetlink_put_metrics(struct sk_buff
*skb, u32 *metrics);
 extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
                   u32 id, long expires, u32 error);

-extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);
+void __rtmsg_ifinfo(int type, struct net_device *dev, unsigned
change, gfp_t flags);
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);

 /* RTNL is used as a global lock for all changes to network configuration  */
 extern void rtnl_lock(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index 0918aad..59b90fe 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5257,7 +5257,7 @@ void __dev_notify_flags(struct net_device *dev,
unsigned int old_flags,
     unsigned int changes = dev->flags ^ old_flags;

     if (gchanges)
-        rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges);
+        __rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC);

     if (changes & IFF_UP) {
         if (dev->flags & IFF_UP)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 4aedf03..5931af9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1984,14 +1984,15 @@ static int rtnl_dump_all(struct sk_buff *skb,
struct netlink_callback *cb)
     return skb->len;
 }

-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
+void __rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
+            gfp_t flags)
 {
     struct net *net = dev_net(dev);
     struct sk_buff *skb;
     int err = -ENOBUFS;
     size_t if_info_size;

-    skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), GFP_KERNEL);
+    skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), flags);
     if (skb == NULL)
         goto errout;

@@ -2002,12 +2003,17 @@ void rtmsg_ifinfo(int type, struct net_device
*dev, unsigned int change)
         kfree_skb(skb);
         goto errout;
     }
-    rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
+    rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
     return;
 errout:
     if (err < 0)
         rtnl_set_sk_err(net, RTNLGRP_LINK, err);
 }
+
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
+{
+    __rtmsg_ifinfo(type, dev, change, GFP_KERNEL);
+}
 EXPORT_SYMBOL(rtmsg_ifinfo);

 static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
--

Nicolas, I've sent you my .config.
Any better ideas?

Thanks
Alex

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
  2013-10-23  5:34   ` Alexei Starovoitov
@ 2013-10-23  7:48     ` Nicolas Dichtel
  2013-10-23 12:18       ` Nicolas Dichtel
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Dichtel @ 2013-10-23  7:48 UTC (permalink / raw)
  To: Alexei Starovoitov, Cong Wang; +Cc: netdev

Le 23/10/2013 07:34, Alexei Starovoitov a écrit :
> On Tue, Oct 22, 2013 at 8:53 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, 22 Oct 2013 at 01:04 GMT, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>
>>> packet_notifier() does rcu_read_lock() before calling into packet_dev_mc() .
>>>
>>> Not sure how to fix it cleanly, other than disabling a notify here.
>>> Any suggestion?
>>>
>>
>> Passing a gfp flag to rtmsg_ifinfo() seems a right fix for me, but I don't
>> know if there is other better way to fix it.
>
> Indeed. rtnl_notify() already accepts gfp_t.
>
> The following diff fixes it for me:
> ---
>   include/linux/rtnetlink.h |    3 ++-
>   net/core/dev.c            |    2 +-
>   net/core/rtnetlink.c      |   12 +++++++++---
>   3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index f28544b..0180523 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -15,7 +15,8 @@ extern int rtnetlink_put_metrics(struct sk_buff
> *skb, u32 *metrics);
>   extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
>                     u32 id, long expires, u32 error);
>
> -extern void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);
> +void __rtmsg_ifinfo(int type, struct net_device *dev, unsigned
> change, gfp_t flags);
Just nitpiking: putting the type and the name on the same line is better 
'unsigned change'.

> +void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change);
>
>   /* RTNL is used as a global lock for all changes to network configuration  */
>   extern void rtnl_lock(void);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 0918aad..59b90fe 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5257,7 +5257,7 @@ void __dev_notify_flags(struct net_device *dev,
> unsigned int old_flags,
>       unsigned int changes = dev->flags ^ old_flags;
>
>       if (gchanges)
> -        rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges);
> +        __rtmsg_ifinfo(RTM_NEWLINK, dev, gchanges, GFP_ATOMIC);
>
>       if (changes & IFF_UP) {
>           if (dev->flags & IFF_UP)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 4aedf03..5931af9 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1984,14 +1984,15 @@ static int rtnl_dump_all(struct sk_buff *skb,
> struct netlink_callback *cb)
>       return skb->len;
>   }
>
> -void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
> +void __rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
> +            gfp_t flags)
'gfp_t flags' should be aligned with 'int type'

>   {
>       struct net *net = dev_net(dev);
>       struct sk_buff *skb;
>       int err = -ENOBUFS;
>       size_t if_info_size;
>
> -    skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), GFP_KERNEL);
> +    skb = nlmsg_new((if_info_size = if_nlmsg_size(dev, 0)), flags);
>       if (skb == NULL)
>           goto errout;
>
> @@ -2002,12 +2003,17 @@ void rtmsg_ifinfo(int type, struct net_device
> *dev, unsigned int change)
>           kfree_skb(skb);
>           goto errout;
>       }
> -    rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, GFP_KERNEL);
> +    rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
>       return;
>   errout:
>       if (err < 0)
>           rtnl_set_sk_err(net, RTNLGRP_LINK, err);
>   }
> +
> +void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change)
> +{
> +    __rtmsg_ifinfo(type, dev, change, GFP_KERNEL);
> +}
>   EXPORT_SYMBOL(rtmsg_ifinfo);
>
>   static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
> --
>
> Nicolas, I've sent you my .config.
I got some pb with my testbed, hence I still didn't reproduce the bug.
I will make more test today, but I think this is the right patch.

> Any better ideas?
No and the patch is right for me (__dev_set_promiscuity() call
audit_log(..., GFP_ATOMIC) already).

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags
  2013-10-23  7:48     ` Nicolas Dichtel
@ 2013-10-23 12:18       ` Nicolas Dichtel
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Dichtel @ 2013-10-23 12:18 UTC (permalink / raw)
  To: Alexei Starovoitov, Cong Wang; +Cc: netdev

Le 23/10/2013 09:48, Nicolas Dichtel a écrit :
> Le 23/10/2013 07:34, Alexei Starovoitov a écrit :
>> On Tue, Oct 22, 2013 at 8:53 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Tue, 22 Oct 2013 at 01:04 GMT, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>>>
[snip]
>> Nicolas, I've sent you my .config.
> I got some pb with my testbed, hence I still didn't reproduce the bug.
> I will make more test today, but I think this is the right patch.
Still not reproduced with your config.
But as I said, I think your patch is the right thing to do.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-23 12:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-22  1:04 BUG: scheduling while atomic dev_set_promiscuity->__dev_notify_flags Alexei Starovoitov
2013-10-22 11:52 ` Nicolas Dichtel
2013-10-23  3:53 ` Cong Wang
2013-10-23  5:34   ` Alexei Starovoitov
2013-10-23  7:48     ` Nicolas Dichtel
2013-10-23 12:18       ` Nicolas Dichtel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox