Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S262128AbUKKCEb (ORCPT ); Wed, 10 Nov 2004 21:04:31 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S262166AbUKKCEb (ORCPT ); Wed, 10 Nov 2004 21:04:31 -0500 Received: from fgwmail5.fujitsu.co.jp ([192.51.44.35]:63906 "EHLO fgwmail5.fujitsu.co.jp") by vger.kernel.org with ESMTP id S262128AbUKKCEL (ORCPT ); Wed, 10 Nov 2004 21:04:11 -0500 Date: Thu, 11 Nov 2004 11:03:05 +0900 From: Keiichiro Tokunaga Subject: Re: [PATCH] kobject: fix double kobject_put in kobject_unregister() In-reply-to: <20041110150400.C13668@unix-os.sc.intel.com> To: Keshavamurthy Anil S Cc: tokunaga.keiich@jp.fujitsu.com, greg@kroah.com, linux-hotplug-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, acpi-devel@lists.sourceforge.net Message-id: <20041111110305.46d1d468.tokunaga.keiich@jp.fujitsu.com> Organization: FUJITSU LIMITED MIME-version: 1.0 X-Mailer: Sylpheed version 0.9.9 (GTK+ 1.2.10; i686-pc-linux-gnu) Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7BIT References: <20041110141923.A13668@unix-os.sc.intel.com> <20041110225421.GA16785@kroah.com> <20041110150400.C13668@unix-os.sc.intel.com> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6836 Lines: 159 On Wed, 10 Nov 2004 15:04:01 -0800 Keshavamurthy Anil S wrote: > On Wed, Nov 10, 2004 at 02:54:21PM -0800, Greg KH wrote: > > On Wed, Nov 10, 2004 at 02:19:23PM -0800, Keshavamurthy Anil S wrote: > > > Hi Greg, > > > > > > This patch fixes the problem where in kobject resources were getting > > > freed when those kobject were still in use due to double kobject_put() > > > getting called in the kobject_unregister() code path. > > > > > > With out this patch kobject_unregister() will have some serious side effects. > > > > > > Please apply. > > > > > > signed-off-by: Anil S Keshavamurthy > > > > > > > > > linux-2.6.10-rc1-mm4-askeshav/lib/kobject.c | 3 ++- > > > 1 files changed, 2 insertions(+), 1 deletion(-) > > > > > > diff -puN lib/kobject.c~kobject_unregister_fix lib/kobject.c > > > --- linux-2.6.10-rc1-mm4/lib/kobject.c~kobject_unregister_fix 2004-11-10 13:43:42.243877455 -0800 > > > +++ linux-2.6.10-rc1-mm4-askeshav/lib/kobject.c 2004-11-10 13:46:30.788797265 -0800 > > > @@ -301,6 +301,8 @@ void kobject_del(struct kobject * kobj) > > > { > > > kobject_hotplug(kobj, KOBJ_REMOVE); > > > sysfs_remove_dir(kobj); > > > + > > > + /* unlink does kobject_put() for us */ > > > unlink(kobj); > > > } > > > > > > @@ -313,7 +315,6 @@ void kobject_unregister(struct kobject * > > > { > > > pr_debug("kobject %s: unregistering\n",kobject_name(kobj)); > > > kobject_del(kobj); > > > - kobject_put(kobj); > > > } > > > > No, this is wrong. Count the add and put in the sequence of: > > kobject_register() > > kobject_unregister() > > > > they are balanced. > > > > You mention you are seeing problems. Have a trace? Example code? > > Here is the call trace when I tried to do kobject_unregiser() > Call Trace: > [] show_stack+0x80/0xa0 > sp=e00000000e3cf730 bsp=e00000000e3c9230 > [] show_regs+0x840/0x880 > sp=e00000000e3cf900 bsp=e00000000e3c91c8 > [] die+0x150/0x1c0 > sp=e00000000e3cf910 bsp=e00000000e3c9188 > [] die_if_kernel+0x40/0x60 > sp=e00000000e3cf910 bsp=e00000000e3c9158 > [] ia64_fault+0x110/0x1060 > sp=e00000000e3cf910 bsp=e00000000e3c9100 > [] ia64_leave_kernel+0x0/0x260 > sp=e00000000e3cfb20 bsp=e00000000e3c9100 > [] strlen+0x20/0x140 > sp=e00000000e3cfcf0 bsp=e00000000e3c90a8 > [] kobject_get_path+0x150/0x1e0 > sp=e00000000e3cfcf0 bsp=e00000000e3c9068 > [] kobject_hotplug+0x350/0x6e0 > sp=e00000000e3cfcf0 bsp=e00000000e3c9008 > [] kobject_del+0x30/0x80 > sp=e00000000e3cfd10 bsp=e00000000e3c8fe0 > [] kobject_unregister+0x20/0x60 > sp=e00000000e3cfd10 bsp=e00000000e3c8fc0 > [] acpi_device_unregister+0x320/0x340 > sp=e00000000e3cfd10 bsp=e00000000e3c8fa0 > [] acpi_bus_remove+0x2f0/0x340 > sp=e00000000e3cfd10 bsp=e00000000e3c8f68 > [] acpi_bus_trim+0x300/0x440 > sp=e00000000e3cfd30 bsp=e00000000e3c8f18 > [] acpi_eject_store+0x110/0x240 > sp=e00000000e3cfde0 bsp=e00000000e3c8ee0 > [] acpi_device_attr_store+0x70/0xa0 > sp=e00000000e3cfe20 bsp=e00000000e3c8ea8 > [] sysfs_write_file+0x270/0x300 > sp=e00000000e3cfe20 bsp=e00000000e3c8e58 > [] vfs_write+0x200/0x2c0 > > I will try to explain the what is happening here. > > I have a sysfs tree like this. > .../_SB/LSB0 > .../_SB/LSB0/CPU5 > .../_SB/LSB0/MEM1 > .../_SB/LSB0/MEM2 > > Say LSB0 is kobj1, CPU5 is kobj2, MEM1 is kobj3, MEM2 is kobj4 > > All I am trying to do is first I will do kobject_unregister(kobj2),then > kobject_unregister(kobj3), then kobject_unregister(kobj4) and now > when I try to do kobject_unregister(kobj1) i.e when I am trying to remove > LSB0 directory, I am seeing the above stack trace. > > The patch I posted fixed this problem. > > Let me know if you need more data. I think the cause is an imbalance between acpi_device_register() and acpi_device_unregister(). acpi_device_register() is supposed to increment a refcount of a target kobject (kobj1) and also a kobject (kobj2) that inherited its kset to the kobj1, but it doesn't today. This is because the kset is not set to kobj1 before calling kboject_init() for kobj1. On the other hand, acpi_device_unregister() decrements the refcount of both kobj1 and kobj2. Thus, kobj2 would be released unexpectedly if kobjects using the kset of kobj2 are released. kobject_init(&device->kobj); strlcpy(device->kobj.name,device->pnp.bus_id,KOBJ_NAME_LEN); if (parent) device->kobj.parent = &parent->kobj; device->kobj.ktype = &ktype_acpi_ns; device->kobj.kset = &acpi_namespace_kset; <-- kset is set here. kobject_add(&device->kobj); I am attaching a patch fixing this. I have added acpi-devel in cc since this might be an ACPI issue. Excuse my cross posting... Thanks, Keiichiro Tokunaga Signed-off-by: Keiichiro Tokunaga Status: Tested on 2.6.10-rc1-mm4 --- linux-2.6.10-rc1-mm4-kei-kei/drivers/acpi/scan.c | 3 +-- 1 files changed, 1 insertion(+), 2 deletions(-) diff -puN drivers/acpi/scan.c~fix_acpi_device_register drivers/acpi/scan.c --- linux-2.6.10-rc1-mm4-kei/drivers/acpi/scan.c~fix_acpi_device_register 2004-11-11 10:11:26.121101371 +0900 +++ linux-2.6.10-rc1-mm4-kei-kei/drivers/acpi/scan.c 2004-11-11 10:11:39.126072074 +0900 @@ -112,13 +112,12 @@ static void acpi_device_register(struct list_add_tail(&device->wakeup_list,&acpi_wakeup_device_list); spin_unlock(&acpi_device_lock); - kobject_init(&device->kobj); strlcpy(device->kobj.name,device->pnp.bus_id,KOBJ_NAME_LEN); if (parent) device->kobj.parent = &parent->kobj; device->kobj.ktype = &ktype_acpi_ns; device->kobj.kset = &acpi_namespace_kset; - kobject_add(&device->kobj); + kobject_register(&device->kobj); create_sysfs_device_files(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/