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
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);
}
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
On Tue, 9 Nov 2004 14:52:45 -0800, Greg KH <[email protected]> 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
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 <[email protected]>
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,
};
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of
>Dmitry Torokhov
>Sent: Tuesday, November 09, 2004 3:19 PM
>To: Greg KH
>Cc: Kay Sievers; [email protected]
>Subject: Re: /sys/devices/system/timer registered twice
>
>On Tue, 9 Nov 2004 14:52:45 -0800, Greg KH <[email protected]> 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_hMb
>4JgIY5u90zA?nav=index.html|src/.|src/arch|src/arch/i386|src/arc
>h/i386/kernel|related/arch/i386/kernel/time.c
>
Yes. It was me :(.
But, do we really need two system devices for timers?. I feel
we can call setup_pit_timer from time.c, whenever pit is being used.
Otherwise, we may have more issues like the order in which these
two resumes are called and the like.
Thanks,
Venki
On Tue, Nov 09, 2004 at 03:41:51PM -0800, Pallipadi, Venkatesh wrote:
>
> But, do we really need two system devices for timers?. I feel
> we can call setup_pit_timer from time.c, whenever pit is being used.
> Otherwise, we may have more issues like the order in which these
> two resumes are called and the like.
I have no idea. Why not work this out with the other system device
authors, as it's obvious people will have both of them loaded at the
same time.
thanks,
greg k-h
>-----Original Message-----
>From: Greg KH [mailto:[email protected]]
>Sent: Tuesday, November 09, 2004 3:44 PM
>To: Pallipadi, Venkatesh
>Cc: [email protected]; Kay Sievers; [email protected]
>Subject: Re: /sys/devices/system/timer registered twice
>
>On Tue, Nov 09, 2004 at 03:41:51PM -0800, Pallipadi, Venkatesh wrote:
>>
>> But, do we really need two system devices for timers?. I feel
>> we can call setup_pit_timer from time.c, whenever pit is being used.
>> Otherwise, we may have more issues like the order in which these
>> two resumes are called and the like.
>
>I have no idea. Why not work this out with the other system device
>authors, as it's obvious people will have both of them loaded at the
>same time.
>
Agreed. We can change the name of the one in timer_pit.c to timer_pit
for now. That will fix the immediate breakage.
I will try to come up with a patch to combine the two, soon.
Thanks,
Venki
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
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: <[email protected]>
---
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: [email protected]
Phone: 1-512-838-1896 Fax:
T/L : 6781896
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
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: <[email protected]>
Applied, thanks.
greg k-h