Hi,
When using the USB serial gadget it regularly occurs that USB packets
arrive at the host in the wrong order. I am using kernel version 3.4
with the corresponding RT patch on an ARM Cortex-A8 platform.
The problem appears to be in file 'drivers/usb/gadget/u_serial.c'.
It seems that kernel thread "irq/92-musb-hdr" and a userspace thread
have a race when sending data, in function 'gs_start_tx'. The kernel
thread calls 'gs_start_tx' from the callback function
'gs_write_complete', a userspace thread through 'gs_write' and
'gs_flush_chars'. The problem is triggered by the fact that the
spinlock 'port_lock' is dropped temporarily in 'gs_start_tx' during
the time 'usb_ep_queue' is executed. There might be good reasons,
which I am unaware off, to do that. So, suppose that the userspace
thread has just released the spinlock and the scheduler selects at
that moment the (normally) higher prioritized kernel thread, which was
waiting at the 'port_lock' in 'gs_write_complete' to run. This will
allow the kernel thread to overtake the sleeping userspace thread and
send out the "next" USB packet first!
I can't reproduce the problem if I disable the RT patch, but I think
that the problem might as well appear, if for instance an interrupt
just happens during the time the lock is dropped in 'gs_start_rx'. But
if you prove me wrong, I'm happy to post it in the RT mailing list
instead.
The provided patch is a diff to the mainline. I've introduced a flag
to mark when there is already a thread busy in the loop emptying the
pool in 'gs_start_tx'. In that case another thread would just skip the
loop. This seems to work for me but it might not be foolproof and not
elegant enough.
By checking the code of the mainline I didn't find any relevant change
that would indicate that the issue has been already solved in the
meantime. For the moment it would be too much work for me to make the
mainline runnable on my platform.
Please CC me and arnout@xxxxxxx as we are not subscribed.
Philip
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index b369292..d2682f5 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -116,6 +116,7 @@ struct gs_port {
int write_allocated;
struct gs_buf port_write_buf;
wait_queue_head_t drain_wait; /* wait while writes drain */
+ bool write_busy;
/* REVISIT this state ... */
struct usb_cdc_line_coding port_line_coding; /* 8-N-1 etc */
@@ -366,7 +367,7 @@ __acquires(&port->port_lock)
int status = 0;
bool do_tty_wake = false;
- while (!list_empty(pool)) {
+ while (!port->write_busy && !list_empty(pool)) {
struct usb_request *req;
int len;
@@ -396,9 +397,11 @@ __acquires(&port->port_lock)
* NOTE that we may keep sending data for a while after
* the TTY closed (dev->ioport->port_tty is NULL).
*/
+ port->write_busy = true;
spin_unlock(&port->port_lock);
status = usb_ep_queue(in, req, GFP_ATOMIC);
spin_lock(&port->port_lock);
+ port->write_busy = false;
if (status) {
pr_debug("%s: %s %s err %d\n",
@@ -770,6 +773,7 @@ static int gs_open(struct tty_struct *tty, struct file *file)
/* Do the "real open" */
spin_lock_irq(&port->port_lock);
+ port->write_busy = false;
/* allocate circular buffer on first open */
if (port->port_write_buf.buf_buf == NULL) {
