linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops
@ 2012-05-28  1:27 Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Use chan instead of void * makes more sense here.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |   10 ++++++----
 net/bluetooth/l2cap_core.c    |   30 +++++++++++++++---------------
 net/bluetooth/l2cap_sock.c    |   16 ++++++++--------
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f44344b..aa2dbc6 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -527,10 +527,12 @@ struct l2cap_chan {
 struct l2cap_ops {
 	char			*name;
 
-	struct l2cap_chan	*(*new_connection) (void *data);
-	int			(*recv) (void *data, struct sk_buff *skb);
-	void			(*close) (void *data);
-	void			(*state_change) (void *data, int state);
+	struct l2cap_chan	*(*new_connection) (struct l2cap_chan *chan);
+	int			(*recv) (struct l2cap_chan * chan,
+					 struct sk_buff *skb);
+	void			(*close) (struct l2cap_chan *chan);
+	void			(*state_change) (struct l2cap_chan *chan,
+						 int state);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index de0dc9e..7edc814 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -180,7 +180,7 @@ static void __l2cap_state_change(struct l2cap_chan *chan, int state)
 						state_to_string(state));
 
 	chan->state = state;
-	chan->ops->state_change(chan->data, state);
+	chan->ops->state_change(chan, state);
 }
 
 static void l2cap_state_change(struct l2cap_chan *chan, int state)
@@ -381,7 +381,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
 
 	l2cap_chan_unlock(chan);
 
-	chan->ops->close(chan->data);
+	chan->ops->close(chan);
 	mutex_unlock(&conn->chan_lock);
 
 	l2cap_chan_put(chan);
@@ -569,7 +569,7 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
 		l2cap_chan_close(chan, ECONNRESET);
 		l2cap_chan_unlock(chan);
 
-		chan->ops->close(chan->data);
+		chan->ops->close(chan);
 	}
 }
 
@@ -1213,7 +1213,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 		goto clean;
 	}
 
-	chan = pchan->ops->new_connection(pchan->data);
+	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
 		goto clean;
 
@@ -1324,7 +1324,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 
 		l2cap_chan_unlock(chan);
 
-		chan->ops->close(chan->data);
+		chan->ops->close(chan);
 		l2cap_chan_put(chan);
 	}
 
@@ -2568,7 +2568,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
 		if (!nskb)
 			continue;
 
-		if (chan->ops->recv(chan->data, nskb))
+		if (chan->ops->recv(chan, nskb))
 			kfree_skb(nskb);
 	}
 
@@ -3411,7 +3411,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		goto response;
 	}
 
-	chan = pchan->ops->new_connection(pchan->data);
+	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
 		goto response;
 
@@ -3420,7 +3420,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	/* Check if we already have channel with that dcid */
 	if (__l2cap_get_chan_by_dcid(conn, scid)) {
 		sock_set_flag(sk, SOCK_ZAPPED);
-		chan->ops->close(chan->data);
+		chan->ops->close(chan);
 		goto response;
 	}
 
@@ -3831,7 +3831,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 
 	l2cap_chan_unlock(chan);
 
-	chan->ops->close(chan->data);
+	chan->ops->close(chan);
 	l2cap_chan_put(chan);
 
 	mutex_unlock(&conn->chan_lock);
@@ -3865,7 +3865,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 
 	l2cap_chan_unlock(chan);
 
-	chan->ops->close(chan->data);
+	chan->ops->close(chan);
 	l2cap_chan_put(chan);
 
 	mutex_unlock(&conn->chan_lock);
@@ -4435,7 +4435,7 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
 		if (chan->sdu)
 			break;
 
-		err = chan->ops->recv(chan->data, skb);
+		err = chan->ops->recv(chan, skb);
 		break;
 
 	case L2CAP_SAR_START:
@@ -4485,7 +4485,7 @@ static int l2cap_reassemble_sdu(struct l2cap_chan *chan, struct sk_buff *skb,
 		if (chan->sdu->len != chan->sdu_len)
 			break;
 
-		err = chan->ops->recv(chan->data, chan->sdu);
+		err = chan->ops->recv(chan, chan->sdu);
 
 		if (!err) {
 			/* Reassembly complete */
@@ -5207,7 +5207,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		if (chan->imtu < skb->len)
 			goto drop;
 
-		if (!chan->ops->recv(chan->data, skb))
+		if (!chan->ops->recv(chan, skb))
 			goto done;
 		break;
 
@@ -5246,7 +5246,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
 	if (chan->imtu < skb->len)
 		goto drop;
 
-	if (!chan->ops->recv(chan->data, skb))
+	if (!chan->ops->recv(chan, skb))
 		return 0;
 
 drop:
@@ -5272,7 +5272,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, u16 cid,
 	if (chan->imtu < skb->len)
 		goto drop;
 
-	if (!chan->ops->recv(chan->data, skb))
+	if (!chan->ops->recv(chan, skb))
 		return 0;
 
 drop:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index d244361..db787f6 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -872,9 +872,9 @@ static int l2cap_sock_release(struct socket *sock)
 	return err;
 }
 
-static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
+static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
-	struct sock *sk, *parent = data;
+	struct sock *sk, *parent = chan->data;
 
 	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
 								GFP_ATOMIC);
@@ -888,10 +888,10 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(void *data)
 	return l2cap_pi(sk)->chan;
 }
 
-static int l2cap_sock_recv_cb(void *data, struct sk_buff *skb)
+static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
 {
 	int err;
-	struct sock *sk = data;
+	struct sock *sk = chan->data;
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 
 	lock_sock(sk);
@@ -924,16 +924,16 @@ done:
 	return err;
 }
 
-static void l2cap_sock_close_cb(void *data)
+static void l2cap_sock_close_cb(struct l2cap_chan *chan)
 {
-	struct sock *sk = data;
+	struct sock *sk = chan->data;
 
 	l2cap_sock_kill(sk);
 }
 
-static void l2cap_sock_state_change_cb(void *data, int state)
+static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
 {
-	struct sock *sk = data;
+	struct sock *sk = chan->data;
 
 	sk->sk_state = state;
 }
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrei Emeltchenko

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This remove a bit more of socket code from l2cap core, this calls set the
SOCK_ZAPPED and do some clean up depending on the socket state.

Reported-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   55 ++++++-------------------------------
 net/bluetooth/l2cap_sock.c    |   61 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 71 insertions(+), 46 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index aa2dbc6..76b0e7e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -530,6 +530,7 @@ struct l2cap_ops {
 	struct l2cap_chan	*(*new_connection) (struct l2cap_chan *chan);
 	int			(*recv) (struct l2cap_chan * chan,
 					 struct sk_buff *skb);
+	void			(*teardown) (struct l2cap_chan *chan, int err);
 	void			(*close) (struct l2cap_chan *chan);
 	void			(*state_change) (struct l2cap_chan *chan,
 						 int state);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7edc814..1f4c720 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -493,9 +493,7 @@ static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 
 static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 {
-	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
-	struct sock *parent = bt_sk(sk)->parent;
 
 	__clear_chan_timer(chan);
 
@@ -511,21 +509,8 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 		hci_conn_put(conn->hcon);
 	}
 
-	lock_sock(sk);
-
-	__l2cap_state_change(chan, BT_CLOSED);
-	sock_set_flag(sk, SOCK_ZAPPED);
-
-	if (err)
-		__l2cap_chan_set_err(chan, err);
-
-	if (parent) {
-		bt_accept_unlink(sk);
-		parent->sk_data_ready(parent, 0);
-	} else
-		sk->sk_state_change(sk);
-
-	release_sock(sk);
+	if (chan->ops->teardown)
+		chan->ops->teardown(chan, err);
 
 	if (test_bit(CONF_NOT_COMPLETE, &chan->conf_state))
 		return;
@@ -554,25 +539,6 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	return;
 }
 
-static void l2cap_chan_cleanup_listen(struct sock *parent)
-{
-	struct sock *sk;
-
-	BT_DBG("parent %p", parent);
-
-	/* Close not yet accepted channels */
-	while ((sk = bt_accept_dequeue(parent, NULL))) {
-		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
-
-		l2cap_chan_lock(chan);
-		__clear_chan_timer(chan);
-		l2cap_chan_close(chan, ECONNRESET);
-		l2cap_chan_unlock(chan);
-
-		chan->ops->close(chan);
-	}
-}
-
 void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 {
 	struct l2cap_conn *conn = chan->conn;
@@ -583,12 +549,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 
 	switch (chan->state) {
 	case BT_LISTEN:
-		lock_sock(sk);
-		l2cap_chan_cleanup_listen(sk);
-
-		__l2cap_state_change(chan, BT_CLOSED);
-		sock_set_flag(sk, SOCK_ZAPPED);
-		release_sock(sk);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan, 0);
 		break;
 
 	case BT_CONNECTED:
@@ -630,9 +592,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 		break;
 
 	default:
-		lock_sock(sk);
-		sock_set_flag(sk, SOCK_ZAPPED);
-		release_sock(sk);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan, 0);
 		break;
 	}
 }
@@ -3419,7 +3380,9 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	/* Check if we already have channel with that dcid */
 	if (__l2cap_get_chan_by_dcid(conn, scid)) {
-		sock_set_flag(sk, SOCK_ZAPPED);
+		if (chan->ops->teardown)
+			chan->ops->teardown(chan, 0);
+
 		chan->ops->close(chan);
 		goto response;
 	}
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index db787f6..3f59463 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -872,6 +872,25 @@ static int l2cap_sock_release(struct socket *sock)
 	return err;
 }
 
+static void l2cap_sock_cleanup_listen(struct sock *parent)
+{
+	struct sock *sk;
+
+	BT_DBG("parent %p", parent);
+
+	/* Close not yet accepted channels */
+	while ((sk = bt_accept_dequeue(parent, NULL))) {
+		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+
+		l2cap_chan_lock(chan);
+		__clear_chan_timer(chan);
+		l2cap_chan_close(chan, ECONNRESET);
+		l2cap_chan_unlock(chan);
+
+		l2cap_sock_kill(sk);
+	}
+}
+
 static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk, *parent = chan->data;
@@ -931,6 +950,47 @@ static void l2cap_sock_close_cb(struct l2cap_chan *chan)
 	l2cap_sock_kill(sk);
 }
 
+static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
+{
+	struct sock *sk = chan->data;
+	struct sock *parent;
+
+	lock_sock(sk);
+
+	parent = bt_sk(sk)->parent;
+
+	sock_set_flag(sk, SOCK_ZAPPED);
+
+	switch (chan->state) {
+	case BT_OPEN:
+	case BT_BOUND:
+	case BT_CLOSED:
+		break;
+	case BT_LISTEN:
+		l2cap_sock_cleanup_listen(sk);
+		sk->sk_state = BT_CLOSED;
+		chan->state = BT_CLOSED;
+
+		break;
+	default:
+		sk->sk_state = BT_CLOSED;
+		chan->state = BT_CLOSED;
+
+		sk->sk_err = err;
+
+		if (parent) {
+			bt_accept_unlink(sk);
+			parent->sk_data_ready(parent, 0);
+		} else {
+			sk->sk_state_change(sk);
+		}
+
+		break;
+	}
+
+	release_sock(sk);
+}
+
 static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state)
 {
 	struct sock *sk = chan->data;
@@ -959,6 +1019,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.new_connection	= l2cap_sock_new_connection_cb,
 	.recv		= l2cap_sock_recv_cb,
 	.close		= l2cap_sock_close_cb,
+	.teardown	= l2cap_sock_teardown_cb,
 	.state_change	= l2cap_sock_state_change_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 };
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready()
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrei Emeltchenko

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

This move socket specific code to l2cap_sock.c.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   18 +++---------------
 net/bluetooth/l2cap_sock.c    |   21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 76b0e7e..c5726c2 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -534,6 +534,7 @@ struct l2cap_ops {
 	void			(*close) (struct l2cap_chan *chan);
 	void			(*state_change) (struct l2cap_chan *chan,
 						 int state);
+	void			(*ready) (struct l2cap_chan *chan);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 1f4c720..5947eb1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -931,26 +931,14 @@ static void l2cap_send_conn_req(struct l2cap_chan *chan)
 
 static void l2cap_chan_ready(struct l2cap_chan *chan)
 {
-	struct sock *sk = chan->sk;
-	struct sock *parent;
-
-	lock_sock(sk);
-
-	parent = bt_sk(sk)->parent;
-
-	BT_DBG("sk %p, parent %p", sk, parent);
-
 	/* This clears all conf flags, including CONF_NOT_COMPLETE */
 	chan->conf_state = 0;
 	__clear_chan_timer(chan);
 
-	__l2cap_state_change(chan, BT_CONNECTED);
-	sk->sk_state_change(sk);
+	chan->state = BT_CONNECTED;
 
-	if (parent)
-		parent->sk_data_ready(parent, 0);
-
-	release_sock(sk);
+	if (chan->ops->ready)
+		chan->ops->ready(chan);
 }
 
 static void l2cap_do_start(struct l2cap_chan *chan)
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 3f59463..5563023 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1014,6 +1014,26 @@ static struct sk_buff *l2cap_sock_alloc_skb_cb(struct l2cap_chan *chan,
 	return skb;
 }
 
+static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
+{
+	struct sock *sk = chan->data;
+	struct sock *parent;
+
+	lock_sock(sk);
+
+	parent = bt_sk(sk)->parent;
+
+	BT_DBG("sk %p, parent %p", sk, parent);
+
+	sk->sk_state = BT_CONNECTED;
+	sk->sk_state_change(sk);
+
+	if (parent)
+		parent->sk_data_ready(parent, 0);
+
+	release_sock(sk);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1021,6 +1041,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.close		= l2cap_sock_close_cb,
 	.teardown	= l2cap_sock_teardown_cb,
 	.state_change	= l2cap_sock_state_change_cb,
+	.ready		= l2cap_sock_ready_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 };
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

These vars are kept in sync so we can use chan->state here.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5947eb1..35e6d7d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1442,21 +1442,17 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		goto done;
 	}
 
-	lock_sock(sk);
-
-	switch (sk->sk_state) {
+	switch (chan->state) {
 	case BT_CONNECT:
 	case BT_CONNECT2:
 	case BT_CONFIG:
 		/* Already connecting */
 		err = 0;
-		release_sock(sk);
 		goto done;
 
 	case BT_CONNECTED:
 		/* Already connected */
 		err = -EISCONN;
-		release_sock(sk);
 		goto done;
 
 	case BT_OPEN:
@@ -1466,13 +1462,12 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 
 	default:
 		err = -EBADFD;
-		release_sock(sk);
 		goto done;
 	}
 
 	/* Set destination address and psm */
+	lock_sock(sk);
 	bacpy(&bt_sk(sk)->dst, dst);
-
 	release_sock(sk);
 
 	chan->psm = psm;
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (2 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  3:59   ` Marcel Holtmann
  2012-05-28  1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Remove socket specific code from l2cap_core.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/l2cap_core.c |   12 ------------
 net/bluetooth/l2cap_sock.c |    6 ++++++
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35e6d7d..d64c836 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1156,12 +1156,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 	lock_sock(parent);
 
-	/* Check for backlog size */
-	if (sk_acceptq_is_full(parent)) {
-		BT_DBG("backlog full %d", parent->sk_ack_backlog);
-		goto clean;
-	}
-
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
 		goto clean;
@@ -3349,12 +3343,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	result = L2CAP_CR_NO_MEM;
 
-	/* Check for backlog size */
-	if (sk_acceptq_is_full(parent)) {
-		BT_DBG("backlog full %d", parent->sk_ack_backlog);
-		goto response;
-	}
-
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
 		goto response;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 5563023..d856cc8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -895,6 +895,12 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk, *parent = chan->data;
 
+	/* Check for backlog size */
+	if (sk_acceptq_is_full(parent)) {
+		BT_DBG("backlog full %d", parent->sk_ack_backlog);
+		return NULL;
+	}
+
 	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
 								GFP_ATOMIC);
 	if (!sk)
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (3 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Remove another socket usage from l2cap_core.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   12 ++++++------
 net/bluetooth/l2cap_sock.c    |    8 ++++++--
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c5726c2..0ea4cf1 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -601,6 +601,7 @@ enum {
 	CONF_LOC_CONF_PEND,
 	CONF_REM_CONF_PEND,
 	CONF_NOT_COMPLETE,
+	CONF_DEFER_SETUP,
 };
 
 #define L2CAP_CONF_MAX_CONF_REQ 2
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d64c836..d0c1ebe 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -569,7 +569,7 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 			struct l2cap_conn_rsp rsp;
 			__u16 result;
 
-			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags))
+			if (test_bit(CONF_DEFER_SETUP, &chan->conf_state))
 				result = L2CAP_CR_SEC_BLOCK;
 			else
 				result = L2CAP_CR_BAD_PSM;
@@ -1056,8 +1056,8 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 
 			if (l2cap_chan_check_security(chan)) {
 				lock_sock(sk);
-				if (test_bit(BT_SK_DEFER_SETUP,
-					     &bt_sk(sk)->flags)) {
+				if (test_bit(CONF_DEFER_SETUP,
+					     &chan->conf_state)) {
 					struct sock *parent = bt_sk(sk)->parent;
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
@@ -3377,7 +3377,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) {
 		if (l2cap_chan_check_security(chan)) {
-			if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
+			if (test_bit(CONF_DEFER_SETUP, &chan->conf_state)) {
 				__l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
@@ -5407,8 +5407,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 			lock_sock(sk);
 
 			if (!status) {
-				if (test_bit(BT_SK_DEFER_SETUP,
-					     &bt_sk(sk)->flags)) {
+				if (test_bit(CONF_DEFER_SETUP,
+					     &chan->conf_state)) {
 					struct sock *parent = bt_sk(sk)->parent;
 					res = L2CAP_CR_PEND;
 					stat = L2CAP_CS_AUTHOR_PEND;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index d856cc8..a421a4e 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -623,10 +623,14 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 			break;
 		}
 
-		if (opt)
+		if (opt) {
 			set_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
-		else
+			set_bit(CONF_DEFER_SETUP, &chan->conf_state);
+		} else {
 			clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags);
+			clear_bit(CONF_DEFER_SETUP, &chan->conf_state);
+		}
+
 		break;
 
 	case BT_FLUSHABLE:
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (4 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  3:54   ` Marcel Holtmann
  2012-05-28  1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

When DEFER_SETUP is set defer() will trigger an authorization request
to the userspace.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   14 ++++++--------
 net/bluetooth/l2cap_sock.c    |   12 ++++++++++++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 0ea4cf1..6d0864e 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -535,6 +535,7 @@ struct l2cap_ops {
 	void			(*state_change) (struct l2cap_chan *chan,
 						 int state);
 	void			(*ready) (struct l2cap_chan *chan);
+	void			(*defer) (struct l2cap_chan *chan);
 	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
 					       unsigned long len, int nb);
 };
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index d0c1ebe..35c2c52 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1058,12 +1058,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 				lock_sock(sk);
 				if (test_bit(CONF_DEFER_SETUP,
 					     &chan->conf_state)) {
-					struct sock *parent = bt_sk(sk)->parent;
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
 					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
-					if (parent)
-						parent->sk_data_ready(parent, 0);
-
+					if(chan->ops->defer)
+						chan->ops->defer(chan->data);
 				} else {
 					__l2cap_state_change(chan, BT_CONFIG);
 					rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
@@ -3381,7 +3379,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 				__l2cap_state_change(chan, BT_CONNECT2);
 				result = L2CAP_CR_PEND;
 				status = L2CAP_CS_AUTHOR_PEND;
-				parent->sk_data_ready(parent, 0);
+				if(chan->ops->defer)
+					chan->ops->defer(chan->data);
 			} else {
 				__l2cap_state_change(chan, BT_CONFIG);
 				result = L2CAP_CR_SUCCESS;
@@ -5409,11 +5408,10 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 			if (!status) {
 				if (test_bit(CONF_DEFER_SETUP,
 					     &chan->conf_state)) {
-					struct sock *parent = bt_sk(sk)->parent;
 					res = L2CAP_CR_PEND;
 					stat = L2CAP_CS_AUTHOR_PEND;
-					if (parent)
-						parent->sk_data_ready(parent, 0);
+					if(chan->ops->defer)
+						chan->ops->defer(chan->data);
 				} else {
 					__l2cap_state_change(chan, BT_CONFIG);
 					res = L2CAP_CR_SUCCESS;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index a421a4e..e81e910 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
 	release_sock(sk);
 }
 
+static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
+{
+	struct sock *sk = chan->data;
+	struct sock *parent;
+
+	parent = bt_sk(sk)->parent;
+
+	if (parent)
+		parent->sk_data_ready(parent, 0);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_sock_new_connection_cb,
@@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
 	.teardown	= l2cap_sock_teardown_cb,
 	.state_change	= l2cap_sock_state_change_cb,
 	.ready		= l2cap_sock_ready_cb,
+	.defer		= l2cap_sock_defer_cb,
 	.alloc_skb	= l2cap_sock_alloc_skb_cb,
 };
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (5 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  4:01   ` Marcel Holtmann
  2012-05-28  1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
  2012-05-28  1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan
  8 siblings, 1 reply; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Move this check to before the channel time creation simplifies the code
and avoid memory allocation if the channel already exist.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
---
 net/bluetooth/l2cap_core.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 35c2c52..584c66f 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3341,21 +3341,16 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	result = L2CAP_CR_NO_MEM;
 
+	/* Check if we already have channel with that dcid */
+	if (__l2cap_get_chan_by_dcid(conn, scid))
+		goto response;
+
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
 		goto response;
 
 	sk = chan->sk;
 
-	/* Check if we already have channel with that dcid */
-	if (__l2cap_get_chan_by_dcid(conn, scid)) {
-		if (chan->ops->teardown)
-			chan->ops->teardown(chan, 0);
-
-		chan->ops->close(chan);
-		goto response;
-	}
-
 	hci_conn_hold(conn->hcon);
 
 	bacpy(&bt_sk(sk)->src, conn->src);
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (6 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
@ 2012-05-28  1:27 ` Gustavo Padovan
  2012-05-28  1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:27 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

bt_accept_enqueue() can be easily placed at the end of
l2cap_sock_new_connection_cb().
There is no problem in moving to bt_accept_enqueue() to an earlier point
in the code, bt_accept_enqueue() only adds the sk to the it parents queue
basically.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c |    4 ----
 net/bluetooth/l2cap_sock.c |    2 ++
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 584c66f..74843d2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1165,8 +1165,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	bacpy(&bt_sk(sk)->src, conn->src);
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 
-	bt_accept_enqueue(parent, sk);
-
 	l2cap_chan_add(conn, chan);
 
 	l2cap_chan_ready(chan);
@@ -3358,8 +3356,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	chan->psm  = psm;
 	chan->dcid = scid;
 
-	bt_accept_enqueue(parent, sk);
-
 	__l2cap_chan_add(conn, chan);
 
 	dcid = chan->scid;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index e81e910..700eef8 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -914,6 +914,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 
 	l2cap_sock_init(sk, parent);
 
+	bt_accept_enqueue(parent, sk);
+
 	return l2cap_pi(sk)->chan;
 }
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c
  2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
                   ` (7 preceding siblings ...)
  2012-05-28  1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
@ 2012-05-28  1:28 ` Gustavo Padovan
  8 siblings, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28  1:28 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

We can lock the parent lock only inside the new_connection() call, then we
just use the l2cap_chan_lock() in core code.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 net/bluetooth/l2cap_core.c |   16 ++++++----------
 net/bluetooth/l2cap_sock.c |    9 ++++++++-
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 74843d2..5920a7c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1139,7 +1139,7 @@ static struct l2cap_chan *l2cap_global_chan_by_scid(int state, u16 cid,
 
 static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 {
-	struct sock *parent, *sk;
+	struct sock *sk;
 	struct l2cap_chan *chan, *pchan;
 
 	BT_DBG("");
@@ -1150,9 +1150,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	if (!pchan)
 		return;
 
-	parent = pchan->sk;
-
-	lock_sock(parent);
+	l2cap_chan_lock(pchan);
 
 	chan = pchan->ops->new_connection(pchan);
 	if (!chan)
@@ -1170,7 +1168,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 	l2cap_chan_ready(chan);
 
 clean:
-	release_sock(parent);
+	l2cap_chan_unlock(pchan);
 }
 
 static void l2cap_conn_ready(struct l2cap_conn *conn)
@@ -3309,7 +3307,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	struct l2cap_conn_req *req = (struct l2cap_conn_req *) data;
 	struct l2cap_conn_rsp rsp;
 	struct l2cap_chan *chan = NULL, *pchan;
-	struct sock *parent, *sk = NULL;
+	struct sock *sk = NULL;
 	int result, status = L2CAP_CS_NO_INFO;
 
 	u16 dcid = 0, scid = __le16_to_cpu(req->scid);
@@ -3324,10 +3322,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		goto sendresp;
 	}
 
-	parent = pchan->sk;
-
 	mutex_lock(&conn->chan_lock);
-	lock_sock(parent);
+	l2cap_chan_lock(pchan);
 
 	/* Check if the ACL is secure enough (if not SDP) */
 	if (psm != __constant_cpu_to_le16(L2CAP_PSM_SDP) &&
@@ -3389,7 +3385,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	}
 
 response:
-	release_sock(parent);
+	l2cap_chan_unlock(pchan);
 	mutex_unlock(&conn->chan_lock);
 
 sendresp:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 700eef8..cfb97ca 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -899,16 +899,21 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 {
 	struct sock *sk, *parent = chan->data;
 
+	lock_sock(parent);
+
 	/* Check for backlog size */
 	if (sk_acceptq_is_full(parent)) {
 		BT_DBG("backlog full %d", parent->sk_ack_backlog);
+		release_sock(parent);
 		return NULL;
 	}
 
 	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
 								GFP_ATOMIC);
-	if (!sk)
+	if (!sk) {
+		release_sock(parent);
 		return NULL;
+	}
 
 	bt_sock_reclassify_lock(sk, BTPROTO_L2CAP);
 
@@ -916,6 +921,8 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
 
 	bt_accept_enqueue(parent, sk);
 
+	release_sock(parent);
+
 	return l2cap_pi(sk)->chan;
 }
 
-- 
1.7.10.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
  2012-05-28  1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
@ 2012-05-28  3:54   ` Marcel Holtmann
  2012-05-28 14:12     ` Andrei Emeltchenko
  2012-05-28 16:23     ` Gustavo Padovan
  0 siblings, 2 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28  3:54 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan

Hi Gustavo,

> When DEFER_SETUP is set defer() will trigger an authorization request
> to the userspace.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  include/net/bluetooth/l2cap.h |    1 +
>  net/bluetooth/l2cap_core.c    |   14 ++++++--------
>  net/bluetooth/l2cap_sock.c    |   12 ++++++++++++
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 0ea4cf1..6d0864e 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -535,6 +535,7 @@ struct l2cap_ops {
>  	void			(*state_change) (struct l2cap_chan *chan,
>  						 int state);
>  	void			(*ready) (struct l2cap_chan *chan);
> +	void			(*defer) (struct l2cap_chan *chan);
>  	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
>  					       unsigned long len, int nb);
>  };
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index d0c1ebe..35c2c52 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1058,12 +1058,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
>  				lock_sock(sk);
>  				if (test_bit(CONF_DEFER_SETUP,
>  					     &chan->conf_state)) {
> -					struct sock *parent = bt_sk(sk)->parent;
>  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
>  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> -					if (parent)
> -						parent->sk_data_ready(parent, 0);
> -
> +					if(chan->ops->defer)

Coding style issue here.

> +						chan->ops->defer(chan->data);
>  				} else {
>  					__l2cap_state_change(chan, BT_CONFIG);
>  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> @@ -3381,7 +3379,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
>  				__l2cap_state_change(chan, BT_CONNECT2);
>  				result = L2CAP_CR_PEND;
>  				status = L2CAP_CS_AUTHOR_PEND;
> -				parent->sk_data_ready(parent, 0);
> +				if(chan->ops->defer)

And here.

> +					chan->ops->defer(chan->data);
>  			} else {
>  				__l2cap_state_change(chan, BT_CONFIG);
>  				result = L2CAP_CR_SUCCESS;
> @@ -5409,11 +5408,10 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
>  			if (!status) {
>  				if (test_bit(CONF_DEFER_SETUP,
>  					     &chan->conf_state)) {
> -					struct sock *parent = bt_sk(sk)->parent;
>  					res = L2CAP_CR_PEND;
>  					stat = L2CAP_CS_AUTHOR_PEND;
> -					if (parent)
> -						parent->sk_data_ready(parent, 0);
> +					if(chan->ops->defer)

And here.

> +						chan->ops->defer(chan->data);
>  				} else {
>  					__l2cap_state_change(chan, BT_CONFIG);
>  					res = L2CAP_CR_SUCCESS;
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index a421a4e..e81e910 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
>  	release_sock(sk);
>  }
>  
> +static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
> +{
> +	struct sock *sk = chan->data;
> +	struct sock *parent;
> +
> +	parent = bt_sk(sk)->parent;
> +
> +	if (parent)
> +		parent->sk_data_ready(parent, 0);
> +}
> +
>  static struct l2cap_ops l2cap_chan_ops = {
>  	.name		= "L2CAP Socket Interface",
>  	.new_connection	= l2cap_sock_new_connection_cb,
> @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
>  	.teardown	= l2cap_sock_teardown_cb,
>  	.state_change	= l2cap_sock_state_change_cb,
>  	.ready		= l2cap_sock_ready_cb,
> +	.defer		= l2cap_sock_defer_cb,
>  	.alloc_skb	= l2cap_sock_alloc_skb_cb,
>  };
>  

In addition can you explain to me how this is suppose to work if defer
is not set. Then we never call ready.

Also this is a pretty bad idea inside the code. We better do something
like l2cap_sock_no_defer like the network subsystem does for general
socket. Since otherwise you keep spreading if (defer) all over the
places.

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c
  2012-05-28  1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
@ 2012-05-28  3:59   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28  3:59 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan

Hi Gustavo,

> Remove socket specific code from l2cap_core.c
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/l2cap_core.c |   12 ------------
>  net/bluetooth/l2cap_sock.c |    6 ++++++
>  2 files changed, 6 insertions(+), 12 deletions(-)

I applied this the first 5 patches to bluetooth-next so far.

<snip>

> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 5563023..d856cc8 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -895,6 +895,12 @@ static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
>  {
>  	struct sock *sk, *parent = chan->data;
>  
> +	/* Check for backlog size */
> +	if (sk_acceptq_is_full(parent)) {
> +		BT_DBG("backlog full %d", parent->sk_ack_backlog);
> +		return NULL;
> +	}
> +
>  	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
>  								GFP_ATOMIC);
>  	if (!sk)

And on a side note, does this still have to be GFP_ATOMIC?

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one
  2012-05-28  1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
@ 2012-05-28  4:01   ` Marcel Holtmann
  0 siblings, 0 replies; 15+ messages in thread
From: Marcel Holtmann @ 2012-05-28  4:01 UTC (permalink / raw)
  To: Gustavo Padovan; +Cc: linux-bluetooth, Gustavo Padovan

Hi Gustavo,

> Move this check to before the channel time creation simplifies the code
> and avoid memory allocation if the channel already exist.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> Acked-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> Acked-by: Marcel Holtmann <marcel@holtmann.org>
> ---
>  net/bluetooth/l2cap_core.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)

in addition I also applied this patch to bluetooth-next tree.

Regards

Marcel



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
  2012-05-28  3:54   ` Marcel Holtmann
@ 2012-05-28 14:12     ` Andrei Emeltchenko
  2012-05-28 16:23     ` Gustavo Padovan
  1 sibling, 0 replies; 15+ messages in thread
From: Andrei Emeltchenko @ 2012-05-28 14:12 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: Gustavo Padovan, linux-bluetooth, Gustavo Padovan

Hi Marcel and Gustavo,

On Mon, May 28, 2012 at 05:54:20AM +0200, Marcel Holtmann wrote:
...

> > @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> >  	.teardown	= l2cap_sock_teardown_cb,
> >  	.state_change	= l2cap_sock_state_change_cb,
> >  	.ready		= l2cap_sock_ready_cb,
> > +	.defer		= l2cap_sock_defer_cb,
> >  	.alloc_skb	= l2cap_sock_alloc_skb_cb,
> >  };
> 
> In addition can you explain to me how this is suppose to work if defer
> is not set. Then we never call ready.

For fixed channel we do not use ready so this would be OK.

> Also this is a pretty bad idea inside the code. We better do something
> like l2cap_sock_no_defer like the network subsystem does for general
> socket. Since otherwise you keep spreading if (defer) all over the
> places.

This look right, I will change my code following your proposal so Gustavo
can invoke those functions directly.

Best regards 
Andrei Emeltchenko 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer()
  2012-05-28  3:54   ` Marcel Holtmann
  2012-05-28 14:12     ` Andrei Emeltchenko
@ 2012-05-28 16:23     ` Gustavo Padovan
  1 sibling, 0 replies; 15+ messages in thread
From: Gustavo Padovan @ 2012-05-28 16:23 UTC (permalink / raw)
  To: Marcel Holtmann; +Cc: linux-bluetooth, Gustavo Padovan

Hi Marcel,

* Marcel Holtmann <marcel@holtmann.org> [2012-05-28 05:54:20 +0200]:

> Hi Gustavo,
> 
> > When DEFER_SETUP is set defer() will trigger an authorization request
> > to the userspace.
> > 
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> > ---
> >  include/net/bluetooth/l2cap.h |    1 +
> >  net/bluetooth/l2cap_core.c    |   14 ++++++--------
> >  net/bluetooth/l2cap_sock.c    |   12 ++++++++++++
> >  3 files changed, 19 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index 0ea4cf1..6d0864e 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -535,6 +535,7 @@ struct l2cap_ops {
> >  	void			(*state_change) (struct l2cap_chan *chan,
> >  						 int state);
> >  	void			(*ready) (struct l2cap_chan *chan);
> > +	void			(*defer) (struct l2cap_chan *chan);
> >  	struct sk_buff		*(*alloc_skb) (struct l2cap_chan *chan,
> >  					       unsigned long len, int nb);
> >  };
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index d0c1ebe..35c2c52 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -1058,12 +1058,10 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
> >  				lock_sock(sk);
> >  				if (test_bit(CONF_DEFER_SETUP,
> >  					     &chan->conf_state)) {
> > -					struct sock *parent = bt_sk(sk)->parent;
> >  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_PEND);
> >  					rsp.status = __constant_cpu_to_le16(L2CAP_CS_AUTHOR_PEND);
> > -					if (parent)
> > -						parent->sk_data_ready(parent, 0);
> > -
> > +					if(chan->ops->defer)
> 
> Coding style issue here.
> 
> > +						chan->ops->defer(chan->data);
> >  				} else {
> >  					__l2cap_state_change(chan, BT_CONFIG);
> >  					rsp.result = __constant_cpu_to_le16(L2CAP_CR_SUCCESS);
> > @@ -3381,7 +3379,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
> >  				__l2cap_state_change(chan, BT_CONNECT2);
> >  				result = L2CAP_CR_PEND;
> >  				status = L2CAP_CS_AUTHOR_PEND;
> > -				parent->sk_data_ready(parent, 0);
> > +				if(chan->ops->defer)
> 
> And here.
> 
> > +					chan->ops->defer(chan->data);
> >  			} else {
> >  				__l2cap_state_change(chan, BT_CONFIG);
> >  				result = L2CAP_CR_SUCCESS;
> > @@ -5409,11 +5408,10 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
> >  			if (!status) {
> >  				if (test_bit(CONF_DEFER_SETUP,
> >  					     &chan->conf_state)) {
> > -					struct sock *parent = bt_sk(sk)->parent;
> >  					res = L2CAP_CR_PEND;
> >  					stat = L2CAP_CS_AUTHOR_PEND;
> > -					if (parent)
> > -						parent->sk_data_ready(parent, 0);
> > +					if(chan->ops->defer)
> 
> And here.
> 
> > +						chan->ops->defer(chan->data);
> >  				} else {
> >  					__l2cap_state_change(chan, BT_CONFIG);
> >  					res = L2CAP_CR_SUCCESS;
> > diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> > index a421a4e..e81e910 100644
> > --- a/net/bluetooth/l2cap_sock.c
> > +++ b/net/bluetooth/l2cap_sock.c
> > @@ -1044,6 +1044,17 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
> >  	release_sock(sk);
> >  }
> >  
> > +static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
> > +{
> > +	struct sock *sk = chan->data;
> > +	struct sock *parent;
> > +
> > +	parent = bt_sk(sk)->parent;
> > +
> > +	if (parent)
> > +		parent->sk_data_ready(parent, 0);
> > +}
> > +
> >  static struct l2cap_ops l2cap_chan_ops = {
> >  	.name		= "L2CAP Socket Interface",
> >  	.new_connection	= l2cap_sock_new_connection_cb,
> > @@ -1052,6 +1063,7 @@ static struct l2cap_ops l2cap_chan_ops = {
> >  	.teardown	= l2cap_sock_teardown_cb,
> >  	.state_change	= l2cap_sock_state_change_cb,
> >  	.ready		= l2cap_sock_ready_cb,
> > +	.defer		= l2cap_sock_defer_cb,
> >  	.alloc_skb	= l2cap_sock_alloc_skb_cb,
> >  };
> >  
> 
> In addition can you explain to me how this is suppose to work if defer
> is not set. Then we never call ready.

If you don't set defer_setup, you probably don't need to call ready. That was the
idea here.

> Also this is a pretty bad idea inside the code. We better do something
> like l2cap_sock_no_defer like the network subsystem does for general
> socket. Since otherwise you keep spreading if (defer) all over the
> places.

Sure, I agree, I'll remove all if (defer) from the code for now. And as
l2cap_sock is still the sole user of this we don't need to add
l2cap_sock_no_defer yet.

	Gustavo

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-05-28 16:23 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-28  1:27 [PATCH -v3 01/10] Bluetooth: Use chan as parameters for l2cap chan ops Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 02/10] Bluetooth: Move clean up code and set of SOCK_ZAPPED to l2cap_sock.c Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 03/10] Bluetooth: Add l2cap_chan->ops->ready() Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 04/10] Bluetooth: Use chan->state instead of sk->sk_state Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 05/10] Bluetooth: Move check for backlog size to l2cap_sock.c Gustavo Padovan
2012-05-28  3:59   ` Marcel Holtmann
2012-05-28  1:27 ` [PATCH -v3 06/10] Bluetooth: Create DEFER_SETUP flag in conf_state Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 07/10] Bluetooth: Add chan->ops->defer() Gustavo Padovan
2012-05-28  3:54   ` Marcel Holtmann
2012-05-28 14:12     ` Andrei Emeltchenko
2012-05-28 16:23     ` Gustavo Padovan
2012-05-28  1:27 ` [PATCH -v3 08/10] Bluetooth: check for already existent channel before create new one Gustavo Padovan
2012-05-28  4:01   ` Marcel Holtmann
2012-05-28  1:27 ` [PATCH -v3 09/10] Bluetooth: Move bt_accept_enqueue() call to l2cap_sock.c Gustavo Padovan
2012-05-28  1:28 ` [PATCH -v3 10/10] Bluetooth: Remove parent socket usage from l2cap_core.c Gustavo Padovan

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).