[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <E5C422A2-82BC-4E9E-B113-835A11AB8CBD@holtmann.org>
Date: Mon, 2 Jun 2008 08:15:03 +0200
From: Marcel Holtmann <marcel@...tmann.org>
To: Dave Young <hidave.darkstar@...il.com>
Cc: davem@...emloft.net, arjan@...ux.intel.com,
akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
linux-bluetooth@...r.kernel.org
Subject: Re: [PATCH] bluetooth: rfcomm_dev_state_change deadlock fix
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@...ux.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@...il.com>
looks good. Thanks for adding a clear comment why we have to do it
this way.
Acked-by: Marcel Holtmann <marcel@...tmann.org>
Regards
Marcel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists