forked from mirrors/linux
		
	[PATCH] USB: add ability for usb-serial drivers to determine if their write urb is currently being used.
This removes a lot of racy and buggy code by trying to check the status of the urb. Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This commit is contained in:
		
							parent
							
								
									f4df0e334a
								
							
						
					
					
						commit
						507ca9bc04
					
				
					 10 changed files with 85 additions and 46 deletions
				
			
		| 
						 | 
				
			
			@ -213,10 +213,14 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
 | 
			
		|||
		return (0);
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if (port->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return (0);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	spin_lock_irqsave(&priv->lock, flags);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -224,6 +228,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
 | 
			
		|||
		/* To much data for buffer. Reset buffer. */
 | 
			
		||||
		priv->wrfilled=0;
 | 
			
		||||
		spin_unlock_irqrestore(&priv->lock, flags);
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
		return (0);
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -268,6 +273,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
 | 
			
		|||
			priv->wrfilled=0;
 | 
			
		||||
			priv->wrsent=0;
 | 
			
		||||
			spin_unlock_irqrestore(&priv->lock, flags);
 | 
			
		||||
			port->write_urb_busy = 0;
 | 
			
		||||
			return 0;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -413,6 +419,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
 | 
			
		|||
 | 
			
		||||
	dbg("%s - port %d", __FUNCTION__, port->number);
 | 
			
		||||
 | 
			
		||||
	port->write_urb_busy = 0;
 | 
			
		||||
	if (urb->status) {
 | 
			
		||||
		dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
 | 
			
		||||
		return;
 | 
			
		||||
| 
						 | 
				
			
			@ -424,12 +431,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
 | 
			
		|||
	if( priv->wrfilled ) {
 | 
			
		||||
		int length, blksize, result;
 | 
			
		||||
 | 
			
		||||
		if (port->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
			dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
			spin_unlock(&priv->lock);
 | 
			
		||||
			return;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		dbg("%s - transmitting data (frame n)", __FUNCTION__);
 | 
			
		||||
 | 
			
		||||
		length = ((priv->wrfilled - priv->wrsent) > port->bulk_out_size) ?
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -174,10 +174,14 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
 | 
			
		|||
 | 
			
		||||
	/* only do something if we have a bulk out endpoint */
 | 
			
		||||
	if (serial->num_bulk_out) {
 | 
			
		||||
		if (port->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
		spin_lock(&port->lock);
 | 
			
		||||
		if (port->write_urb_busy) {
 | 
			
		||||
			spin_unlock(&port->lock);
 | 
			
		||||
			dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
			return (0);
 | 
			
		||||
			return 0;
 | 
			
		||||
		}
 | 
			
		||||
		port->write_urb_busy = 1;
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
		count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -195,17 +199,20 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
 | 
			
		|||
				     usb_serial_generic_write_bulk_callback), port);
 | 
			
		||||
 | 
			
		||||
		/* send the data out the bulk port */
 | 
			
		||||
		port->write_urb_busy = 1;
 | 
			
		||||
		result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
 | 
			
		||||
		if (result)
 | 
			
		||||
		if (result) {
 | 
			
		||||
			dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
 | 
			
		||||
		else
 | 
			
		||||
			/* don't have to grab the lock here, as we will retry if != 0 */
 | 
			
		||||
			port->write_urb_busy = 0;
 | 
			
		||||
		} else
 | 
			
		||||
			result = count;
 | 
			
		||||
 | 
			
		||||
		return result;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	/* no bulk out, so return 0 bytes written */
 | 
			
		||||
	return (0);
 | 
			
		||||
	return 0;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
int usb_serial_generic_write_room (struct usb_serial_port *port)
 | 
			
		||||
| 
						 | 
				
			
			@ -216,7 +223,7 @@ int usb_serial_generic_write_room (struct usb_serial_port *port)
 | 
			
		|||
	dbg("%s - port %d", __FUNCTION__, port->number);
 | 
			
		||||
 | 
			
		||||
	if (serial->num_bulk_out) {
 | 
			
		||||
		if (port->write_urb->status != -EINPROGRESS)
 | 
			
		||||
		if (port->write_urb_busy)
 | 
			
		||||
			room = port->bulk_out_size;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -232,7 +239,7 @@ int usb_serial_generic_chars_in_buffer (struct usb_serial_port *port)
 | 
			
		|||
	dbg("%s - port %d", __FUNCTION__, port->number);
 | 
			
		||||
 | 
			
		||||
	if (serial->num_bulk_out) {
 | 
			
		||||
		if (port->write_urb->status == -EINPROGRESS)
 | 
			
		||||
		if (port->write_urb_busy)
 | 
			
		||||
			chars = port->write_urb->transfer_buffer_length;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -291,6 +298,7 @@ void usb_serial_generic_write_bulk_callback (struct urb *urb, struct pt_regs *re
 | 
			
		|||
 | 
			
		||||
	dbg("%s - port %d", __FUNCTION__, port->number);
 | 
			
		||||
 | 
			
		||||
	port->write_urb_busy = 0;
 | 
			
		||||
	if (urb->status) {
 | 
			
		||||
		dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
 | 
			
		||||
		return;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -818,11 +818,6 @@ static void ipaq_write_gather(struct usb_serial_port *port)
 | 
			
		|||
	struct ipaq_packet	*pkt, *tmp;
 | 
			
		||||
	struct urb		*urb = port->write_urb;
 | 
			
		||||
 | 
			
		||||
	if (urb->status == -EINPROGRESS) {
 | 
			
		||||
		/* Should never happen */
 | 
			
		||||
		err("%s - flushing while urb is active !", __FUNCTION__);
 | 
			
		||||
		return;
 | 
			
		||||
	}
 | 
			
		||||
	room = URBDATA_SIZE;
 | 
			
		||||
	list_for_each_entry_safe(pkt, tmp, &priv->queue, list) {
 | 
			
		||||
		count = min(room, (int)(pkt->len - pkt->written));
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -400,9 +400,14 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
 | 
			
		|||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	/* Racy and broken, FIXME properly! */
 | 
			
		||||
	if (port->write_urb->status == -EINPROGRESS)
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	count = min(count, port->bulk_out_size);
 | 
			
		||||
	memcpy(port->bulk_out_buffer, buf, count);
 | 
			
		||||
| 
						 | 
				
			
			@ -418,6 +423,7 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
 | 
			
		|||
 | 
			
		||||
	ret = usb_submit_urb(port->write_urb, GFP_ATOMIC);
 | 
			
		||||
	if (ret != 0) {
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
		dbg("%s - usb_submit_urb(write bulk) failed with error = %d", __FUNCTION__, ret);
 | 
			
		||||
		return ret;
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -341,10 +341,14 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
 | 
			
		|||
	if (count == 0)
 | 
			
		||||
		return 0;
 | 
			
		||||
 | 
			
		||||
	if (port->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
		dbg ("%s - already writing", __FUNCTION__);
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	transfer_buffer = port->write_urb->transfer_buffer;
 | 
			
		||||
	transfer_size = min(count, port->bulk_out_size - 1);
 | 
			
		||||
| 
						 | 
				
			
			@ -374,9 +378,10 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
 | 
			
		|||
	port->write_urb->transfer_flags = URB_ZERO_PACKET;
 | 
			
		||||
 | 
			
		||||
	result = usb_submit_urb (port->write_urb, GFP_ATOMIC);
 | 
			
		||||
	if (result)
 | 
			
		||||
	if (result) {
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
		dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
 | 
			
		||||
	else
 | 
			
		||||
	} else
 | 
			
		||||
		result = transfer_size;
 | 
			
		||||
 | 
			
		||||
	return result;
 | 
			
		||||
| 
						 | 
				
			
			@ -388,6 +393,7 @@ static void ir_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
 | 
			
		|||
 | 
			
		||||
	dbg("%s - port %d", __FUNCTION__, port->number);
 | 
			
		||||
 | 
			
		||||
	port->write_urb_busy = 0;
 | 
			
		||||
	if (urb->status) {
 | 
			
		||||
		dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
 | 
			
		||||
		return;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -520,9 +520,13 @@ static int keyspan_pda_write(struct usb_serial_port *port,
 | 
			
		|||
	   the TX urb is in-flight (wait until it completes)
 | 
			
		||||
	   the device is full (wait until it says there is room)
 | 
			
		||||
	*/
 | 
			
		||||
	if (port->write_urb->status == -EINPROGRESS || priv->tx_throttled ) {
 | 
			
		||||
		return( 0 );
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy || priv->tx_throttled) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	/* At this point the URB is in our control, nobody else can submit it
 | 
			
		||||
	   again (the only sudden transition was the one from EINPROGRESS to
 | 
			
		||||
| 
						 | 
				
			
			@ -593,6 +597,8 @@ static int keyspan_pda_write(struct usb_serial_port *port,
 | 
			
		|||
 | 
			
		||||
	rc = count;
 | 
			
		||||
exit:
 | 
			
		||||
	if (rc < 0)
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
	return rc;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -602,6 +608,7 @@ static void keyspan_pda_write_bulk_callback (struct urb *urb, struct pt_regs *re
 | 
			
		|||
	struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
 | 
			
		||||
	struct keyspan_pda_private *priv;
 | 
			
		||||
 | 
			
		||||
	port->write_urb_busy = 0;
 | 
			
		||||
	priv = usb_get_serial_port_data(port);
 | 
			
		||||
 | 
			
		||||
	/* queue up a wakeup at scheduler time */
 | 
			
		||||
| 
						 | 
				
			
			@ -631,7 +638,7 @@ static int keyspan_pda_chars_in_buffer (struct usb_serial_port *port)
 | 
			
		|||
 | 
			
		||||
	/* when throttled, return at least WAKEUP_CHARS to tell select() (via
 | 
			
		||||
	   n_tty.c:normal_poll() ) that we're not writeable. */
 | 
			
		||||
	if( port->write_urb->status == -EINPROGRESS || priv->tx_throttled )
 | 
			
		||||
	if (port->write_urb_busy || priv->tx_throttled)
 | 
			
		||||
		return 256;
 | 
			
		||||
	return 0;
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -254,10 +254,15 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
 | 
			
		|||
		dbg("%s - write request of 0 bytes", __FUNCTION__);
 | 
			
		||||
		return (0);
 | 
			
		||||
	}
 | 
			
		||||
	if (wport->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return (0);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count;
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -275,9 +280,10 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
 | 
			
		|||
 | 
			
		||||
	wport->write_urb->dev = serial->dev;
 | 
			
		||||
	result = usb_submit_urb(wport->write_urb, GFP_ATOMIC);
 | 
			
		||||
	if (result)
 | 
			
		||||
	if (result) {
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
		err("%s - failed submitting write urb, error %d", __FUNCTION__, result);
 | 
			
		||||
	else
 | 
			
		||||
	} else
 | 
			
		||||
		result = count;
 | 
			
		||||
 | 
			
		||||
	return result;
 | 
			
		||||
| 
						 | 
				
			
			@ -291,7 +297,7 @@ static int omninet_write_room (struct usb_serial_port *port)
 | 
			
		|||
 | 
			
		||||
	int room = 0; // Default: no room
 | 
			
		||||
 | 
			
		||||
	if (wport->write_urb->status != -EINPROGRESS)
 | 
			
		||||
	if (wport->write_urb_busy)
 | 
			
		||||
		room = wport->bulk_out_size - OMNINET_HEADERLEN;
 | 
			
		||||
 | 
			
		||||
//	dbg("omninet_write_room returns %d", room);
 | 
			
		||||
| 
						 | 
				
			
			@ -306,6 +312,7 @@ static void omninet_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
 | 
			
		|||
 | 
			
		||||
//	dbg("omninet_write_bulk_callback, port %0x\n", port);
 | 
			
		||||
 | 
			
		||||
	port->write_urb_busy = 0;
 | 
			
		||||
	if (urb->status) {
 | 
			
		||||
		dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
 | 
			
		||||
		return;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -299,10 +299,14 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
 | 
			
		|||
		dbg ("%s - write request of 0 bytes", __FUNCTION__);
 | 
			
		||||
		return (0);
 | 
			
		||||
	}
 | 
			
		||||
	if (port->write_urb->status == -EINPROGRESS) {
 | 
			
		||||
		dbg ("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return (0);
 | 
			
		||||
	spin_lock(&port->lock);
 | 
			
		||||
	if (port->write_urb_busy) {
 | 
			
		||||
		spin_unlock(&port->lock);
 | 
			
		||||
		dbg("%s - already writing", __FUNCTION__);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
	port->write_urb_busy = 1;
 | 
			
		||||
	spin_unlock(&port->lock);
 | 
			
		||||
 | 
			
		||||
	packet_length = port->bulk_out_size;	// get max packetsize
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -354,6 +358,7 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
 | 
			
		|||
#endif
 | 
			
		||||
	port->write_urb->dev = port->serial->dev;
 | 
			
		||||
	if ((result = usb_submit_urb (port->write_urb, GFP_KERNEL))) {
 | 
			
		||||
		port->write_urb_busy = 0;
 | 
			
		||||
		err ("%s - failed submitting write urb, error %d", __FUNCTION__, result);
 | 
			
		||||
		return 0;
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			@ -368,7 +373,7 @@ static int safe_write_room (struct usb_serial_port *port)
 | 
			
		|||
 | 
			
		||||
	dbg ("%s", __FUNCTION__);
 | 
			
		||||
 | 
			
		||||
	if (port->write_urb->status != -EINPROGRESS)
 | 
			
		||||
	if (port->write_urb_busy)
 | 
			
		||||
		room = port->bulk_out_size - (safe ? 2 : 0);
 | 
			
		||||
 | 
			
		||||
	if (room) {
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1047,6 +1047,7 @@ int usb_serial_probe(struct usb_interface *interface,
 | 
			
		|||
		memset(port, 0x00, sizeof(struct usb_serial_port));
 | 
			
		||||
		port->number = i + serial->minor;
 | 
			
		||||
		port->serial = serial;
 | 
			
		||||
		spin_lock_init(&port->lock);
 | 
			
		||||
		INIT_WORK(&port->work, usb_serial_port_softint, port);
 | 
			
		||||
		serial->port[i] = port;
 | 
			
		||||
	}
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -69,6 +69,7 @@
 | 
			
		|||
 * usb_serial_port: structure for the specific ports of a device.
 | 
			
		||||
 * @serial: pointer back to the struct usb_serial owner of this port.
 | 
			
		||||
 * @tty: pointer to the corresponding tty for this port.
 | 
			
		||||
 * @lock: spinlock to grab when updating portions of this structure.
 | 
			
		||||
 * @number: the number of the port (the minor number).
 | 
			
		||||
 * @interrupt_in_buffer: pointer to the interrupt in buffer for this port.
 | 
			
		||||
 * @interrupt_in_urb: pointer to the interrupt in struct urb for this port.
 | 
			
		||||
| 
						 | 
				
			
			@ -98,6 +99,7 @@
 | 
			
		|||
struct usb_serial_port {
 | 
			
		||||
	struct usb_serial *	serial;
 | 
			
		||||
	struct tty_struct *	tty;
 | 
			
		||||
	spinlock_t		lock;
 | 
			
		||||
	unsigned char		number;
 | 
			
		||||
 | 
			
		||||
	unsigned char *		interrupt_in_buffer;
 | 
			
		||||
| 
						 | 
				
			
			@ -117,6 +119,7 @@ struct usb_serial_port {
 | 
			
		|||
	unsigned char *		bulk_out_buffer;
 | 
			
		||||
	int			bulk_out_size;
 | 
			
		||||
	struct urb *		write_urb;
 | 
			
		||||
	int			write_urb_busy;
 | 
			
		||||
	__u8			bulk_out_endpointAddress;
 | 
			
		||||
 | 
			
		||||
	wait_queue_head_t	write_wait;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue