netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jεan Sacren" <sakiwit@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: nicolas.dichtel@6wind.com, Daniel Borkmann <daniel@iogearbox.net>,
	David Miller <davem@davemloft.net>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH v2 net] net: do not use rcu in rtnl_dump_ifinfo()
Date: Fri, 27 Feb 2015 17:43:57 -0700	[thread overview]
Message-ID: <20150228004357.GA28631@mail.gmail.com> (raw)
In-Reply-To: <1425058970.5130.32.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 27 Feb 2015 09:42:50 -0800
>
> From: Eric Dumazet <edumazet@google.com>
> 
> We did a failed attempt in the past to only use rcu in rtnl dump
> operations (commit e67f88dd12f6 "net: dont hold rtnl mutex during
> netlink dump callbacks")
> 
> Now that dumps are holding RTNL anyway, there is no need to also
> use rcu locking, as it forbids any scheduling ability, like
> GFP_KERNEL allocations that controlling path should use instead
> of GFP_ATOMIC whenever possible.
> 
> This should fix following splat Cong Wang reported :
> 
>  [ INFO: suspicious RCU usage. ]
>  3.19.0+ #805 Tainted: G        W      
> 
>  include/linux/rcupdate.h:538 Illegal context switch in RCU read-side critical section!
>  
>  other info that might help us debug this:
>  
>  
>  rcu_scheduler_active = 1, debug_locks = 0
>  2 locks held by ip/771:
>   #0:  (rtnl_mutex){+.+.+.}, at: [<ffffffff8182b8f4>] netlink_dump+0x21/0x26c
>   #1:  (rcu_read_lock){......}, at: [<ffffffff817d785b>] rcu_read_lock+0x0/0x6e
>  
>  stack backtrace:
>  CPU: 3 PID: 771 Comm: ip Tainted: G        W       3.19.0+ #805
>  Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>   0000000000000001 ffff8800d51e7718 ffffffff81a27457 0000000029e729e6
>   ffff8800d6108000 ffff8800d51e7748 ffffffff810b539b ffffffff820013dd
>   00000000000001c8 0000000000000000 ffff8800d7448088 ffff8800d51e7758
>  Call Trace:
>   [<ffffffff81a27457>] dump_stack+0x4c/0x65
>   [<ffffffff810b539b>] lockdep_rcu_suspicious+0x107/0x110
>   [<ffffffff8109796f>] rcu_preempt_sleep_check+0x45/0x47
>   [<ffffffff8109e457>] ___might_sleep+0x1d/0x1cb
>   [<ffffffff8109e67d>] __might_sleep+0x78/0x80
>   [<ffffffff814b9b1f>] idr_alloc+0x45/0xd1
>   [<ffffffff810cb7ab>] ? rcu_read_lock_held+0x3b/0x3d
>   [<ffffffff814b9f9d>] ? idr_for_each+0x53/0x101
>   [<ffffffff817c1383>] alloc_netid+0x61/0x69
>   [<ffffffff817c14c3>] __peernet2id+0x79/0x8d
>   [<ffffffff817c1ab7>] peernet2id+0x13/0x1f
>   [<ffffffff817d8673>] rtnl_fill_ifinfo+0xa8d/0xc20
>   [<ffffffff810b17d9>] ? __lock_is_held+0x39/0x52
>   [<ffffffff817d894f>] rtnl_dump_ifinfo+0x149/0x213
>   [<ffffffff8182b9c2>] netlink_dump+0xef/0x26c
>   [<ffffffff8182bcba>] netlink_recvmsg+0x17b/0x2c5
>   [<ffffffff817b0adc>] __sock_recvmsg+0x4e/0x59
>   [<ffffffff817b1b40>] sock_recvmsg+0x3f/0x51
>   [<ffffffff817b1f9a>] ___sys_recvmsg+0xf6/0x1d9
>   [<ffffffff8115dc67>] ? handle_pte_fault+0x6e1/0xd3d
>   [<ffffffff8100a3a0>] ? native_sched_clock+0x35/0x37
>   [<ffffffff8109f45b>] ? sched_clock_local+0x12/0x72
>   [<ffffffff8109f6ac>] ? sched_clock_cpu+0x9e/0xb7
>   [<ffffffff810cb7ab>] ? rcu_read_lock_held+0x3b/0x3d
>   [<ffffffff811abde8>] ? __fcheck_files+0x4c/0x58
>   [<ffffffff811ac556>] ? __fget_light+0x2d/0x52
>   [<ffffffff817b376f>] __sys_recvmsg+0x42/0x60
>   [<ffffffff817b379f>] SyS_recvmsg+0x12/0x1c
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: commit 0c7aecd4bde4b7302 ("netns: add rtnl cmd to add and get peer netns ids")

- The word 'commit' is not supposed to be there.

- ${SHA1} is supposed to be 12-digit-long.

  reply	other threads:[~2015-02-28  0:46 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-27  6:32 [Patch net] netns: avoid allocating idr when dumping info Cong Wang
2015-02-27  8:01 ` Eric Dumazet
2015-02-27 16:25   ` Nicolas Dichtel
2015-02-27 16:55     ` Eric Dumazet
2015-02-27 17:07     ` [PATCH] net: do not use rcu in rtnl_dump_ifinfo() Eric Dumazet
2015-02-27 17:19       ` Daniel Borkmann
2015-02-27 17:40         ` Eric Dumazet
2015-02-27 18:35         ` Cong Wang
2015-02-27 17:42       ` [PATCH v2 net] " Eric Dumazet
2015-02-28  0:43         ` Jεan Sacren [this message]
2015-02-28  1:05           ` Eric Dumazet
2015-03-01  5:29         ` David Miller
2015-02-27 17:28     ` [Patch net] netns: avoid allocating idr when dumping info Cong Wang
2015-02-27 17:48       ` Nicolas Dichtel
2015-02-28  0:56         ` Cong Wang
2015-02-28  6:17           ` Cong Wang
2015-02-27 18:04       ` Eric Dumazet
2015-02-28  0:48         ` Cong Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150228004357.GA28631@mail.gmail.com \
    --to=sakiwit@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eric.dumazet@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.dichtel@6wind.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).