iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

usb: xhci: dbc: Fix lockdep warning

Browse Source 
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: dfba2174dc ("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>
This commit is contained in:
Lu Baolu 2018-03-08 17:17:15 +02:00 committed by Greg Kroah-Hartman
parent 97ef0faf57
commit a098dc8b03
2 changed files with 24 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
drivers/usb/host/xhci-dbgcap.c
View File

@ -328,13 +328,14 @@ dbc_ep_do_queue(struct dbc_ep *dep, struct dbc_request *req)
int dbc_ep_queue(struct dbc_ep *dep, struct dbc_request *req, int dbc_ep_queue(struct dbc_ep *dep, struct dbc_request *req,
gfp_t gfp_flags) gfp_t gfp_flags)
{ {
unsigned long flags;
struct xhci_dbc *dbc = dep->dbc; struct xhci_dbc *dbc = dep->dbc;
int ret = -ESHUTDOWN; int ret = -ESHUTDOWN;
spin_lock(&dbc->lock); spin_lock_irqsave(&dbc->lock, flags);
if (dbc->state == DS_CONFIGURED) if (dbc->state == DS_CONFIGURED)
ret = dbc_ep_do_queue(dep, req); ret = dbc_ep_do_queue(dep, req);
spin_unlock(&dbc->lock); spin_unlock_irqrestore(&dbc->lock, flags);
mod_delayed_work(system_wq, &dbc->event_work, 0); mod_delayed_work(system_wq, &dbc->event_work, 0);
@ -521,15 +522,16 @@ static void xhci_do_dbc_stop(struct xhci_hcd *xhci)
static int xhci_dbc_start(struct xhci_hcd *xhci) static int xhci_dbc_start(struct xhci_hcd *xhci)
{ {
int ret; int ret;
unsigned long flags;
struct xhci_dbc *dbc = xhci->dbc; struct xhci_dbc *dbc = xhci->dbc;
WARN_ON(!dbc); WARN_ON(!dbc);
pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller); pm_runtime_get_sync(xhci_to_hcd(xhci)->self.controller);
spin_lock(&dbc->lock); spin_lock_irqsave(&dbc->lock, flags);
ret = xhci_do_dbc_start(xhci); ret = xhci_do_dbc_start(xhci);
spin_unlock(&dbc->lock); spin_unlock_irqrestore(&dbc->lock, flags);
if (ret) { if (ret) {
pm_runtime_put(xhci_to_hcd(xhci)->self.controller); pm_runtime_put(xhci_to_hcd(xhci)->self.controller);
@ -541,6 +543,7 @@ static int xhci_dbc_start(struct xhci_hcd *xhci)
static void xhci_dbc_stop(struct xhci_hcd *xhci) static void xhci_dbc_stop(struct xhci_hcd *xhci)
{ {
unsigned long flags;
struct xhci_dbc *dbc = xhci->dbc; struct xhci_dbc *dbc = xhci->dbc;
struct dbc_port *port = &dbc->port; struct dbc_port *port = &dbc->port;
@ -551,9 +554,9 @@ static void xhci_dbc_stop(struct xhci_hcd *xhci)
if (port->registered) if (port->registered)
xhci_dbc_tty_unregister_device(xhci); xhci_dbc_tty_unregister_device(xhci);
spin_lock(&dbc->lock); spin_lock_irqsave(&dbc->lock, flags);
xhci_do_dbc_stop(xhci); xhci_do_dbc_stop(xhci);
spin_unlock(&dbc->lock); spin_unlock_irqrestore(&dbc->lock, flags);
pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller); pm_runtime_put_sync(xhci_to_hcd(xhci)->self.controller);
} }
@ -779,14 +782,15 @@ static void xhci_dbc_handle_events(struct work_struct *work)
int ret; int ret;
enum evtreturn evtr; enum evtreturn evtr;
struct xhci_dbc *dbc; struct xhci_dbc *dbc;
unsigned long flags;
struct xhci_hcd *xhci; struct xhci_hcd *xhci;
dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work); dbc = container_of(to_delayed_work(work), struct xhci_dbc, event_work);
xhci = dbc->xhci; xhci = dbc->xhci;
spin_lock(&dbc->lock); spin_lock_irqsave(&dbc->lock, flags);
evtr = xhci_dbc_do_handle_events(dbc); evtr = xhci_dbc_do_handle_events(dbc);
spin_unlock(&dbc->lock); spin_unlock_irqrestore(&dbc->lock, flags);
switch (evtr) { switch (evtr) {
case EVT_GSER: case EVT_GSER:

20
drivers/usb/host/xhci-dbgtty.c
View File

@ -92,21 +92,23 @@ static void dbc_start_rx(struct dbc_port *port)
static void static void
dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req) dbc_read_complete(struct xhci_hcd *xhci, struct dbc_request *req)
{ {
unsigned long flags;
struct xhci_dbc *dbc = xhci->dbc; struct xhci_dbc *dbc = xhci->dbc;
struct dbc_port *port = &dbc->port; struct dbc_port *port = &dbc->port;
spin_lock(&port->port_lock); spin_lock_irqsave(&port->port_lock, flags);
list_add_tail(&req->list_pool, &port->read_queue); list_add_tail(&req->list_pool, &port->read_queue);
tasklet_schedule(&port->push); tasklet_schedule(&port->push);
spin_unlock(&port->port_lock); spin_unlock_irqrestore(&port->port_lock, flags);
} }
static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req) static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
{ {
unsigned long flags;
struct xhci_dbc *dbc = xhci->dbc; struct xhci_dbc *dbc = xhci->dbc;
struct dbc_port *port = &dbc->port; struct dbc_port *port = &dbc->port;
spin_lock(&port->port_lock); spin_lock_irqsave(&port->port_lock, flags);
list_add(&req->list_pool, &port->write_pool); list_add(&req->list_pool, &port->write_pool);
switch (req->status) { switch (req->status) {
case 0: case 0:
@ -119,7 +121,7 @@ static void dbc_write_complete(struct xhci_hcd *xhci, struct dbc_request *req)
req->status); req->status);
break; break;
} }
spin_unlock(&port->port_lock); spin_unlock_irqrestore(&port->port_lock, flags);
} }
static void xhci_dbc_free_req(struct dbc_ep *dep, struct dbc_request *req) static 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)
{ {
struct dbc_request *req; struct dbc_request *req;
struct tty_struct *tty; struct tty_struct *tty;
unsigned long flags;
bool do_push = false; bool do_push = false;
bool disconnect = false; bool disconnect = false;
struct dbc_port *port = (void *)_port; struct dbc_port *port = (void *)_port;
struct list_head *queue = &port->read_queue; struct list_head *queue = &port->read_queue;
spin_lock_irq(&port->port_lock); spin_lock_irqsave(&port->port_lock, flags);
tty = port->port.tty; tty = port->port.tty;
while (!list_empty(queue)) { while (!list_empty(queue)) {
req = list_first_entry(queue, struct dbc_request, list_pool); req = list_first_entry(queue, struct dbc_request, list_pool);
@ -392,16 +395,17 @@ static void dbc_rx_push(unsigned long _port)
if (!disconnect) if (!disconnect)
dbc_start_rx(port); dbc_start_rx(port);
spin_unlock_irq(&port->port_lock); spin_unlock_irqrestore(&port->port_lock, flags);
} }
static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty) static int dbc_port_activate(struct tty_port *_port, struct tty_struct *tty)
{ {
unsigned long flags;
struct dbc_port *port = container_of(_port, struct dbc_port, port); struct dbc_port *port = container_of(_port, struct dbc_port, port);
spin_lock_irq(&port->port_lock); spin_lock_irqsave(&port->port_lock, flags);
dbc_start_rx(port); dbc_start_rx(port);
spin_unlock_irq(&port->port_lock); spin_unlock_irqrestore(&port->port_lock, flags);
return 0; return 0;
} }