* Re: bluetooth: fix deadlock on device reset and power down [not found] ` <20120402090337.GB29687@aemeltch-MOBL1> @ 2012-04-02 9:17 ` Alexander Holler 2012-04-02 10:16 ` Alexander Holler 0 siblings, 1 reply; 3+ messages in thread From: Alexander Holler @ 2012-04-02 9:17 UTC (permalink / raw) To: Andrei Emeltchenko Cc: David Herrmann, linux-bluetooth, linux-kernel, Gustavo F. Padovan, stable Am 02.04.2012 11:03, schrieb Andrei Emeltchenko: > Hi guys, > > On Mon, Apr 02, 2012 at 10:44:43AM +0200, David Herrmann wrote: >> Hi Andrei and Alexander >> >> On Mon, Apr 2, 2012 at 10:29 AM, Alexander Holler <holler@ahsoftware.de> wrote: >>> Am 02.04.2012 08:55, schrieb Andrei Emeltchenko: >>>> Hi Alexander, >>>> >>>> On Sat, Mar 31, 2012 at 03:23:38PM +0200, Alexander Holler wrote: >>>>> I've experienced a deadlock on shutdown using kernel 3.3 and tracked >>>>> it down. Because I'm not very familiar with the bluetooth stack I'm >>>>> not sure if the below patch is correct, but it fixed the problem >>>>> here. >>>> >>>> Could you please attach deadlock dump? >>>> >>>>> >>>>> Commit 09fd0de5bd8f8ef3317e5365f92f1a13dcd89aa9 introduced a deadlock: >>>>> >>>>> bluetoothd calls ioctl HCIDEVDOWN >>>>> hci_sock_ioctl() >>>>> hci_dev_close() >>>>> hci_dev_do_close() >>>>> hci_dev_lock(hdev); >>>>> inquiry_cache_flush(); >>>>> hci_conn_hash_flush(); >>>>> hci_conn_del() >>>>> cancel_delayed_work_sync() >>>>> hci_conn_timeout() >>>>> hci_dev_lock(hdev); /* DEADLOCK */ >>>> >>>> I am actually not sure that hci_conn_timeout locks hdev. Why do you think >>>> so? >>> >>> By reading the source, printk and suffering through the deadlock. It's >>> especially painfull when using a bt-keyboard and systemd, because >>> systemd tries 4 times (~ some minutes) to kill bluetoothd before it >>> marks the service as failed and finally continues to shut down. >> >> hci_conn_timeout does lock the device. See the source. But the problem > > I think you need to check commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 > > commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 > Author: Andre Guedes <andre.guedes@openbossa.org> > Date: Fri Jan 27 19:42:03 2012 -0300 > > Bluetooth: Remove unneeded locking > > We don't need locking hdev in hci_conn_timeout() since it doesn't > access any hdev's shared resources, it basically queues HCI commands. So if the locks in hci_conn_timeout() aren't needed, your commit which removes them should go to the stable tree because it fixes a painful deadlock. Regards, Alexander ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: bluetooth: fix deadlock on device reset and power down 2012-04-02 9:17 ` bluetooth: fix deadlock on device reset and power down Alexander Holler @ 2012-04-02 10:16 ` Alexander Holler 2012-04-02 13:52 ` Andre Guedes 0 siblings, 1 reply; 3+ messages in thread From: Alexander Holler @ 2012-04-02 10:16 UTC (permalink / raw) To: Andrei Emeltchenko Cc: David Herrmann, linux-bluetooth, linux-kernel, Gustavo F. Padovan, stable, Andre Guedes Am 02.04.2012 11:17, schrieb Alexander Holler: > Am 02.04.2012 11:03, schrieb Andrei Emeltchenko: >> Hi guys, >> >> On Mon, Apr 02, 2012 at 10:44:43AM +0200, David Herrmann wrote: >>> Hi Andrei and Alexander >>> >>> On Mon, Apr 2, 2012 at 10:29 AM, Alexander Holler<holler@ahsoftware.de> wrote: >>>> Am 02.04.2012 08:55, schrieb Andrei Emeltchenko: >>>>> Hi Alexander, >>>>> >>>>> On Sat, Mar 31, 2012 at 03:23:38PM +0200, Alexander Holler wrote: >>>>>> I've experienced a deadlock on shutdown using kernel 3.3 and tracked >>>>>> it down. Because I'm not very familiar with the bluetooth stack I'm >>>>>> not sure if the below patch is correct, but it fixed the problem >>>>>> here. >>>>> >>>>> Could you please attach deadlock dump? >>>>> >>>>>> >>>>>> Commit 09fd0de5bd8f8ef3317e5365f92f1a13dcd89aa9 introduced a deadlock: >>>>>> >>>>>> bluetoothd calls ioctl HCIDEVDOWN >>>>>> hci_sock_ioctl() >>>>>> hci_dev_close() >>>>>> hci_dev_do_close() >>>>>> hci_dev_lock(hdev); >>>>>> inquiry_cache_flush(); >>>>>> hci_conn_hash_flush(); >>>>>> hci_conn_del() >>>>>> cancel_delayed_work_sync() >>>>>> hci_conn_timeout() >>>>>> hci_dev_lock(hdev); /* DEADLOCK */ >>>>> >>>>> I am actually not sure that hci_conn_timeout locks hdev. Why do you think >>>>> so? >>>> >>>> By reading the source, printk and suffering through the deadlock. It's >>>> especially painfull when using a bt-keyboard and systemd, because >>>> systemd tries 4 times (~ some minutes) to kill bluetoothd before it >>>> marks the service as failed and finally continues to shut down. >>> >>> hci_conn_timeout does lock the device. See the source. But the problem >> >> I think you need to check commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 >> >> commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 >> Author: Andre Guedes<andre.guedes@openbossa.org> >> Date: Fri Jan 27 19:42:03 2012 -0300 >> >> Bluetooth: Remove unneeded locking >> >> We don't need locking hdev in hci_conn_timeout() since it doesn't >> access any hdev's shared resources, it basically queues HCI commands. > > So if the locks in hci_conn_timeout() aren't needed, your commit which > removes them should go to the stable tree because it fixes a painful > deadlock. Oh, sorry, that patch is not from you, your first name is only similiar. I've added the author to cc, just in case the lock might still be needed in 3.3 if no other patches (from 3.4) besides that one are applied. Regards, Alexander ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: bluetooth: fix deadlock on device reset and power down 2012-04-02 10:16 ` Alexander Holler @ 2012-04-02 13:52 ` Andre Guedes 0 siblings, 0 replies; 3+ messages in thread From: Andre Guedes @ 2012-04-02 13:52 UTC (permalink / raw) To: Alexander Holler Cc: Andrei Emeltchenko, David Herrmann, linux-bluetooth, linux-kernel, Gustavo F. Padovan, stable Hi all, On Mon, Apr 2, 2012 at 7:16 AM, Alexander Holler <holler@ahsoftware.de> wrote: > Am 02.04.2012 11:17, schrieb Alexander Holler: > >> Am 02.04.2012 11:03, schrieb Andrei Emeltchenko: >>> >>> Hi guys, >>> >>> On Mon, Apr 02, 2012 at 10:44:43AM +0200, David Herrmann wrote: >>>> >>>> Hi Andrei and Alexander >>>> >>>> On Mon, Apr 2, 2012 at 10:29 AM, Alexander Holler<holler@ahsoftware.de> >>>> �wrote: >>>>> >>>>> Am 02.04.2012 08:55, schrieb Andrei Emeltchenko: >>>>>> >>>>>> Hi Alexander, >>>>>> >>>>>> On Sat, Mar 31, 2012 at 03:23:38PM +0200, Alexander Holler wrote: >>>>>>> >>>>>>> I've experienced a deadlock on shutdown using kernel 3.3 and tracked >>>>>>> it down. Because I'm not very familiar with the bluetooth stack I'm >>>>>>> not sure if the below patch is correct, but it fixed the problem >>>>>>> here. >>>>>> >>>>>> >>>>>> Could you please attach deadlock dump? >>>>>> >>>>>>> >>>>>>> Commit 09fd0de5bd8f8ef3317e5365f92f1a13dcd89aa9 introduced a >>>>>>> deadlock: >>>>>>> >>>>>>> bluetoothd calls ioctl HCIDEVDOWN >>>>>>> � � hci_sock_ioctl() >>>>>>> � � � � hci_dev_close() >>>>>>> � � � � � � hci_dev_do_close() >>>>>>> � � � � � � � � hci_dev_lock(hdev); >>>>>>> � � � � � � � � inquiry_cache_flush(); >>>>>>> � � � � � � � � hci_conn_hash_flush(); >>>>>>> � � � � � � � � � � hci_conn_del() >>>>>>> � � � � � � � � � � � � cancel_delayed_work_sync() >>>>>>> � � � � � � � � � � � � � � hci_conn_timeout() >>>>>>> � � � � � � � � � � � � � � � � hci_dev_lock(hdev); /* DEADLOCK */ >>>>>> >>>>>> >>>>>> I am actually not sure that hci_conn_timeout locks hdev. Why do you >>>>>> think >>>>>> so? >>>>> >>>>> >>>>> By reading the source, printk and suffering through the deadlock. It's >>>>> especially painfull when using a bt-keyboard and systemd, because >>>>> systemd tries 4 times (~ some minutes) to kill bluetoothd before it >>>>> marks the service as failed and finally continues to shut down. >>>> >>>> >>>> hci_conn_timeout does lock the device. See the source. But the problem >>> >>> >>> I think you need to check commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 >>> >>> commit e72acc13c770a82b4ce4a07e9716f29320eae0f8 >>> Author: Andre Guedes<andre.guedes@openbossa.org> >>> Date: � Fri Jan 27 19:42:03 2012 -0300 >>> >>> � � Bluetooth: Remove unneeded locking >>> >>> � � We don't need locking hdev in hci_conn_timeout() since it doesn't >>> � � access any hdev's shared resources, it basically queues HCI commands. >> >> >> So if the locks in hci_conn_timeout() aren't needed, your commit which >> removes them should go to the stable tree because it fixes a painful >> deadlock. > > > Oh, sorry, that patch is not from you, your first name is only similiar. > I've added the author to cc, just in case the lock might still be needed in > 3.3 if no other patches (from 3.4) besides that one are applied. It is still applicable. It was applied to bluetooth-next tree a week after 3.3 merge window was closed. This is the reason why it is not present in 3.3. BR, Andre ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-02 13:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4F77055A.2070502@ahsoftware.de>
[not found] ` <20120402065525.GA29687@aemeltch-MOBL1>
[not found] ` <4F796377.2050008@ahsoftware.de>
[not found] ` <CANq1E4RCQvQX6deGOTjdhbEkAahZD9MmAmfgz-Y-+wDfRtx0dw@mail.gmail.com>
[not found] ` <20120402090337.GB29687@aemeltch-MOBL1>
2012-04-02 9:17 ` bluetooth: fix deadlock on device reset and power down Alexander Holler
2012-04-02 10:16 ` Alexander Holler
2012-04-02 13:52 ` Andre Guedes
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox