bug in init_i82365 wrt sysfs

bug in init_i82365 wrt sysfs

[Arnaldo Carvalho de Melo]

Unable to handle kernel NULL pointer dereference at virtual address 00000010
 printing eip:
c0192fc9
*pde = 00000000
Oops: 0000 [#1]
CPU:    1
EIP:    0060:[<c0192fc9>]    Not tainted
EFLAGS: 00010282
EIP is at sysfs_add_file+0x9/0xa0
eax: 00000000   ebx: 0000023c   ecx: 00000001   edx: c2817ff0
esi: 00000001   edi: 00000000   ebp: c0d89f70   esp: c0d89f64
ds: 007b   es: 007b   ss: 0068
Process modprobe (pid: 481, threadinfo=c0d88000 task=c0e759b0)
Stack: 0000023c 00000001 c2817fe8 c0d89f80 c01dbf4c 00000000 c2817154 c0d89fa0
       c2805188 c2817fe8 c2817154 00000000 c029e8b8 c0d88000 c28172e0 c0d89fbc
       c0139cff 0804e738 0804e738 0804e738 0804e0e0 00000000 c0d88000 c0109897
Call Trace:
 [<c01dbf4c>] class_device_create_file+0x1c/0x30
 [<c2805188>] init_i82365+0xe8/0x1e8 [i82365]
 [<c0139cff>] sys_init_module+0x15f/0x2f0
 [<c0109897>] syscall_call+0x7/0xb

Code: 8b 77 10 6a 77 68 ab 88 27 c0 e8 e8 a2 f8 ff 8d 5e 70 58 89

Re: bug in init_i82365 wrt sysfs

[Arnaldo Carvalho de Melo]

Em Fri, Oct 10, 2003 at 12:59:40AM -0300, Arnaldo C. Melo escreveu:
> Unable to handle kernel NULL pointer dereference at virtual address 00000010
>  printing eip:
> c0192fc9
> *pde = 00000000
> Oops: 0000 [#1]
> CPU:    1
> EIP:    0060:[<c0192fc9>]    Not tainted
> EFLAGS: 00010282
> EIP is at sysfs_add_file+0x9/0xa0
> eax: 00000000   ebx: 0000023c   ecx: 00000001   edx: c2817ff0
> esi: 00000001   edi: 00000000   ebp: c0d89f70   esp: c0d89f64
> ds: 007b   es: 007b   ss: 0068
> Process modprobe (pid: 481, threadinfo=c0d88000 task=c0e759b0)
> Stack: 0000023c 00000001 c2817fe8 c0d89f80 c01dbf4c 00000000 c2817154 c0d89fa0
>        c2805188 c2817fe8 c2817154 00000000 c029e8b8 c0d88000 c28172e0 c0d89fbc
>        c0139cff 0804e738 0804e738 0804e738 0804e0e0 00000000 c0d88000 c0109897
> Call Trace:
>  [<c01dbf4c>] class_device_create_file+0x1c/0x30
>  [<c2805188>] init_i82365+0xe8/0x1e8 [i82365]
>  [<c0139cff>] sys_init_module+0x15f/0x2f0
>  [<c0109897>] syscall_call+0x7/0xb
> 
> Code: 8b 77 10 6a 77 68 ab 88 27 c0 e8 e8 a2 f8 ff 8d 5e 70 58 89

Sorry, hit the send key too fast, here is what I researched:

We should use the same approach net/core/net-sysfs.c is using, i.e:

static struct class net_class = {
        .name = "net",
        .release = netdev_release,
#ifdef CONFIG_HOTPLUG
        .hotplug = netdev_hotplug,
#endif
};

.
.
.

        class_dev->class = &net_class;
        class_dev->class_data = net;

Adjusted to pcmcia, of course, its late here, going to bed, tomorrow I'll
try to fix this one, if nobody beats me to it, I'll be happy to test patches
done while I'm asleep 8)

- Arnaldo

Re: bug in init_i82365 wrt sysfs

[Russell King]

On Fri, Oct 10, 2003 at 12:59:40AM -0300, Arnaldo Carvalho de Melo wrote:
> Call Trace:
>  [<c01dbf4c>] class_device_create_file+0x1c/0x30
>  [<c2805188>] init_i82365+0xe8/0x1e8 [i82365]
>  [<c0139cff>] sys_init_module+0x15f/0x2f0
>  [<c0109897>] syscall_call+0x7/0xb

This oops has been caused by the need to register the class before
registering any objects against it.  Unfortunately, the class needs
to be registered asynchronously in a separate thread to avoid driver
model deadlock with yenta with cardbus cards inserted or standard
PCMCIA cards not being detected correctly due to a race.

I think the only real solution is to remove the class_device_create_file
calls from all socket drivers.  This is just a simple commenting out of
the calls, and should be suitable for the remainder of the -test kernels.

Due to the number of cases that we're encountering with PCMCIA, I'm
beginning to wonder if the driver model could be fixed to be more kind
to PCMCIA by avoiding some of these ordering dependencies.  None of this
would be a problem if the driver model would allow PCI device drivers to
register PCI devices while their probe or remove functions were executing.

Below is a completely untested patch.  Arnaldo - please test whether this
fixes your problem.

===== drivers/pcmcia/i82365.c 1.46 vs edited =====
--- 1.46/drivers/pcmcia/i82365.c	Sun Sep 28 00:04:09 2003
+++ edited/drivers/pcmcia/i82365.c	Fri Oct 10 08:59:26 2003
@@ -1211,6 +1211,7 @@
     return 0;
 } /* i365_set_mem_map */
 
+#if 0 /* driver model ordering issue */
 /*======================================================================
 
     Routines for accessing socket information and register dumps via
@@ -1250,6 +1251,7 @@
 
 static CLASS_DEVICE_ATTR(exca, S_IRUGO, show_exca, NULL);
 static CLASS_DEVICE_ATTR(info, S_IRUGO, show_info, NULL);
+#endif
 
 /*====================================================================*/
 
@@ -1414,10 +1416,12 @@
 			    pcmcia_unregister_socket(&socket[i].socket);
 		    break;
 	    }
