* sleeping while atomic in dwc3_gadget_start @ 2013-06-26 21:52 Stephen Boyd 2013-06-27 6:58 ` Felipe Balbi 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2013-06-26 21:52 UTC (permalink / raw) To: Felipe Balbi; +Cc: Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman Hi, I'm getting the folllowing BUG message on bootup with 3.10-rc5 BUG: sleeping function called from invalid context at mm/slub.c:926 in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) and I suspect this problem was introduced in commit 8698e2acf (usb: dwc3: gadget: introduce and use enable/disable irq methods). Is there a fix for this problem? Can we just move the irq request outside the spinlock? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sleeping while atomic in dwc3_gadget_start 2013-06-26 21:52 sleeping while atomic in dwc3_gadget_start Stephen Boyd @ 2013-06-27 6:58 ` Felipe Balbi 2013-06-27 16:57 ` Stephen Boyd 0 siblings, 1 reply; 6+ messages in thread From: Felipe Balbi @ 2013-06-27 6:58 UTC (permalink / raw) To: Stephen Boyd Cc: Felipe Balbi, Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman [-- Attachment #1: Type: text/plain, Size: 3221 bytes --] On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: > Hi, > > I'm getting the folllowing BUG message on bootup with 3.10-rc5 > > BUG: sleeping function called from invalid context at mm/slub.c:926 > in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 > [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) > [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) > [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) > [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) > [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) > [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) > > and I suspect this problem was introduced in commit 8698e2acf > (usb: dwc3: gadget: introduce and use enable/disable irq > methods). Is there a fix for this problem? Can we just move the > irq request outside the spinlock? nice :-) how about this ? diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b5e5b35..a8e6b8b 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1508,6 +1508,15 @@ static int dwc3_gadget_start(struct usb_gadget *g, int irq; u32 reg; + irq = platform_get_irq(to_platform_device(dwc->dev), 0); + ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, + IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc); + if (ret) { + dev_err(dwc->dev, "failed to request irq #%d --> %d\n", + irq, ret); + goto err0; + } + spin_lock_irqsave(&dwc->lock, flags); if (dwc->gadget_driver) { @@ -1515,7 +1524,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, dwc->gadget.name, dwc->gadget_driver->driver.name); ret = -EBUSY; - goto err0; + goto err1; } dwc->gadget_driver = driver; @@ -1551,41 +1560,33 @@ static int dwc3_gadget_start(struct usb_gadget *g, ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false); if (ret) { dev_err(dwc->dev, "failed to enable %s\n", dep->name); - goto err0; + goto err1; } dep = dwc->eps[1]; ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false); if (ret) { dev_err(dwc->dev, "failed to enable %s\n", dep->name); - goto err1; + goto err2; } /* begin to receive SETUP packets */ dwc->ep0state = EP0_SETUP_PHASE; dwc3_ep0_out_start(dwc); - irq = platform_get_irq(to_platform_device(dwc->dev), 0); - ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, - IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc); - if (ret) { - dev_err(dwc->dev, "failed to request irq #%d --> %d\n", - irq, ret); - goto err1; - } - dwc3_gadget_enable_irq(dwc); spin_unlock_irqrestore(&dwc->lock, flags); return 0; -err1: +err2: __dwc3_gadget_ep_disable(dwc->eps[0]); -err0: +err1: spin_unlock_irqrestore(&dwc->lock, flags); +err0: return ret; } -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sleeping while atomic in dwc3_gadget_start 2013-06-27 6:58 ` Felipe Balbi @ 2013-06-27 16:57 ` Stephen Boyd 2013-06-28 10:58 ` Felipe Balbi 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2013-06-27 16:57 UTC (permalink / raw) To: balbi; +Cc: Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman On 06/26/13 23:58, Felipe Balbi wrote: > On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: >> Hi, >> >> I'm getting the folllowing BUG message on bootup with 3.10-rc5 >> >> BUG: sleeping function called from invalid context at mm/slub.c:926 >> in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 >> [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) >> [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) >> [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) >> [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) >> [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) >> [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) >> >> and I suspect this problem was introduced in commit 8698e2acf >> (usb: dwc3: gadget: introduce and use enable/disable irq >> methods). Is there a fix for this problem? Can we just move the >> irq request outside the spinlock? > nice :-) > > how about this ? If start fails do you call stop? I believe the answer is no, so we'll need to free_irq() somewhere along the error path. Or we can request it after the spin_unlock()? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sleeping while atomic in dwc3_gadget_start 2013-06-27 16:57 ` Stephen Boyd @ 2013-06-28 10:58 ` Felipe Balbi 2013-06-29 2:02 ` Stephen Boyd 0 siblings, 1 reply; 6+ messages in thread From: Felipe Balbi @ 2013-06-28 10:58 UTC (permalink / raw) To: Stephen Boyd Cc: balbi, Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman [-- Attachment #1: Type: text/plain, Size: 4783 bytes --] Hi, On Thu, Jun 27, 2013 at 09:57:52AM -0700, Stephen Boyd wrote: > On 06/26/13 23:58, Felipe Balbi wrote: > > On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: > >> Hi, > >> > >> I'm getting the folllowing BUG message on bootup with 3.10-rc5 > >> > >> BUG: sleeping function called from invalid context at mm/slub.c:926 > >> in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 > >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 > >> [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) > >> [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) > >> [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) > >> [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) > >> [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) > >> [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) > >> > >> and I suspect this problem was introduced in commit 8698e2acf > >> (usb: dwc3: gadget: introduce and use enable/disable irq > >> methods). Is there a fix for this problem? Can we just move the > >> irq request outside the spinlock? > > nice :-) > > > > how about this ? > > If start fails do you call stop? I believe the answer is no, so we'll > need to free_irq() somewhere along the error path. Or we can request it > after the spin_unlock()? good point here's v2: From 1a7896c5481646e533ca09ba60918831d9849ed8 Mon Sep 17 00:00:00 2001 From: Felipe Balbi <balbi@ti.com> Date: Thu, 27 Jun 2013 10:00:18 +0300 Subject: [PATCH] usb: dwc3: gadget: don't request IRQs in atomic We cannot request an IRQ with spinlocks held as that would trigger a sleeping inside spinlock warning. Cc: <stable@vger.kernel.org> # v3.10 Reported-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/usb/dwc3/gadget.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b5e5b35..bb0d312c 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1508,6 +1508,15 @@ static int dwc3_gadget_start(struct usb_gadget *g, int irq; u32 reg; + irq = platform_get_irq(to_platform_device(dwc->dev), 0); + ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, + IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc); + if (ret) { + dev_err(dwc->dev, "failed to request irq #%d --> %d\n", + irq, ret); + goto err0; + } + spin_lock_irqsave(&dwc->lock, flags); if (dwc->gadget_driver) { @@ -1515,7 +1524,7 @@ static int dwc3_gadget_start(struct usb_gadget *g, dwc->gadget.name, dwc->gadget_driver->driver.name); ret = -EBUSY; - goto err0; + goto err1; } dwc->gadget_driver = driver; @@ -1551,41 +1560,35 @@ static int dwc3_gadget_start(struct usb_gadget *g, ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false); if (ret) { dev_err(dwc->dev, "failed to enable %s\n", dep->name); - goto err0; + goto err1; } dep = dwc->eps[1]; ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false); if (ret) { dev_err(dwc->dev, "failed to enable %s\n", dep->name); - goto err1; + goto err2; } /* begin to receive SETUP packets */ dwc->ep0state = EP0_SETUP_PHASE; dwc3_ep0_out_start(dwc); - irq = platform_get_irq(to_platform_device(dwc->dev), 0); - ret = request_threaded_irq(irq, dwc3_interrupt, dwc3_thread_interrupt, - IRQF_SHARED | IRQF_ONESHOT, "dwc3", dwc); - if (ret) { - dev_err(dwc->dev, "failed to request irq #%d --> %d\n", - irq, ret); - goto err1; - } - dwc3_gadget_enable_irq(dwc); spin_unlock_irqrestore(&dwc->lock, flags); return 0; -err1: +err2: __dwc3_gadget_ep_disable(dwc->eps[0]); -err0: +err1: spin_unlock_irqrestore(&dwc->lock, flags); +err0: + free_irq(irq, dwc); + return ret; } @@ -1599,9 +1602,6 @@ static int dwc3_gadget_stop(struct usb_gadget *g, spin_lock_irqsave(&dwc->lock, flags); dwc3_gadget_disable_irq(dwc); - irq = platform_get_irq(to_platform_device(dwc->dev), 0); - free_irq(irq, dwc); - __dwc3_gadget_ep_disable(dwc->eps[0]); __dwc3_gadget_ep_disable(dwc->eps[1]); @@ -1609,6 +1609,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g, spin_unlock_irqrestore(&dwc->lock, flags); + irq = platform_get_irq(to_platform_device(dwc->dev), 0); + free_irq(irq, dwc); + return 0; } -- 1.8.2.1 -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sleeping while atomic in dwc3_gadget_start 2013-06-28 10:58 ` Felipe Balbi @ 2013-06-29 2:02 ` Stephen Boyd 2013-07-01 12:16 ` Felipe Balbi 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2013-06-29 2:02 UTC (permalink / raw) To: balbi; +Cc: Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman On 06/28/13 03:58, Felipe Balbi wrote: > Hi, > > On Thu, Jun 27, 2013 at 09:57:52AM -0700, Stephen Boyd wrote: >> On 06/26/13 23:58, Felipe Balbi wrote: >>> On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: >>>> Hi, >>>> >>>> I'm getting the folllowing BUG message on bootup with 3.10-rc5 >>>> >>>> BUG: sleeping function called from invalid context at mm/slub.c:926 >>>> in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 >>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 >>>> [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) >>>> [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) >>>> [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) >>>> [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) >>>> [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) >>>> [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) >>>> >>>> and I suspect this problem was introduced in commit 8698e2acf >>>> (usb: dwc3: gadget: introduce and use enable/disable irq >>>> methods). Is there a fix for this problem? Can we just move the >>>> irq request outside the spinlock? >>> nice :-) >>> >>> how about this ? >> If start fails do you call stop? I believe the answer is no, so we'll >> need to free_irq() somewhere along the error path. Or we can request it >> after the spin_unlock()? > good point here's v2: Ok looks good to me. I hope that platform_get_irq() doesn't fail, otherwise we're in for a nasty surprise. Maybe we should add a check in request_irq() for that case. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sleeping while atomic in dwc3_gadget_start 2013-06-29 2:02 ` Stephen Boyd @ 2013-07-01 12:16 ` Felipe Balbi 0 siblings, 0 replies; 6+ messages in thread From: Felipe Balbi @ 2013-07-01 12:16 UTC (permalink / raw) To: Stephen Boyd Cc: balbi, Vivek Gautam, linux-usb, linux-kernel, Greg Kroah-Hartman [-- Attachment #1: Type: text/plain, Size: 2084 bytes --] On Fri, Jun 28, 2013 at 07:02:32PM -0700, Stephen Boyd wrote: > On 06/28/13 03:58, Felipe Balbi wrote: > > Hi, > > > > On Thu, Jun 27, 2013 at 09:57:52AM -0700, Stephen Boyd wrote: > >> On 06/26/13 23:58, Felipe Balbi wrote: > >>> On Wed, Jun 26, 2013 at 02:52:56PM -0700, Stephen Boyd wrote: > >>>> Hi, > >>>> > >>>> I'm getting the folllowing BUG message on bootup with 3.10-rc5 > >>>> > >>>> BUG: sleeping function called from invalid context at mm/slub.c:926 > >>>> in_atomic(): 1, irqs_disabled(): 128, pid: 1, name: swapper/0 > >>>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.10.0-rc5-gee3e35b-09316-ge78f3b35 #643 > >>>> [<c0014220>] (unwind_backtrace+0x0/0x120) from [<c001212c>] (show_stack+0x10/0x14) > >>>> [<c001212c>] (show_stack+0x10/0x14) from [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) > >>>> [<c0143750>] (kmem_cache_alloc_trace+0x3c/0x210) from [<c00e0c60>] (request_threaded_irq+0x88/0x11c) > >>>> [<c00e0c60>] (request_threaded_irq+0x88/0x11c) from [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) > >>>> [<c03bf53c>] (dwc3_gadget_start+0x198/0x200) from [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) > >>>> [<c03f7a5c>] (udc_bind_to_driver+0x70/0xd8) from [<c03f7b50>] (usb_gadget_probe_driver+0x8c/0xb8) > >>>> > >>>> and I suspect this problem was introduced in commit 8698e2acf > >>>> (usb: dwc3: gadget: introduce and use enable/disable irq > >>>> methods). Is there a fix for this problem? Can we just move the > >>>> irq request outside the spinlock? > >>> nice :-) > >>> > >>> how about this ? > >> If start fails do you call stop? I believe the answer is no, so we'll > >> need to free_irq() somewhere along the error path. Or we can request it > >> after the spin_unlock()? > > good point here's v2: > > Ok looks good to me. I hope that platform_get_irq() doesn't fail, > otherwise we're in for a nasty surprise. Maybe we should add a check in > request_irq() for that case. I don't think we would ever fall into that situation. And if we do, our data (struct resource or DT) is f-ed up anyway :-) -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-01 12:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-06-26 21:52 sleeping while atomic in dwc3_gadget_start Stephen Boyd 2013-06-27 6:58 ` Felipe Balbi 2013-06-27 16:57 ` Stephen Boyd 2013-06-28 10:58 ` Felipe Balbi 2013-06-29 2:02 ` Stephen Boyd 2013-07-01 12:16 ` Felipe Balbi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).