[OOPS] 2.6.0-test11 sysfs

[OOPS] 2.6.0-test11 sysfs

[Guennadi Liakhovetski]

Hello

Just installed 2.6.0-test11 on a Toshiba notebook, and  booting / loading
PCMCIA produces the following Oops:

Linux Kernel Card Services
  options:  [pci] [pm]
Intel PCIC probe:
  Intel i82365sl B step ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
    host opts [0]: none
    host opts [1]: none
    ISA irqs (scanned) = 3,4,5,7,9,10<6>    PCI card interrupts, status change on irq 10
Unable to handle kernel NULL pointer dereference at virtual address 0000000c
 printing eip:
c01785a4
*pde = 00000000
Oops: 0000 [#1]
CPU:    0
EIP:    0060:[<c01785a4>]    Not tainted
EFLAGS: 00010282
EIP is at sysfs_add_file+0xc/0x80
eax: 00000000   ebx: c40370ec   ecx: 00000001   edx: 00000000
esi: 00000001   edi: c4037e94   ebp: c1f7ff64   esp: c1f7ff58
ds: 007b   es: 007b   ss: 0068
Process modprobe (pid: 212, threadinfo=c1f7e000 task=c1f5cd20)
Stack: c40370ec 00000001 c4036f44 c1f7ff74 c0178633 00000000 c4037e94 c1f7ff84
       c01e3524 c40370f4 c4037e94 c1f7ffa4 c403b0b0 c40370ec c4037e94 c1f7e000
       c4038020 c02b3040 00000224 c1f7ffbc c01323ab 40134000 0804bb2f bffffbbc
Call Trace:
 [<c0178633>] sysfs_create_file+0x1b/0x28
 [<c01e3524>] class_device_create_file+0x1c/0x20
 [<c403b0b0>] init_i82365+0x12c/0x1bc [i82365]
 [<c01323ab>] sys_init_module+0xfb/0x200
 [<c010a037>] syscall_call+0x7/0xb

Code: 8b 42 0c 8d 48 6c ff 48 6c 0f 88 3e 01 00 00 8b 07 50 52 e8
 <6>cs: IO port probe 0x0c00-0x0cff: clean.
cs: IO port probe 0x0800-0x08ff: clean.
cs: IO port probe 0x0100-0x04ff: excluding 0x220-0x22f 0x330-0x337 0x378-0x37f 0x388-0x38f 0x3c0-0x3e7 0x480-0x48f 0x4d0-0x4d7
cs: IO port probe 0x0a00-0x0aff: clean.

Right, you guessed it - there was no /sys directory:-) Shouldn't lead to
an Oops though... Is it known already?

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

Re: [OOPS] 2.6.0-test11 sysfs

[Matthew Reppert]

On Tue, 2003-12-09 at 11:35, Guennadi Liakhovetski wrote:
> Hello
> 
> Just installed 2.6.0-test11 on a Toshiba notebook, and  booting / loading
> PCMCIA produces the following Oops:
> 
> Linux Kernel Card Services
>   options:  [pci] [pm]
> Intel PCIC probe:
>   Intel i82365sl B step ISA-to-PCMCIA at port 0x3e0 ofs 0x00, 2 sockets
>     host opts [0]: none
>     host opts [1]: none
>     ISA irqs (scanned) = 3,4,5,7,9,10<6>    PCI card interrupts, status change on irq 10
> Unable to handle kernel NULL pointer dereference at virtual address 0000000c
>  printing eip:
> c01785a4
> *pde = 00000000
> Oops: 0000 [#1]
> CPU:    0
> EIP:    0060:[<c01785a4>]    Not tainted
> EFLAGS: 00010282
> EIP is at sysfs_add_file+0xc/0x80
> eax: 00000000   ebx: c40370ec   ecx: 00000001   edx: 00000000
> esi: 00000001   edi: c4037e94   ebp: c1f7ff64   esp: c1f7ff58
> ds: 007b   es: 007b   ss: 0068
> Process modprobe (pid: 212, threadinfo=c1f7e000 task=c1f5cd20)
> Stack: c40370ec 00000001 c4036f44 c1f7ff74 c0178633 00000000 c4037e94 c1f7ff84
>        c01e3524 c40370f4 c4037e94 c1f7ffa4 c403b0b0 c40370ec c4037e94 c1f7e000
>        c4038020 c02b3040 00000224 c1f7ffbc c01323ab 40134000 0804bb2f bffffbbc
> Call Trace:
>  [<c0178633>] sysfs_create_file+0x1b/0x28
>  [<c01e3524>] class_device_create_file+0x1c/0x20
>  [<c403b0b0>] init_i82365+0x12c/0x1bc [i82365]
>  [<c01323ab>] sys_init_module+0xfb/0x200
>  [<c010a037>] syscall_call+0x7/0xb
> 
> Code: 8b 42 0c 8d 48 6c ff 48 6c 0f 88 3e 01 00 00 8b 07 50 52 e8
>  <6>cs: IO port probe 0x0c00-0x0cff: clean.
> cs: IO port probe 0x0800-0x08ff: clean.
> cs: IO port probe 0x0100-0x04ff: excluding 0x220-0x22f 0x330-0x337 0x378-0x37f 0x388-0x38f 0x3c0-0x3e7 0x480-0x48f 0x4d0-0x4d7
> cs: IO port probe 0x0a00-0x0aff: clean.
> 
> Right, you guessed it - there was no /sys directory:-) Shouldn't lead to
> an Oops though... Is it known already?

Hi,

Try this patch. (Patrick, is this the sane thing to do? And is it worth
it? If so, I can do similar things to the other sysfs_create_* functions
if you would like.)

Matt



  Avoid an oops if we try to add a file when sysfs mount point doesn't
exist.



diff -puN fs/sysfs/file.c~sysfs-add-file-oops fs/sysfs/file.c
--- linux-2.6.0-test11/fs/sysfs/file.c~sysfs-add-file-oops	2003-12-09
11:46:58.025205656 -0600
+++ linux-2.6.0-test11-arashi/fs/sysfs/file.c	2003-12-09
11:47:28.781529984 -0600
@@ -374,7 +374,7 @@ int sysfs_add_file(struct dentry * dir, 
 
 int sysfs_create_file(struct kobject * kobj, const struct attribute *
attr)
 {
-	if (kobj && attr)
+	if (kobj && kobj->dentry && attr)
 		return sysfs_add_file(kobj->dentry,attr);
 	return -EINVAL;
 }

_

Re: [OOPS] 2.6.0-test11 sysfs

[Guennadi Liakhovetski]

On Tue, 9 Dec 2003, Matthew Reppert wrote:

> Try this patch. (Patrick, is this the sane thing to do? And is it worth
> it? If so, I can do similar things to the other sysfs_create_* functions
> if you would like.)

Thanks, willl, try it and report back tomorrow. Meanwhile, I created the
mount point and added a record to fstab... And then I've got it again...
Will it also be addressed with your patch? Funny - with directory but
without the record in fstab (so, sysfs not mounted) it doesn't oops...

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

Re: [OOPS] 2.6.0-test11 sysfs

[Russell King]

On Tue, Dec 09, 2003 at 11:57:28AM -0600, Matthew Reppert wrote:
> Try this patch. (Patrick, is this the sane thing to do? And is it worth
> it? If so, I can do similar things to the other sysfs_create_* functions
> if you would like.)

Actually the "right" thing to do is to drop the file creation stuff from
i82365; due to an interaction between sysfs and pcmcia, we can't register
class device files in the initialisation path.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core

Re: [OOPS] 2.6.0-test11 sysfs

[Guennadi Liakhovetski]

On Tue, 9 Dec 2003, Russell King wrote:

> On Tue, Dec 09, 2003 at 11:57:28AM -0600, Matthew Reppert wrote:
> > Try this patch. (Patrick, is this the sane thing to do? And is it worth
> > it? If so, I can do similar things to the other sysfs_create_* functions
> > if you would like.)
>
> Actually the "right" thing to do is to drop the file creation stuff from
> i82365; due to an interaction between sysfs and pcmcia, we can't register
> class device files in the initialisation path.

Without commenting on the "right thing to do", I can only witness, that
the patch from Matthew does remove the Oops (or at least I couldn't
reproduce it any more).

Also, Russell, as you are maintaining PCMCIA code as well, I might just
remind, that the i82365 driver doesn't have the release method, which
produces a nice kernel-message with a backtrace on unloading of the
module. Or does that driver have a separate maintainer?

Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany

Re: [OOPS] 2.6.0-test11 sysfs

[Maneesh Soni]

On Tue, Dec 09, 2003 at 09:19:52PM +0000, Russell King wrote:
> On Tue, Dec 09, 2003 at 11:57:28AM -0600, Matthew Reppert wrote:
> > Try this patch. (Patrick, is this the sane thing to do? And is it worth
> > it? If so, I can do similar things to the other sysfs_create_* functions
> > if you would like.)
> 
> Actually the "right" thing to do is to drop the file creation stuff from
> i82365; due to an interaction between sysfs and pcmcia, we can't register
> class device files in the initialisation path.
> 

Hi Russell,

How is the following patch? It moves the complete() call in the "pccardd" 
thread after class_device_register(), so that in init_i82365() when
pcmcia_register_socket() is done we are sure that class device is 
registered before creating the attribute files.

I must be missing lots of things as I could not understand the interaction
you mentioned, but still after seeing the code I did this patch. 

IMHO we should propogate the error code if returned by class_device_register(), 
back to pcmcia_register_socket() so that in case of error we can fail the 
pcmcia_registera_socket(). For this purpose, I used socket->driver_data() in
pccardd() and also checking it in pcmcia_register_socket(). 


Thanks
Maneesh



o pcmcia_register_socket() should be woken up only after registering the
  class device so that it doesnot create attribute files before registering
  the class device.

o The error, if returned from class_device_register() is propogated back
  to pcmcia_register_socket() using the ->driver_data field in the
  pcmcia_socket. pcmcia_register_socket() is failed if socket->driver_data
  has error in it, so that the driver init routine can process the failuer
  properly.



 drivers/pcmcia/cs.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)