+#if 0 /* driver model ordering issue */
 	   class_device_create_file(&socket[i].socket.dev,
 			   	    &class_device_attr_info);
 	   class_device_create_file(&socket[i].socket.dev,
 			   	    &class_device_attr_exca);
+#endif
     }
 
     /* Finally, schedule a polling interrupt */


-- 
Russell King (rmk@arm.linux.org.uk)	http://www.arm.linux.org.uk/personal/
      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: bug in init_i82365 wrt sysfs

[Arnaldo Carvalho de Melo]

Em Fri, Oct 10, 2003 at 09:01:04AM +0100, Russell King escreveu:
> On Fri, Oct 10, 2003 at 12:59:40AM -0300, Arnaldo Carvalho de Melo wrote:
> > Call Trace:
> >  [<c01dbf4c>] class_device_create_file+0x1c/0x30
> >  [<c2805188>] init_i82365+0xe8/0x1e8 [i82365]
> >  [<c0139cff>] sys_init_module+0x15f/0x2f0
> >  [<c0109897>] syscall_call+0x7/0xb
> 
> This oops has been caused by the need to register the class before
> registering any objects against it.  Unfortunately, the class needs
> to be registered asynchronously in a separate thread to avoid driver
> model deadlock with yenta with cardbus cards inserted or standard
> PCMCIA cards not being detected correctly due to a race.
> 
> I think the only real solution is to remove the class_device_create_file
> calls from all socket drivers.  This is just a simple commenting out of
> the calls, and should be suitable for the remainder of the -test kernels.
> 
> Due to the number of cases that we're encountering with PCMCIA, I'm
> beginning to wonder if the driver model could be fixed to be more kind
> to PCMCIA by avoiding some of these ordering dependencies.  None of this
> would be a problem if the driver model would allow PCI device drivers to
> register PCI devices while their probe or remove functions were executing.
> 
> Below is a completely untested patch.  Arnaldo - please test whether this
> fixes your problem.

Yes, it fixes, thanks.

- Arnaldo

Re: bug in init_i82365 wrt sysfs

[Patrick Mochel]

> Due to the number of cases that we're encountering with PCMCIA, I'm
> beginning to wonder if the driver model could be fixed to be more kind
> to PCMCIA by avoiding some of these ordering dependencies.  None of this
> would be a problem if the driver model would allow PCI device drivers to
> register PCI devices while their probe or remove functions were executing.

I am open to that suggestion, but after doing some tinkering, I've found 
that it would be very painful. 

The problem is that we hold the bus's rwsem across the driver binding 
sequence. This includes walking either the list of devices or drivers 
regsitered with the bus and calling ->probe().

We could make the locking more fine-grained, and only hold it when 
actually iterating over the list, and dropping it on each iteration. 
However, I'm strongly opposed to it because it gets ugly and fragile 
very quickly. I'd much rather protect the entire iteration, since it's 
easy to understand. 

So, that means you simply cannot register devices that are on the same bus
from inside a ->probe() function, or unregister them in ->remove(). At
least not the way it is. Any suggestions?

Thanks,


	Pat

Re: bug in init_i82365 wrt sysfs

[Russell King]

On Tue, Oct 14, 2003 at 04:13:10PM -0700, Patrick Mochel wrote:
> So, that means you simply cannot register devices that are on the same bus
> from inside a ->probe() function, or unregister them in ->remove(). At
> least not the way it is. Any suggestions?

No.  It means that PCMCIA/Cardbus will remain to be fairly fragile
with an over-complex initialisation to work around this problem.

-- 
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