aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLu Baolu <baolu.lu@linux.intel.com>2018-03-08 10:17:15 -0500
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2018-03-08 12:06:53 -0500
commita098dc8b03d1c7284ed98d921224fffa15b13ece (patch)
tree4bfd0bd7ff912a89b5f3367b599aa1e3130aaccf
parent97ef0faf575e03b352553f92c9430cb4c0431436 (diff)
usb: xhci: dbc: Fix lockdep warning
The xHCI DbC implementation might enter a deadlock situation because there is no sufficient protection against the shared data between process and softirq contexts. This can lead to the following lockdep warnings. This patch changes to use spin_{,un}lock_irq{save,restore} to avoid potential deadlock. [ 528.248084] ================================ [ 528.252914] WARNING: inconsistent lock state [ 528.257756] 4.15.0-rc1+ #1630 Not tainted [ 528.262305] -------------------------------- [ 528.267145] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 528.273953] ksoftirqd/1/17 [HC0[0]:SC1[1]:HE0:SE0] takes: [ 528.280075] (&(&port->port_lock)->rlock){+.?.}, at: [<ffffffff815396a8>] dbc_rx_push+0x38/0x1c0 [ 528.290043] {SOFTIRQ-ON-W} state was registered at: [ 528.295570] _raw_spin_lock+0x2f/0x40 [ 528.299818] dbc_write_complete+0x27/0xa0 [ 528.304458] xhci_dbc_giveback+0xd1/0x200 [ 528.309098] xhci_dbc_flush_endpoint_requests+0x50/0x70 [ 528.315116] xhci_dbc_handle_events+0x696/0x7b0 [ 528.320349] process_one_work+0x1ee/0x6e0 [ 528.324988] worker_thread+0x4a/0x430 [ 528.329236] kthread+0x13e/0x170 [ 528.332992] ret_from_fork+0x24/0x30 [ 528.337141] irq event stamp: 2861 [ 528.340897] hardirqs last enabled at (2860): [<ffffffff810674ea>] tasklet_action+0x6a/0x250 [ 528.350460] hardirqs last disabled at (2861): [<ffffffff817dc1ef>] _raw_spin_lock_irq+0xf/0x40 [ 528.360219] softirqs last enabled at (2852): [<ffffffff817e0e8c>] __do_softirq+0x3dc/0x4f9 [ 528.369683] softirqs last disabled at (2857): [<ffffffff8106805b>] run_ksoftirqd+0x1b/0x60 [ 528.379048] [ 528.379048] other info that might help us debug this: [ 528.386443] Possible unsafe locking scenario: [ 528.386443] [ 528.393150] CPU0 [ 528.395917] ---- [ 528.398687] lock(&(&port->port_lock)->rlock); [ 528.403821] <Interrupt> [ 528.406786] lock(&(&port->port_lock)->rlock); [ 528.412116] [ 528.412116] *** DEADLOCK *** [ 528.412116] [ 528.418825] no locks held by ksoftirqd/1/17. [ 528.423662] [ 528.423662] stack backtrace: [ 528.428598] CPU: 1 PID: 17 Comm: ksoftirqd/1 Not tainted 4.15.0-rc1+ #1630 [ 528.436387] Call Trace: [ 528.439158] dump_stack+0x5e/0x8e [ 528.442914] print_usage_bug+0x1fc/0x220 [ 528.447357] mark_lock+0x4db/0x5a0 [ 528.451210] __lock_acquire+0x726/0x1130 [ 528.455655] ? __lock_acquire+0x557/0x1130 [ 528.460296] lock_acquire+0xa2/0x200 [ 528.464347] ? dbc_rx_push+0x38/0x1c0 [ 528.468496] _raw_spin_lock_irq+0x35/0x40 [ 528.473038] ? dbc_rx_push+0x38/0x1c0 [ 528.477186] dbc_rx_push+0x38/0x1c0 [ 528.481139] tasklet_action+0x1d2/0x250 [ 528.485483] __do_softirq+0x1dc/0x4f9 [ 528.489630] run_ksoftirqd+0x1b/0x60 [ 528.493682] smpboot_thread_fn+0x179/0x270 [ 528.498324] kthread+0x13e/0x170 [ 528.501981] ? sort_range+0x20/0x20 [ 528.505933] ? kthread_delayed_work_timer_fn+0x80/0x80 [ 528.511755] ret_from_fork+0x24/0x30 Fixes: dfba2174dc42 ("usb: xhci: Add DbC support in xHCI driver") Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/usb/host/xhci-dbgcap.c20
-rw-r--r--drivers/usb/host/xhci-dbgtty.c20
2 files changed, 24 insertions, 16 deletions
diff --git a/drivers/usb/host/xhci-dbgcap.c b/drivers/usb/host/xhci-dbgcap.c
index a1ab8acf39ba..c359bae7b754 100644
--- a/drivers/usb/host/xhci-dbgcap.c
+++ b/drivers/usb/host/xhci-dbgcap.c
@@ -328,13 +328,14 @@ dbc_ep_do_queue(struct dbc_ep *dep, struct dbc_request *req)
328int dbc_ep_queue(struct dbc_ep *dep, struct dbc_request *req, 328int dbc_ep_queue(struct dbc_ep *dep, struct dbc_request *req,
329 gfp_t gfp_flags) 329 gfp_t gfp_flags)
330{ 330{
331 unsigned long flags;
331 struct xhci_dbc *dbc = dep->dbc; 332 struct xhci_dbc *dbc = dep->dbc;
332 int ret = -ESHUTDOWN; 333 int ret = -ESHUTDOWN;
333 334
334 spin_lock(&dbc->lock); 335 spin_lock_irqsave(&dbc->lock, flags);
335 if (dbc->state == DS_CONFIGURED) 336 if (dbc->state == DS_CONFIGURED)
336 ret = dbc_ep_do_queue(dep, req); 337 ret = dbc_ep_do_queue(dep, req);
337 spin_unlock(&dbc->lock); 338 spin_unlock_irqrestore(&dbc->lock, flags);
338 339
339 mod_delayed_work(system_wq, &dbc->event_work, 0); 340 mod_delayed_work(system_wq, &dbc->event_work, 0);
340 341
@@ -521,15 +522,16 @@ static void xhci_do_dbc_stop(struct xhci_hcd *xhci)
521static int xhci_dbc_start(struct xhci_hcd *xhci) 522static int xhci_dbc_start(struct xhci_hcd *xhci)
522{ 523{
523 int ret; 524 int ret;
525 unsigned long flags;
524 struct xhci_dbc *dbc = xhci->dbc; 526 struct xhci_dbc *dbc = xhci->dbc;
525 527
526 WARN_ON(!dbc); 528 WARN_ON(!dbc);
527 529
528 pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller); 530 pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller);
529 531
530 spin_lock(&dbc->lock); 532 spin_lock_irqsave(&dbc->lock, flags);
531 ret = xhci_do_dbc_start(xhci); 533 ret = xhci_do_dbc_start(xhci);
532 spin_unlock(&dbc->lock); 534 spin_unlock_irqrestore(&dbc->lock, flags);
533 535
534 if (ret) { 536 if (ret) {
535 pm_runtime_put(xhci_to_hcd(xhci)->self.controller); 537 pm_runtime_put(xhci_to_hcd(xhci)->self.controller);
@@ -541,6 +543,7 @@ static int xhci_dbc_start(struct xhci_hcd *xhci)
541 543
542static void xhci_dbc_stop(struct xhci_hcd *xhci) 544static void xhci_dbc_stop(struct xhci_hcd *xhci)
543{ 545{
546 unsigned long flags;
544 struct xhci_dbc *dbc = xhci->dbc; 547 struct xhci_dbc *dbc = xhci->dbc;
545 struct dbc_port *port = &dbc->port; 548 struct dbc_port *port = &dbc->port;
546 549
@@ -551,9 +554,9 @@ static void xhci_dbc_stop(struct xhci_hcd *xhci)
551 if (port->registered) 554 if (port->registered)
552 xhci_dbc_tty_unregister_device(xhci); 555 xhci_dbc_tty_unregister_device(xhci);
553 556
554 spin_lock(&dbc->lock); 557 spin_lock_irqsave(&dbc->lock, flags);
555 xhci_do_dbc_stop(xhci); 558 xhci_do_dbc_stop(xhci);
556 spin_unlock(&dbc->lock); 559 spin_unlock_irqrestore(&dbc->lock, flags);
557 560
558 pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller); 561 pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller);
559} 562}
@@ -779,14 +782,15 @@ static void xhci_dbc_handle_events(struct work_struct *work)
779 int ret; 782 int ret;
780 enum evtreturn evtr; 783 enum evtreturn evtr;
781 struct xhci_dbc *dbc; 784 struct xhci_dbc *dbc;
785 unsigned long flags;
782 struct xhci_hcd *xhci; 786 struct xhci_hcd *xhci;
783 787
784 dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work); 788 dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
785 xhci = dbc->xhci; 789 xhci = dbc->xhci;
786 790
787 spin_lock(&dbc->lock); 791 spin_lock_irqsave(&dbc->lock, flags);
788 evtr = xhci_dbc_do_handle_events(dbc); 792 evtr = xhci_dbc_do_handle_events(dbc);
789 spin_unlock(&dbc->lock); 793 spin_unlock_irqrestore(&dbc->lock, flags);
790 794
791 switch (evtr) { 795 switch (evtr) {
792 case EVT_GSER: 796 case EVT_GSER:
diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c
index 8d47b6fbf973..75f0b92694ba 100644
--- a/drivers/usb/host/xhci-dbgtty.c
+++ b/drivers/usb/host/xhci-dbgtty.c
@@ -92,21 +92,23 @@ static void dbc_start_rx(struct dbc_port *port)
92static void 92static void
93dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req) 93dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req)
94{ 94{
95 unsigned long flags;
95 struct xhci_dbc *dbc = xhci->dbc; 96 struct xhci_dbc *dbc = xhci->dbc;
96 struct dbc_port *port = &dbc->port; 97 struct dbc_port *port = &dbc->port;
97 98
98 spin_lock(&port->port_lock); 99 spin_lock_irqsave(&port->port_lock, flags);
99 list_add_tail(&req->list_pool, &port->read_queue); 100 list_add_tail(&req->list_pool, &port->read_queue);
100 tasklet_schedule(&port->push); 101 tasklet_schedule(&port->push);
101 spin_unlock(&port->port_lock); 102 spin_unlock_irqrestore(&port->port_lock, flags);
102} 103}
103 104
104static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req) 105static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
105{ 106{
107 unsigned long flags;
106 struct xhci_dbc *dbc = xhci->dbc; 108 struct xhci_dbc *dbc = xhci->dbc;
107 struct dbc_port *port = &dbc->port; 109 struct dbc_port *port = &dbc->port;
108 110
109 spin_lock(&port->port_lock); 111 spin_lock_irqsave(&port->port_lock, flags);
110 list_add(&req->list_pool, &port->write_pool); 112 list_add(&req->list_pool, &port->write_pool);
111 switch (req->status) { 113 switch (req->status) {
112 case 0: 114 case 0:
@@ -119,7 +121,7 @@ static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
119 req->status); 121 req->status);
120 break; 122 break;
121 } 123 }
122 spin_unlock(&port->port_lock); 124 spin_unlock_irqrestore(&port->port_lock, flags);
123} 125}
124 126
125static void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req) 127static void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req)
@@ -327,12 +329,13 @@ static void dbc_rx_push(unsigned long _port)
327{ 329{
328 struct dbc_request *req; 330 struct dbc_request *req;
329 struct tty_struct *tty; 331 struct tty_struct *tty;
332 unsigned long flags;
330 bool do_push = false; 333 bool do_push = false;
331 bool disconnect = false; 334 bool disconnect = false;
332 struct dbc_port *port = (void *)_port; 335 struct dbc_port *port = (void *)_port;
333 struct list_head *queue = &port->read_queue; 336 struct list_head *queue = &port->read_queue;
334 337
335 spin_lock_irq(&port->port_lock); 338 spin_lock_irqsave(&port->port_lock, flags);
336 tty = port->port.tty; 339 tty = port->port.tty;
337 while (!list_empty(queue)) { 340 while (!list_empty(queue)) {
338 req = list_first_entry(queue, struct dbc_request, list_pool); 341 req = list_first_entry(queue, struct dbc_request, list_pool);
@@ -392,16 +395,17 @@ static void dbc_rx_push(unsigned long _port)
392 if (!disconnect) 395 if (!disconnect)
393 dbc_start_rx(port); 396 dbc_start_rx(port);
394 397
395 spin_unlock_irq(&port->port_lock); 398 spin_unlock_irqrestore(&port->port_lock, flags);
396} 399}
397 400
398static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty) 401static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty)
399{ 402{
403 unsigned long flags;
400 struct dbc_port *port = container_of(_port, struct dbc_port, port); 404 struct dbc_port *port = container_of(_port, struct dbc_port, port);
401 405
402 spin_lock_irq(&port->port_lock); 406 spin_lock_irqsave(&port->port_lock, flags);
403 dbc_start_rx(port); 407 dbc_start_rx(port);
404 spin_unlock_irq(&port->port_lock); 408 spin_unlock_irqrestore(&port->port_lock, flags);
405 409
406 return 0; 410 return 0;
407} 411}