With recent changes in AOSP, adb is using asynchronous io, which causes the following crash usually on a reboot:
[ 184.278302] BUG: scheduling while atomic: ksoftirqd/0/9/0x00000104 [ 184.284617] Modules linked in: wl18xx wlcore snd_soc_hdmi_codec wlcore_sdio tcpci_rt1711h tcpci tcpm typec adv7511 cec dwc3 phy_hi3660_usb3 snd_soc_simple_card snd_soc_a [ 184.316034] Preemption disabled at: [ 184.316072] [<ffffff8008081de4>] __do_softirq+0x64/0x398 [ 184.324953] CPU: 0 PID: 9 Comm: ksoftirqd/0 Tainted: G S 4.19.43-00669-g8e4970572c43-dirty #356 [ 184.334963] Hardware name: HiKey960 (DT) [ 184.338892] Call trace: [ 184.341352] dump_backtrace+0x0/0x158 [ 184.345025] show_stack+0x14/0x20 [ 184.348355] dump_stack+0x80/0xa4 [ 184.351685] __schedule_bug+0x6c/0xc0 [ 184.355363] __schedule+0x64c/0x978 [ 184.358863] schedule+0x2c/0x90 [ 184.362053] dwc3_gadget_ep_dequeue+0x274/0x388 [dwc3] [ 184.367210] usb_ep_dequeue+0x24/0xf8 [ 184.370884] ffs_aio_cancel+0x3c/0x80 [ 184.374561] free_ioctx_users+0x40/0x148 [ 184.378500] percpu_ref_switch_to_atomic_rcu+0x180/0x1c0 [ 184.383830] rcu_process_callbacks+0x24c/0x5d8 [ 184.388283] __do_softirq+0x13c/0x398 [ 184.391959] run_ksoftirqd+0x3c/0x48 [ 184.395549] smpboot_thread_fn+0x220/0x288 [ 184.399660] kthread+0x12c/0x130 [ 184.402901] ret_from_fork+0x10/0x1c
This happens as usb_ep_dequeue can be called in interrupt context, and dwc3_gadget_ep_dequeue() then calls wait_event_lock_irq() which can sleep.
Upstream kernels are not affected due to the change fec9095bdef4 ("dwc3: gadget: remove wait_end_transfer") which removes the wait_even_lock_irq code. Unfortunately that change has a number of dependencies, which I'm submitting here.
Also, to match upstream, in this series I've reverted one change that was backported to -stable, to replace it with the cherry-picked upstream commit (as the dependencies are now there)
This issue also affects 4.14,4.9 and I believe 4.4 kernels, however I don't know how to best backport this functionality that far back. Help from the maintainers would be very much appreciated!
Feedback and comments would be welcome!
thanks -john
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y
Felipe Balbi (7): usb: dwc3: gadget: combine unaligned and zero flags usb: dwc3: gadget: track number of TRBs per request usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue() usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs() usb: dwc3: gadget: introduce cancelled_list usb: dwc3: gadget: move requests to cancelled_list usb: dwc3: gadget: remove wait_end_transfer
Jack Pham (1): usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup
John Stultz (1): Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup"
drivers/usb/dwc3/core.h | 15 ++-- drivers/usb/dwc3/gadget.c | 158 +++++++++++++------------------------- drivers/usb/dwc3/gadget.h | 15 ++++ 3 files changed, 75 insertions(+), 113 deletions(-)
From: Felipe Balbi felipe.balbi@linux.intel.com
commit 1a22ec643580626f439c8583edafdcc73798f2fb upstream
Both flags are used for the same purpose in dwc3: appending an extra TRB at the end to deal with controller requirements. By combining both flags into one, we make it clear that the situation is the same and that they should be treated equally.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit 1a22ec643580626f439c8583edafdcc73798f2fb) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/core.h | 7 +++---- drivers/usb/dwc3/gadget.c | 18 +++++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 5bfb62533e0f..4872cba8699b 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -847,11 +847,11 @@ struct dwc3_hwparams { * @epnum: endpoint number to which this request refers * @trb: pointer to struct dwc3_trb * @trb_dma: DMA address of @trb - * @unaligned: true for OUT endpoints with length not divisible by maxp + * @needs_extra_trb: true when request needs one extra TRB (either due to ZLP + * or unaligned OUT) * @direction: IN or OUT direction flag * @mapped: true when request has been dma-mapped * @started: request is started - * @zero: wants a ZLP */ struct dwc3_request { struct usb_request request; @@ -867,11 +867,10 @@ struct dwc3_request { struct dwc3_trb *trb; dma_addr_t trb_dma;
- unsigned unaligned:1; + unsigned needs_extra_trb:1; unsigned direction:1; unsigned mapped:1; unsigned started:1; - unsigned zero:1; };
/* diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 65ba1038b111..4894fed1441c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1070,7 +1070,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep, struct dwc3 *dwc = dep->dwc; struct dwc3_trb *trb;
- req->unaligned = true; + req->needs_extra_trb = true;
/* prepare normal TRB */ dwc3_prepare_one_trb(dep, req, true, i); @@ -1114,7 +1114,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep, struct dwc3 *dwc = dep->dwc; struct dwc3_trb *trb;
- req->unaligned = true; + req->needs_extra_trb = true;
/* prepare normal TRB */ dwc3_prepare_one_trb(dep, req, true, 0); @@ -1130,7 +1130,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep, struct dwc3 *dwc = dep->dwc; struct dwc3_trb *trb;
- req->zero = true; + req->needs_extra_trb = true;
/* prepare normal TRB */ dwc3_prepare_one_trb(dep, req, true, 0); @@ -1412,7 +1412,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, dwc3_ep_inc_deq(dep); }
- if (r->unaligned || r->zero) { + if (r->needs_extra_trb) { trb = r->trb + r->num_pending_sgs + 1; trb->ctrl &= ~DWC3_TRB_CTRL_HWO; dwc3_ep_inc_deq(dep); @@ -1423,7 +1423,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, trb->ctrl &= ~DWC3_TRB_CTRL_HWO; dwc3_ep_inc_deq(dep);
- if (r->unaligned || r->zero) { + if (r->needs_extra_trb) { trb = r->trb + 1; trb->ctrl &= ~DWC3_TRB_CTRL_HWO; dwc3_ep_inc_deq(dep); @@ -2252,7 +2252,8 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, * with one TRB pending in the ring. We need to manually clear HWO bit * from that TRB. */ - if ((req->zero || req->unaligned) && !(trb->ctrl & DWC3_TRB_CTRL_CHN)) { + + if (req->needs_extra_trb && !(trb->ctrl & DWC3_TRB_CTRL_CHN)) { trb->ctrl &= ~DWC3_TRB_CTRL_HWO; return 1; } @@ -2329,11 +2330,10 @@ static int dwc3_gadget_ep_cleanup_completed_request(struct dwc3_ep *dep, ret = dwc3_gadget_ep_reclaim_trb_linear(dep, req, event, status);
- if (req->unaligned || req->zero) { + if (req->needs_extra_trb) { ret = dwc3_gadget_ep_reclaim_trb_linear(dep, req, event, status); - req->unaligned = false; - req->zero = false; + req->needs_extra_trb = false; }
req->request.actual = req->request.length - req->remaining;
From: Felipe Balbi felipe.balbi@linux.intel.com
commit 09fe1f8d7e2f461275b1cdd832f2cfa5e9be346d upstream
This will help us remove the wait_event() from our ->dequeue().
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit 09fe1f8d7e2f461275b1cdd832f2cfa5e9be346d) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/core.h | 3 +++ drivers/usb/dwc3/gadget.c | 6 ++++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 4872cba8699b..0de78cb29f2c 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -847,6 +847,7 @@ struct dwc3_hwparams { * @epnum: endpoint number to which this request refers * @trb: pointer to struct dwc3_trb * @trb_dma: DMA address of @trb + * @num_trbs: number of TRBs used by this request * @needs_extra_trb: true when request needs one extra TRB (either due to ZLP * or unaligned OUT) * @direction: IN or OUT direction flag @@ -867,6 +868,8 @@ struct dwc3_request { struct dwc3_trb *trb; dma_addr_t trb_dma;
+ unsigned num_trbs; + unsigned needs_extra_trb:1; unsigned direction:1; unsigned mapped:1; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 4894fed1441c..019643a6ce9d 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1043,6 +1043,8 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep, req->trb_dma = dwc3_trb_dma_offset(dep, trb); }
+ req->num_trbs++; + __dwc3_prepare_one_trb(dep, trb, dma, length, chain, node, stream_id, short_not_ok, no_interrupt); } @@ -1077,6 +1079,7 @@ static void dwc3_prepare_one_trb_sg(struct dwc3_ep *dep,
/* Now prepare one extra TRB to align transfer size */ trb = &dep->trb_pool[dep->trb_enqueue]; + req->num_trbs++; __dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp - rem, false, 1, req->request.stream_id, @@ -1121,6 +1124,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
/* Now prepare one extra TRB to align transfer size */ trb = &dep->trb_pool[dep->trb_enqueue]; + req->num_trbs++; __dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, maxp - rem, false, 1, req->request.stream_id, req->request.short_not_ok, @@ -1137,6 +1141,7 @@ static void dwc3_prepare_one_trb_linear(struct dwc3_ep *dep,
/* Now prepare one extra TRB to handle ZLP */ trb = &dep->trb_pool[dep->trb_enqueue]; + req->num_trbs++; __dwc3_prepare_one_trb(dep, trb, dwc->bounce_addr, 0, false, 1, req->request.stream_id, req->request.short_not_ok, @@ -2233,6 +2238,7 @@ static int dwc3_gadget_ep_reclaim_completed_trb(struct dwc3_ep *dep, dwc3_ep_inc_deq(dep);
trace_dwc3_complete_trb(dep, trb); + req->num_trbs--;
/* * If we're in the middle of series of chained TRBs and we
From: Felipe Balbi felipe.balbi@linux.intel.com
commit c3acd59014148470dc58519870fbc779785b4bf7 upstream
Now that we track how many TRBs a request uses, it's easier to skip over them in case of a call to usb_ep_dequeue(). Let's do so and simplify the code a bit.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit c3acd59014148470dc58519870fbc779785b4bf7) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/gadget.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 019643a6ce9d..cb6dfea5d5e7 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1370,6 +1370,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, break; } if (r == req) { + int i; + /* wait until it is processed */ dwc3_stop_active_transfer(dep, true);
@@ -1407,32 +1409,12 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (!r->trb) goto out0;
- if (r->num_pending_sgs) { + for (i = 0; i < r->num_trbs; i++) { struct dwc3_trb *trb; - int i = 0; - - for (i = 0; i < r->num_pending_sgs; i++) { - trb = r->trb + i; - trb->ctrl &= ~DWC3_TRB_CTRL_HWO; - dwc3_ep_inc_deq(dep); - } - - if (r->needs_extra_trb) { - trb = r->trb + r->num_pending_sgs + 1; - trb->ctrl &= ~DWC3_TRB_CTRL_HWO; - dwc3_ep_inc_deq(dep); - } - } else { - struct dwc3_trb *trb = r->trb;
+ trb = r->trb + i; trb->ctrl &= ~DWC3_TRB_CTRL_HWO; dwc3_ep_inc_deq(dep); - - if (r->needs_extra_trb) { - trb = r->trb + 1; - trb->ctrl &= ~DWC3_TRB_CTRL_HWO; - dwc3_ep_inc_deq(dep); - } } goto out1; } @@ -1443,8 +1425,6 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, }
out1: - /* giveback the request */ - dwc3_gadget_giveback(dep, req, -ECONNRESET);
out0:
From: Felipe Balbi felipe.balbi@linux.intel.com
commit 7746a8dfb3f9c91b3a0b63a1d5c2664410e6498d upstream
Extract the logic for skipping over TRBs to its own function. This makes the code slightly more readable and makes it easier to move this call to its final resting place as a following patch.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit 7746a8dfb3f9c91b3a0b63a1d5c2664410e6498d) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/gadget.c | 61 +++++++++++++++------------------------ 1 file changed, 24 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index cb6dfea5d5e7..f0c3b08ff7c6 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1343,6 +1343,29 @@ static int dwc3_gadget_ep_queue(struct usb_ep *ep, struct usb_request *request, return ret; }
+static void dwc3_gadget_ep_skip_trbs(struct dwc3_ep *dep, struct dwc3_request *req) +{ + int i; + + /* + * If request was already started, this means we had to + * stop the transfer. With that we also need to ignore + * all TRBs used by the request, however TRBs can only + * be modified after completion of END_TRANSFER + * command. So what we do here is that we wait for + * END_TRANSFER completion and only after that, we jump + * over TRBs by clearing HWO and incrementing dequeue + * pointer. + */ + for (i = 0; i < req->num_trbs; i++) { + struct dwc3_trb *trb; + + trb = req->trb + i; + trb->ctrl &= ~DWC3_TRB_CTRL_HWO; + dwc3_ep_inc_deq(dep); + } +} + static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, struct usb_request *request) { @@ -1370,38 +1393,8 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, break; } if (r == req) { - int i; - /* wait until it is processed */ dwc3_stop_active_transfer(dep, true); - - /* - * If request was already started, this means we had to - * stop the transfer. With that we also need to ignore - * all TRBs used by the request, however TRBs can only - * be modified after completion of END_TRANSFER - * command. So what we do here is that we wait for - * END_TRANSFER completion and only after that, we jump - * over TRBs by clearing HWO and incrementing dequeue - * pointer. - * - * Note that we have 2 possible types of transfers here: - * - * i) Linear buffer request - * ii) SG-list based request - * - * SG-list based requests will have r->num_pending_sgs - * set to a valid number (> 0). Linear requests, - * normally use a single TRB. - * - * For each of these two cases, if r->unaligned flag is - * set, one extra TRB has been used to align transfer - * size to wMaxPacketSize. - * - * All of these cases need to be taken into - * consideration so we don't mess up our TRB ring - * pointers. - */ wait_event_lock_irq(dep->wait_end_transfer, !(dep->flags & DWC3_EP_END_TRANSFER_PENDING), dwc->lock); @@ -1409,13 +1402,7 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (!r->trb) goto out0;
- for (i = 0; i < r->num_trbs; i++) { - struct dwc3_trb *trb; - - trb = r->trb + i; - trb->ctrl &= ~DWC3_TRB_CTRL_HWO; - dwc3_ep_inc_deq(dep); - } + dwc3_gadget_ep_skip_trbs(dep, r); goto out1; } dev_err(dwc->dev, "request %pK was not queued to %s\n",
From: Felipe Balbi felipe.balbi@linux.intel.com
commit d5443bbf5fc8f8389cce146b1fc2987cdd229d12 upstream
This list will host cancelled requests who still have TRBs being processed.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit d5443bbf5fc8f8389cce146b1fc2987cdd229d12) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/core.h | 2 ++ drivers/usb/dwc3/gadget.c | 1 + drivers/usb/dwc3/gadget.h | 15 +++++++++++++++ 3 files changed, 18 insertions(+)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 0de78cb29f2c..24f0b108b7f6 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -636,6 +636,7 @@ struct dwc3_event_buffer { /** * struct dwc3_ep - device side endpoint representation * @endpoint: usb endpoint + * @cancelled_list: list of cancelled requests for this endpoint * @pending_list: list of pending requests for this endpoint * @started_list: list of started requests on this endpoint * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete @@ -659,6 +660,7 @@ struct dwc3_event_buffer { */ struct dwc3_ep { struct usb_ep endpoint; + struct list_head cancelled_list; struct list_head pending_list; struct list_head started_list;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index f0c3b08ff7c6..34331a9fb584 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2146,6 +2146,7 @@ static int dwc3_gadget_init_endpoint(struct dwc3 *dwc, u8 epnum)
INIT_LIST_HEAD(&dep->pending_list); INIT_LIST_HEAD(&dep->started_list); + INIT_LIST_HEAD(&dep->cancelled_list);
return 0; } diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h index 2aacd1afd9ff..023a473648eb 100644 --- a/drivers/usb/dwc3/gadget.h +++ b/drivers/usb/dwc3/gadget.h @@ -79,6 +79,21 @@ static inline void dwc3_gadget_move_started_request(struct dwc3_request *req) list_move_tail(&req->list, &dep->started_list); }
+/** + * dwc3_gadget_move_cancelled_request - move @req to the cancelled_list + * @req: the request to be moved + * + * Caller should take care of locking. This function will move @req from its + * current list to the endpoint's cancelled_list. + */ +static inline void dwc3_gadget_move_cancelled_request(struct dwc3_request *req) +{ + struct dwc3_ep *dep = req->dep; + + req->started = false; + list_move_tail(&req->list, &dep->cancelled_list); +} + void dwc3_gadget_giveback(struct dwc3_ep *dep, struct dwc3_request *req, int status);
From: Felipe Balbi felipe.balbi@linux.intel.com
commit d4f1afe5e896c18ae01099a85dab5e1a198bd2a8 upstream
Whenever we have a request in flight, we can move it to the cancelled list and later simply iterate over that list and skip over any TRBs we find.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit d4f1afe5e896c18ae01099a85dab5e1a198bd2a8) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/gadget.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 34331a9fb584..25cdce359736 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1366,6 +1366,17 @@ static void dwc3_gadget_ep_skip_trbs(struct dwc3_ep *dep, struct dwc3_request *r } }
+static void dwc3_gadget_ep_cleanup_cancelled_requests(struct dwc3_ep *dep) +{ + struct dwc3_request *req; + struct dwc3_request *tmp; + + list_for_each_entry_safe(req, tmp, &dep->cancelled_list, list) { + dwc3_gadget_ep_skip_trbs(dep, req); + dwc3_gadget_giveback(dep, req, -ECONNRESET); + } +} + static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, struct usb_request *request) { @@ -1402,8 +1413,9 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (!r->trb) goto out0;
- dwc3_gadget_ep_skip_trbs(dep, r); - goto out1; + dwc3_gadget_move_cancelled_request(req); + dwc3_gadget_ep_cleanup_cancelled_requests(dep); + goto out0; } dev_err(dwc->dev, "request %pK was not queued to %s\n", request, ep->name); @@ -1411,7 +1423,6 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, goto out0; }
-out1: dwc3_gadget_giveback(dep, req, -ECONNRESET);
out0:
From: Felipe Balbi felipe.balbi@linux.intel.com
commit fec9095bdef4e7c988adb603d0d4f92ee735d4a1 upstream
Now that we have a list of cancelled requests, we can skip over TRBs when END_TRANSFER command completes.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit fec9095bdef4e7c988adb603d0d4f92ee735d4a1) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/core.h | 3 --- drivers/usb/dwc3/gadget.c | 40 +-------------------------------------- 2 files changed, 1 insertion(+), 42 deletions(-)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 24f0b108b7f6..131028501752 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -639,7 +639,6 @@ struct dwc3_event_buffer { * @cancelled_list: list of cancelled requests for this endpoint * @pending_list: list of pending requests for this endpoint * @started_list: list of started requests on this endpoint - * @wait_end_transfer: wait_queue_head_t for waiting on End Transfer complete * @lock: spinlock for endpoint request queue traversal * @regs: pointer to first endpoint register * @trb_pool: array of transaction buffers @@ -664,8 +663,6 @@ struct dwc3_ep { struct list_head pending_list; struct list_head started_list;
- wait_queue_head_t wait_end_transfer; - spinlock_t lock; void __iomem *regs;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 25cdce359736..879f652c5580 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -640,8 +640,6 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action) reg |= DWC3_DALEPENA_EP(dep->number); dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
- init_waitqueue_head(&dep->wait_end_transfer); - if (usb_endpoint_xfer_control(desc)) goto out;
@@ -1406,15 +1404,11 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep, if (r == req) { /* wait until it is processed */ dwc3_stop_active_transfer(dep, true); - wait_event_lock_irq(dep->wait_end_transfer, - !(dep->flags & DWC3_EP_END_TRANSFER_PENDING), - dwc->lock);
if (!r->trb) goto out0;
dwc3_gadget_move_cancelled_request(req); - dwc3_gadget_ep_cleanup_cancelled_requests(dep); goto out0; } dev_err(dwc->dev, "request %pK was not queued to %s\n", @@ -1915,8 +1909,6 @@ static int dwc3_gadget_stop(struct usb_gadget *g) { struct dwc3 *dwc = gadget_to_dwc(g); unsigned long flags; - int epnum; - u32 tmo_eps = 0;
spin_lock_irqsave(&dwc->lock, flags);
@@ -1925,36 +1917,6 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
__dwc3_gadget_stop(dwc);
- for (epnum = 2; epnum < DWC3_ENDPOINTS_NUM; epnum++) { - struct dwc3_ep *dep = dwc->eps[epnum]; - int ret; - - if (!dep) - continue; - - if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) - continue; - - ret = wait_event_interruptible_lock_irq_timeout(dep->wait_end_transfer, - !(dep->flags & DWC3_EP_END_TRANSFER_PENDING), - dwc->lock, msecs_to_jiffies(5)); - - if (ret <= 0) { - /* Timed out or interrupted! There's nothing much - * we can do so we just log here and print which - * endpoints timed out at the end. - */ - tmo_eps |= 1 << epnum; - dep->flags &= DWC3_EP_END_TRANSFER_PENDING; - } - } - - if (tmo_eps) { - dev_err(dwc->dev, - "end transfer timed out on endpoints 0x%x [bitmap]\n", - tmo_eps); - } - out: dwc->gadget_driver = NULL; spin_unlock_irqrestore(&dwc->lock, flags); @@ -2451,7 +2413,7 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
if (cmd == DWC3_DEPCMD_ENDTRANSFER) { dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING; - wake_up(&dep->wait_end_transfer); + dwc3_gadget_ep_cleanup_cancelled_requests(dep); } break; case DWC3_DEPEVT_STREAMEVT:
This reverts commit 25ad17d692ad54c3c33b2a31e5ce2a82e38de14e, as with other patches backported to -stable, we can now apply the actual upstream commit that matches this.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/gadget.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 879f652c5580..843586f20572 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -177,8 +177,6 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep, req->started = false; list_del(&req->list); req->remaining = 0; - req->unaligned = false; - req->zero = false;
if (req->request.status == -EINPROGRESS) req->request.status = status;
Hi John,
On Thu, Jun 27, 2019 at 08:52:39PM +0000, John Stultz wrote:
This reverts commit 25ad17d692ad54c3c33b2a31e5ce2a82e38de14e, as with other patches backported to -stable, we can now apply the actual upstream commit that matches this.
Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: John Stultz john.stultz@linaro.org
drivers/usb/dwc3/gadget.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 879f652c5580..843586f20572 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -177,8 +177,6 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep, req->started = false; list_del(&req->list); req->remaining = 0;
- req->unaligned = false;
- req->zero = false;
Given that these structure members are removed in Patch 1/9, wouldn't having these lines remain until this revert patch present compilation errors when applying the patches in this series individually?
For bisectability would it be better to fix-up Patch 1 to also convert these two flags to req->needs_extra_trb in one shot? Alternatively you could sandwich Patch 1 between Patch 8 & 9.
Thanks, Jack
On Thu, Jun 27, 2019 at 10:54 PM Jack Pham jackp@codeaurora.org wrote:
On Thu, Jun 27, 2019 at 08:52:39PM +0000, John Stultz wrote:
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 879f652c5580..843586f20572 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -177,8 +177,6 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep, req->started = false; list_del(&req->list); req->remaining = 0;
req->unaligned = false;req->zero = false;Given that these structure members are removed in Patch 1/9, wouldn't having these lines remain until this revert patch present compilation errors when applying the patches in this series individually?
For bisectability would it be better to fix-up Patch 1 to also convert these two flags to req->needs_extra_trb in one shot? Alternatively you could sandwich Patch 1 between Patch 8 & 9.
Ah. Good point! I'll respin this. Thanks for looking this over!
thanks -john
From: Jack Pham jackp@codeaurora.org
commit bd6742249b9ca918565e4e3abaa06665e587f4b5 upstream
OUT endpoint requests may somtimes have this flag set when preparing to be submitted to HW indicating that there is an additional TRB chained to the request for alignment purposes. If that request is removed before the controller can execute the transfer (e.g. ep_dequeue/ep_disable), the request will not go through the dwc3_gadget_ep_cleanup_completed_request() handler and will not have its needs_extra_trb flag cleared when dwc3_gadget_giveback() is called. This same request could be later requeued for a new transfer that does not require an extra TRB and if it is successfully completed, the cleanup and TRB reclamation will incorrectly process the additional TRB which belongs to the next request, and incorrectly advances the TRB dequeue pointer, thereby messing up calculation of the next requeust's actual/remaining count when it completes.
The right thing to do here is to ensure that the flag is cleared before it is given back to the function driver. A good place to do that is in dwc3_gadget_del_and_unmap_request().
Fixes: c6267a51639b ("usb: dwc3: gadget: align transfers to wMaxPacketSize") Cc: Fei Yang fei.yang@intel.com Cc: Sam Protsenko semen.protsenko@linaro.org Cc: Felipe Balbi balbi@kernel.org Cc: linux-usb@vger.kernel.org Cc: stable@vger.kernel.org # 4.19.y Signed-off-by: Jack Pham jackp@codeaurora.org Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com (cherry picked from commit bd6742249b9ca918565e4e3abaa06665e587f4b5) Signed-off-by: John Stultz john.stultz@linaro.org --- drivers/usb/dwc3/gadget.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 843586f20572..e7122b5199d2 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -177,6 +177,7 @@ static void dwc3_gadget_del_and_unmap_request(struct dwc3_ep *dep, req->started = false; list_del(&req->list); req->remaining = 0; + req->needs_extra_trb = false;
if (req->request.status == -EINPROGRESS) req->request.status = status;
With recent changes in AOSP, adb is using asynchronous io, which causes the following crash usually on a reboot:
[ 184.278302] BUG: scheduling while atomic: ksoftirqd/0/9/0x00000104 [ 184.284617] Modules linked in: wl18xx wlcore snd_soc_hdmi_codec wlcore_sdio tcpci_rt1711h tcpci tcpm typec adv7511 cec dwc3 phy_hi3660_usb3 snd_soc_simple_card snd_soc_a [ 184.316034] Preemption disabled at: [ 184.316072] [<ffffff8008081de4>] __do_softirq+0x64/0x398 [ 184.324953] CPU: 0 PID: 9 Comm: ksoftirqd/0 Tainted: G S 4.19.43- 00669-g8e4970572c43-dirty #356 [ 184.334963] Hardware name: HiKey960 (DT) [ 184.338892] Call trace: [ 184.341352] dump_backtrace+0x0/0x158 [ 184.345025] show_stack+0x14/0x20 [ 184.348355] dump_stack+0x80/0xa4 [ 184.351685] __schedule_bug+0x6c/0xc0 [ 184.355363] __schedule+0x64c/0x978 [ 184.358863] schedule+0x2c/0x90 [ 184.362053] dwc3_gadget_ep_dequeue+0x274/0x388 [dwc3]
This happens as usb_ep_dequeue can be called in interrupt context, and dwc3_gadget_ep_dequeue() then calls wait_event_lock_irq() which can sleep.
Upstream kernels are not affected due to the change fec9095bdef4 ("dwc3: gadget: remove wait_end_transfer") which removes the wait_even_lock_irq code. Unfortunately that change has a number of dependencies, which I'm submitting here.
Also, to match upstream, in this series I've reverted one change that was backported to -stable, to replace it with the cherry-picked upstream commit (as the dependencies are now there)
This issue also affects 4.14,4.9 and I believe 4.4 kernels, however I don't know how to best backport this functionality that far back. Help from the maintainers would be very much appreciated!
Feedback and comments would be welcome!
thanks -john
I confirm that this patch series fixes crash seen on reboot. Considering that many Android platforms use 4.19 stable kernel with latest AOSP codebase, it would be really helpful if these patches are merged to 4.19 stable.
Thanks, Saranya
On Fri, Jun 28, 2019 at 3:10 AM Gopal, Saranya saranya.gopal@intel.com wrote:
With recent changes in AOSP, adb is using asynchronous io, which causes the following crash usually on a reboot:
[ 184.278302] BUG: scheduling while atomic: ksoftirqd/0/9/0x00000104 [ 184.284617] Modules linked in: wl18xx wlcore snd_soc_hdmi_codec wlcore_sdio tcpci_rt1711h tcpci tcpm typec adv7511 cec dwc3 phy_hi3660_usb3 snd_soc_simple_card snd_soc_a [ 184.316034] Preemption disabled at: [ 184.316072] [<ffffff8008081de4>] __do_softirq+0x64/0x398 [ 184.324953] CPU: 0 PID: 9 Comm: ksoftirqd/0 Tainted: G S 4.19.43- 00669-g8e4970572c43-dirty #356 [ 184.334963] Hardware name: HiKey960 (DT) [ 184.338892] Call trace: [ 184.341352] dump_backtrace+0x0/0x158 [ 184.345025] show_stack+0x14/0x20 [ 184.348355] dump_stack+0x80/0xa4 [ 184.351685] __schedule_bug+0x6c/0xc0 [ 184.355363] __schedule+0x64c/0x978 [ 184.358863] schedule+0x2c/0x90 [ 184.362053] dwc3_gadget_ep_dequeue+0x274/0x388 [dwc3]
This happens as usb_ep_dequeue can be called in interrupt context, and dwc3_gadget_ep_dequeue() then calls wait_event_lock_irq() which can sleep.
Upstream kernels are not affected due to the change fec9095bdef4 ("dwc3: gadget: remove wait_end_transfer") which removes the wait_even_lock_irq code. Unfortunately that change has a number of dependencies, which I'm submitting here.
Also, to match upstream, in this series I've reverted one change that was backported to -stable, to replace it with the cherry-picked upstream commit (as the dependencies are now there)
This issue also affects 4.14,4.9 and I believe 4.4 kernels, however I don't know how to best backport this functionality that far back. Help from the maintainers would be very much appreciated!
Feedback and comments would be welcome!
thanks -john
I confirm that this patch series fixes crash seen on reboot. Considering that many Android platforms use 4.19 stable kernel with latest AOSP codebase, it would be really helpful if these patches are merged to 4.19 stable.
Thanks so much for the testing! Do let me know if you come across any ideas on how to cleanly resolve this for 4.14/4.9/4.4!
thanks -john
linux-stable-mirror@lists.linaro.org
-
Gopal, Saranya -
Jack Pham -
John Stultz