I should have noticed this before making changes to this driver, but
there is really something wrong with the locking in wdm_pre_reset/
wdm_post_reset. And it is there before any of my changes.
The unmodified stable linux-3.2.y cdc-wdm does:
static int wdm_pre_reset(struct usb_interface *intf)
{
struct wdm_device *desc = usb_get_intfdata(intf);
mutex_lock(&desc->lock);
kill_urbs(desc);
/*
* we notify everybody using poll of
* an exceptional situation
* must be done before recovery lest a spontaneous
* message from the device is lost
*/
spin_lock_irq(&desc->iuspin);
desc->rerr = -EINTR;
spin_unlock_irq(&desc->iuspin);
wake_up_all(&desc->wait);
return 0;
}
The problem here is that wdm_read or wdm_write may be blocking on the
desc->wait waitqueue while holding the desc->lock mutex. Which means
that wdm_pre_reset will have to wait for something else to wake up
wdm_read or wdm_write before it can finish. This is the same problem
which used to be present in wdm_disconnect.
Now I don't know how I am supposed to trigger a reset, but loading
usb-storage seems to do the trick for devices with mass storage
interfaces. So I'm using that as a trigger.
I am testing with cdc-wdm from the stable linux-3.2.y branch
(i.e. without any of my recent changes) with the following diff to add
two debug printk's and allow binding to the Huawei modem (I will happily
demonstrate this on the Ericsson modem without any diff at all, if
someone tells me how to trigger a reset):
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index efe6849..a338175 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -630,7 +630,7 @@ static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id)
struct usb_cdc_dmm_desc *dmhd;
u8 *buffer = intf->altsetting->extra;
int buflen = intf->altsetting->extralen;
- u16 maxcom = 0;
+ u16 maxcom = 512;
if (!buffer)
goto out;
@@ -676,8 +676,8 @@ next_desc:
rv = -EINVAL;
iface = intf->cur_altsetting;
- if (iface->desc.bNumEndpoints != 1)
- goto err;
+// if (iface->desc.bNumEndpoints != 1)
+// goto err;
ep = &iface->endpoint[0].desc;
if (!ep || !usb_endpoint_is_int_in(ep))
goto err;
@@ -854,6 +854,7 @@ static int wdm_pre_reset(struct usb_interface *intf)
{
struct wdm_device *desc = usb_get_intfdata(intf);
+ dev_dbg(&intf->dev, "%s()\n", __func__);
mutex_lock(&desc->lock);
kill_urbs(desc);
@@ -875,6 +876,7 @@ static int wdm_post_reset(struct usb_interface *intf)
struct wdm_device *desc = usb_get_intfdata(intf);
int rv;
+ dev_dbg(&intf->dev, "%s()\n", __func__);
rv = recover_from_urb_loss(desc);
mutex_unlock(&desc->lock);
return 0;
First, if not doing a blocking read or write then the reset work just
fine:
Jan 31 10:05:48 nemi kernel: [20438.808049] cdc_wdm 2-1:1.3: wdm2_suspend
Jan 31 10:05:48 nemi kernel: [20438.808140] option: option_instat_callback: error -2
Jan 31 10:06:01 nemi kernel: [20451.477056] Initializing USB Mass Storage driver...
Jan 31 10:06:01 nemi kernel: [20451.524742] cdc_wdm 2-1:1.3: wdm2_resume
Jan 31 10:06:01 nemi kernel: [20451.524853] scsi26 : usb-storage 2-1:1.4
Jan 31 10:06:01 nemi kernel: [20451.525924] scsi27 : usb-storage 2-1:1.5
Jan 31 10:06:01 nemi kernel: [20451.526828] scsi28 : usb-storage 2-2:1.0
Jan 31 10:06:01 nemi kernel: [20451.529000] usbcore: registered new interface driver usb-storage
Jan 31 10:06:01 nemi kernel: [20451.529005] USB Mass Storage support registered.
Jan 31 10:06:02 nemi kernel: [20452.525875] scsi 26:0:0:0: CD-ROM HUAWEI Mass Storage 2.31 PQ: 0 ANSI: 0
Jan 31 10:06:02 nemi kernel: [20452.529103] scsi 28:0:0:0: Direct-Access SEMC Mass Storage 0001 PQ: 0 ANSI: 2
Jan 31 10:06:02 nemi kernel: [20452.529598] scsi 28:0:0:1: CD-ROM SEMC CD-ROM 0001 PQ: 0 ANSI: 2
Jan 31 10:06:23 nemi kernel: [20473.824273] option: option_instat_callback: error -108
Jan 31 10:06:23 nemi kernel: [20473.824409] option1 ttyUSB2: GSM modem (1-port) converter now disconnected from ttyUSB2
Jan 31 10:06:23 nemi kernel: [20473.824439] option 2-1:1.0: device disconnected
Jan 31 10:06:23 nemi kernel: [20473.825406] option1 ttyUSB1: GSM modem (1-port) converter now disconnected from ttyUSB1
Jan 31 10:06:23 nemi kernel: [20473.825434] option 2-1:1.1: device disconnected
Jan 31 10:06:23 nemi kernel: [20473.826926] option1 ttyUSB0: GSM modem (1-port) converter now disconnected from ttyUSB0
Jan 31 10:06:23 nemi kernel: [20473.826956] option 2-1:1.2: device disconnected
Jan 31 10:06:23 nemi kernel: [20473.826963] cdc_wdm 2-1:1.3: wdm_pre_reset()
Jan 31 10:06:24 nemi kernel: [20473.936060] usb 2-1: reset high-speed USB device number 11 using ehci_hcd
Jan 31 10:06:24 nemi kernel: [20474.071217] cdc_wdm 2-1:1.3: wdm_post_reset()
Jan 31 10:06:24 nemi kernel: [20474.071270] option 2-1:1.2: GSM modem (1-port) converter detected
Jan 31 10:06:24 nemi kernel: [20474.072067] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB0
Jan 31 10:06:24 nemi kernel: [20474.072104] option 2-1:1.1: GSM modem (1-port) converter detected
Jan 31 10:06:24 nemi kernel: [20474.073510] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB1
Jan 31 10:06:24 nemi kernel: [20474.073554] option 2-1:1.0: GSM modem (1-port) converter detected
Jan 31 10:06:24 nemi kernel: [20474.074178] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB2
Jan 31 10:06:24 nemi kernel: [20474.080716] scsi 27:0:0:0: Direct-Access HUAWEI SD Storage 2.31 PQ: 0 ANSI: 2
Jan 31 10:06:24 nemi kernel: [20474.084822] sd 27:0:0:0: [sdb] Attached SCSI removable disk
Jan 31 10:06:24 nemi kernel: [20474.098566] sr1: scsi-1 drive
Jan 31 10:06:24 nemi kernel: [20474.098786] sr 26:0:0:0: Attached scsi CD-ROM sr1
Jan 31 10:06:24 nemi kernel: [20474.101687] sr2: scsi3-mmc drive: 0x/0x caddy
Jan 31 10:06:24 nemi kernel: [20474.101867] sr 28:0:0:1: Attached scsi CD-ROM sr2
Jan 31 10:06:24 nemi kernel: [20474.102905] sd 28:0:0:0: [sdc] Attached SCSI removable disk
But if I run "cat /dev/cdc-wdm2" while loading usb-storage then
everything just stops at the pre_reset, as expected:
Jan 31 10:07:03 nemi kernel: [20513.014686] Initializing USB Mass Storage driver...
Jan 31 10:07:03 nemi kernel: [20513.015083] scsi29 : usb-storage 2-1:1.4
Jan 31 10:07:03 nemi kernel: [20513.015344] scsi30 : usb-storage 2-1:1.5
Jan 31 10:07:03 nemi kernel: [20513.015722] scsi31 : usb-storage 2-2:1.0
Jan 31 10:07:03 nemi kernel: [20513.016890] usbcore: registered new interface driver usb-storage
Jan 31 10:07:03 nemi kernel: [20513.016895] USB Mass Storage support registered.
Jan 31 10:07:04 nemi kernel: [20514.013654] scsi 29:0:0:0: CD-ROM HUAWEI Mass Storage 2.31 PQ: 0 ANSI: 0
Jan 31 10:07:04 nemi kernel: [20514.018318] scsi 31:0:0:0: Direct-Access SEMC Mass Storage 0001 PQ: 0 ANSI: 2
Jan 31 10:07:04 nemi kernel: [20514.023467] sr1: scsi-1 drive
Jan 31 10:07:04 nemi kernel: [20514.023990] sr 29:0:0:0: Attached scsi CD-ROM sr1
Jan 31 10:07:04 nemi kernel: [20514.024491] scsi 31:0:0:1: CD-ROM SEMC CD-ROM 0001 PQ: 0 ANSI: 2
Jan 31 10:07:24 nemi kernel: [20534.880287] option: option_instat_callback: error -108
Jan 31 10:07:24 nemi kernel: [20534.880422] option1 ttyUSB2: GSM modem (1-port) converter now disconnected from ttyUSB2
Jan 31 10:07:24 nemi kernel: [20534.880453] option 2-1:1.0: device disconnected
Jan 31 10:07:24 nemi kernel: [20534.880549] option1 ttyUSB1: GSM modem (1-port) converter now disconnected from ttyUSB1
Jan 31 10:07:24 nemi kernel: [20534.880576] option 2-1:1.1: device disconnected
Jan 31 10:07:24 nemi kernel: [20534.881411] option1 ttyUSB0: GSM modem (1-port) converter now disconnected from ttyUSB0
Jan 31 10:07:24 nemi kernel: [20534.881440] option 2-1:1.2: device disconnected
Jan 31 10:07:24 nemi kernel: [20534.881448] cdc_wdm 2-1:1.3: wdm_pre_reset()
The device appears completely dead at this point. lsusb will hang, the
option driver is disconnected, and these drivers are just waiting:
nemi:/tmp# ls -l /sys/bus/usb/devices/2-1/*/driver
lrwxrwxrwx 1 root root 0 Jan 31 10:37 /sys/bus/usb/devices/2-1/2-1:1.3/driver -> ../../../../../../bus/usb/drivers/cdc_wdm
lrwxrwxrwx 1 root root 0 Jan 31 10:07 /sys/bus/usb/devices/2-1/2-1:1.4/driver -> ../../../../../../bus/usb/drivers/usb-storage
lrwxrwxrwx 1 root root 0 Jan 31 10:37 /sys/bus/usb/devices/2-1/2-1:1.5/driver -> ../../../../../../bus/usb/drivers/usb-storage
Killing the blocking read revives the device again:
bjorn@nemi:~$ cat /dev/cdc-wdm2
^C
Jan 31 10:38:41 nemi kernel: [22411.768051] usb 2-1: reset high-speed USB device number 11 using ehci_hcd
Jan 31 10:38:41 nemi kernel: [22411.903182] cdc_wdm 2-1:1.3: wdm_post_reset()
Jan 31 10:38:41 nemi kernel: [22411.903222] cdc_wdm 2-1:1.3: wdm_release: cleanup
Jan 31 10:38:41 nemi kernel: [22411.903263] option 2-1:1.2: GSM modem (1-port) converter detected
Jan 31 10:38:41 nemi kernel: [22411.904037] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB0
Jan 31 10:38:41 nemi kernel: [22411.904080] option 2-1:1.1: GSM modem (1-port) converter detected
Jan 31 10:38:42 nemi kernel: [22411.905843] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB1
Jan 31 10:38:42 nemi kernel: [22411.905889] option 2-1:1.0: GSM modem (1-port) converter detected
Jan 31 10:38:42 nemi kernel: [22411.906970] usb 2-1: GSM modem (1-port) converter now attached to ttyUSB2
Jan 31 10:38:42 nemi kernel: [22411.912936] scsi 30:0:0:0: Direct-Access HUAWEI SD Storage 2.31 PQ: 0 ANSI: 2
Jan 31 10:38:42 nemi kernel: [22411.918662] sr2: scsi3-mmc drive: 0x/0x caddy
Jan 31 10:38:42 nemi kernel: [22411.918906] sr 31:0:0:1: Attached scsi CD-ROM sr2
Jan 31 10:38:42 nemi kernel: [22411.919403] sd 31:0:0:0: [sdc] Attached SCSI removable disk
Jan 31 10:38:42 nemi kernel: [22411.919530] sd 30:0:0:0: [sdb] Attached SCSI removable disk
The problem was not changed by my split in separate read and write
locks, as I made sure to take both locks at this point.
Now the problem can be "fixed" like it was in disconnect, by reordering.
But that's really not a good solution here. We just end up racing
against the next reader or writer waiting for the lock with a fair
chance of winning, but still with a chance of losing...
If we're going to take the read/write mutex in pre_reset then we really
need to first set a flag telling wdm_read and wdm_write not to block
with the mutex held, like we do in disconnect. But all this seems
unnecessarily complex. Do we really need to take the locks in reset?
What problem is this trying to solve? Is it avoiding submitting an urb
in wdm_write? In that case, why not just set the WDM_IN_USE flag?
Bjørn
--
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
