linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue
@ 2019-06-27 20:52 John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 1/9] usb: dwc3: gadget: combine unaligned and zero flags John Stultz
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable; +Cc: John Stultz, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb

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(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 1/9] usb: dwc3: gadget: combine unaligned and zero flags
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 2/9] usb: dwc3: gadget: track number of TRBs per request John Stultz
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 2/9] usb: dwc3: gadget: track number of TRBs per request
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 1/9] usb: dwc3: gadget: combine unaligned and zero flags John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 3/9] usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue() John Stultz
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 3/9] usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue()
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 1/9] usb: dwc3: gadget: combine unaligned and zero flags John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 2/9] usb: dwc3: gadget: track number of TRBs per request John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 4/9] usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs() John Stultz
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 4/9] usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs()
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (2 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 3/9] usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue() John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 5/9] usb: dwc3: gadget: introduce cancelled_list John Stultz
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 5/9] usb: dwc3: gadget: introduce cancelled_list
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (3 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 4/9] usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs() John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 6/9] usb: dwc3: gadget: move requests to cancelled_list John Stultz
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 6/9] usb: dwc3: gadget: move requests to cancelled_list
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (4 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 5/9] usb: dwc3: gadget: introduce cancelled_list John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 7/9] usb: dwc3: gadget: remove wait_end_transfer John Stultz
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 7/9] usb: dwc3: gadget: remove wait_end_transfer
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (5 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 6/9] usb: dwc3: gadget: move requests to cancelled_list John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-27 20:52 ` [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup" John Stultz
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Felipe Balbi, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	John Stultz

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:
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup"
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (6 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 7/9] usb: dwc3: gadget: remove wait_end_transfer John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-28  5:54   ` Jack Pham
  2019-06-27 20:52 ` [PATCH 4.19.y 9/9] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup John Stultz
  2019-06-28 10:10 ` [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue Gopal, Saranya
  9 siblings, 1 reply; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable; +Cc: John Stultz, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb

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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4.19.y 9/9] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (7 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup" John Stultz
@ 2019-06-27 20:52 ` John Stultz
  2019-06-28 10:10 ` [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue Gopal, Saranya
  9 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-27 20:52 UTC (permalink / raw)
  To: stable
  Cc: Jack Pham, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb,
	Felipe Balbi, John Stultz

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;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup"
  2019-06-27 20:52 ` [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup" John Stultz
@ 2019-06-28  5:54   ` Jack Pham
  2019-06-28 17:34     ` John Stultz
  0 siblings, 1 reply; 14+ messages in thread
From: Jack Pham @ 2019-06-28  5:54 UTC (permalink / raw)
  To: John Stultz; +Cc: stable, Fei Yang, Sam Protsenko, Felipe Balbi, linux-usb

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
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue
  2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
                   ` (8 preceding siblings ...)
  2019-06-27 20:52 ` [PATCH 4.19.y 9/9] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup John Stultz
@ 2019-06-28 10:10 ` Gopal, Saranya
  2019-06-28 18:14   ` John Stultz
  9 siblings, 1 reply; 14+ messages in thread
From: Gopal, Saranya @ 2019-06-28 10:10 UTC (permalink / raw)
  To: John Stultz, stable; +Cc: Yang, Fei, Sam Protsenko, Felipe Balbi, linux-usb

> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup"
  2019-06-28  5:54   ` Jack Pham
@ 2019-06-28 17:34     ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-28 17:34 UTC (permalink / raw)
  To: Jack Pham; +Cc: stable, Fei Yang, Sam Protsenko, Felipe Balbi, Linux USB List

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue
  2019-06-28 10:10 ` [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue Gopal, Saranya
@ 2019-06-28 18:14   ` John Stultz
  0 siblings, 0 replies; 14+ messages in thread
From: John Stultz @ 2019-06-28 18:14 UTC (permalink / raw)
  To: Gopal, Saranya; +Cc: stable, Yang, Fei, Sam Protsenko, Felipe Balbi, linux-usb

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-06-28 18:14 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 20:52 [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 1/9] usb: dwc3: gadget: combine unaligned and zero flags John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 2/9] usb: dwc3: gadget: track number of TRBs per request John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 3/9] usb: dwc3: gadget: use num_trbs when skipping TRBs on ->dequeue() John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 4/9] usb: dwc3: gadget: extract dwc3_gadget_ep_skip_trbs() John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 5/9] usb: dwc3: gadget: introduce cancelled_list John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 6/9] usb: dwc3: gadget: move requests to cancelled_list John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 7/9] usb: dwc3: gadget: remove wait_end_transfer John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 8/9] Revert "usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup" John Stultz
2019-06-28  5:54   ` Jack Pham
2019-06-28 17:34     ` John Stultz
2019-06-27 20:52 ` [PATCH 4.19.y 9/9] usb: dwc3: gadget: Clear req->needs_extra_trb flag on cleanup John Stultz
2019-06-28 10:10 ` [PATCH 4.19.y 0/9] Fix scheduling while atomic in dwc3_gadget_ep_dequeue Gopal, Saranya
2019-06-28 18:14   ` John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).