From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754989AbaELQTQ (ORCPT ); Mon, 12 May 2014 12:19:16 -0400 Received: from usevmg20.ericsson.net ([198.24.6.45]:58714 "EHLO usevmg20.ericsson.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751442AbaELQTN (ORCPT ); Mon, 12 May 2014 12:19:13 -0400 X-Greylist: delayed 901 seconds by postgrey-1.27 at vger.kernel.org; Mon, 12 May 2014 12:19:13 EDT X-AuditID: c618062d-f79c96d000001cfc-6c-5370a1b7e05f Message-ID: <5370F0F0.1020606@ericsson.com> Date: Mon, 12 May 2014 12:04:00 -0400 From: Jon Maloy User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 MIME-Version: 1.0 To: Willy Tarreau , , CC: Ying Xue , Paul Gortmaker , "David S. Miller" Subject: Re: [ 072/143] tipc: fix lockdep warning during bearer initialization References: <20140512003203.627061916@1wt.eu> In-Reply-To: <20140512003203.627061916@1wt.eu> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-Originating-IP: [147.117.188.8] X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrELMWRmVeSWpSXmKPExsUyuXRPiO72hQXBBj+OiVvMOd/CYnF51xw2 i2t7jzNbLNj4iNHi+Pw5jBaPr19ndmDzOD19P7PHlpU3mTw+b5LzWL9lK1MASxSXTUpqTmZZ apG+XQJXxoT5z9kLNppVbF18irGB8atGFyMnh4SAicT0v0dZIGwxiQv31rN1MXJxCAkcZZRo bfnEDuFsY5S48+A7G0gVr4C2ROvkSWAdLAKqEn//fGMCsdkENCReTutgBLFFBcIk2i/MZIao F5Q4OfMJWL2IQITEizOvwOqZBeoldj96CWYLC/hJrLq8HKxGSEBXYmLDB7A5nAJ6Em9OTGSF qNeTmHK1hRHClpfY/nYOM0S9ssTcD9OYID5QlHhx/CfTBEahWUhWz0LSPgtJ+wJG5lWMHKXF qWW56UYGmxiBgX5Mgk13B+Oel5aHGAU4GJV4eBfMLggWYk0sK67MPcQozcGiJM5b8CU2WEgg PbEkNTs1tSC1KL6oNCe1+BAjEwenVAOjy8qb18ueP5t6pbbKKv131c73jd89GLxc+p7br1W7 dfDzD9d5T2IOn1MVSNV9vOBdYnydXdOELTJLVlxOLrh6w1xP6ezl3Uyx1Wee22x1TL/8on73 lzbZGq3D87w0NKzPKn6a3vDu4t2jrFzHX7lwqH/XzZzifIjf/+6lbl2niicLzC7GvlbZp8RS nJFoqMVcVJwIABBGeZVVAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This one is obsolete. tipc_net_lock does not exist in the current code. It was removed in commit 7216cd949c9bd56a4ccd952c624ab68f8c9aa0a4("tipc: purge tipc_net_lock lock") Regards ///jon On 05/11/2014 08:33 PM, Willy Tarreau wrote: > 2.6.32-longterm review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Ying Xue > > [ Upstream commit 4225a398c1352a7a5c14dc07277cb5cc4473983b ] > > When the lockdep validator is enabled, it will report the below > warning when we enable a TIPC bearer: > > [ INFO: possible irq lock inversion dependency detected ] > --------------------------------------------------------- > Possible interrupt unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(ptype_lock); > local_irq_disable(); > lock(tipc_net_lock); > lock(ptype_lock); > > lock(tipc_net_lock); > > *** DEADLOCK *** > > the shortest dependencies between 2nd lock and 1st lock: > -> (ptype_lock){+.+...} ops: 10 { > [...] > SOFTIRQ-ON-W at: > [] __lock_acquire+0x528/0x13e0 > [] lock_acquire+0x90/0x100 > [] _raw_spin_lock+0x38/0x50 > [] dev_add_pack+0x3a/0x60 > [] arp_init+0x1a/0x48 > [] inet_init+0x181/0x27e > [] do_one_initcall+0x34/0x170 > [] kernel_init+0x110/0x1b2 > [] kernel_thread_helper+0x6/0x10 > [...] > ... key at: [] ptype_lock+0x10/0x20 > ... acquired at: > [] lock_acquire+0x90/0x100 > [] _raw_spin_lock+0x38/0x50 > [] dev_add_pack+0x3a/0x60 > [] enable_bearer+0xf2/0x140 [tipc] > [] tipc_enable_bearer+0x1ba/0x450 [tipc] > [] tipc_cfg_do_cmd+0x5c4/0x830 [tipc] > [] handle_cmd+0x42/0xd0 [tipc] > [] genl_rcv_msg+0x232/0x280 > [] netlink_rcv_skb+0x86/0xb0 > [] genl_rcv+0x1c/0x30 > [] netlink_unicast+0x174/0x1f0 > [] netlink_sendmsg+0x1eb/0x2d0 > [] sock_aio_write+0x161/0x170 > [] do_sync_write+0xac/0xf0 > [] vfs_write+0x156/0x170 > [] sys_write+0x42/0x70 > [] sysenter_do_call+0x12/0x38 > [...] > } > -> (tipc_net_lock){+..-..} ops: 4 { > [...] > IN-SOFTIRQ-R at: > [] __lock_acquire+0x64a/0x13e0 > [] lock_acquire+0x90/0x100 > [] _raw_read_lock_bh+0x3d/0x50 > [] tipc_recv_msg+0x1d/0x830 [tipc] > [] recv_msg+0x3f/0x50 [tipc] > [] __netif_receive_skb+0x22a/0x590 > [] netif_receive_skb+0x2b/0xf0 > [] pcnet32_poll+0x292/0x780 > [] net_rx_action+0xfa/0x1e0 > [] __do_softirq+0xae/0x1e0 > [...] > } > >>>From the log, we can see three different call chains between > CPU0 and CPU1: > > Time 0 on CPU0: > > kernel_init()->inet_init()->dev_add_pack() > > At time 0, the ptype_lock is held by CPU0 in dev_add_pack(); > > Time 1 on CPU1: > > tipc_enable_bearer()->enable_bearer()->dev_add_pack() > > At time 1, tipc_enable_bearer() first holds tipc_net_lock, and then > wants to take ptype_lock to register TIPC protocol handler into the > networking stack. But the ptype_lock has been taken by dev_add_pack() > on CPU0, so at this time the dev_add_pack() running on CPU1 has to be > busy looping. > > Time 2 on CPU0: > > netif_receive_skb()->recv_msg()->tipc_recv_msg() > > At time 2, an incoming TIPC packet arrives at CPU0, hence > tipc_recv_msg() will be invoked. In tipc_recv_msg(), it first wants > to hold tipc_net_lock. At the moment, below scenario happens: > > On CPU0, below is our sequence of taking locks: > > lock(ptype_lock)->lock(tipc_net_lock) > > On CPU1, our sequence of taking locks looks like: > > lock(tipc_net_lock)->lock(ptype_lock) > > Obviously deadlock may happen in this case. > > But please note the deadlock possibly doesn't occur at all when the > first TIPC bearer is enabled. Before enable_bearer() -- running on > CPU1 does not hold ptype_lock, so the TIPC receive handler (i.e. > recv_msg()) is not registered successfully via dev_add_pack(), so > the tipc_recv_msg() cannot be called by recv_msg() even if a TIPC > message comes to CPU0. But when the second TIPC bearer is > registered, the deadlock can perhaps really happen. > > To fix it, we will push the work of registering TIPC protocol > handler into workqueue context. After the change, both paths taking > ptype_lock are always in process contexts, thus, the deadlock should > never occur. > > Signed-off-by: Ying Xue > Signed-off-by: Jon Maloy > Signed-off-by: Paul Gortmaker > Signed-off-by: David S. Miller > Signed-off-by: Willy Tarreau > --- > net/tipc/eth_media.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c > index 524ba56..22453a8 100644 > --- a/net/tipc/eth_media.c > +++ b/net/tipc/eth_media.c > @@ -56,6 +56,7 @@ struct eth_bearer { > struct tipc_bearer *bearer; > struct net_device *dev; > struct packet_type tipc_packet_type; > + struct work_struct setup; > }; > > static struct eth_bearer eth_bearers[MAX_ETH_BEARERS]; > @@ -122,6 +123,17 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev, > } > > /** > + * setup_bearer - setup association between Ethernet bearer and interface > + */ > +static void setup_bearer(struct work_struct *work) > +{ > + struct eth_bearer *eb_ptr = > + container_of(work, struct eth_bearer, setup); > + > + dev_add_pack(&eb_ptr->tipc_packet_type); > +} > + > +/** > * enable_bearer - attach TIPC bearer to an Ethernet interface > */ > > @@ -157,7 +169,8 @@ static int enable_bearer(struct tipc_bearer *tb_ptr) > eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr; > INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list)); > dev_hold(dev); > - dev_add_pack(&eb_ptr->tipc_packet_type); > + INIT_WORK(&eb_ptr->setup, setup_bearer); > + schedule_work(&eb_ptr->setup); > } > > /* Associate TIPC bearer with Ethernet bearer */ >