Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762572AbXEKQZf (ORCPT ); Fri, 11 May 2007 12:25:35 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1759759AbXEKQZ0 (ORCPT ); Fri, 11 May 2007 12:25:26 -0400 Received: from smtp.osdl.org ([65.172.181.24]:52423 "EHLO smtp.osdl.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759746AbXEKQZZ (ORCPT ); Fri, 11 May 2007 12:25:25 -0400 Date: Fri, 11 May 2007 09:25:19 -0700 From: Stephen Hemminger To: Kenji Kaneshige Cc: netdev@vger.kernel.org, linux-kernel , Andrew Morton Subject: Re: [BUG][PATCH] Fix race condition about network device name allocation Message-ID: <20070511092519.1f34ab34@freepuppy> In-Reply-To: <1178862045.3979.33.camel@kane-linux> References: <1178862045.3979.33.camel@kane-linux> Organization: Linux Foundation X-Mailer: Sylpheed-Claws 2.6.0 (GTK+ 2.10.11; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7251 Lines: 160 On Fri, 11 May 2007 14:40:45 +0900 Kenji Kaneshige wrote: > Hi, > > I encountered the following error when I was hot-plugging network card > using pci hotplug driver. > > kobject_add failed for eth8 with -EEXIST, don't try to register things with the same name in the same directory. > > Call Trace: > [] show_stack+0x40/0xa0 > sp=e0000000652cfbf0 bsp=e0000000652c9450 > [] dump_stack+0x30/0x60 > sp=e0000000652cfdc0 bsp=e0000000652c9438 > [] kobject_shadow_add+0x3b0/0x440 > sp=e0000000652cfdc0 bsp=e0000000652c93f0 > [] kobject_add+0x30/0x60 > sp=e0000000652cfdc0 bsp=e0000000652c93d0 > [] device_add+0x160/0xf40 > sp=e0000000652cfdc0 bsp=e0000000652c9358 > [] netdev_register_sysfs+0xc0/0xe0 > sp=e0000000652cfdc0 bsp=e0000000652c9328 > [] register_netdevice+0x600/0x7e0 > sp=e0000000652cfdc0 bsp=e0000000652c92e8 > [] register_netdev+0x70/0xc0 > sp=e0000000652cfdd0 bsp=e0000000652c92c0 > [] e1000_probe+0x1710/0x1a00 [e1000] > sp=e0000000652cfdd0 bsp=e0000000652c9258 > [] pci_device_probe+0xc0/0x120 > sp=e0000000652cfde0 bsp=e0000000652c9218 > [] really_probe+0x1c0/0x300 > sp=e0000000652cfde0 bsp=e0000000652c91c8 > [] driver_probe_device+0x1a0/0x1c0 > sp=e0000000652cfde0 bsp=e0000000652c9198 > [] __device_attach+0x30/0x60 > sp=e0000000652cfde0 bsp=e0000000652c9170 > [] bus_for_each_drv+0x80/0x120 > sp=e0000000652cfde0 bsp=e0000000652c9138 > [] device_attach+0xa0/0x100 > sp=e0000000652cfe00 bsp=e0000000652c9110 > [] bus_attach_device+0x60/0xe0 > sp=e0000000652cfe00 bsp=e0000000652c90e0 > [] device_add+0x950/0xf40 > sp=e0000000652cfe00 bsp=e0000000652c9068 > [] pci_bus_add_device+0x20/0x100 > sp=e0000000652cfe00 bsp=e0000000652c9040 > [] pci_bus_add_devices+0x50/0x320 > sp=e0000000652cfe00 bsp=e0000000652c9010 > [] shpchp_configure_device+0x460/0x4a0 [shpchp] > sp=e0000000652cfe00 bsp=e0000000652c8fc0 > [] board_added+0x720/0x8c0 [shpchp] > sp=e0000000652cfe00 bsp=e0000000652c8f80 > [] shpchp_enable_slot+0x920/0xa40 [shpchp] > sp=e0000000652cfe10 bsp=e0000000652c8f30 > [] shpchp_sysfs_enable_slot+0x120/0x220 [shpchp] > sp=e0000000652cfe20 bsp=e0000000652c8ef8 > [] enable_slot+0x90/0xc0 [shpchp] > sp=e0000000652cfe20 bsp=e0000000652c8ed0 > [] power_write_file+0x1e0/0x2a0 [pci_hotplug] > sp=e0000000652cfe20 bsp=e0000000652c8ea0 > [] hotplug_slot_attr_store+0x60/0xa0 [pci_hotplug] > sp=e0000000652cfe20 bsp=e0000000652c8e68 > [] sysfs_write_file+0x270/0x300 > sp=e0000000652cfe20 bsp=e0000000652c8e18 > [] vfs_write+0x1b0/0x300 > sp=e0000000652cfe20 bsp=e0000000652c8dc0 > [] sys_write+0x70/0xe0 > sp=e0000000652cfe20 bsp=e0000000652c8d48 > [] ia64_ret_from_syscall+0x0/0x20 > sp=e0000000652cfe30 bsp=e0000000652c8d48 > [] __kernel_syscall_via_break+0x0/0x20 > sp=e0000000652d0000 bsp=e0000000652c8d48 > > This error is caused by the race condition between dev_alloc_name() > and unregistering net device. The dev_alloc_name() function checks > dev_base list to find a suitable id. The net device is removed from > the dev_base list when net device is unregistered. Those are done with > rtnl lock held, so there is no problem. However, removing sysfs entry > is delayed until netdev_run_todo() which is outside rtnl lock. Because > of this, dev_alloc_name() can create a device name which is duplicated > with the existing sysfs entry. > > The following patch fixes this by changing dev_alloc_name() function > to check not only dev_base list but also todo list and snapshot list > to find a suitable id. If some devices exists on the todo list or > snapshot list, sysfs entries corresponding to them are not deleted > yet. > > Thanks, > Kenji Kaneshige How about the following (untested), instead. It hold a removes the sysfs entries earlier (on unregister_netdevice), but holds a kobject reference. Then when todo runs the actual last put free happens. --- net/core/dev.c | 10 ++++++---- net/core/net-sysfs.c | 8 +++++++- 2 files changed, 13 insertions(+), 5 deletions(-) --- net-2.6.orig/net/core/dev.c 2007-05-11 09:10:10.000000000 -0700 +++ net-2.6/net/core/dev.c 2007-05-11 09:21:01.000000000 -0700 @@ -3245,7 +3245,6 @@ void netdev_run_todo(void) continue; } - netdev_unregister_sysfs(dev); dev->reg_state = NETREG_UNREGISTERED; netdev_wait_allrefs(dev); @@ -3256,11 +3255,11 @@ void netdev_run_todo(void) BUG_TRAP(!dev->ip6_ptr); BUG_TRAP(!dev->dn_ptr); - /* It must be the very last action, - * after this 'dev' may point to freed up memory. - */ if (dev->destructor) dev->destructor(dev); + + /* Free network device */ + kobject_put(&dev->dev.kobj); } out: @@ -3411,6 +3410,9 @@ void unregister_netdevice(struct net_dev /* Notifier chain MUST detach us from master device. */ BUG_TRAP(!dev->master); + /* Remove entries from sysfs */ + netdev_unregister_sysfs(dev); + /* Finish processing unregister after unlock */ net_set_todo(dev); --- net-2.6.orig/net/core/net-sysfs.c 2007-05-11 09:10:14.000000000 -0700 +++ net-2.6/net/core/net-sysfs.c 2007-05-11 09:14:45.000000000 -0700 @@ -456,9 +456,15 @@ static struct class net_class = { #endif }; +/* Delete sysfs entries but hold kobject reference until after all + * netdev references are gone. + */ void netdev_unregister_sysfs(struct net_device * net) { - device_del(&(net->dev)); + struct device *dev = &(net->dev); + + kobject_get(&dev->kobj); + device_del(dev); } /* Create sysfs entries for network device. */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/