From cb4d5ce588c5ff68e0fdd30370a0e6bc2c0a736b Mon Sep 17 00:00:00 2001 From: OGAWA Hirofumi Date: Tue, 3 Jan 2017 18:28:50 +0200 Subject: xhci: Use delayed_work instead of timer for command timeout This is preparation to fix abort operation race (See "xhci: Fix race related to abort operation"). To make timeout sleepable, use delayed_work instead of timer. [change a newly added pending timer fix to pending work -Mathias] Signed-off-by: OGAWA Hirofumi Signed-off-by: Mathias Nyman Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/host/xhci.h') diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 8ccc11a974b8..b54c486df956 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1568,7 +1568,7 @@ struct xhci_hcd { #define CMD_RING_STATE_STOPPED (1 << 2) struct list_head cmd_list; unsigned int cmd_ring_reserved_trbs; - struct timer_list cmd_timer; + struct delayed_work cmd_timer; struct xhci_command *current_cmd; struct xhci_ring *event_ring; struct xhci_erst erst; @@ -1934,7 +1934,7 @@ void xhci_queue_config_ep_quirk(struct xhci_hcd *xhci, unsigned int slot_id, unsigned int ep_index, struct xhci_dequeue_state *deq_state); void xhci_stop_endpoint_command_watchdog(unsigned long arg); -void xhci_handle_command_timeout(unsigned long data); +void xhci_handle_command_timeout(struct work_struct *work); void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, unsigned int slot_id, unsigned int ep_index, unsigned int stream_id); -- cgit v1.2.3 From 1c111b6c3844a142e03bcfc2fa17bfbdea08e9dc Mon Sep 17 00:00:00 2001 From: OGAWA Hirofumi Date: Tue, 3 Jan 2017 18:28:51 +0200 Subject: xhci: Fix race related to abort operation Current abort operation has race. xhci_handle_command_timeout() xhci_abort_cmd_ring() xhci_write_64(CMD_RING_ABORT) xhci_handshake(5s) do { check CMD_RING_RUNNING udelay(1) ... COMP_CMD_ABORT event COMP_CMD_STOP event xhci_handle_stopped_cmd_ring() restart cmd_ring CMD_RING_RUNNING become 1 again } while () return -ETIMEDOUT xhci_write_64(CMD_RING_ABORT) /* can abort random command */ To do abort operation correctly, we have to wait both of COMP_CMD_STOP event and negation of CMD_RING_RUNNING. But like above, while timeout handler is waiting negation of CMD_RING_RUNNING, event handler can restart cmd_ring. So timeout handler never be notice negation of CMD_RING_RUNNING, and retry of CMD_RING_ABORT can abort random command (BTW, I guess retry of CMD_RING_ABORT was workaround of this race). To fix this race, this moves xhci_handle_stopped_cmd_ring() to xhci_abort_cmd_ring(). And timeout handler waits COMP_CMD_STOP event. At this point, timeout handler is owner of cmd_ring, and safely restart cmd_ring by using xhci_handle_stopped_cmd_ring(). [FWIW, as bonus, this way would be easily extend to add CMD_RING_PAUSE operation] [locks edited as patch is rebased on other locking fixes -Mathias] Signed-off-by: OGAWA Hirofumi Signed-off-by: Mathias Nyman Signed-off-by: Greg Kroah-Hartman --- drivers/usb/host/xhci.h | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/usb/host/xhci.h') diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index b54c486df956..2d7b6374b58d 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1569,6 +1569,7 @@ struct xhci_hcd { struct list_head cmd_list; unsigned int cmd_ring_reserved_trbs; struct delayed_work cmd_timer; + struct completion cmd_ring_stop_completion; struct xhci_command *current_cmd; struct xhci_ring *event_ring; struct xhci_erst erst; -- cgit v1.2.3