USB: serial: remove multi-urb write from generic driver
Remove multi-urb write from the generic driver and simplify the prepare_write_buffer prototype: int (*prepare_write_buffer)(struct usb_serial_port *port, void *dest, size_t size); The default implementation simply fills dest with data from port write fifo but drivers can override it if they need to process the outgoing data (e.g. add headers). Turn ftdi_sio into a generic fifo-based driver, which lowers CPU usage significantly for small writes while retaining maximum throughput. Signed-off-by: Johan Hovold <jhovold@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This commit is contained in:
		
					parent
					
						
							
								27c7acf220
							
						
					
				
			
			
				commit
				
					
						c23e5fc1f7
					
				
			
		
					 4 changed files with 47 additions and 175 deletions
				
			
		|  | @ -84,9 +84,10 @@ static const struct usb_device_id id_table[] = { | |||
| MODULE_DEVICE_TABLE(usb, id_table); | ||||
| 
 | ||||
| static int aircable_prepare_write_buffer(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *src, size_t count) | ||||
| 						void *dest, size_t size) | ||||
| { | ||||
| 	unsigned char *buf = *dest; | ||||
| 	int count; | ||||
| 	unsigned char *buf = dest; | ||||
| 
 | ||||
| 	count = kfifo_out_locked(&port->write_fifo, buf + HCI_HEADER_LENGTH, | ||||
| 					size - HCI_HEADER_LENGTH, &port->lock); | ||||
|  | @ -185,8 +186,6 @@ static struct usb_serial_driver aircable_device = { | |||
| 	.id_table = 		id_table, | ||||
| 	.num_ports =		1, | ||||
| 	.bulk_out_size =	HCI_COMPLETE_FRAME, | ||||
| 	/* Must modify prepare_write_buffer if multi_urb_write is changed. */ | ||||
| 	.multi_urb_write =	0, | ||||
| 	.probe =		aircable_probe, | ||||
| 	.process_read_urb =	aircable_process_read_urb, | ||||
| 	.prepare_write_buffer =	aircable_prepare_write_buffer, | ||||
|  |  | |||
|  | @ -781,7 +781,7 @@ static void ftdi_close(struct usb_serial_port *port); | |||
| static void ftdi_dtr_rts(struct usb_serial_port *port, int on); | ||||
| static void ftdi_process_read_urb(struct urb *urb); | ||||
| static int ftdi_prepare_write_buffer(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *buf, size_t count); | ||||
| 						void *dest, size_t size); | ||||
| static void ftdi_set_termios(struct tty_struct *tty, | ||||
| 			struct usb_serial_port *port, struct ktermios *old); | ||||
| static int  ftdi_tiocmget(struct tty_struct *tty, struct file *file); | ||||
|  | @ -808,8 +808,7 @@ static struct usb_serial_driver ftdi_sio_device = { | |||
| 	.id_table =		id_table_combined, | ||||
| 	.num_ports =		1, | ||||
| 	.bulk_in_size =		512, | ||||
| 	/* Must modify prepare_write_buffer if multi_urb_write is changed. */ | ||||
| 	.multi_urb_write =	1, | ||||
| 	.bulk_out_size =	256, | ||||
| 	.probe =		ftdi_sio_probe, | ||||
| 	.port_probe =		ftdi_sio_port_probe, | ||||
| 	.port_remove =		ftdi_sio_port_remove, | ||||
|  | @ -1531,15 +1530,6 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port) | |||
| 		quirk->port_probe(priv); | ||||
| 
 | ||||
| 	priv->port = port; | ||||
| 
 | ||||
| 	/* Free port's existing write urb and transfer buffer. */ | ||||
| 	if (port->write_urb) { | ||||
| 		usb_free_urb(port->write_urb); | ||||
| 		port->write_urb = NULL; | ||||
| 	} | ||||
| 	kfree(port->bulk_out_buffer); | ||||
| 	port->bulk_out_buffer = NULL; | ||||
| 
 | ||||
| 	usb_set_serial_port_data(port, priv); | ||||
| 
 | ||||
| 	ftdi_determine_type(port); | ||||
|  | @ -1734,8 +1724,7 @@ static void ftdi_close(struct usb_serial_port *port) | |||
| 
 | ||||
| 	dbg("%s", __func__); | ||||
| 
 | ||||
| 	/* shutdown our bulk read */ | ||||
| 	usb_kill_urb(port->read_urb); | ||||
| 	usb_serial_generic_close(port); | ||||
| 	kref_put(&priv->kref, ftdi_sio_priv_release); | ||||
| } | ||||
| 
 | ||||
