[   40.467381] =============================================
[   40.473013] [ INFO: possible recursive locking detected ]
[   40.478651] 4.6.0-08691-g7f3db9a #37 Not tainted
[   40.483466] ---------------------------------------------
[   40.489098] usb/733 is trying to acquire lock:
[   40.493734]  (&(&dev->lock)->rlock){-.....}, at: [<bf129288>] 
ep0_complete+0x18/0xdc [gadgetfs]
[   40.502882]
[   40.502882] but task is already holding lock:
[   40.508967]  (&(&dev->lock)->rlock){-.....}, at: [<bf12a420>] 
ep0_read+0x20/0x5e0 [gadgetfs]
[   40.517811]
[   40.517811] other info that might help us debug this:
[   40.524623]  Possible unsafe locking scenario:
[   40.524623]
[   40.530798]        CPU0
[   40.533346]        ----
[   40.535894]   lock(&(&dev->lock)->rlock);
[   40.540088]   lock(&(&dev->lock)->rlock);
[   40.544284]
[   40.544284]  *** DEADLOCK ***
[   40.544284]
[   40.550461]  May be due to missing lock nesting notation
[   40.550461]
[   40.557544] 2 locks held by usb/733:
[   40.561271]  #0:  (&f->f_pos_lock){+.+.+.}, at: [<c02a6114>] 
__fdget_pos+0x40/0x48
[   40.569219]  #1:  (&(&dev->lock)->rlock){-.....}, at: [<bf12a420>] 
ep0_read+0x20/0x5e0 [gadgetfs]
[   40.578523]
[   40.578523] stack backtrace:
[   40.583075] CPU: 0 PID: 733 Comm: usb Not tainted 4.6.0-08691-g7f3db9a #37
[   40.590246] Hardware name: Generic AM33XX (Flattened Device Tree)
[   40.596625] [<c010ffbc>] (unwind_backtrace) from [<c010c1bc>] 
(show_stack+0x10/0x14)
[   40.604718] [<c010c1bc>] (show_stack) from [<c04207fc>] 
(dump_stack+0xb0/0xe4)
[   40.612267] [<c04207fc>] (dump_stack) from [<c01886ec>] 
(__lock_acquire+0xf68/0x1994)
[   40.620440] [<c01886ec>] (__lock_acquire) from [<c0189528>] 
(lock_acquire+0xd8/0x238)
[   40.628621] [<c0189528>] (lock_acquire) from [<c06ad6b4>] 
(_raw_spin_lock_irqsave+0x38/0x4c)
[   40.637440] [<c06ad6b4>] (_raw_spin_lock_irqsave) from [<bf129288>] 
(ep0_complete+0x18/0xdc [gadgetfs])
[   40.647339] [<bf129288>] (ep0_complete [gadgetfs]) from [<bf10a728>] 
(musb_g_giveback+0x118/0x1b0 [musb_hdrc])
[   40.657842] [<bf10a728>] (musb_g_giveback [musb_hdrc]) from [<bf108768>] 
(musb_g_ep0_queue+0x16c/0x188 [musb_hdrc])
[   40.668772] [<bf108768>] (musb_g_ep0_queue [musb_hdrc]) from [<bf12a944>] 
(ep0_read+0x544/0x5e0 [gadgetfs])
[   40.678963] [<bf12a944>] (ep0_read [gadgetfs]) from [<c0284470>] 
(__vfs_read+0x20/0x110)
[   40.687414] [<c0284470>] (__vfs_read) from [<c0285324>] (vfs_read+0x88/0x114)
[   40.694864] [<c0285324>] (vfs_read) from [<c0286150>] (SyS_read+0x44/0x9c)
[   40.702051] [<c0286150>] (SyS_read) from [<c0107820>] 
(ret_fast_syscall+0x0/0x1c)

This is caused by the spinlock bug in ep0_read().
Fix the two other deadlock sources in gadgetfs_setup() too.

Cc: <sta...@vger.kernel.org> # v3.16+
Signed-off-by: Bin Liu <b-...@ti.com>
---
v3:
  - update commit comments mentioning the fix in gadgetfs_setup()
  - use spin_(un)lock() for fix in gadgetfs_setup(), instead of
    spin_(un)lock_irq(), since locked by spin_lock()
v2:
  - lock protects setup_req(), only unlock for usb_ep_queue()
  - use GFP_KERNEL in usb_ep_queue()
  - fix the same in two places in gadgetfs_setup() too

 drivers/usb/gadget/legacy/inode.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c 
b/drivers/usb/gadget/legacy/inode.c
index e64479f..aa3707b 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -938,8 +938,11 @@ ep0_read (struct file *fd, char __user *buf, size_t len, 
loff_t *ptr)
                        struct usb_ep           *ep = dev->gadget->ep0;
                        struct usb_request      *req = dev->req;
 
-                       if ((retval = setup_req (ep, req, 0)) == 0)
-                               retval = usb_ep_queue (ep, req, GFP_ATOMIC);
+                       if ((retval = setup_req (ep, req, 0)) == 0) {
+                               spin_unlock_irq (&dev->lock);
+                               retval = usb_ep_queue (ep, req, GFP_KERNEL);
+                               spin_lock_irq (&dev->lock);
+                       }
                        dev->state = STATE_DEV_CONNECTED;
 
                        /* assume that was SET_CONFIGURATION */
@@ -1457,8 +1460,11 @@ delegate:
                                                        w_length);
                                if (value < 0)
                                        break;
+
+                               spin_unlock (&dev->lock);
                                value = usb_ep_queue (gadget->ep0, dev->req,
-                                                       GFP_ATOMIC);
+                                                       GFP_KERNEL);
+                               spin_lock (&dev->lock);
                                if (value < 0) {
                                        clean_req (gadget->ep0, dev->req);
                                        break;
@@ -1481,11 +1487,14 @@ delegate:
        if (value >= 0 && dev->state != STATE_DEV_SETUP) {
                req->length = value;
                req->zero = value < w_length;
-               value = usb_ep_queue (gadget->ep0, req, GFP_ATOMIC);
+
+               spin_unlock (&dev->lock);
+               value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL);
                if (value < 0) {
                        DBG (dev, "ep_queue --> %d\n", value);
                        req->status = 0;
                }
+               return value;
        }
 
        /* device stalls when value < 0 */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to