lkml.org 
[lkml]   [2008]   [May]   [31]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
    /
    DateSat, 31 May 2008 17:16:28 +0800
    From"Dave Young" <>
    SubjectRe: [PATCH] rfcomm deadlock fix
    Sorry, I sent a wrong version, I will sent again after a while.
    
    On Sat, May 31, 2008 at 5:09 PM, Dave Young <hidave.darkstar@gmail.com> wrote:
    > 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@linux.intel.com> for the effort in commit
    > 4c8411f8c115def968820a4df6658ccfd55d7f1a
    > but he missed the rfcomm_dev_state_change lock issue.
    >
    > Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
    >
    > ---
    > net/bluetooth/rfcomm/tty.c  |    9 +++++++++
    > 1 files changed, 9 insertions(+)
    >
    > 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 16:02:38.000000000 +0800
    > @@ -566,11 +566,20 @@ static void rfcomm_dev_state_change(stru
    >        if (dlc->state == BT_CLOSED) {
    >                if (!dev->tty) {
    >                        if (test_bit(RFCOMM_RELEASE_ONHUP, &dev->flags)) {
    > +                               /* 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
    > +                                */
    > +                               rfcomm_dlc_unlock(dlc);
    >                                if (rfcomm_dev_get(dev->id) == NULL)
    >                                        return;
    >
    >                                rfcomm_dev_del(dev);
    >                                rfcomm_dev_put(dev);
    > +                               rfcomm_dlc_lock(dlc);
    >                        }
    >                } else
    >                        tty_hangup(dev->tty);
    >
    
    
    
    -- 
    ---
    Regards
    dave
    
    
    \
     
     \ /
      Last update: 2008-05-31 11:19    [from the cache]
    ©2003-2008