From: Gustavo Padovan <padovan@profusion.mobi>
To: Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCHv1 12/14] Bluetooth: Change locking logic in security_cfm
Date: Tue, 21 Feb 2012 15:56:03 -0200 [thread overview]
Message-ID: <20120221175602.GC22229@joana> (raw)
In-Reply-To: <1329821707-11817-13-git-send-email-Andrei.Emeltchenko.news@gmail.com>
* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-02-21 12:55:05 +0200]:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
> Change bh_ locking functions to mutex_locks since we can now sleep.
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
> net/bluetooth/l2cap_core.c | 17 ++++++++++-------
> 1 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 7baee68..49aa226 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -4572,9 +4572,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> mutex_lock(&conn->chan_lock);
>
> list_for_each_entry(chan, &conn->chan_l, list) {
> - struct sock *sk = chan->sk;
> -
> - bh_lock_sock(sk);
> + l2cap_chan_lock(chan);
>
> BT_DBG("chan->scid %d", chan->scid);
>
> @@ -4584,19 +4582,19 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> l2cap_chan_ready(chan);
> }
>
> - bh_unlock_sock(sk);
> + l2cap_chan_unlock(chan);
> continue;
> }
>
> if (test_bit(CONF_CONNECT_PEND, &chan->conf_state)) {
> - bh_unlock_sock(sk);
> + l2cap_chan_unlock(chan);
> continue;
> }
>
> if (!status && (chan->state == BT_CONNECTED ||
> chan->state == BT_CONFIG)) {
> l2cap_check_encryption(chan, encrypt);
> - bh_unlock_sock(sk);
> + l2cap_chan_unlock(chan);
> continue;
> }
>
> @@ -4617,9 +4615,12 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
> }
> } else if (chan->state == BT_CONNECT2) {
> + struct sock *sk = chan->sk;
> struct l2cap_conn_rsp rsp;
> __u16 res, stat;
>
> + lock_sock(sk);
> +
> if (!status) {
> if (bt_sk(sk)->defer_setup) {
> struct sock *parent = bt_sk(sk)->parent;
> @@ -4640,6 +4641,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> stat = L2CAP_CS_NO_INFO;
> }
>
> + release_sock(sk);
> +
> rsp.scid = cpu_to_le16(chan->dcid);
> rsp.dcid = cpu_to_le16(chan->scid);
> rsp.result = cpu_to_le16(res);
> @@ -4648,7 +4651,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> sizeof(rsp), &rsp);
> }
>
> - bh_unlock_sock(sk);
> + l2cap_chan_unlock(chan);
I don't like the idea of this locking change in multiple separate patches, it
could put the stack in a broken and racy state if we stop in the middle of
these patches.
I would say to merge all these patches into one, they were a lot reviewed
alone, there is no problem in create a big one with all the changes together.
Gustavo
next prev parent reply other threads:[~2012-02-21 17:56 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-21 10:54 [PATCHv1 00/14] Bluetooth: Change socket lock to l2cap_chan lock Andrei Emeltchenko
2012-02-21 10:54 ` [PATCHv1 01/14] Bluetooth: trivial: Fix long line Andrei Emeltchenko
2012-02-21 10:54 ` [PATCHv1 02/14] Bluetooth: Revert to mutexes from RCU list Andrei Emeltchenko
2012-02-21 10:54 ` [PATCHv1 03/14] Bluetooth: Add l2cap_chan_lock Andrei Emeltchenko
2012-02-21 16:51 ` Ulisses Furquim
2012-02-21 17:25 ` Gustavo Padovan
2012-02-21 10:54 ` [PATCHv1 04/14] Bluetooth: Add locked and unlocked state_change Andrei Emeltchenko
2012-02-21 16:52 ` Ulisses Furquim
2012-02-21 17:26 ` Gustavo Padovan
2012-02-21 10:54 ` [PATCHv1 05/14] Bluetooth: Add socket error function Andrei Emeltchenko
2012-02-21 16:57 ` Ulisses Furquim
2012-02-21 17:27 ` Gustavo Padovan
2012-02-21 10:54 ` [PATCHv1 06/14] Bluetooth: Add unlocked __l2cap_chan_add function Andrei Emeltchenko
2012-02-21 17:01 ` Ulisses Furquim
2012-02-22 9:25 ` Andrei Emeltchenko
2012-02-21 17:28 ` Gustavo Padovan
2012-02-21 10:55 ` [PATCHv1 07/14] Bluetooth: Use chan lock in timers Andrei Emeltchenko
2012-02-21 17:06 ` Ulisses Furquim
2012-02-21 10:55 ` [PATCHv1 08/14] Bluetooth: Use chan lock in L2CAP sig commands Andrei Emeltchenko
2012-02-21 17:18 ` Ulisses Furquim
2012-02-21 17:57 ` Gustavo Padovan
2012-02-21 10:55 ` [PATCHv1 09/14] Bluetooth: Use chan lock in L2CAP conn start Andrei Emeltchenko
2012-02-21 17:20 ` Ulisses Furquim
2012-02-21 10:55 ` [PATCHv1 10/14] Bluetooth: Use chan lock in receiving data Andrei Emeltchenko
2012-02-21 20:28 ` Ulisses Furquim
2012-02-21 10:55 ` [PATCHv1 11/14] Bluetooth: Change locking logic for conn/chan ready Andrei Emeltchenko
2012-02-21 17:31 ` Ulisses Furquim
2012-02-21 10:55 ` [PATCHv1 12/14] Bluetooth: Change locking logic in security_cfm Andrei Emeltchenko
2012-02-21 17:32 ` Ulisses Furquim
2012-02-21 17:56 ` Gustavo Padovan [this message]
2012-02-21 20:44 ` Andrei Emeltchenko
2012-02-21 21:18 ` Ulisses Furquim
2012-02-21 10:55 ` [PATCHv1 13/14] Bluetooth: Use l2cap chan lock in socket connect Andrei Emeltchenko
2012-02-21 17:36 ` Ulisses Furquim
2012-02-21 17:46 ` Gustavo Padovan
2012-02-22 9:53 ` Andrei Emeltchenko
2012-02-21 10:55 ` [PATCHv1 14/14] Bluetooth: Remove socket lock check Andrei Emeltchenko
2012-02-21 17:37 ` Ulisses Furquim
2012-02-22 9:59 ` Andrei Emeltchenko
2012-02-21 17:51 ` Gustavo Padovan
2012-02-21 19:24 ` [PATCHv1 00/14] Bluetooth: Change socket lock to l2cap_chan lock Johan Hedberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120221175602.GC22229@joana \
--to=padovan@profusion.mobi \
--cc=Andrei.Emeltchenko.news@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).