From patchwork Sun Oct 6 03:13:14 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 280823 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 9B4CD2C00AA for ; Sun, 6 Oct 2013 13:13:31 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753464Ab3JFCN2 (ORCPT ); Sat, 5 Oct 2013 22:13:28 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:56858 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752903Ab3JFCN1 (ORCPT ); Sat, 5 Oct 2013 22:13:27 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1VSdqY-0000Qf-GV; Sat, 05 Oct 2013 20:13:26 -0600 Received: from c-98-207-154-105.hsd1.ca.comcast.net ([98.207.154.105] helo=eric-ThinkPad-X220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1VSdqX-0003Wc-5F; Sat, 05 Oct 2013 20:13:26 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Francesco Ruggeri Cc: netdev@vger.kernel.org References: <5250c0b6.45dc420a.738b.6a58@mx.google.com> Date: Sat, 05 Oct 2013 19:13:14 -0700 In-Reply-To: <5250c0b6.45dc420a.738b.6a58@mx.google.com> (Francesco Ruggeri's message of "Sat, 5 Oct 2013 06:18:22 -0700") Message-ID: <87a9inb0zp.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 X-XM-AID: U2FsdGVkX1+3HaXU1mgtbXdi+8bBq0Gkc4VIzSLwDk8= X-SA-Exim-Connect-IP: 98.207.154.105 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sa04.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.6 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE, T_TM2_M_HEADER_IN_MSG, T_TooManySym_01, XMSolicitRefs_0, XMSubLong autolearn=disabled version=3.3.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 0.0 T_TM2_M_HEADER_IN_MSG BODY: T_TM2_M_HEADER_IN_MSG * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.4366] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.1 XMSolicitRefs_0 Weightloss drug X-Spam-DCC: XMission; sa04 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Francesco Ruggeri X-Spam-Relay-Country: Subject: Re: [PATCH net-next] net: Separate the close_list and the unreg_list X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Wed, 14 Nov 2012 14:26:46 -0700) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Francesco Ruggeri writes: > Hi Eric, > I am running some more extensive tests with this patch and I ran into > the crash below. > > It may be caused by > > @@ -1301,7 +1301,7 @@ int dev_close(struct net_device *dev) > if (dev->flags & IFF_UP) { > LIST_HEAD(single); > > - list_add(&dev->unreg_list, &single); > + list_add(&dev->close_list, &single); > dev_close_many(&single); > list_del(&single); > } > > dev_close_many expects net_devices to be linked by unreg_list. > Let me know what you think. Yes. There does appear to be a logic problem there. Grr. It looks like the least error prone approach is to simply have dev_close_many consume it's list. Can you verify this incremental change fixes it for you? This changes all of the callers to build the close_list before calling dev_close_many. Which makes it safe to muck with the close list however we want. --- -- 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 --git a/net/core/dev.c b/net/core/dev.c index c8db0bfc36d6..fa0b2b06c1a6 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1362,16 +1362,15 @@ static int __dev_close(struct net_device *dev) static int dev_close_many(struct list_head *head) { struct net_device *dev, *tmp; - LIST_HEAD(many); - /* rollback_registered_many needs the original unmodified list */ - list_for_each_entry(dev, head, unreg_list) - if (dev->flags & IFF_UP) - list_add_tail(&dev->close_list, &many); + /* Remove the devices that don't need to be closed */ + list_for_each_entry_safe(dev, tmp, head, close_list) + if (!(dev->flags & IFF_UP)) + list_del_init(&dev->close_list); - __dev_close_many(&many); + __dev_close_many(head); - list_for_each_entry_safe(dev, tmp, &many, close_list) { + list_for_each_entry_safe(dev, tmp, head, close_list) { rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_UP|IFF_RUNNING); call_netdevice_notifiers(NETDEV_DOWN, dev); list_del_init(&dev->close_list); @@ -5439,6 +5438,7 @@ static void net_set_todo(struct net_device *dev) static void rollback_registered_many(struct list_head *head) { struct net_device *dev, *tmp; + LIST_HEAD(close_head); BUG_ON(dev_boot_phase); ASSERT_RTNL(); @@ -5461,7 +5461,9 @@ static void rollback_registered_many(struct list_head *head) } /* If device is running, close it first. */ - dev_close_many(head); + list_for_each_entry(dev, head, unreg_list) + list_add_tail(&dev->close_list, &close_head); + dev_close_many(&close_head); list_for_each_entry(dev, head, unreg_list) { /* And unlink it from device chain. */