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
done 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 be a bug as well?).
Keeping the original write_urb, buffer and flag for now as there are
other drivers depending on them.
FIXME: no need to store pointers to urb buffers (can be reached from urbs)
Signed-off-by: Johan Hovold <jhovold@xxxxxxxxx>
---
drivers/usb/serial/generic.c | 58 +++++++++++++++++++++++++-------------
drivers/usb/serial/usb-serial.c | 33 ++++++++++++++++++++++
include/linux/usb/serial.h | 9 ++++++
3 files changed, 80 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 65f13c8..afc61aa 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -142,6 +142,7 @@ static void generic_cleanup(struct usb_serial_port *port)
{
struct usb_serial *serial = port->serial;
unsigned long flags;
+ int i;
dbg("%s - port %d", __func__, port->number);
@@ -149,6 +150,8 @@ static void generic_cleanup(struct usb_serial_port *port)
/* shutdown any bulk transfers that might be going on */
if (port->bulk_out_size) {
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);
kfifo_reset_out(&port->write_fifo);
@@ -257,46 +260,56 @@ err_urb:
* usb_serial_generic_write_start - kick off an URB write
* @port: Pointer to the &struct usb_serial_port data
*
- * Returns the number of bytes queued on success. This will be zero if there
- * was nothing to send. Otherwise, it returns a negative errno value
+ * Returns zero on success, or a negative errno value
*/
static int usb_serial_generic_write_start(struct usb_serial_port *port)
{
- int result;
- int count;
+ struct urb *urb;
+ int count, result;
unsigned long flags;
+ unsigned i;
+ if (test_and_set_bit_lock(USB_SERIAL_WRITE_BUSY, &port->flags))
+ return 0;
+retry:
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);
return 0;
}
- port->write_urb_busy = 1;
+ i = (unsigned)find_first_bit(&port->write_urbs_free,
+ ARRAY_SIZE(port->write_urbs));
spin_unlock_irqrestore(&port->lock, flags);
+ urb = port->write_urbs[i];
count = port->serial->type->prepare_write_buffer(port,
- &port->write_urb->transfer_buffer,
- port->bulk_out_size, NULL, 0);
- usb_serial_debug_data(debug, &port->dev, __func__,
- count, port->write_urb->transfer_buffer);
- port->write_urb->transfer_buffer_length = count;
-
- /* send the data out the bulk port */
- result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
+ &urb->transfer_buffer,
+ port->bulk_out_size, NULL, 0);
+ urb->transfer_buffer_length = count;
+ usb_serial_debug_data(debug, &port->dev, __func__, count,
+ urb->transfer_buffer);
+ result = usb_submit_urb(urb, GFP_ATOMIC);
if (result) {
dev_err(&port->dev, "%s - error submitting urb: %d\n",
__func__, result);
- /* don't have to grab the lock here, as we will
- retry if != 0 */
- port->write_urb_busy = 0;
+ clear_bit_unlock(USB_SERIAL_WRITE_BUSY, &port->flags);
return result;
}
+ clear_bit(i, &port->write_urbs_free);
spin_lock_irqsave(&port->lock, flags);
port->tx_bytes += count;
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;
}
/**
@@ -460,6 +473,7 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
unsigned long flags;
struct usb_serial_port *port = urb->context;
int status = urb->status;
+ unsigned i;
dbg("%s - port %d", __func__, port->number);
@@ -471,9 +485,13 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb)
port->tx_urbs--;
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;
- port->write_urb_busy = 0;
+ set_bit(i, &port->write_urbs_free);
spin_unlock_irqrestore(&port->lock, flags);
if (status) {
@@ -567,7 +585,7 @@ int usb_serial_generic_resume(struct usb_serial *serial)
c++;
}
- if (port->write_urb) {
+ if (port->bulk_out_size) {
r = usb_serial_generic_write_start(port);
if (r < 0)
c++;
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 8249fd8..941c2d4 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -548,8 +548,12 @@ static void usb_serial_port_work(struct work_struct *work)
static void kill_traffic(struct usb_serial_port *port)
{
+ int i;
+
usb_kill_urb(port->read_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.
* Some drivers submit the read_urb in the
@@ -568,6 +572,7 @@ static void kill_traffic(struct usb_serial_port *port)
static void port_release(struct device *dev)
{
struct usb_serial_port *port = to_usb_serial_port(dev);
+ int i;
dbg ("%s - %s", __func__, dev_name(dev));
@@ -582,6 +587,10 @@ static void port_release(struct device *dev)
usb_free_urb(port->write_urb);
usb_free_urb(port->interrupt_in_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);
kfree(port->bulk_in_buffer);
kfree(port->bulk_out_buffer);
@@ -920,6 +929,8 @@ int usb_serial_probe(struct usb_interface *interface,
}
for (i = 0; i < num_bulk_out; ++i) {
+ int j;
+
endpoint = bulk_out_endpoint[i];
port = serial->port[i];
port->write_urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -945,6 +956,28 @@ int usb_serial_probe(struct usb_interface *interface,
endpoint->bEndpointAddress),
port->bulk_out_buffer, buffer_size,
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) {
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index a4c99ea..b88afbc 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -35,6 +35,10 @@ enum port_dev_state {
PORT_UNREGISTERING,
};
+/* USB serial flags. */
+#define USB_SERIAL_WRITE_BUSY 0
+/* FIXME: could add throttle flags */
+
/**
* usb_serial_port: structure for the specific ports of a device.
* @serial: pointer back to the struct usb_serial owner of this port.
@@ -100,6 +104,11 @@ struct usb_serial_port {
int write_urb_busy;
__u8 bulk_out_endpointAddress;
+ unsigned char *bulk_out_buffers[2];
+ struct urb *write_urbs[2];
+ unsigned long write_urbs_free;
+ unsigned long flags;
+
int tx_bytes;
int tx_urbs;
--
1.7.0.2
--
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
