From: Baolin Wang <baolin.wang@linaro.org>
To: mathias.nyman@intel.com, gregkh@linuxfoundation.org
Cc: baolu.lu@linux.intel.com, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, broonie@kernel.org,
baolin.wang@linaro.org
Subject: [PATCH 2/2] usb: host: xhci: Handle the right timeout command
Date: Mon, 5 Dec 2016 15:51:50 +0800 [thread overview]
Message-ID: <0c39bfed1cf6f7b747e702aa841f82c9d2140f27.1480922249.git.baolin.wang@linaro.org> (raw)
In-Reply-To: <b0157454dce55d1288c7e097ddffc3d6c81351b2.1480922249.git.baolin.wang@linaro.org>
In-Reply-To: <b0157454dce55d1288c7e097ddffc3d6c81351b2.1480922249.git.baolin.wang@linaro.org>
If a command event is found on the event ring during an interrupt,
we need to stop the command timer with del_timer(). Since del_timer()
can fail if the timer is running and waiting on the xHCI lock, then
it maybe get the wrong timeout command in xhci_handle_command_timeout()
if host fetched a new command and updated the xhci->current_cmd in
handle_cmd_completion(). For this situation, we need a way to signal
to the command timer that everything is fine and it should exit.
We should introduce a counter (xhci->current_cmd_pending) for the number
of pending commands. If we need to cancel the command timer and del_timer()
succeeds, we decrement the number of pending commands. If del_timer() fails,
we leave the number of pending commands alone.
For handling timeout command, in xhci_handle_command_timeout() we will check
the counter after decrementing it, if the counter (xhci->current_cmd_pending)
is 0, which means xhci->current_cmd is the right timeout command. If the
counter (xhci->current_cmd_pending) is greater than 0, which means current
timeout command has been handled by host and host has fetched new command as
xhci->current_cmd, then just return and wait for new current command.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
drivers/usb/host/xhci-ring.c | 29 ++++++++++++++++++++++++++++-
drivers/usb/host/xhci.h | 1 +
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9965a4c..edc9ac2 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1253,6 +1253,7 @@ static void xhci_handle_stopped_cmd_ring(struct xhci_hcd *xhci,
if ((xhci->cmd_ring->dequeue != xhci->cmd_ring->enqueue) &&
!(xhci->xhc_state & XHCI_STATE_DYING)) {
xhci->current_cmd = cur_cmd;
+ xhci->current_cmd_pending++;
mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT);
xhci_ring_cmd_db(xhci);
}
@@ -1269,11 +1270,27 @@ void xhci_handle_command_timeout(unsigned long data)
xhci = (struct xhci_hcd *) data;
spin_lock_irqsave(&xhci->lock, flags);
+ xhci->current_cmd_pending--;
+
if (!xhci->current_cmd) {
spin_unlock_irqrestore(&xhci->lock, flags);
return;
}
+ /*
+ * If the current_cmd_pending is 0, which means current command is
+ * timeout.
+ *
+ * If the current_cmd_pending is greater than 0, which means current
+ * timeout command has been handled by host and host has fetched new
+ * command as xhci->current_cmd, then just return and wait for new
+ * current command.
+ */
+ if (xhci->current_cmd_pending > 0) {
+ spin_unlock_irqrestore(&xhci->lock, flags);
+ return;
+ }
+
if (xhci->current_cmd->status == COMP_CMD_ABORT)
second_timeout = true;
xhci->current_cmd->status = COMP_CMD_ABORT;
@@ -1282,6 +1299,8 @@ void xhci_handle_command_timeout(unsigned long data)
hw_ring_state = xhci_read_64(xhci, &xhci->op_regs->cmd_ring);
if ((xhci->cmd_ring_state & CMD_RING_STATE_RUNNING) &&
(hw_ring_state & CMD_RING_RUNNING)) {
+ /* Will add command timer again to wait for abort event */
+ xhci->current_cmd_pending++;
spin_unlock_irqrestore(&xhci->lock, flags);
xhci_dbg(xhci, "Command timeout\n");
ret = xhci_abort_cmd_ring(xhci);
@@ -1336,7 +1355,13 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);
- del_timer(&xhci->cmd_timer);
+ /*
+ * If the command timer is running on another CPU, we don't decrement
+ * current_cmd_pending, since we didn't successfully stop the command
+ * timer.
+ */
+ if (del_timer(&xhci->cmd_timer))
+ xhci->current_cmd_pending--;
trace_xhci_cmd_completion(cmd_trb, (struct xhci_generic_trb *) event);
@@ -1427,6 +1452,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci,
if (cmd->cmd_list.next != &xhci->cmd_list) {
xhci->current_cmd = list_entry(cmd->cmd_list.next,
struct xhci_command, cmd_list);
+ xhci->current_cmd_pending++;
mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT);
} else if (xhci->current_cmd == cmd) {
xhci->current_cmd = NULL;
@@ -3927,6 +3953,7 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd,
if (xhci->cmd_list.next == &cmd->cmd_list &&
!timer_pending(&xhci->cmd_timer)) {
xhci->current_cmd = cmd;
+ xhci->current_cmd_pending++;
mod_timer(&xhci->cmd_timer, jiffies + XHCI_CMD_DEFAULT_TIMEOUT);
}
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 9dbaacf..5d81257 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1567,6 +1567,7 @@ struct xhci_hcd {
unsigned int cmd_ring_reserved_trbs;
struct timer_list cmd_timer;
struct xhci_command *current_cmd;
+ u32 current_cmd_pending;
struct xhci_ring *event_ring;
struct xhci_erst erst;
/* Scratchpad */
--
1.7.9.5
next prev parent reply other threads:[~2016-12-05 8:01 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-05 7:51 [PATCH 1/2] usb: host: xhci: Fix possible wild pointer when handling abort command Baolin Wang
2016-12-05 7:51 ` Baolin Wang [this message]
2016-12-12 15:52 ` [PATCH 2/2] usb: host: xhci: Handle the right timeout command Mathias Nyman
2016-12-13 3:21 ` Baolin Wang
2016-12-19 10:33 ` Mathias Nyman
2016-12-19 11:34 ` Baolin Wang
2016-12-19 12:13 ` Mathias Nyman
2016-12-20 3:23 ` Baolin Wang
2016-12-20 4:29 ` Lu Baolu
2016-12-20 6:06 ` Baolin Wang
2016-12-20 6:39 ` Lu Baolu
2016-12-20 6:46 ` Baolin Wang
2016-12-20 7:18 ` Lu Baolu
2016-12-20 7:30 ` Baolin Wang
2016-12-20 15:13 ` Mathias Nyman
2016-12-21 2:22 ` Baolin Wang
2016-12-21 13:00 ` Mathias Nyman
2016-12-27 3:07 ` Baolin Wang
2017-01-02 14:57 ` Mathias Nyman
2017-01-03 6:20 ` Baolin Wang
2016-12-21 6:17 ` Lu Baolu
2016-12-21 12:48 ` Mathias Nyman
2016-12-21 14:10 ` OGAWA Hirofumi
2016-12-21 15:04 ` Mathias Nyman
2016-12-21 15:18 ` OGAWA Hirofumi
2016-12-22 1:46 ` Lu Baolu
2016-12-23 12:54 ` Mathias Nyman
2016-12-22 1:43 ` Lu Baolu
2016-12-21 6:57 ` Lu Baolu
2016-12-21 12:57 ` Mathias Nyman
2016-12-22 1:39 ` Lu Baolu
2016-12-05 14:08 ` [PATCH 1/2] usb: host: xhci: Fix possible wild pointer when handling abort command Mathias Nyman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c39bfed1cf6f7b747e702aa841f82c9d2140f27.1480922249.git.baolin.wang@linaro.org \
--to=baolin.wang@linaro.org \
--cc=baolu.lu@linux.intel.com \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).