|  | @ -1747,40 +1736,34 @@ static void ftdi_close(struct usb_serial_port *port) | |||
|  * The new devices do not require this byte | ||||
|  */ | ||||
| static int ftdi_prepare_write_buffer(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *src, size_t count) | ||||
| 						void *dest, size_t size) | ||||
| { | ||||
| 	struct ftdi_private *priv; | ||||
| 	unsigned char *buffer; | ||||
| 	int len; | ||||
| 	int count; | ||||
| 	unsigned long flags; | ||||
| 
 | ||||
| 	priv = usb_get_serial_port_data(port); | ||||
| 
 | ||||
| 	len = count; | ||||
| 	if (priv->chip_type == SIO && count != 0) | ||||
| 		len += ((count - 1) / (priv->max_packet_size - 1)) + 1; | ||||
| 
 | ||||
| 	buffer = kmalloc(len, GFP_ATOMIC); | ||||
| 	if (!buffer) { | ||||
| 		dev_err(&port->dev, "%s - could not allocate buffer\n", | ||||
| 				__func__); | ||||
| 		return -ENOMEM; | ||||
| 	} | ||||
| 
 | ||||
| 	if (priv->chip_type == SIO) { | ||||
| 		int i, msg_len; | ||||
| 		unsigned char *buffer = dest; | ||||
| 		int i, len, c; | ||||
| 
 | ||||
| 		for (i = 0; i < len; i += priv->max_packet_size) { | ||||
| 			msg_len = min_t(int, len - i, priv->max_packet_size) - 1; | ||||
| 			buffer[i] = (msg_len << 2) + 1; | ||||
| 			memcpy(&buffer[i + 1], src, msg_len); | ||||
| 			src += msg_len; | ||||
| 		count = 0; | ||||
| 		spin_lock_irqsave(&port->lock, flags); | ||||
| 		for (i = 0; i < size - 1; i += priv->max_packet_size) { | ||||
| 			len = min_t(int, size - i, priv->max_packet_size) - 1; | ||||
| 			buffer[i] = (len << 2) + 1; | ||||
| 			c = kfifo_out(&port->write_fifo, &buffer[i + 1], len); | ||||
| 			if (!c) | ||||
| 				break; | ||||
| 			count += c + 1; | ||||
| 		} | ||||
| 		spin_unlock_irqrestore(&port->lock, flags); | ||||
| 	} else { | ||||
| 		memcpy(buffer, src, count); | ||||
| 		count = kfifo_out_locked(&port->write_fifo, dest, size, | ||||
| 								&port->lock); | ||||
| 	} | ||||
| 
 | ||||
| 	*dest = buffer; | ||||
| 
 | ||||
| 	return count; | ||||
| } | ||||
| 
 | ||||
|  |  | |||
|  | @ -26,8 +26,6 @@ | |||
| 
 | ||||
| static int debug; | ||||
| 
 | ||||
| #define MAX_TX_URBS		40 | ||||
| 
 | ||||
| #ifdef CONFIG_USB_SERIAL_GENERIC | ||||
| 
 | ||||
| static int generic_probe(struct usb_interface *interface, | ||||
|  | @ -172,90 +170,9 @@ void usb_serial_generic_close(struct usb_serial_port *port) | |||
| EXPORT_SYMBOL_GPL(usb_serial_generic_close); | ||||
| 
 | ||||
| int usb_serial_generic_prepare_write_buffer(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *src, size_t count) | ||||
| 						void *dest, size_t size) | ||||
| { | ||||
| 	if (!*dest) { | ||||
| 		size = count; | ||||
| 		*dest = kmalloc(count, GFP_ATOMIC); | ||||
| 		if (!*dest) { | ||||
| 			dev_err(&port->dev, "%s - could not allocate buffer\n", | ||||
| 					__func__); | ||||
| 			return -ENOMEM; | ||||
| 		} | ||||
| 	} | ||||
| 	if (src) { | ||||
| 		count = size; | ||||
| 		memcpy(*dest, src, size); | ||||
| 	} else { | ||||
| 		count = kfifo_out_locked(&port->write_fifo, *dest, size, | ||||
| 								&port->lock); | ||||
| 	} | ||||
| 	return count; | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(usb_serial_generic_prepare_write_buffer); | ||||
| 
 | ||||
| static int usb_serial_multi_urb_write(struct tty_struct *tty, | ||||
| 	struct usb_serial_port *port, const unsigned char *buf, int count) | ||||
| { | ||||
| 	unsigned long flags; | ||||
| 	struct urb *urb; | ||||
| 	void *buffer; | ||||
| 	int status; | ||||
| 
 | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	if (port->tx_urbs == MAX_TX_URBS) { | ||||
| 		spin_unlock_irqrestore(&port->lock, flags); | ||||
| 		dbg("%s - write limit hit", __func__); | ||||
| 		return 0; | ||||
| 	} | ||||
| 	port->tx_urbs++; | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	urb = usb_alloc_urb(0, GFP_ATOMIC); | ||||
| 	if (!urb) { | ||||
| 		dev_err(&port->dev, "%s - no free urbs available\n", __func__); | ||||
| 		status = -ENOMEM; | ||||
| 		goto err_urb; | ||||
| 	} | ||||
| 
 | ||||
| 	buffer = NULL; | ||||
| 	count = min_t(int, count, PAGE_SIZE); | ||||
| 	count = port->serial->type->prepare_write_buffer(port, &buffer, 0, | ||||
| 								buf, count); | ||||
| 	if (count < 0) { | ||||
| 		status = count; | ||||
| 		goto err_buf; | ||||
| 	} | ||||
| 	usb_serial_debug_data(debug, &port->dev, __func__, count, buffer); | ||||
| 	usb_fill_bulk_urb(urb, port->serial->dev, | ||||
| 			usb_sndbulkpipe(port->serial->dev, | ||||
| 					port->bulk_out_endpointAddress), | ||||
| 			buffer, count, | ||||
| 			port->serial->type->write_bulk_callback, port); | ||||
| 
 | ||||
| 	status = usb_submit_urb(urb, GFP_ATOMIC); | ||||
| 	if (status) { | ||||
| 		dev_err(&port->dev, "%s - error submitting urb: %d\n", | ||||
| 				__func__, status); | ||||
| 		goto err; | ||||
| 	} | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	port->tx_bytes += urb->transfer_buffer_length; | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	usb_free_urb(urb); | ||||
| 
 | ||||
| 	return count; | ||||
| err: | ||||
| 	kfree(buffer); | ||||
| err_buf: | ||||
| 	usb_free_urb(urb); | ||||
| err_urb: | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	port->tx_urbs--; | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	return status; | ||||
| 	return kfifo_out_locked(&port->write_fifo, dest, size, &port->lock); | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  | @ -286,8 +203,8 @@ retry: | |||
| 
 | ||||
| 	urb = port->write_urbs[i]; | ||||
| 	count = port->serial->type->prepare_write_buffer(port, | ||||
| 						&urb->transfer_buffer, | ||||
| 						port->bulk_out_size, NULL, 0); | ||||
| 						urb->transfer_buffer, | ||||
| 						port->bulk_out_size); | ||||
| 	urb->transfer_buffer_length = count; | ||||
| 	usb_serial_debug_data(debug, &port->dev, __func__, count, | ||||
| 						urb->transfer_buffer); | ||||
|  | @ -328,7 +245,6 @@ retry: | |||
| int usb_serial_generic_write(struct tty_struct *tty, | ||||
| 	struct usb_serial_port *port, const unsigned char *buf, int count) | ||||
| { | ||||
| 	struct usb_serial *serial = port->serial; | ||||
| 	int result; | ||||
| 
 | ||||
| 	dbg("%s - port %d", __func__, port->number); | ||||
|  | @ -340,23 +256,18 @@ int usb_serial_generic_write(struct tty_struct *tty, | |||
| 	if (!count) | ||||
| 		return 0; | ||||
| 
 | ||||
| 	if (serial->type->multi_urb_write) | ||||
| 		return usb_serial_multi_urb_write(tty, port, buf, count); | ||||
| 
 | ||||
| 	count = kfifo_in_locked(&port->write_fifo, buf, count, &port->lock); | ||||
| 	result = usb_serial_generic_write_start(port); | ||||
| 	if (result) | ||||
| 		return result; | ||||
| 
 | ||||
| 	if (result >= 0) | ||||
| 		result = count; | ||||
| 
 | ||||
| 	return result; | ||||
| 	return count; | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(usb_serial_generic_write); | ||||
| 
 | ||||
| int usb_serial_generic_write_room(struct tty_struct *tty) | ||||
| { | ||||
| 	struct usb_serial_port *port = tty->driver_data; | ||||
| 	struct usb_serial *serial = port->serial; | ||||
| 	unsigned long flags; | ||||
| 	int room; | ||||
| 
 | ||||
|  | @ -366,10 +277,7 @@ int usb_serial_generic_write_room(struct tty_struct *tty) | |||
| 		return 0; | ||||
| 
 | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	if (serial->type->multi_urb_write) | ||||
| 		room = (MAX_TX_URBS - port->tx_urbs) * PAGE_SIZE; | ||||
| 	else | ||||
| 		room = kfifo_avail(&port->write_fifo); | ||||
| 	room = kfifo_avail(&port->write_fifo); | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	dbg("%s - returns %d", __func__, room); | ||||
|  | @ -379,7 +287,6 @@ int usb_serial_generic_write_room(struct tty_struct *tty) | |||
| int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) | ||||
| { | ||||
| 	struct usb_serial_port *port = tty->driver_data; | ||||
| 	struct usb_serial *serial = port->serial; | ||||
| 	unsigned long flags; | ||||
| 	int chars; | ||||
| 
 | ||||
|  | @ -389,10 +296,7 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) | |||
| 		return 0; | ||||
| 
 | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	if (serial->type->multi_urb_write) | ||||
| 		chars = port->tx_bytes; | ||||
| 	else | ||||
| 		chars = kfifo_len(&port->write_fifo) + port->tx_bytes; | ||||
| 	chars = kfifo_len(&port->write_fifo) + port->tx_bytes; | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	dbg("%s - returns %d", __func__, chars); | ||||
|  | @ -479,35 +383,25 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) | |||
| 
 | ||||
| 	dbg("%s - port %d", __func__, port->number); | ||||
| 
 | ||||
| 	if (port->serial->type->multi_urb_write) { | ||||
| 		kfree(urb->transfer_buffer); | ||||
| 	for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) | ||||
| 		if (port->write_urbs[i] == urb) | ||||
| 			break; | ||||
| 
 | ||||
| 	spin_lock_irqsave(&port->lock, flags); | ||||
| 	port->tx_bytes -= urb->transfer_buffer_length; | ||||
| 	set_bit(i, &port->write_urbs_free); | ||||
| 	spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 	if (status) { | ||||
| 		dbg("%s - non-zero urb status: %d", __func__, status); | ||||
| 
 | ||||
| 		spin_lock_irqsave(&port->lock, flags); | ||||
| 		port->tx_bytes -= urb->transfer_buffer_length; | ||||
| 		port->tx_urbs--; | ||||
| 		kfifo_reset_out(&port->write_fifo); | ||||
| 		spin_unlock_irqrestore(&port->lock, flags); | ||||
| 	} else { | ||||
| 		for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) | ||||
| 			if (port->write_urbs[i] == urb) | ||||
| 				break; | ||||
| 
 | ||||
| 		spin_lock_irqsave(&port->lock, flags); | ||||
| 		port->tx_bytes -= urb->transfer_buffer_length; | ||||
| 		set_bit(i, &port->write_urbs_free); | ||||
| 		spin_unlock_irqrestore(&port->lock, flags); | ||||
| 
 | ||||
| 		if (status) { | ||||
| 			spin_lock_irqsave(&port->lock, flags); | ||||
| 			kfifo_reset_out(&port->write_fifo); | ||||
| 			spin_unlock_irqrestore(&port->lock, flags); | ||||
| 		} else { | ||||
| 			usb_serial_generic_write_start(port); | ||||
| 		} | ||||
| 		usb_serial_generic_write_start(port); | ||||
| 	} | ||||
| 
 | ||||
| 	if (status) | ||||
| 		dbg("%s - non-zero urb status: %d", __func__, status); | ||||
| 
 | ||||
| 	usb_serial_port_softint(port); | ||||
| } | ||||
| EXPORT_SYMBOL_GPL(usb_serial_generic_write_bulk_callback); | ||||
|  |  | |||
|  | @ -67,7 +67,6 @@ enum port_dev_state { | |||
|  * @write_urbs: pointers to the bulk out urbs for this port | ||||
|  * @write_urbs_free: status bitmap the for bulk out urbs | ||||
|  * @tx_bytes: number of bytes currently in host stack queues | ||||
|  * @tx_urbs: number of urbs currently in host stack queues | ||||
|  * @bulk_out_endpointAddress: endpoint address for the bulk out pipe for this | ||||
|  *	port. | ||||
|  * @flags: usb serial port flags | ||||
|  | @ -112,7 +111,6 @@ struct usb_serial_port { | |||
| 	__u8			bulk_out_endpointAddress; | ||||
| 
 | ||||
| 	int			tx_bytes; | ||||
| 	int			tx_urbs; | ||||
| 
 | ||||
| 	unsigned long		flags; | ||||
| 	wait_queue_head_t	write_wait; | ||||
|  | @ -238,8 +236,6 @@ struct usb_serial_driver { | |||
| 	struct usb_driver	*usb_driver; | ||||
| 	struct usb_dynids	dynids; | ||||
| 
 | ||||
| 	unsigned char		multi_urb_write:1; | ||||
| 
 | ||||
| 	size_t			bulk_in_size; | ||||
| 	size_t			bulk_out_size; | ||||
| 
 | ||||
|  | @ -291,7 +287,7 @@ struct usb_serial_driver { | |||
| 	void (*process_read_urb)(struct urb *urb); | ||||
| 	/* Called by the generic write implementation */ | ||||
| 	int (*prepare_write_buffer)(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *src, size_t count); | ||||
| 						void *dest, size_t size); | ||||
| }; | ||||
| #define to_usb_serial_driver(d) \ | ||||
| 	container_of(d, struct usb_serial_driver, driver) | ||||
|  | @ -345,7 +341,7 @@ extern int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, | |||
| 						 gfp_t mem_flags); | ||||
| extern void usb_serial_generic_process_read_urb(struct urb *urb); | ||||
| extern int usb_serial_generic_prepare_write_buffer(struct usb_serial_port *port, | ||||
| 		void **dest, size_t size, const void *src, size_t count); | ||||
| 						void *dest, size_t size); | ||||
| extern int usb_serial_handle_sysrq_char(struct tty_struct *tty, | ||||
| 					struct usb_serial_port *port, | ||||
| 					unsigned int ch); | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Johan Hovold
				Johan Hovold