diff -puN drivers/pcmcia/cs.c~pcmcia-sysfs-initialisation-fix drivers/pcmcia/cs.c
--- linux-2.6.0-test11/drivers/pcmcia/cs.c~pcmcia-sysfs-initialisation-fix	2003-12-10 15:27:41.000000000 +0530
+++ linux-2.6.0-test11-maneesh/drivers/pcmcia/cs.c	2003-12-10 16:27:33.000000000 +0530
@@ -356,6 +356,10 @@ int pcmcia_register_socket(struct pcmcia
 
 	wait_for_completion(&socket->thread_done);
 	BUG_ON(!socket->thread);
+	if (IS_ERR(socket->driver_data)) {
+		ret = PTR_ERR(socket->driver_data);
+		goto err;
+	}
 	pcmcia_parse_events(socket, SS_DETECT);
 
 	return 0;
@@ -772,19 +776,21 @@ static int pccardd(void *__skt)
 
 	daemonize("pccardd");
 	skt->thread = current;
-	complete(&skt->thread_done);
-
-	skt->socket = dead_socket;
-	skt->ops->init(skt);
-	skt->ops->set_socket(skt, &skt->socket);
 
 	/* register with the device core */
 	ret = class_device_register(&skt->dev);
 	if (ret) {
 		printk(KERN_WARNING "PCMCIA: unable to register socket 0x%p\n",
 			skt);
+		skt->driver_data = ERR_PTR(ret);
 	}
 
+	complete(&skt->thread_done);
+
+	skt->socket = dead_socket;
+	skt->ops->init(skt);
+	skt->ops->set_socket(skt, &skt->socket);
+
 	add_wait_queue(&skt->thread_wait, &wait);
 	for (;;) {
 		unsigned long flags;

_

-- 
Maneesh Soni
Linux Technology Center, 
IBM Software Lab, Bangalore, India
email: maneesh@in.ibm.com
Phone: 91-80-5044999 Fax: 91-80-5268553
T/L : 9243696

Re: [OOPS] 2.6.0-test11 sysfs

[Russell King]

On Wed, Dec 10, 2003 at 04:36:33PM +0530, Maneesh Soni wrote:
> How is the following patch? It moves the complete() call in the "pccardd" 
> thread after class_device_register(), so that in init_i82365() when
> pcmcia_register_socket() is done we are sure that class device is 
> registered before creating the attribute files.

You'll either deadlock if you have a Cardbus card in the slot at
initialisation, or break standard PCMCIA cards due to a race.

The PCMCIA card race is the exact reason why class_device_register
was moved out of pcmcia_register_socket() in the first place; we
tried adding locks, but that just caused deadlock.

Basically, the only way this can be properly solved is to find some
solution to sysfs's restriction that you can't add devices for a
particular bus type from the same bus type driver's probe method.

IOW, you can't add PCI devices from a PCI device drivers probe method,
and you can't remove PCI devices from a PCI device drivers remove
method.

Pat decided that it was too risky/too late to try to fix this for 2.6,
so our only option now is to disable the problematical bits of code.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
                 2.6 Serial core