usb: detect stop endpoint race using pending timer instead of counter. [1/1]

PD#SWPL-13177

Problem:
stop endpoint race

Solution:
A counter was used to find out if the stop endpoint completion raced with
the stop endpoint timeout timer. This was needed in case the stop ep
completion failed to delete the timer as it was running on anoter cpu.

The EP_STOP_CMD_PENDING flag was not enough as a new stop endpoint command
may be queued between the command completion and timeout function, which
would set the flag back.

Instead of the separate counter that was used we can detect the race by
checking both the STOP_EP_PENDING flag and timer_pending in the timeout
function.

Verify:
franklin

Change-Id: Ie958ffd530a6bd176d0cf451894a5bd4dece38da
Signed-off-by: Yue Wang <yue.wang@amlogic.com>
This commit is contained in:
Yue Wang
2019-08-23 15:19:27 +08:00
committed by Luke Go
parent d6bff5638e
commit 6d76b270cc
3 changed files with 51 additions and 3 deletions

View File

@@ -393,8 +393,13 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci,
* pointer command pending because the device can choose to start any
* stream once the endpoint is on the HW schedule.
*/
#ifdef CONFIG_AMLOGIC_USB
if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING)
|| (ep_state & EP_HALTED))
#else
if ((ep_state & EP_HALT_PENDING) || (ep_state & SET_DEQ_PENDING) ||
(ep_state & EP_HALTED))
#endif
return;
writel(DB_VALUE(ep_index, stream_id), db_addr);
/* The CPU has better things to do at this point than wait for a
@@ -632,13 +637,19 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci,
struct xhci_virt_ep *ep)
{
#ifdef CONFIG_AMLOGIC_USB
ep->ep_state &= ~EP_STOP_CMD_PENDING;
/* Can't del_timer_sync in interrupt */
del_timer(&ep->stop_cmd_timer);
#else
ep->ep_state &= ~EP_HALT_PENDING;
/* Can't del_timer_sync in interrupt, so we attempt to cancel. If the
/* Can't del_timer_sync in interrupt, so we attempt to cancel. If the
* timer is running on another CPU, we don't decrement stop_cmds_pending
* (since we didn't successfully stop the watchdog timer).
*/
if (del_timer(&ep->stop_cmd_timer))
ep->stop_cmds_pending--;
#endif
}
/* Must be called with xhci->lock held in interrupt context */
@@ -902,10 +913,15 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
* simple flag to say whether there is a pending stop endpoint command for a
* particular endpoint.
*
#ifdef CONFIG_AMLOGIC_USB
* Instead we use a combination of that flag and checking if a new timer is
* pending.
#else
* Instead we use a combination of that flag and a counter for the number of
* pending stop endpoint commands. If the timer is the tail end of the last
* stop endpoint command, and the endpoint's command is still pending, we assume
* the host is dying.
#endif
*/
void xhci_stop_endpoint_command_watchdog(unsigned long arg)
{
@@ -919,12 +935,21 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
spin_lock_irqsave(&xhci->lock, flags);
#ifdef CONFIG_AMLOGIC_USB
/* bail out if cmd completed but raced with stop ep watchdog timer.*/
if (!(ep->ep_state & EP_STOP_CMD_PENDING) ||
timer_pending(&ep->stop_cmd_timer)) {
#else
ep->stop_cmds_pending--;
if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) {
xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
"Stop EP timer ran, but no command pending, "
"exiting.");
#endif
spin_unlock_irqrestore(&xhci->lock, flags);
#ifdef CONFIG_AMLOGIC_USB
xhci_dbg(xhci, "Stop EP timer raced with cmd completion, exit");
#endif
return;
}
@@ -933,7 +958,12 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg)
/* Oops, HC is dead or dying or at least not responding to the stop
* endpoint command.
*/
xhci->xhc_state |= XHCI_STATE_DYING;
#ifdef CONFIG_AMLOGIC_USB
ep->ep_state &= ~EP_STOP_CMD_PENDING;
#endif
/* Disable interrupts from the host controller and start halting it */
xhci_quiesce(xhci);
spin_unlock_irqrestore(&xhci->lock, flags);

View File

@@ -1595,14 +1595,22 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
/* Queue a stop endpoint command, but only if this is
* the first cancellation to be handled.
*/
#ifdef CONFIG_AMLOGIC_USB
if (!(ep->ep_state & EP_STOP_CMD_PENDING)) {
#else
if (!(ep->ep_state & EP_HALT_PENDING)) {
#endif
command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
if (!command) {
ret = -ENOMEM;
goto done;
}
#ifdef CONFIG_AMLOGIC_USB
ep->ep_state |= EP_STOP_CMD_PENDING;
#else
ep->ep_state |= EP_HALT_PENDING;
ep->stop_cmds_pending++;
#endif
ep->stop_cmd_timer.expires = jiffies +
XHCI_STOP_EP_CMD_TIMEOUT * HZ;
add_timer(&ep->stop_cmd_timer);
@@ -3648,7 +3656,11 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
/* Stop any wayward timer functions (which may grab the lock) */
for (i = 0; i < 31; ++i) {
#ifdef CONFIG_AMLOGIC_USB
virt_dev->eps[i].ep_state &= ~EP_STOP_CMD_PENDING;
#else
virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING;
#endif
del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
}

View File

@@ -917,7 +917,11 @@ struct xhci_virt_ep {
unsigned int ep_state;
#define SET_DEQ_PENDING (1 << 0)
#define EP_HALTED (1 << 1) /* For stall handling */
#define EP_HALT_PENDING (1 << 2) /* For URB cancellation */
#ifdef CONFIG_AMLOGIC_USB
#define EP_STOP_CMD_PENDING (1 << 2) /* For URB cancellation */
#else
#define EP_HALT_PENDING (1 << 2) /* For URB cancellation */
#endif
/* Transitioning the endpoint to using streams, don't enqueue URBs */
#define EP_GETTING_STREAMS (1 << 3)
#define EP_HAS_STREAMS (1 << 4)
@@ -929,7 +933,9 @@ struct xhci_virt_ep {
unsigned int stopped_stream;
/* Watchdog timer for stop endpoint command to cancel URBs */
struct timer_list stop_cmd_timer;
int stop_cmds_pending;
#ifndef CONFIG_AMLOGIC_USB
int stop_cmds_pending;
#endif
struct xhci_hcd *xhci;
/* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue
* command. We'll need to update the ring's dequeue segment and dequeue