aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPhilip Oberstaller <Philip.Oberstaller@septentrio.com>2015-03-27 12:42:18 -0400
committerFelipe Balbi <balbi@ti.com>2015-04-27 15:44:29 -0400
commit3e9d3d2efc677b501b12512cab5adb4f32a0673a (patch)
tree99374f697bbd56447003e3f42d9cbf8baafcd0c5
parentf286d487e9283a42a8844659bb5552b3f1bf6a7d (diff)
usb: gadget: serial: fix re-ordering of tx data
When a single thread is sending out data over the gadget serial port, gs_start_tx() will be called both from the sender context and from the write completion. Since the port lock is released before the packet is queued, the order in which the URBs are submitted is not guaranteed. E.g. sending thread completion (interrupt) gs_write() LOCK gs_write_complete() LOCK (wait) gs_start_tx() req1 = list_entry(pool->next) UNLOCK LOCK (acquired) gs_start_tx() req2 = list_entry(pool->next) UNLOCK usb_ep_queue(req2) usb_ep_queue(req1) I.e., req2 is submitted before req1 but it contains the data that comes after req1. To reproduce, use SMP with sending thread and completion pinned to different CPUs, or use PREEMPT_RT, and add the following delay just before the call to usb_ep_queue(): if (port->write_started > 0 && !list_empty(pool)) udelay(1000); To work around this problem, make sure that only one thread is running through the gs_start_tx() loop with an extra flag write_busy. Since gs_start_tx() is always called with the port lock held, no further synchronisation is needed. The original caller will continue through the loop when the request was successfully submitted. Signed-off-by: Philip Oberstaller <Philip.Oberstaller@septentrio.com> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Signed-off-by: Felipe Balbi <balbi@ti.com>
-rw-r--r--drivers/usb/gadget/function/u_serial.c5
1 files changed, 4 insertions, 1 deletions
diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index 89179ab20c10..7ee057930ae7 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -113,6 +113,7 @@ struct gs_port {
113 int write_allocated; 113 int write_allocated;
114 struct gs_buf port_write_buf; 114 struct gs_buf port_write_buf;
115 wait_queue_head_t drain_wait; /* wait while writes drain */ 115 wait_queue_head_t drain_wait; /* wait while writes drain */
116 bool write_busy;
116 117
117 /* REVISIT this state ... */ 118 /* REVISIT this state ... */
118 struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */ 119 struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
@@ -363,7 +364,7 @@ __acquires(&port->port_lock)
363 int status = 0; 364 int status = 0;
364 bool do_tty_wake = false; 365 bool do_tty_wake = false;
365 366
366 while (!list_empty(pool)) { 367 while (!port->write_busy && !list_empty(pool)) {
367 struct usb_request *req; 368 struct usb_request *req;
368 int len; 369 int len;
369 370
@@ -393,9 +394,11 @@ __acquires(&port->port_lock)
393 * NOTE that we may keep sending data for a while after 394 * NOTE that we may keep sending data for a while after
394 * the TTY closed (dev->ioport->port_tty is NULL). 395 * the TTY closed (dev->ioport->port_tty is NULL).
395 */ 396 */
397 port->write_busy = true;
396 spin_unlock(&port->port_lock); 398 spin_unlock(&port->port_lock);
397 status = usb_ep_queue(in, req, GFP_ATOMIC); 399 status = usb_ep_queue(in, req, GFP_ATOMIC);
398 spin_lock(&port->port_lock); 400 spin_lock(&port->port_lock);
401 port->write_busy = false;
399 402
400 if (status) { 403 if (status) {
401 pr_debug("%s: %s %s err %d\n", 404 pr_debug("%s: %s %s err %d\n",