From: Andrei Emeltchenko <[email protected]>
Change is needed to remove dependency on sk when possible
before introducing l2cap channel lock.
Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/l2cap_core.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 69e8490..c330cdc 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -907,9 +907,9 @@ clean:
release_sock(parent);
}
-static void l2cap_chan_ready(struct sock *sk)
+static void l2cap_chan_ready(struct l2cap_chan *chan)
{
- struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+ struct sock *sk = chan->sk;
struct sock *parent = bt_sk(sk)->parent;
BT_DBG("sk %p, parent %p", sk, parent);
@@ -945,7 +945,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
if (conn->hcon->type == LE_LINK) {
if (smp_conn_security(conn, chan->sec_level))
- l2cap_chan_ready(sk);
+ l2cap_chan_ready(chan);
} else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
__clear_chan_timer(chan);
@@ -2874,7 +2874,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
if (chan->mode == L2CAP_MODE_ERTM)
l2cap_ertm_init(chan);
- l2cap_chan_ready(sk);
+ l2cap_chan_ready(chan);
goto unlock;
}
@@ -3005,7 +3005,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
if (chan->mode == L2CAP_MODE_ERTM)
l2cap_ertm_init(chan);
- l2cap_chan_ready(sk);
+ l2cap_chan_ready(chan);
}
done:
@@ -4524,7 +4524,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
if (chan->scid == L2CAP_CID_LE_DATA) {
if (!status && encrypt) {
chan->sec_level = hcon->sec_level;
- l2cap_chan_ready(sk);
+ l2cap_chan_ready(chan);
}
bh_unlock_sock(sk);
--
1.7.4.1
Hi Andrei,
On Thu, Jan 26, 2012 at 7:00 AM, Emeltchenko Andrei
<[email protected]> wrote:
> Hi Ulisses,
>
> On Thu, Jan 26, 2012 at 01:04:06AM -0200, Ulisses Furquim wrote:
>> > From: Andrei Emeltchenko <[email protected]>
>> >
>> > Change is needed to remove dependency on sk when possible
>> > before introducing l2cap channel lock.
>>
>> What's the overall idea? We used to rely on sk lock for mutual
>> exclusion, right? (please correct me if I'm wrong) I'm seeing some
>> patches from you to change from sk to chan but introducing another
>> lock might shake things a bit so that's why I'm asking for the big
>> picture, if you have thought this through already.
>
> Basically it is known that current implementation of some higher level
> protocols like RFCOMM using kernel sockets is not the right way and shall
> be changed at some point.
>
> I've implemented basic A2MP protocol as a kernel socket and Marcel gave m=
e
> suggestion to move from sockets to internal L2CAP functions.
>
> I've done this and since everything is locked with sk and obviously we do
> not have socket I have to use following constructions:
>
> <------8<-------------------
> | =A0if (sk)
> | =A0 =A0 =A0 =A0 =A0lock_sock(sk)
> | =A0...
> | =A0if(sk)
> | =A0 =A0 =A0 =A0 =A0release_sock(sk)
> |
> <------8<-------------------
>
> which does not look nice and might be racy. Then comes idea to change
> socket lock for L2CAP protocol to e.g. l2cap_channel_{lock,unlock}.
Will you completely change one lock for the other?
> I have code which works but some parts looks like hacks so I try to polis=
h
> it a bit and send as RFC. Please give me comments if you think this might
> be done other way.
Ok, let's see your patch series on that. I haven't thought about it
and seeing your proposal might be good.
Best regards,
--=20
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
Hi Ulisses,
On Thu, Jan 26, 2012 at 01:04:06AM -0200, Ulisses Furquim wrote:
> > From: Andrei Emeltchenko <[email protected]>
> >
> > Change is needed to remove dependency on sk when possible
> > before introducing l2cap channel lock.
>
> What's the overall idea? We used to rely on sk lock for mutual
> exclusion, right? (please correct me if I'm wrong) I'm seeing some
> patches from you to change from sk to chan but introducing another
> lock might shake things a bit so that's why I'm asking for the big
> picture, if you have thought this through already.
Basically it is known that current implementation of some higher level
protocols like RFCOMM using kernel sockets is not the right way and shall
be changed at some point.
I've implemented basic A2MP protocol as a kernel socket and Marcel gave me
suggestion to move from sockets to internal L2CAP functions.
I've done this and since everything is locked with sk and obviously we do
not have socket I have to use following constructions:
<------8<-------------------
| if (sk)
| lock_sock(sk)
| ...
| if(sk)
| release_sock(sk)
|
<------8<-------------------
which does not look nice and might be racy. Then comes idea to change
socket lock for L2CAP protocol to e.g. l2cap_channel_{lock,unlock}.
I have code which works but some parts looks like hacks so I try to polish
it a bit and send as RFC. Please give me comments if you think this might
be done other way.
Best regards
Andrei Emeltchenko
Hi Andrei,
On Wed, Jan 25, 2012 at 12:09 PM, Emeltchenko Andrei
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Change is needed to remove dependency on sk when possible
> before introducing l2cap channel lock.
What's the overall idea? We used to rely on sk lock for mutual
exclusion, right? (please correct me if I'm wrong) I'm seeing some
patches from you to change from sk to chan but introducing another
lock might shake things a bit so that's why I'm asking for the big
picture, if you have thought this through already.
Thanks,
-- Ulisses
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> ?net/bluetooth/l2cap_core.c | ? 12 ++++++------
> ?1 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 69e8490..c330cdc 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -907,9 +907,9 @@ clean:
> ? ? ? ?release_sock(parent);
> ?}
>
> -static void l2cap_chan_ready(struct sock *sk)
> +static void l2cap_chan_ready(struct l2cap_chan *chan)
> ?{
> - ? ? ? struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> + ? ? ? struct sock *sk = chan->sk;
> ? ? ? ?struct sock *parent = bt_sk(sk)->parent;
>
> ? ? ? ?BT_DBG("sk %p, parent %p", sk, parent);
> @@ -945,7 +945,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
>
> ? ? ? ? ? ? ? ?if (conn->hcon->type == LE_LINK) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (smp_conn_security(conn, chan->sec_level))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? l2cap_chan_ready(sk);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? l2cap_chan_ready(chan);
>
> ? ? ? ? ? ? ? ?} else if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
> ? ? ? ? ? ? ? ? ? ? ? ?__clear_chan_timer(chan);
> @@ -2874,7 +2874,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> ? ? ? ? ? ? ? ?if (chan->mode == L2CAP_MODE_ERTM)
> ? ? ? ? ? ? ? ? ? ? ? ?l2cap_ertm_init(chan);
>
> - ? ? ? ? ? ? ? l2cap_chan_ready(sk);
> + ? ? ? ? ? ? ? l2cap_chan_ready(chan);
> ? ? ? ? ? ? ? ?goto unlock;
> ? ? ? ?}
>
> @@ -3005,7 +3005,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
> ? ? ? ? ? ? ? ?if (chan->mode == ?L2CAP_MODE_ERTM)
> ? ? ? ? ? ? ? ? ? ? ? ?l2cap_ertm_init(chan);
>
> - ? ? ? ? ? ? ? l2cap_chan_ready(sk);
> + ? ? ? ? ? ? ? l2cap_chan_ready(chan);
> ? ? ? ?}
>
> ?done:
> @@ -4524,7 +4524,7 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> ? ? ? ? ? ? ? ?if (chan->scid == L2CAP_CID_LE_DATA) {
> ? ? ? ? ? ? ? ? ? ? ? ?if (!status && encrypt) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?chan->sec_level = hcon->sec_level;
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? l2cap_chan_ready(sk);
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? l2cap_chan_ready(chan);
> ? ? ? ? ? ? ? ? ? ? ? ?}
>
> ? ? ? ? ? ? ? ? ? ? ? ?bh_unlock_sock(sk);
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs