Commit 3e9d3d2e authored by Philip Oberstaller's avatar Philip Oberstaller Committed by Felipe Balbi

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: default avatarPhilip Oberstaller <Philip.Oberstaller@septentrio.com>
Signed-off-by: default avatarArnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: default avatarFelipe Balbi <balbi@ti.com>
parent f286d487
...@@ -113,6 +113,7 @@ struct gs_port { ...@@ -113,6 +113,7 @@ struct gs_port {
int write_allocated; int write_allocated;
struct gs_buf port_write_buf; struct gs_buf port_write_buf;
wait_queue_head_t drain_wait; /* wait while writes drain */ wait_queue_head_t drain_wait; /* wait while writes drain */
bool write_busy;
/* REVISIT this state ... */ /* REVISIT this state ... */
struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */ struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
...@@ -363,7 +364,7 @@ __acquires(&port->port_lock) ...@@ -363,7 +364,7 @@ __acquires(&port->port_lock)
int status = 0; int status = 0;
bool do_tty_wake = false; bool do_tty_wake = false;
while (!list_empty(pool)) { while (!port->write_busy && !list_empty(pool)) {
struct usb_request *req; struct usb_request *req;
int len; int len;
...@@ -393,9 +394,11 @@ __acquires(&port->port_lock) ...@@ -393,9 +394,11 @@ __acquires(&port->port_lock)
* NOTE that we may keep sending data for a while after * NOTE that we may keep sending data for a while after
* the TTY closed (dev->ioport->port_tty is NULL). * the TTY closed (dev->ioport->port_tty is NULL).
*/ */
port->write_busy = true;
spin_unlock(&port->port_lock); spin_unlock(&port->port_lock);
status = usb_ep_queue(in, req, GFP_ATOMIC); status = usb_ep_queue(in, req, GFP_ATOMIC);
spin_lock(&port->port_lock); spin_lock(&port->port_lock);
port->write_busy = false;
if (status) { if (status) {
pr_debug("%s: %s %s err %d\n", pr_debug("%s: %s %s err %d\n",
......
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