Commit 27c7acf2 authored by Johan Hovold's avatar Johan Hovold Committed by Greg Kroah-Hartman

USB: serial: reimplement generic fifo-based writes

Reimplement fifo-based writes in the generic driver using a multiple
pre-allocated urb scheme.

In contrast to multi-urb writes, no allocations (of urbs or buffers) are
made during run-time and there is less pressure on the host stack
queues as currently only two urbs are used (implementation is generic
and can handle more than two urbs as well, though).

Initial tests using ftdi_sio show that the implementation achieves the
same (maximum) throughput at high baudrates as multi-urb writes. The CPU
usage is much lower than for multi-urb writes for small write requests
and only slightly higher for large (e.g. 2k) requests (due to extra copy
via fifo?).

Also outperforms multi-urb writes for small write requests on an
embedded arm-9 system, where multi-urb writes are CPU-bound at high
baudrates (perf reveals that a lot of time is spent in the host stack
enqueue function -- could perhaps be a bug as well).

Keeping the original write_urb, buffer and flag for now as there are
other drivers depending on them.
Signed-off-by: default avatarJohan Hovold <jhovold@gmail.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent 4272568b
/* /*
* USB Serial Converter Generic functions * USB Serial Converter Generic functions
* *
* Copyright (C) 2010 Johan Hovold (jhovold@gmail.com)
* Copyright (C) 1999 - 2002 Greg Kroah-Hartman (greg@kroah.com) * Copyright (C) 1999 - 2002 Greg Kroah-Hartman (greg@kroah.com)
* *
* This program is free software; you can redistribute it and/or * This program is free software; you can redistribute it and/or
...@@ -143,6 +144,7 @@ static void generic_cleanup(struct usb_serial_port *port) ...@@ -143,6 +144,7 @@ static void generic_cleanup(struct usb_serial_port *port)
{ {
struct usb_serial *serial = port->serial; struct usb_serial *serial = port->serial;
unsigned long flags; unsigned long flags;
int i;
dbg("%s - port %d", __func__, port->number); dbg("%s - port %d", __func__, port->number);
...@@ -150,6 +152,8 @@ static void generic_cleanup(struct usb_serial_port *port) ...@@ -150,6 +152,8 @@ static void generic_cleanup(struct usb_serial_port *port)
/* shutdown any bulk transfers that might be going on */ /* shutdown any bulk transfers that might be going on */
if (port->bulk_out_size) { if (port->bulk_out_size) {
usb_kill_urb(port->write_urb); usb_kill_urb(port->write_urb);
for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
usb_kill_urb(port->write_urbs[i]);
spin_lock_irqsave(&port->lock, flags); spin_lock_irqsave(&port->lock, flags);
kfifo_reset_out(&port->write_fifo); kfifo_reset_out(&port->write_fifo);
...@@ -258,46 +262,56 @@ static int usb_serial_multi_urb_write(struct tty_struct *tty, ...@@ -258,46 +262,56 @@ static int usb_serial_multi_urb_write(struct tty_struct *tty,
* usb_serial_generic_write_start - kick off an URB write * usb_serial_generic_write_start - kick off an URB write
* @port: Pointer to the &struct usb_serial_port data * @port: Pointer to the &struct usb_serial_port data
* *
* Returns the number of bytes queued on success. This will be zero if there * Returns zero on success, or a negative errno value
* was nothing to send. Otherwise, it returns a negative errno value
*/ */
static int usb_serial_generic_write_start(struct usb_serial_port *port) static int usb_serial_generic_write_start(struct usb_serial_port *port)
{ {
int result; struct urb *urb;
int count; int count, result;
unsigned long flags; unsigned long flags;
int i;
if (test_and_set_bit_lock(USB_SERIAL_WRITE_BUSY, &port->flags))
return 0;
retry:
spin_lock_irqsave(&port->lock, flags); spin_lock_irqsave(&port->lock, flags);
if (port->write_urb_busy || !kfifo_len(&port->write_fifo)) { if (!port->write_urbs_free || !kfifo_len(&port->write_fifo)) {
clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
spin_unlock_irqrestore(&port->lock, flags); spin_unlock_irqrestore(&port->lock, flags);
return 0; return 0;
} }
port->write_urb_busy = 1; i = (int)find_first_bit(&port->write_urbs_free,
ARRAY_SIZE(port->write_urbs));
spin_unlock_irqrestore(&port->lock, flags); spin_unlock_irqrestore(&port->lock, flags);
urb = port->write_urbs[i];
count = port->serial->type->prepare_write_buffer(port, count = port->serial->type->prepare_write_buffer(port,
&port->write_urb->transfer_buffer, &urb->transfer_buffer,
port->bulk_out_size, NULL, 0); port->bulk_out_size, NULL, 0);
usb_serial_debug_data(debug, &port->dev, __func__, urb->transfer_buffer_length = count;
count, port->write_urb->transfer_buffer); usb_serial_debug_data(debug, &port->dev, __func__, count,
port->write_urb->transfer_buffer_length = count; urb->transfer_buffer);
result = usb_submit_urb(urb, GFP_ATOMIC);
/* send the data out the bulk port */
result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
if (result) { if (result) {
dev_err(&port->dev, "%s - error submitting urb: %d\n", dev_err(&port->dev, "%s - error submitting urb: %d\n",
__func__, result); __func__, result);
/* don't have to grab the lock here, as we will clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
retry if != 0 */
port->write_urb_busy = 0;
return result; return result;
} }
clear_bit(i, &port->write_urbs_free);
spin_lock_irqsave(&port->lock, flags); spin_lock_irqsave(&port->lock, flags);
port->tx_bytes += count; port->tx_bytes += count;
spin_unlock_irqrestore(&port->lock, flags); spin_unlock_irqrestore(&port->lock, flags);
return count; /* Try sending off another urb, unless in irq context (in which case
* there will be no free urb). */
if (!in_irq())
goto retry;
clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
return 0;
} }
/** /**
...@@ -461,6 +475,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) ...@@ -461,6 +475,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
unsigned long flags; unsigned long flags;
struct usb_serial_port *port = urb->context; struct usb_serial_port *port = urb->context;
int status = urb->status; int status = urb->status;
int i;
dbg("%s - port %d", __func__, port->number); dbg("%s - port %d", __func__, port->number);
...@@ -472,9 +487,13 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) ...@@ -472,9 +487,13 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
port->tx_urbs--; port->tx_urbs--;
spin_unlock_irqrestore(&port->lock, flags); spin_unlock_irqrestore(&port->lock, flags);
} else { } else {
for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
if (port->write_urbs[i] == urb)
break;
spin_lock_irqsave(&port->lock, flags); spin_lock_irqsave(&port->lock, flags);
port->tx_bytes -= urb->transfer_buffer_length; port->tx_bytes -= urb->transfer_buffer_length;
port->write_urb_busy = 0; set_bit(i, &port->write_urbs_free);
spin_unlock_irqrestore(&port->lock, flags); spin_unlock_irqrestore(&port->lock, flags);
if (status) { if (status) {
...@@ -576,7 +595,7 @@ int usb_serial_generic_resume(struct usb_serial *serial) ...@@ -576,7 +595,7 @@ int usb_serial_generic_resume(struct usb_serial *serial)
c++; c++;
} }
if (port->write_urb) { if (port->bulk_out_size) {
r = usb_serial_generic_write_start(port); r = usb_serial_generic_write_start(port);
if (r < 0) if (r < 0)
c++; c++;
......
...@@ -548,8 +548,12 @@ static void usb_serial_port_work(struct work_struct *work) ...@@ -548,8 +548,12 @@ static void usb_serial_port_work(struct work_struct *work)
static void kill_traffic(struct usb_serial_port *port) static void kill_traffic(struct usb_serial_port *port)
{ {
int i;
usb_kill_urb(port->read_urb); usb_kill_urb(port->read_urb);
usb_kill_urb(port->write_urb); usb_kill_urb(port->write_urb);
for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i)
usb_kill_urb(port->write_urbs[i]);
/* /*
* This is tricky. * This is tricky.
* Some drivers submit the read_urb in the * Some drivers submit the read_urb in the
...@@ -568,6 +572,7 @@ static void kill_traffic(struct usb_serial_port *port) ...@@ -568,6 +572,7 @@ static void kill_traffic(struct usb_serial_port *port)
static void port_release(struct device *dev) static void port_release(struct device *dev)
{ {
struct usb_serial_port *port = to_usb_serial_port(dev); struct usb_serial_port *port = to_usb_serial_port(dev);
int i;
dbg ("%s - %s", __func__, dev_name(dev)); dbg ("%s - %s", __func__, dev_name(dev));
...@@ -582,6 +587,10 @@ static void port_release(struct device *dev) ...@@ -582,6 +587,10 @@ static void port_release(struct device *dev)
usb_free_urb(port->write_urb); usb_free_urb(port->write_urb);
usb_free_urb(port->interrupt_in_urb); usb_free_urb(port->interrupt_in_urb);
usb_free_urb(port->interrupt_out_urb); usb_free_urb(port->interrupt_out_urb);
for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) {
usb_free_urb(port->write_urbs[i]);
kfree(port->bulk_out_buffers[i]);
}
kfifo_free(&port->write_fifo); kfifo_free(&port->write_fifo);
kfree(port->bulk_in_buffer); kfree(port->bulk_in_buffer);
kfree(port->bulk_out_buffer); kfree(port->bulk_out_buffer);
...@@ -920,6 +929,8 @@ int usb_serial_probe(struct usb_interface *interface, ...@@ -920,6 +929,8 @@ int usb_serial_probe(struct usb_interface *interface,
} }
for (i = 0; i < num_bulk_out; ++i) { for (i = 0; i < num_bulk_out; ++i) {
int j;
endpoint = bulk_out_endpoint[i]; endpoint = bulk_out_endpoint[i];
port = serial->port[i]; port = serial->port[i];
port->write_urb = usb_alloc_urb(0, GFP_KERNEL); port->write_urb = usb_alloc_urb(0, GFP_KERNEL);
...@@ -945,6 +956,28 @@ int usb_serial_probe(struct usb_interface *interface, ...@@ -945,6 +956,28 @@ int usb_serial_probe(struct usb_interface *interface,
endpoint->bEndpointAddress), endpoint->bEndpointAddress),
port->bulk_out_buffer, buffer_size, port->bulk_out_buffer, buffer_size,
serial->type->write_bulk_callback, port); serial->type->write_bulk_callback, port);
for (j = 0; j < ARRAY_SIZE(port->write_urbs); ++j) {
set_bit(j, &port->write_urbs_free);
port->write_urbs[j] = usb_alloc_urb(0, GFP_KERNEL);
if (!port->write_urbs[j]) {
dev_err(&interface->dev,
"No free urbs available\n");
goto probe_error;
}
port->bulk_out_buffers[j] = kmalloc(buffer_size,
GFP_KERNEL);
if (!port->bulk_out_buffers[j]) {
dev_err(&interface->dev,
"Couldn't allocate bulk_out_buffer\n");
goto probe_error;
}
usb_fill_bulk_urb(port->write_urbs[j], dev,
usb_sndbulkpipe(dev,
endpoint->bEndpointAddress),
port->bulk_out_buffers[j], buffer_size,
serial->type->write_bulk_callback,
port);
}
} }
if (serial->type->read_int_callback) { if (serial->type->read_int_callback) {
......
...@@ -35,6 +35,9 @@ enum port_dev_state { ...@@ -35,6 +35,9 @@ enum port_dev_state {
PORT_UNREGISTERING, PORT_UNREGISTERING,
}; };
/* USB serial flags */
#define USB_SERIAL_WRITE_BUSY 0
/** /**
* usb_serial_port: structure for the specific ports of a device. * usb_serial_port: structure for the specific ports of a device.
* @serial: pointer back to the struct usb_serial owner of this port. * @serial: pointer back to the struct usb_serial owner of this port.
...@@ -60,10 +63,14 @@ enum port_dev_state { ...@@ -60,10 +63,14 @@ enum port_dev_state {
* @write_urb: pointer to the bulk out struct urb for this port. * @write_urb: pointer to the bulk out struct urb for this port.
* @write_fifo: kfifo used to buffer outgoing data * @write_fifo: kfifo used to buffer outgoing data
* @write_urb_busy: port`s writing status * @write_urb_busy: port`s writing status
* @bulk_out_buffers: pointers to the bulk out buffers for this port
* @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_bytes: number of bytes currently in host stack queues
* @tx_urbs: number of urbs 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 * @bulk_out_endpointAddress: endpoint address for the bulk out pipe for this
* port. * port.
* @flags: usb serial port flags
* @write_wait: a wait_queue_head_t used by the port. * @write_wait: a wait_queue_head_t used by the port.
* @work: work queue entry for the line discipline waking up. * @work: work queue entry for the line discipline waking up.
* @throttled: nonzero if the read urb is inactive to throttle the device * @throttled: nonzero if the read urb is inactive to throttle the device
...@@ -98,11 +105,16 @@ struct usb_serial_port { ...@@ -98,11 +105,16 @@ struct usb_serial_port {
struct urb *write_urb; struct urb *write_urb;
struct kfifo write_fifo; struct kfifo write_fifo;
int write_urb_busy; int write_urb_busy;
unsigned char *bulk_out_buffers[2];
struct urb *write_urbs[2];
unsigned long write_urbs_free;
__u8 bulk_out_endpointAddress; __u8 bulk_out_endpointAddress;
int tx_bytes; int tx_bytes;
int tx_urbs; int tx_urbs;
unsigned long flags;
wait_queue_head_t write_wait; wait_queue_head_t write_wait;
struct work_struct work; struct work_struct work;
char throttled; char throttled;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment