On Sun, Jun 1, 2008 at 3:06 PM, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote: > Hi Dave, > >> There's logic in __rfcomm_dlc_close: >> rfcomm_dlc_lock(d); >> d->state = BT_CLOSED; >> d->state_changed(d, err); >> rfcomm_dlc_unlock(d); >> >> In rfcomm_dev_state_change, it's possible that rfcomm_dev_put try to take >> the >> dlc lock, then we will deadlock. >> >> Here fixed it by unlock dlc before rfcomm_dev_get in >> rfcomm_dev_state_change. >> >> why not unlock just before rfcomm_dev_put? it's because there's another >> problem. >> rfcomm_dev_get/rfcomm_dev_del will take rfcomm_dev_lock, but in >> rfcomm_dev_add >> the lock order is : rfcomm_dev_lock --> dlc lock >> >> so I unlock dlc before the taken of rfcomm_dev_lock. >> >> Actually it's a regression caused by commit >> 1905f6c736cb618e07eca0c96e60e3c024023428, the dlc state_change could be >> two >> callbacks : rfcomm_sk_state_change and rfcomm_dev_state_change. I missed >> the rfcomm_sk_state_change that time. >> >> Thanks Arjan van de Ven <arjan@xxxxxxxxxxxxxxx> for the effort in commit >> 4c8411f8c115def968820a4df6658ccfd55d7f1a >> but he missed the rfcomm_dev_state_change lock issue. >> >> Signed-off-by: Dave Young <hidave.darkstar@xxxxxxxxx> >> >> --- >> net/bluetooth/rfcomm/tty.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff -upr linux/net/bluetooth/rfcomm/tty.c >> linux.new/net/bluetooth/rfcomm/tty.c >> --- linux/net/bluetooth/rfcomm/tty.c 2008-05-30 15:46:33.000000000 >> +0800 >> +++ linux.new/net/bluetooth/rfcomm/tty.c 2008-05-30 >> 17:08:30.000000000 +0800 >> @@ -566,11 +566,22 @@ static void rfcomm_dev_state_change(stru >> if (dlc->state == BT_CLOSED) { >> if (!dev->tty) { >> if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) { >> - if (rfcomm_dev_get(dev->id) == NULL) >> + /* Drop DLC lock here to avoid deadlock >> + * 1. rfcomm_dev_get will take >> rfcomm_dev_lock >> + * but in rfcomm_dev_add there's lock >> order: >> + * rfcomm_dev_lock -> dlc lock >> + * 2. rfcomm_dev_put will deaklock if it's >> + * the last reference > > I meant this one :) s/deaklock/deadlock/ Fixed, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html