ehci calling put_device from irq handler

ehci calling put_device from irq handler

[Greg KH]

We can not call put_device() from irq context :(

I added a "might_sleep()" to the driver core and get the following from
the ehci driver.  Any thoughts?

thanks,

greg k-h


Debug: sleeping function called from invalid context at drivers/base/core.c:343
in_atomic():1, irqs_disabled():0
 [<c012006d>] __might_sleep+0x9e/0xa6
 [<c0270400>] put_device+0x1c/0x35
 [<f883ebde>] usb_hcd_giveback_urb+0x23/0x84 [usbcore]
 [<f8820f40>] ehci_urb_done+0x8d/0xcf [ehci_hcd]
 [<f8821138>] qh_completions+0x1b6/0x359 [ehci_hcd]
 [<f882208d>] scan_async+0x8a/0x13f [ehci_hcd]
 [<f8824e77>] ehci_work+0x59/0xd2 [ehci_hcd]
 [<f88255d7>] ehci_irq+0x185/0x336 [ehci_hcd]
 [<c0106a10>] do_gettimeofday+0x1e/0xbf
 [<c0127ae5>] getnstimeofday+0x14/0x2a
 [<c0139f6c>] ktime_get_ts+0x5e/0x66
 [<c0139ea3>] ktime_get+0x1b/0x43
 [<c013a521>] hrtimer_run_queues+0x50/0xf9
 [<f883ec6f>] usb_hcd_irq+0x30/0x68 [usbcore]
 [<c0144b18>] handle_IRQ_event+0x39/0x6d
 [<c0144bd4>] __do_IRQ+0x88/0xfc
 [<c0105789>] do_IRQ+0x19/0x24
 [<c0103b82>] common_interrupt+0x1a/0x20
 [<c01012a5>] mwait_idle+0x2a/0x34
 [<c0101103>] cpu_idle+0x7f/0xb4
 [<c041e898>] start_kernel+0x166/0x17f
 [<c041e2b6>] unknown_bootoption+0x0/0x1e0

Re: [linux-usb-devel] ehci calling put_device from irq handler

[Alan Stern]

On Wed, 18 Jan 2006, Greg KH wrote:

> We can not call put_device() from irq context :(
> 
> I added a "might_sleep()" to the driver core and get the following from
> the ehci driver.  Any thoughts?

In principle the put_device and corresponding get_device calls aren't
needed.  We don't release a usb_device structure until after disabling all
its endpoints, and disabling an endpoint will wait until all the URBs for
that endpoint have completed.  So there's no reason to keep a reference to
the device structure for each URB.

I see that uhci-hcd is guilty of the same thing (reference acquired for 
each QH, released while holding a spinlock).  Probably each of the 
host controller drivers is.

Alan Stern

Re: [linux-usb-devel] ehci calling put_device from irq handler

[Greg KH]

On Wed, Jan 18, 2006 at 04:54:04PM -0500, Alan Stern wrote:
> On Wed, 18 Jan 2006, Greg KH wrote:
> 
> > We can not call put_device() from irq context :(
> > 
> > I added a "might_sleep()" to the driver core and get the following from
> > the ehci driver.  Any thoughts?
> 
> In principle the put_device and corresponding get_device calls aren't
> needed.  We don't release a usb_device structure until after disabling all
> its endpoints, and disabling an endpoint will wait until all the URBs for
> that endpoint have completed.  So there's no reason to keep a reference to
> the device structure for each URB.
> 
> I see that uhci-hcd is guilty of the same thing (reference acquired for 
> each QH, released while holding a spinlock).  Probably each of the 
> host controller drivers is.

Great, care to make up a patch to fix this?

:)

thanks,

greg k-h

Re: [linux-usb-devel] ehci calling put_device from irq handler

[David Brownell]

On Wednesday 18 January 2006 1:54 pm, Alan Stern wrote:
> On Wed, 18 Jan 2006, Greg KH wrote:
> 
> > We can not call put_device() from irq context :(
> > 
> > I added a "might_sleep()" to the driver core and get the following from
> > the ehci driver.  Any thoughts?
> 
> In principle the put_device and corresponding get_device calls aren't
> needed.  We don't release a usb_device structure until after disabling all
> its endpoints, and disabling an endpoint will wait until all the URBs for
> that endpoint have completed.  So there's no reason to keep a reference to
> the device structure for each URB.
> 
> I see that uhci-hcd is guilty of the same thing (reference acquired for 
> each QH, released while holding a spinlock).  Probably each of the 
> host controller drivers is.

I think it dated from the days before we had any mechanism for
disabling endpoints properly, and was (important!) insurance
against the inevitable "driver disconnect() didn't work right"
class of bugs.

So it should be safe to remove those calls now.

- Dave