On Fri, Mar 23, 2012 at 11:12:26AM -0400, Alan Stern wrote:
> On Fri, 23 Mar 2012, Johan Hovold wrote:
> > On Fri, Mar 23, 2012 at 01:11:57PM +0100, Johan Hovold wrote:
> > > Hi Alan and Greg,
> > >
> > > There seems to be a problem with the new API for serial driver
> > > registration/deregistration.
> > >
> > > I get the following oops with usb-next when unloading a usb-serial
> > > driver (verified with both ftdi_sio and pl2303) while the device is
> > > connected:
[...]
> > > The problem appears to be related to the reversed order of
> > > deregistration (usb-serial driver is now deregistered before usb
> > > driver).
> > >
> > > I've verified my suspicion in so far as commit 97b6b6d2339f67
> > > ("usb-serial: use new registration API in [d-h]* drivers") in usb-next
> > > is indeed the commit introducing the regression for ftdi_sio.
>
> It's a little difficult to see what's going wrong, and I don't have my
> serial dongle here to test with now. In particular, I don't see why
> the WARNING's stack trace shows bus_remove_device() calling
> device_release_driver() -- by that point the driver should already have
> been unbound from the port device. That should have happened when
> usb_serial_deregister_drivers() called usb_serial_deregister().
>
> Perhaps the problem is this code in usb_serial_disconnect:
>
> /* Make sure the port is bound so that the
> * driver's port_remove method is called.
> */
> if (!port->dev.driver) {
> int rc;
>
> port->dev.driver =
> &serial->type->driver;
> rc = device_bind_driver(&port->dev);
> }
> port->dev_state = PORT_UNREGISTERING;
> device_del(&port->dev);
> port->dev_state = PORT_UNREGISTERED;
>
> It does look like it could cause this sort of thing to happen. I wrote
> that code to handle the case where the disconnect raced with the
> initial probing, so some of the ports might not have gotten probed yet;
> I don't remember the details.
>
> Still, maybe it's not necessary any more. If you remove that "if"
> statement, does it fix the problem?
I'm afraid I don't have time to debug this further today, but I did a
quick test removing the if statement above. It didn't oops at unload,
but instead I got the following warning when the module was reloaded:
[ 291.257618] drivers/usb/serial/usb-serial.c: usb_serial_probe - registering ttyUSB0
[ 291.257830] ------------[ cut here ]------------
[ 291.257854] WARNING: at fs/sysfs/dir.c:481 sysfs_add_one+0x95/0xd0()
[ 291.257883] Hardware name: Vostro 1520
[ 291.257893] sysfs: cannot create duplicate filename '/dev/char/188:0'
[ 291.257903] Modules linked in: pl2303(+) usbserial [last unloaded: usbserial]
[ 291.257955] Pid: 1111, comm: modprobe Not tainted 3.3.0-rc7+ #89
[ 291.257978] Call Trace:
[ 291.257996] [<c102d862>] warn_slowpath_common+0x72/0xa0
[ 291.258115] [<c113c495>] ? sysfs_add_one+0x95/0xd0
[ 291.258129] [<c113c495>] ? sysfs_add_one+0x95/0xd0
[ 291.258143] [<c102d933>] warn_slowpath_fmt+0x33/0x40
[ 291.258157] [<c113c495>] sysfs_add_one+0x95/0xd0
[ 291.258186] [<c113d4b9>] sysfs_do_create_link+0xe9/0x1e0
[ 291.258202] [<c113d5e7>] sysfs_create_link+0x17/0x20
[ 291.258218] [<c12ebfc3>] device_add+0x483/0x700
[ 291.258235] [<c12f6003>] ? pm_runtime_init+0x113/0x120
[ 291.258262] [<c100303a>] ? do_iret_error+0x6a/0xa0
[ 291.258276] [<c12ec257>] device_register+0x17/0x20
[ 291.258289] [<c12ec317>] device_create_vargs+0xb7/0xd0
[ 291.258304] [<c12ec35d>] device_create+0x2d/0x30
[ 291.258334] [<c125fc74>] tty_register_device+0x84/0x100
[ 291.258349] [<c113bb68>] ? sysfs_add_file+0x18/0x20
[ 291.258370] [<f8118d3d>] usb_serial_device_probe+0xad/0xe0 [usbserial]
[ 291.258404] [<c113d5e7>] ? sysfs_create_link+0x17/0x20
[ 291.258420] [<c12ee14f>] driver_probe_device+0x8f/0x2f0
[ 291.258449] [<c105adbb>] ? sub_preempt_count+0x7b/0xb0
[ 291.258465] [<c1457b5c>] ? _raw_spin_unlock+0x2c/0x50
[ 291.258480] [<c12ee499>] __device_attach+0x49/0x60
[ 291.258496] [<c12ec923>] bus_for_each_drv+0x53/0x80
[ 291.258525] [<c12ee582>] device_attach+0x92/0xb0
[ 291.258538] [<c12ee450>] ? __driver_attach+0xa0/0xa0
[ 291.258552] [<c12ed587>] bus_probe_device+0x77/0xa0
[ 291.258566] [<c12ec176>] device_add+0x636/0x700
[ 291.258594] [<c1203af1>] ? kvasprintf+0x41/0x50
[ 291.258612] [<f8117927>] usb_serial_probe+0x1187/0x1300 [usbserial]
[ 291.258632] [<f8118700>] ? usb_serial_generic_unthrottle+0xa0/0xa0 [usbserial]
[ 291.258651] [<f8129360>] ? pl2303_process_read_urb+0x220/0x220 [pl2303]
[ 291.258682] [<c12f6110>] ? pm_runtime_enable+0x20/0x80
[ 291.258696] [<c1457b15>] ? _raw_spin_unlock_irqrestore+0x55/0x70
[ 291.258711] [<c1058dcb>] ? get_parent_ip+0xb/0x40
[ 291.258736] [<c105adbb>] ? sub_preempt_count+0x7b/0xb0
[ 291.258751] [<c12f6138>] ? pm_runtime_enable+0x48/0x80
[ 291.258767] [<c1338390>] usb_probe_interface+0xf0/0x240
[ 291.258793] [<c113d5e7>] ? sysfs_create_link+0x17/0x20
[ 291.258809] [<c12ee1f2>] driver_probe_device+0x132/0x2f0
[ 291.258826] [<c12ee449>] __driver_attach+0x99/0xa0
[ 291.258854] [<c12ec9a3>] bus_for_each_dev+0x53/0x80
[ 291.258868] [<c12edf0e>] driver_attach+0x1e/0x20
[ 291.258882] [<c12ee3b0>] ? driver_probe_device+0x2f0/0x2f0
[ 291.258912] [<f8115aef>] usb_serial_register_drivers+0x1ef/0x450 [usbserial]
[ 291.258928] [<c108c264>] ? tracepoint_module_notify+0xd4/0x190
[ 291.258947] [<f812d012>] pl2303_driver_init+0x12/0x14 [pl2303]
[ 291.258974] [<c1001124>] do_one_initcall+0x34/0x170
[ 291.258987] [<f812d000>] ? 0xf812cfff
[ 291.259001] [<c1080bb1>] sys_init_module+0x131/0x11b0
[ 291.259059] [<c1458510>] sysenter_do_call+0x12/0x36
[ 291.259071] ---[ end trace 83cc1d559b1b8d7c ]---
[ 291.259366] usb 6-1: pl2303 converter now attached to ttyUSB0
/Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
