From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from comal.ext.ti.com ([198.47.26.152]:37104 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754648AbcEZQgZ (ORCPT ); Thu, 26 May 2016 12:36:25 -0400 Date: Thu, 26 May 2016 11:36:22 -0500 From: Bin Liu To: Felipe Balbi CC: , Subject: Re: [PATCH v2] usb: gadget: fix spinlock dead lock in gadgetfs Message-ID: <20160526163622.GB26723@uda0271908> References: <1464203914-17339-1-git-send-email-b-liu@ti.com> <20160525192458.GA12150@uda0271908> <87bn3tqqpd.fsf@linux.intel.com> <20160526153842.GA26723@uda0271908> <87mvnclr97.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <87mvnclr97.fsf@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: Hi, On Thu, May 26, 2016 at 07:26:44PM +0300, Felipe Balbi wrote: > > >> Bin Liu writes: > >> > On Wed, May 25, 2016 at 02:18:34PM -0500, Bin Liu wrote: > >> >> [ 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: [] ep0_complete+0x18/0xdc [gadgetfs] > >> >> [ 40.502882] > >> >> [ 40.502882] but task is already holding lock: > >> >> [ 40.508967] (&(&dev->lock)->rlock){-.....}, at: [] 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: [] __fdget_pos+0x40/0x48 > >> >> [ 40.569219] #1: (&(&dev->lock)->rlock){-.....}, at: [] 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] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > >> >> [ 40.604718] [] (show_stack) from [] (dump_stack+0xb0/0xe4) > >> >> [ 40.612267] [] (dump_stack) from [] (__lock_acquire+0xf68/0x1994) > >> >> [ 40.620440] [] (__lock_acquire) from [] (lock_acquire+0xd8/0x238) > >> >> [ 40.628621] [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x38/0x4c) > >> >> [ 40.637440] [] (_raw_spin_lock_irqsave) from [] (ep0_complete+0x18/0xdc [gadgetfs]) > >> >> [ 40.647339] [] (ep0_complete [gadgetfs]) from [] (musb_g_giveback+0x118/0x1b0 [musb_hdrc]) > >> >> [ 40.657842] [] (musb_g_giveback [musb_hdrc]) from [] (musb_g_ep0_queue+0x16c/0x188 [musb_hdrc]) > >> >> [ 40.668772] [] (musb_g_ep0_queue [musb_hdrc]) from [] (ep0_read+0x544/0x5e0 [gadgetfs]) > >> >> [ 40.678963] [] (ep0_read [gadgetfs]) from [] (__vfs_read+0x20/0x110) > >> >> [ 40.687414] [] (__vfs_read) from [] (vfs_read+0x88/0x114) > >> >> [ 40.694864] [] (vfs_read) from [] (SyS_read+0x44/0x9c) > >> >> [ 40.702051] [] (SyS_read) from [] (ret_fast_syscall+0x0/0x1c) > >> >> > >> >> Cc: # v3.16+ > >> >> Signed-off-by: Bin Liu > >> >> --- > >> >> 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 > >> > > >> > Never mind. Sent the patch too soon. It gives the following problem. > >> > > >> > [ 179.810367] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2724 > >> > _raw_spin_unlock_irq+0x24/0x2c > >> > [ 179.819719] DEBUG_LOCKS_WARN_ON(current->hardirq_context) > >> > >> thanks for letting us know. I'm dropping this from my queue. Please > >> resend final patch once you have it ;-) > > > > It turned out to be a very easy fix ;) > > > > I have v3 ready, but checkpatch.pl complains the followings. I don't > > think this patch should fix them, since the whole driver is coded like > > that. Any comments? > > Let's fix it with a follow-up patch. Can you do that one too, btw? I can > do it, if you don't wanna deal with that; but I'm also open to receiving > free patches heh I'd like to do it, but I have a pile of other suff to do. (I still remember I owe you a g-zero test with dwc3...) Regards, -Bin.