On SMP systems there is a synchronization issue in the gadget
serial driver which potentially causes a packet swap on some
occasions. In the gadget serial driver implementation, a transfer
is queued to the controller on the following two occasions.
1. Application requesting to send data through gadget serial driver:
2. Previously scheduled transfer is complete and a new transfer is
triggered from the callback function:
On SMP systems, sometimes there is a packet swap since the port lock
is being dropped briefly during queueing. On some occasions, as both
the completion routine and the application are trying to queue the data
at the same time, due to the lock being dropped, the packets gets
swapped before they get queued to the endpoint.
CPU1 CPU2
----------------------------------+----------------------------------
| u_serial submits request1
| to controller
|
| controller processes request1
| and sends it over the bus
|
Interrupt received for request1 |
completion. Completion forwarded |
to u_serial. |
|
|
gs_write_complete is invoked |
for request1 completion from |
interrupt context |
|
PORT LOCK ACQUIRED |
u_serial tries to queue request |
from the completion context by |
calling gs_start_tx. request2 |
is fetched from the list. |
| u_serial is ready to submit
| next request from the
| application context
| WAITING FOR LOCK
|
gs_start_tx releases port lock |
before request is queued to |
the endpoint. |
PORT LOCK RELEASED |
|
| Application context sneaks in
| and acquires lock in gs_write
| PORT LOCK ACQUIRED
| Fetches request3 from the list
| PORT LOCK RELEASED
| Queues request3 to endpoint
| PORT LOCK ACQUIRED
| Completes the request queueing
| request3 is queued ahead of request2
| PORT LOCK RELEASED
|
request2 is queued to the |
endpoint from the interrupt |
context after request3 is queued.|
PORT LOCK ACQUIRED BACK |
Signed-off-by: Balakumar Rajendran <balakumar.rajendran@xxxxxxxxxxxxxx>
Signed-off-by: Praveena Nadahally <praveen.nadahally@xxxxxxxxxxxxxx>
Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
---
drivers/usb/gadget/u_serial.c | 15 ++++++++-------
1 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/u_serial.c b/drivers/usb/gadget/u_serial.c
index 5b3f5ff..0bcc8cd 100644
--- a/drivers/usb/gadget/u_serial.c
+++ b/drivers/usb/gadget/u_serial.c
@@ -384,16 +384,17 @@ __acquires(&port->port_lock)
port->port_num, len, *((u8 *)req->buf),
*((u8 *)req->buf+1), *((u8 *)req->buf+2));
- /* Drop lock while we call out of driver; completions
- * could be issued while we do so. Disconnection may
- * happen too; maybe immediately before we queue this!
+ /*
+ * Do not drop lock here. It causes packet swaps in
+ * SMP systems when both the interrupt context and the
+ * application context and trying to queue data. Packets
+ * get queued in wrong order sometimes.
*
- * NOTE that we may keep sending data for a while after
- * the TTY closed (dev->ioport->port_tty is NULL).
+ * TODO: For musb no issue is observed during the
+ * disconnect sequence as well. Needs to be verified
+ * with other controllers.
*/
- spin_unlock(&port->port_lock);
status = usb_ep_queue(in, req, GFP_ATOMIC);
- spin_lock(&port->port_lock);
if (status) {
pr_debug("%s: %s %s err %d\n",
--
1.7.4.3
--
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
