* /sys/devices/system/timer registered twice @ 2004-11-09 19:30 Kay Sievers 2004-11-09 19:39 ` Greg KH 2004-11-09 22:52 ` Greg KH 0 siblings, 2 replies; 9+ messages in thread From: Kay Sievers @ 2004-11-09 19:30 UTC (permalink / raw) To: linux-kernel; +Cc: Greg KH Hi, I got this on a Centrino box with the latest bk: [kay@pim linux.kay]$ ls -l /sys/devices/system/ total 0 drwxr-xr-x 7 root root 0 Nov 8 15:12 . drwxr-xr-x 5 root root 0 Nov 8 15:12 .. drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter ?--------- ? ? ? ? ? timer It is caused by registering two devices with the name "timer" from: arch/i386/kernel/time.c arch/i386/kernel/timers/timer_pit.c If I change one of the names, I get two correct looking sysfs entries. Greg, shouldn't the driver core prevent the corruption of the first device if another one tries to register with the same name? Thanks, Kay ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-09 19:30 /sys/devices/system/timer registered twice Kay Sievers @ 2004-11-09 19:39 ` Greg KH 2004-11-10 2:25 ` Kay Sievers 2004-11-09 22:52 ` Greg KH 1 sibling, 1 reply; 9+ messages in thread From: Greg KH @ 2004-11-09 19:39 UTC (permalink / raw) To: Kay Sievers; +Cc: linux-kernel On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > Hi, > I got this on a Centrino box with the latest bk: > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > total 0 > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > ?--------- ? ? ? ? ? timer > > > It is caused by registering two devices with the name "timer" from: > > arch/i386/kernel/time.c > arch/i386/kernel/timers/timer_pit.c > > If I change one of the names, I get two correct looking sysfs entries. > > Greg, shouldn't the driver core prevent the corruption of the first > device if another one tries to register with the same name? Yes, we should handle this. Can you try the patch below? I just sent it to Linus, as it fixes a bug that was recently introduced. The second registration should fail, and this patch will make it fail, and recover properly. thanks, greg k-h --- a/lib/kobject.c 2004-11-05 10:06:33 -08:00 +++ b/lib/kobject.c 2004-11-08 23:58:02 -08:00 @@ -181,10 +181,10 @@ int kobject_add(struct kobject * kobj) error = create_dir(kobj); if (error) { + /* Does the kobject_put() for us */ unlink(kobj); if (parent) kobject_put(parent); - kobject_put(kobj); } else { kobject_hotplug(kobj, KOBJ_ADD); } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-09 19:39 ` Greg KH @ 2004-11-10 2:25 ` Kay Sievers 2004-11-10 22:36 ` Maneesh Soni 0 siblings, 1 reply; 9+ messages in thread From: Kay Sievers @ 2004-11-10 2:25 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel On Tue, Nov 09, 2004 at 11:39:47AM -0800, Greg KH wrote: > On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > > Hi, > > I got this on a Centrino box with the latest bk: > > > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > > total 0 > > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > > ?--------- ? ? ? ? ? timer > > > > > > It is caused by registering two devices with the name "timer" from: > > > > arch/i386/kernel/time.c > > arch/i386/kernel/timers/timer_pit.c > > > > If I change one of the names, I get two correct looking sysfs entries. > > > > Greg, shouldn't the driver core prevent the corruption of the first > > device if another one tries to register with the same name? > > Yes, we should handle this. Can you try the patch below? I just sent > it to Linus, as it fixes a bug that was recently introduced. > > The second registration should fail, and this patch will make it fail, > and recover properly. Yes, the registration fails. But it seems that the second call to create_dir(kobj) with a kobject with the same name and parent corrupts the dentry from the first call. To test it, I just called create_dir(kobj) a second time for my video driver and the sysfs entry of the successful registered kobject was corrupted: [kay@pim ~]$ ls -la /sys/class/video4linux/ total 0 drwxr-xr-x 3 root root 0 Nov 10 02:53 . drwxr-xr-x 18 root root 0 Nov 10 02:53 .. ?--------- ? ? ? ? ? video0 Thanks, Kay ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-10 2:25 ` Kay Sievers @ 2004-11-10 22:36 ` Maneesh Soni 2004-11-11 0:12 ` Kay Sievers 2004-11-12 20:50 ` Greg KH 0 siblings, 2 replies; 9+ messages in thread From: Maneesh Soni @ 2004-11-10 22:36 UTC (permalink / raw) To: Kay Sievers; +Cc: Greg KH, linux-kernel On Wed, Nov 10, 2004 at 03:25:35AM +0100, Kay Sievers wrote: > On Tue, Nov 09, 2004 at 11:39:47AM -0800, Greg KH wrote: > > On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > > > Hi, > > > I got this on a Centrino box with the latest bk: > > > > > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > > > total 0 > > > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > > > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > > > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > > > ?--------- ? ? ? ? ? timer > > > > > > > > > It is caused by registering two devices with the name "timer" from: > > > > > > arch/i386/kernel/time.c > > > arch/i386/kernel/timers/timer_pit.c > > > > > > If I change one of the names, I get two correct looking sysfs entries. > > > > > > Greg, shouldn't the driver core prevent the corruption of the first > > > device if another one tries to register with the same name? > > > > Yes, we should handle this. Can you try the patch below? I just sent > > it to Linus, as it fixes a bug that was recently introduced. > > > > The second registration should fail, and this patch will make it fail, > > and recover properly. > > Yes, the registration fails. But it seems that the second call to > create_dir(kobj) with a kobject with the same name and parent corrupts > the dentry from the first call. > > To test it, I just called create_dir(kobj) a second time for my video > driver and the sysfs entry of the successful registered kobject was > corrupted: > > [kay@pim ~]$ ls -la /sys/class/video4linux/ > total 0 > drwxr-xr-x 3 root root 0 Nov 10 02:53 . > drwxr-xr-x 18 root root 0 Nov 10 02:53 .. > ?--------- ? ? ? ? ? video0 > Thanks for reporting this bug. I think the problem here is doing d_drop() for the existing directory dentry in create_dir() for error case. It should not be done for EEXIST. Could you please try the same test with the following patch applied. Thanks Maneesh o Do not release existing directory if the new directory happens to be a duplicate directory. Thanks to Kay Sievers for the testcase. Signed-off-by: <maneesh@in.ibm.com> --- linux-2.6.10-rc1-bk20-maneesh/fs/sysfs/dir.c | 2 +- 1 files changed, 1 insertion(+), 1 deletion(-) diff -puN fs/sysfs/dir.c~fix-dropping-existing-dir fs/sysfs/dir.c --- linux-2.6.10-rc1-bk20/fs/sysfs/dir.c~fix-dropping-existing-dir 2004-11-10 16:26:14.000000000 -0600 +++ linux-2.6.10-rc1-bk20-maneesh/fs/sysfs/dir.c 2004-11-10 16:27:42.000000000 -0600 @@ -111,7 +111,7 @@ static int create_dir(struct kobject * k d_rehash(*d); } } - if (error) + if (error && (error != -EEXIST)) d_drop(*d); dput(*d); } else _ -- Maneesh Soni Linux Technology Center, IBM Austin email: maneesh@in.ibm.com Phone: 1-512-838-1896 Fax: T/L : 6781896 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-10 22:36 ` Maneesh Soni @ 2004-11-11 0:12 ` Kay Sievers 2004-11-12 20:50 ` Greg KH 1 sibling, 0 replies; 9+ messages in thread From: Kay Sievers @ 2004-11-11 0:12 UTC (permalink / raw) To: Maneesh Soni; +Cc: Greg KH, linux-kernel On Wed, Nov 10, 2004 at 04:36:29PM -0600, Maneesh Soni wrote: > On Wed, Nov 10, 2004 at 03:25:35AM +0100, Kay Sievers wrote: > > On Tue, Nov 09, 2004 at 11:39:47AM -0800, Greg KH wrote: > > > On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > > > > Hi, > > > > I got this on a Centrino box with the latest bk: > > > > > > > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > > > > total 0 > > > > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > > > > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > > > > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > > > > ?--------- ? ? ? ? ? timer > > > > > > > > > > > > It is caused by registering two devices with the name "timer" from: > > > > > > > > arch/i386/kernel/time.c > > > > arch/i386/kernel/timers/timer_pit.c > > > > > > > > If I change one of the names, I get two correct looking sysfs entries. > > > > > > > > Greg, shouldn't the driver core prevent the corruption of the first > > > > device if another one tries to register with the same name? > > > > > > Yes, we should handle this. Can you try the patch below? I just sent > > > it to Linus, as it fixes a bug that was recently introduced. > > > > > > The second registration should fail, and this patch will make it fail, > > > and recover properly. > > > > Yes, the registration fails. But it seems that the second call to > > create_dir(kobj) with a kobject with the same name and parent corrupts > > the dentry from the first call. > > > > To test it, I just called create_dir(kobj) a second time for my video > > driver and the sysfs entry of the successful registered kobject was > > corrupted: > > > > [kay@pim ~]$ ls -la /sys/class/video4linux/ > > total 0 > > drwxr-xr-x 3 root root 0 Nov 10 02:53 . > > drwxr-xr-x 18 root root 0 Nov 10 02:53 .. > > ?--------- ? ? ? ? ? video0 > > > > Thanks for reporting this bug. I think the problem here is doing d_drop() > for the existing directory dentry in create_dir() for error case. It should > not be done for EEXIST. Could you please try the same test with the following > patch applied. My second create_dir(), also and the double "timer" registration do not corrupt the sysfs directory from the first call anymore. Nice fix! Thanks, Kay ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-10 22:36 ` Maneesh Soni 2004-11-11 0:12 ` Kay Sievers @ 2004-11-12 20:50 ` Greg KH 1 sibling, 0 replies; 9+ messages in thread From: Greg KH @ 2004-11-12 20:50 UTC (permalink / raw) To: Maneesh Soni; +Cc: Kay Sievers, linux-kernel On Wed, Nov 10, 2004 at 04:36:29PM -0600, Maneesh Soni wrote: > On Wed, Nov 10, 2004 at 03:25:35AM +0100, Kay Sievers wrote: > > On Tue, Nov 09, 2004 at 11:39:47AM -0800, Greg KH wrote: > > > On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > > > > Hi, > > > > I got this on a Centrino box with the latest bk: > > > > > > > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > > > > total 0 > > > > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > > > > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > > > > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > > > > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > > > > ?--------- ? ? ? ? ? timer > > > > > > > > > > > > It is caused by registering two devices with the name "timer" from: > > > > > > > > arch/i386/kernel/time.c > > > > arch/i386/kernel/timers/timer_pit.c > > > > > > > > If I change one of the names, I get two correct looking sysfs entries. > > > > > > > > Greg, shouldn't the driver core prevent the corruption of the first > > > > device if another one tries to register with the same name? > > > > > > Yes, we should handle this. Can you try the patch below? I just sent > > > it to Linus, as it fixes a bug that was recently introduced. > > > > > > The second registration should fail, and this patch will make it fail, > > > and recover properly. > > > > Yes, the registration fails. But it seems that the second call to > > create_dir(kobj) with a kobject with the same name and parent corrupts > > the dentry from the first call. > > > > To test it, I just called create_dir(kobj) a second time for my video > > driver and the sysfs entry of the successful registered kobject was > > corrupted: > > > > [kay@pim ~]$ ls -la /sys/class/video4linux/ > > total 0 > > drwxr-xr-x 3 root root 0 Nov 10 02:53 . > > drwxr-xr-x 18 root root 0 Nov 10 02:53 .. > > ?--------- ? ? ? ? ? video0 > > > > Thanks for reporting this bug. I think the problem here is doing d_drop() > for the existing directory dentry in create_dir() for error case. It should > not be done for EEXIST. Could you please try the same test with the following > patch applied. > > Thanks > Maneesh > > o Do not release existing directory if the new directory happens to be a > duplicate directory. Thanks to Kay Sievers for the testcase. > > Signed-off-by: <maneesh@in.ibm.com> Applied, thanks. greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-09 19:30 /sys/devices/system/timer registered twice Kay Sievers 2004-11-09 19:39 ` Greg KH @ 2004-11-09 22:52 ` Greg KH 2004-11-09 23:19 ` Dmitry Torokhov 1 sibling, 1 reply; 9+ messages in thread From: Greg KH @ 2004-11-09 22:52 UTC (permalink / raw) To: Kay Sievers, dtor_core; +Cc: linux-kernel On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > Hi, > I got this on a Centrino box with the latest bk: > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > total 0 > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > ?--------- ? ? ? ? ? timer > > > It is caused by registering two devices with the name "timer" from: > > arch/i386/kernel/time.c > arch/i386/kernel/timers/timer_pit.c > > If I change one of the names, I get two correct looking sysfs entries. > > Greg, shouldn't the driver core prevent the corruption of the first > device if another one tries to register with the same name? Hm, this looks like an issue for Dmitry, as there shouldn't be too sysdev_class structures with the same name, right? thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: /sys/devices/system/timer registered twice 2004-11-09 22:52 ` Greg KH @ 2004-11-09 23:19 ` Dmitry Torokhov 2004-11-09 23:30 ` [PATCH] timer: fix up problem where two sysdev_class devices had the same name Greg KH 0 siblings, 1 reply; 9+ messages in thread From: Dmitry Torokhov @ 2004-11-09 23:19 UTC (permalink / raw) To: Greg KH; +Cc: Kay Sievers, linux-kernel On Tue, 9 Nov 2004 14:52:45 -0800, Greg KH <greg@kroah.com> wrote: > > > On Tue, Nov 09, 2004 at 08:30:43PM +0100, Kay Sievers wrote: > > Hi, > > I got this on a Centrino box with the latest bk: > > > > [kay@pim linux.kay]$ ls -l /sys/devices/system/ > > total 0 > > drwxr-xr-x 7 root root 0 Nov 8 15:12 . > > drwxr-xr-x 5 root root 0 Nov 8 15:12 .. > > drwxr-xr-x 3 root root 0 Nov 8 15:12 cpu > > drwxr-xr-x 3 root root 0 Nov 8 15:12 i8259 > > drwxr-xr-x 2 root root 0 Nov 8 15:12 ioapic > > drwxr-xr-x 3 root root 0 Nov 8 15:12 irqrouter > > ?--------- ? ? ? ? ? timer > > > > > > It is caused by registering two devices with the name "timer" from: > > > > arch/i386/kernel/time.c > > arch/i386/kernel/timers/timer_pit.c > > > > If I change one of the names, I get two correct looking sysfs entries. > > > > Greg, shouldn't the driver core prevent the corruption of the first > > device if another one tries to register with the same name? > > Hm, this looks like an issue for Dmitry, as there shouldn't be too > sysdev_class structures with the same name, right? > I agree, but I think you got the wrong man here ;) You need to talk to Venkatesh. http://linux.bkbits.net:8080/linux-2.5/cset@41810e4aGZ0E5bn_hMb4JgIY5u90zA?nav=index.html|src/.|src/arch|src/arch/i386|src/arch/i386/kernel|related/arch/i386/kernel/time.c -- Dmitry ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] timer: fix up problem where two sysdev_class devices had the same name. 2004-11-09 23:19 ` Dmitry Torokhov @ 2004-11-09 23:30 ` Greg KH 0 siblings, 0 replies; 9+ messages in thread From: Greg KH @ 2004-11-09 23:30 UTC (permalink / raw) To: torvalds, Andrew Morton, venkatesh.pallipadi Cc: Kay Sievers, linux-kernel, dtor_core Thanks to Kay Sievers for reporting this. Was caused by a change from Venkatesh Pallipadi as seen at: http://linux.bkbits.net:8080/linux-2.5/cset@41810e4aGZ0E5bn_hMb4JgIY5u90zA Signed-off-by: Greg Kroah-Hartman <greg@kroah.com> diff -Nru a/arch/i386/kernel/timers/timer_pit.c b/arch/i386/kernel/timers/timer_pit.c --- a/arch/i386/kernel/timers/timer_pit.c 2004-11-09 15:25:54 -08:00 +++ b/arch/i386/kernel/timers/timer_pit.c 2004-11-09 15:25:54 -08:00 @@ -181,7 +181,7 @@ } static struct sysdev_class timer_sysclass = { - set_kset_name("timer"), + set_kset_name("timer_pit"), .resume = timer_resume, }; ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2004-11-12 21:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2004-11-09 19:30 /sys/devices/system/timer registered twice Kay Sievers 2004-11-09 19:39 ` Greg KH 2004-11-10 2:25 ` Kay Sievers 2004-11-10 22:36 ` Maneesh Soni 2004-11-11 0:12 ` Kay Sievers 2004-11-12 20:50 ` Greg KH 2004-11-09 22:52 ` Greg KH 2004-11-09 23:19 ` Dmitry Torokhov 2004-11-09 23:30 ` [PATCH] timer: fix up problem where two sysdev_class devices had the same name Greg KH
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome