Re: [Patch 2/2] usb/serial/mct_usb232: add delay on close

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 5 Dec 2009 23:08:46 +0000
Alan Cox <alan@xxxxxxxxxxxxxxxxxxx> wrote:

> > +	msleep(2);
> 
> NAK but only to the implementation - if this is dependant upon the amount
> of data then we should compute a delay based upon the queue size. Quite a
> few drivers have a need for this and it's part of the standard port close
> handling as a result.
> 
> Simply set port->port.drain_delay = 256;  /* good first guess */

Apropos this, I have a question for you.

I ran some tests and captured usbmon traces, and noticed that we definitely
kill the URB in progress, like this:
(echo "0123456789ABCDEFGHIJ" >/dev/ttyUSB0):

2f1cf4b8 1.597852 S Bo:1:003:2 - 16 = 30313233 34353637 38394142 43444546
2f1cf4b8 1.598048 C Bo:1:003:2 0 16 >
2f1cf4b8 1.598052 S Bo:1:003:2 - 4 = 4748494a
2f1cf4b8 1.598176 C Bo:1:003:2 -2 0

The code -2 is ENOENT which means that the URB has completed by the
time it was killed, but the callback wasn't delivered yet, so the "C"
message returns with this error.

So, just as a test, I added this:

@@ -514,8 +547,36 @@ static void mct_u232_dtr_rts(struct usb_serial_port *port, int on)
 
 static void mct_u232_close(struct usb_serial_port *port)
 {
+	unsigned long flags;
+	long timeout;
+	wait_queue_t wait;
+	struct tty_struct *tty;
+
 	dbg("%s port %d", __func__, port->number);
 
+ /* P3 */ printk("mct_u232_close: waiting on pid %d\n", (int) current->pid);
+	/* wait for data to drain from the buffer */
+	timeout = 10 * HZ;
+	tty = tty_port_tty_get(&port->port);
+	init_waitqueue_entry(&wait, current);
+	add_wait_queue(&tty->write_wait, &wait);
+	for (;;) {
+		set_current_state(TASK_INTERRUPTIBLE);
+		spin_lock_irqsave(&port->lock, flags);
+		if (!port->write_urb_busy ||
+		    timeout == 0 || signal_pending(current) ||
+		    port->serial->disconnected)
+			break;
+		spin_unlock_irqrestore(&port->lock, flags);
+		timeout = schedule_timeout(timeout);
+	}
+/* P3 */ /* if (port->write_urb_busy) { */
+   printk("mct_u232_close: losing timeout %ld data %p pending %d\n", timeout, usb_get_intfdata(port->serial->interface), signal_pending(current));
+/* } */
+	set_current_state(TASK_RUNNING);
+	spin_unlock_irqrestore(&port->lock, flags);
+	remove_wait_queue(&tty->write_wait, &wait);
+
 	if (port->serial->dev) {
 		/* shutdown our urbs */
 		usb_kill_urb(port->write_urb);

It looked fairly innocous to me, but the result was quite unexpected:
 - module has use count of 1
 - ttyUSB0 cannot be opened anymore (open returns -EIO)

The question: how in the world can the above cause it?

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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux