linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/6] Bluetooth: Add l2cap_chan_ops abstraction
@ 2011-06-02 16:15 Gustavo F. Padovan
  2011-06-02 16:15 ` [PATCH 2/6] Bluetooth: add recv() callback to l2cap_chan_ops Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

Add an abstraction layer between L2CAP core and its users (only
l2cap_sock.c now). The first function implemented is new_connection() that
replaces calls to l2cap_sock_alloc() in l2cap_core.c

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |   12 +++++++++---
 net/bluetooth/l2cap_core.c    |   17 +++++++----------
 net/bluetooth/l2cap_sock.c    |   29 +++++++++++++++++++++++++++--
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index a2dcbff..95e44df 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -353,6 +353,15 @@ struct l2cap_chan {
 
 	struct list_head list;
 	struct list_head global_l;
+
+	struct l2cap_ops *ops;
+};
+
+struct l2cap_ops {
+	char		*name;
+	void		*data;
+
+	struct l2cap_chan	*(*new_connection) (void *data);
 };
 
 struct l2cap_conn {
@@ -459,9 +468,6 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm);
 int l2cap_add_scid(struct l2cap_chan *chan,  __u16 scid);
 
 void l2cap_sock_kill(struct sock *sk);
-void l2cap_sock_init(struct sock *sk, struct sock *parent);
-struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock,
-							int proto, gfp_t prio);
 
 struct l2cap_chan *l2cap_chan_create(struct sock *sk);
 void l2cap_chan_close(struct l2cap_chan *chan, int reason);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 233ecdc..6849e74 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -837,18 +837,16 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 		goto clean;
 	}
 
-	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, GFP_ATOMIC);
-	if (!sk)
+	chan = pchan->ops->new_connection(pchan->ops->data);
+	if (!chan)
 		goto clean;
 
-	chan = l2cap_pi(sk)->chan;
+	sk = chan->sk;
 
 	write_lock_bh(&conn->chan_lock);
 
 	hci_conn_hold(conn->hcon);
 
-	l2cap_sock_init(sk, parent);
-
 	bacpy(&bt_sk(sk)->src, conn->src);
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 
@@ -2323,10 +2321,12 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		goto response;
 	}
 
-	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP, GFP_ATOMIC);
-	if (!sk)
+	chan = pchan->ops->new_connection(pchan->ops->data);
+	if (!chan)
 		goto response;
 
+	sk = chan->sk;
+
 	write_lock_bh(&conn->chan_lock);
 
 	/* Check if we already have channel with that dcid */
@@ -2339,9 +2339,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	hci_conn_hold(conn->hcon);
 
-	chan = l2cap_pi(sk)->chan;
-
-	l2cap_sock_init(sk, parent);
 	bacpy(&bt_sk(sk)->src, conn->src);
 	bacpy(&bt_sk(sk)->dst, conn->dst);
 	chan->psm  = psm;
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 1337811..79926a4 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -31,6 +31,8 @@
 #include <net/bluetooth/l2cap.h>
 
 static const struct proto_ops l2cap_sock_ops;
+static void l2cap_sock_init(struct sock *sk, struct sock *parent);
+static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio);
 
 static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
 {
@@ -739,6 +741,25 @@ static int l2cap_sock_release(struct socket *sock)
 	return err;
 }
 
+static struct l2cap_chan *l2cap_chan_new_connection_cb(void *data)
+{
+	struct sock *sk, *parent = data;
+
+	sk = l2cap_sock_alloc(sock_net(parent), NULL, BTPROTO_L2CAP,
+								GFP_ATOMIC);
+	if (!sk)
+		return NULL;
+
+	l2cap_sock_init(sk, parent);
+
+	return l2cap_pi(sk)->chan;
+}
+
+static struct l2cap_ops l2cap_chan_ops = {
+	.name		= "L2CAP Socket Interface",
+	.new_connection	= l2cap_chan_new_connection_cb,
+};
+
 static void l2cap_sock_destruct(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
@@ -747,7 +768,7 @@ static void l2cap_sock_destruct(struct sock *sk)
 	skb_queue_purge(&sk->sk_write_queue);
 }
 
-void l2cap_sock_init(struct sock *sk, struct sock *parent)
+static void l2cap_sock_init(struct sock *sk, struct sock *parent)
 {
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 	struct l2cap_chan *chan = pi->chan;
@@ -772,6 +793,7 @@ void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		chan->role_switch = pchan->role_switch;
 		chan->force_reliable = pchan->force_reliable;
 		chan->flushable = pchan->flushable;
+		chan->ops = pchan->ops;
 	} else {
 
 		switch (sk->sk_type) {
@@ -802,6 +824,9 @@ void l2cap_sock_init(struct sock *sk, struct sock *parent)
 		chan->role_switch = 0;
 		chan->force_reliable = 0;
 		chan->flushable = BT_FLUSHABLE_OFF;
+
+		l2cap_chan_ops.data = sk;
+		chan->ops = &l2cap_chan_ops;
 	}
 
 	/* Default config options */
@@ -814,7 +839,7 @@ static struct proto l2cap_proto = {
 	.obj_size	= sizeof(struct l2cap_pinfo)
 };
 
-struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio)
+static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int proto, gfp_t prio)
 {
 	struct sock *sk;
 	struct l2cap_chan *chan;
-- 
1.7.5.1


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

* [PATCH 2/6] Bluetooth: add recv() callback to l2cap_chan_ops
  2011-06-02 16:15 [PATCH 1/6] Bluetooth: Add l2cap_chan_ops abstraction Gustavo F. Padovan
@ 2011-06-02 16:15 ` Gustavo F. Padovan
  2011-06-02 16:15   ` [PATCH 3/6] Bluetooth: add close() " Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

This abstracts the call to sock_queue_recv_skb() into
l2cap_chan_ops->recv().

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |    1 +
 net/bluetooth/l2cap_core.c    |   16 ++++++++--------
 net/bluetooth/l2cap_sock.c    |    8 ++++++++
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 95e44df..e625f09 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -362,6 +362,7 @@ struct l2cap_ops {
 	void		*data;
 
 	struct l2cap_chan	*(*new_connection) (void *data);
+	int			(*recv) (void *data, struct sk_buff *skb);
 };
 
 struct l2cap_conn {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 6849e74..b5327f3 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1692,7 +1692,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb)
 		if (!nskb)
 			continue;
 
-		if (sock_queue_rcv_skb(sk, nskb))
+		if (chan->ops->recv(chan->ops->data, nskb))
 			kfree_skb(nskb);
 	}
 	read_unlock(&conn->chan_lock);
@@ -3118,7 +3118,7 @@ static int l2cap_ertm_reassembly_sdu(struct l2cap_chan *chan, struct sk_buff *sk
 		if (chan->conn_state & L2CAP_CONN_SAR_SDU)
 			goto drop;
 
-		return sock_queue_rcv_skb(chan->sk, skb);
+		return chan->ops->recv(chan->ops->data, skb);
 
 	case L2CAP_SDU_START:
 		if (chan->conn_state & L2CAP_CONN_SAR_SDU)
@@ -3184,7 +3184,7 @@ static int l2cap_ertm_reassembly_sdu(struct l2cap_chan *chan, struct sk_buff *sk
 			return -ENOMEM;
 		}
 
-		err = sock_queue_rcv_skb(chan->sk, _skb);
+		err = chan->ops->recv(chan->ops->data, _skb);
 		if (err < 0) {
 			kfree_skb(_skb);
 			chan->conn_state |= L2CAP_CONN_SAR_RETRY;
@@ -3352,7 +3352,7 @@ static int l2cap_streaming_reassembly_sdu(struct l2cap_chan *chan, struct sk_buf
 			break;
 		}
 
-		err = sock_queue_rcv_skb(chan->sk, skb);
+		err = chan->ops->recv(chan->ops->data, skb);
 		if (!err)
 			return 0;
 
@@ -3413,7 +3413,7 @@ static int l2cap_streaming_reassembly_sdu(struct l2cap_chan *chan, struct sk_buf
 
 		if (chan->partial_sdu_len == chan->sdu_len) {
 			_skb = skb_clone(chan->sdu, GFP_ATOMIC);
-			err = sock_queue_rcv_skb(chan->sk, _skb);
+			err = chan->ops->recv(chan->ops->data, _skb);
 			if (err < 0)
 				kfree_skb(_skb);
 		}
@@ -3880,7 +3880,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
 		if (chan->imtu < skb->len)
 			goto drop;
 
-		if (!sock_queue_rcv_skb(sk, skb))
+		if (!chan->ops->recv(chan->ops->data, skb))
 			goto done;
 		break;
 
@@ -3958,7 +3958,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
 	if (l2cap_pi(sk)->chan->imtu < skb->len)
 		goto drop;
 
-	if (!sock_queue_rcv_skb(sk, skb))
+	if (!chan->ops->recv(chan->ops->data, skb))
 		goto done;
 
 drop:
@@ -3991,7 +3991,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, __le16 cid, struct
 	if (l2cap_pi(sk)->chan->imtu < skb->len)
 		goto drop;
 
-	if (!sock_queue_rcv_skb(sk, skb))
+	if (!chan->ops->recv(chan->ops->data, skb))
 		goto done;
 
 drop:
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 79926a4..ba6d4f1 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -755,9 +755,17 @@ static struct l2cap_chan *l2cap_chan_new_connection_cb(void *data)
 	return l2cap_pi(sk)->chan;
 }
 
+static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
+{
+	struct sock *sk = data;
+
+	return sock_queue_rcv_skb(sk, skb);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_chan_new_connection_cb,
+	.recv		= l2cap_chan_recv_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.7.5.1


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

* [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops
  2011-06-02 16:15 ` [PATCH 2/6] Bluetooth: add recv() callback to l2cap_chan_ops Gustavo F. Padovan
@ 2011-06-02 16:15   ` Gustavo F. Padovan
  2011-06-02 16:15     ` [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan Gustavo F. Padovan
  2011-06-02 21:30     ` [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops Mat Martineau
  0 siblings, 2 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

close() calls l2cap_sock_kill() on l2cap_sock.c

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |    3 +--
 net/bluetooth/l2cap_core.c    |   17 +++++++++--------
 net/bluetooth/l2cap_sock.c    |   10 +++++++++-
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e625f09..af2f171 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -363,6 +363,7 @@ struct l2cap_ops {
 
 	struct l2cap_chan	*(*new_connection) (void *data);
 	int			(*recv) (void *data, struct sk_buff *skb);
+	void			(*close) (void *data);
 };
 
 struct l2cap_conn {
@@ -468,8 +469,6 @@ int __l2cap_wait_ack(struct sock *sk);
 int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm);
 int l2cap_add_scid(struct l2cap_chan *chan,  __u16 scid);
 
-void l2cap_sock_kill(struct sock *sk);
-
 struct l2cap_chan *l2cap_chan_create(struct sock *sk);
 void l2cap_chan_close(struct l2cap_chan *chan, int reason);
 void l2cap_chan_destroy(struct l2cap_chan *chan);
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b5327f3..c645e03 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -254,7 +254,7 @@ static void l2cap_chan_timeout(unsigned long arg)
 
 	bh_unlock_sock(sk);
 
-	l2cap_sock_kill(sk);
+	chan->ops->close(chan->ops->data);
 	sock_put(sk);
 }
 
@@ -391,11 +391,12 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
 
 	/* Close not yet accepted channels */
 	while ((sk = bt_accept_dequeue(parent, NULL))) {
-		l2cap_chan_clear_timer(l2cap_pi(sk)->chan);
+		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+		l2cap_chan_clear_timer(chan);
 		lock_sock(sk);
-		l2cap_chan_close(l2cap_pi(sk)->chan, ECONNRESET);
+		l2cap_chan_close(chan, ECONNRESET);
 		release_sock(sk);
-		l2cap_sock_kill(sk);
+		chan->ops->close(chan->ops->data);
 	}
 
 	parent->sk_state = BT_CLOSED;
@@ -988,7 +989,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 		bh_lock_sock(sk);
 		l2cap_chan_del(chan, err);
 		bh_unlock_sock(sk);
-		l2cap_sock_kill(sk);
+		chan->ops->close(chan->ops->data);
 	}
 
 	if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT)
@@ -2333,7 +2334,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	if (__l2cap_get_chan_by_dcid(conn, scid)) {
 		write_unlock_bh(&conn->chan_lock);
 		sock_set_flag(sk, SOCK_ZAPPED);
-		l2cap_sock_kill(sk);
+		chan->ops->close(chan->ops->data);
 		goto response;
 	}
 
@@ -2706,7 +2707,7 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 	l2cap_chan_del(chan, ECONNRESET);
 	bh_unlock_sock(sk);
 
-	l2cap_sock_kill(sk);
+	chan->ops->close(chan->ops->data);
 	return 0;
 }
 
@@ -2740,7 +2741,7 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 	l2cap_chan_del(chan, 0);
 	bh_unlock_sock(sk);
 
-	l2cap_sock_kill(sk);
+	chan->ops->close(chan->ops->data);
 	return 0;
 }
 
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index ba6d4f1..23cf902 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
 /* Kill socket (only if zapped and orphan)
  * Must be called on unlocked socket.
  */
-void l2cap_sock_kill(struct sock *sk)
+static void l2cap_sock_kill(struct sock *sk)
 {
 	if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
 		return;
@@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
 	return sock_queue_rcv_skb(sk, skb);
 }
 
+static void l2cap_chan_close_cb(void *data)
+{
+	struct sock *sk = data;
+
+	l2cap_sock_kill(sk);
+}
+
 static struct l2cap_ops l2cap_chan_ops = {
 	.name		= "L2CAP Socket Interface",
 	.new_connection	= l2cap_chan_new_connection_cb,
 	.recv		= l2cap_chan_recv_cb,
+	.close		= l2cap_chan_close_cb,
 };
 
 static void l2cap_sock_destruct(struct sock *sk)
-- 
1.7.5.1


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

* [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan
  2011-06-02 16:15   ` [PATCH 3/6] Bluetooth: add close() " Gustavo F. Padovan
@ 2011-06-02 16:15     ` Gustavo F. Padovan
  2011-06-02 16:15       ` [PATCH 5/6] Bluetooth: Make timer functions generic Gustavo F. Padovan
  2011-06-02 21:30     ` [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops Mat Martineau
  1 sibling, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

struct l2cap_chan has now its own refcnt that is compatible with the
socket refcnt, i.e., we won't see sk_refcnt = 0 and chan->refcnt > 0.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |    2 ++
 net/bluetooth/l2cap_core.c    |   29 ++++++++++++++++++++---------
 2 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index af2f171..c620a92 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -287,6 +287,8 @@ struct l2cap_chan {
 
 	struct l2cap_conn	*conn;
 
+	atomic_t	refcnt;
+
 	__le16		psm;
 	__u16		dcid;
 	__u16		scid;
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index c645e03..dd8da43 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -78,6 +78,18 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
 static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb);
 
 /* ---- L2CAP channels ---- */
+
+static inline void chan_hold(struct l2cap_chan *c)
+{
+	atomic_inc(&c->refcnt);
+}
+
+static inline void chan_put(struct l2cap_chan *c)
+{
+	if (atomic_dec_and_test(&c->refcnt))
+		kfree(c);
+}
+
 static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
 {
 	struct l2cap_chan *c;
@@ -213,7 +225,7 @@ static void l2cap_chan_set_timer(struct l2cap_chan *chan, long timeout)
        BT_DBG("chan %p state %d timeout %ld", chan->sk, chan->sk->sk_state,
 								 timeout);
        if (!mod_timer(&chan->chan_timer, jiffies + timeout))
-	       sock_hold(chan->sk);
+	       chan_hold(chan);
 }
 
 static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
@@ -221,7 +233,7 @@ static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
        BT_DBG("chan %p state %d", chan, chan->sk->sk_state);
 
        if (timer_pending(&chan->chan_timer) && del_timer(&chan->chan_timer))
-	       __sock_put(chan->sk);
+	       chan_put(chan);
 }
 
 static void l2cap_chan_timeout(unsigned long arg)
@@ -238,7 +250,7 @@ static void l2cap_chan_timeout(unsigned long arg)
 		/* sk is owned by user. Try again later */
 		l2cap_chan_set_timer(chan, HZ / 5);
 		bh_unlock_sock(sk);
-		sock_put(sk);
+		chan_put(chan);
 		return;
 	}
 
@@ -255,7 +267,7 @@ static void l2cap_chan_timeout(unsigned long arg)
 	bh_unlock_sock(sk);
 
 	chan->ops->close(chan->ops->data);
-	sock_put(sk);
+	chan_put(chan);
 }
 
 struct l2cap_chan *l2cap_chan_create(struct sock *sk)
@@ -274,6 +286,8 @@ struct l2cap_chan *l2cap_chan_create(struct sock *sk)
 
 	setup_timer(&chan->chan_timer, l2cap_chan_timeout, (unsigned long) chan);
 
+	atomic_set(&chan->refcnt, 1);
+
 	return chan;
 }
 
@@ -283,13 +297,11 @@ void l2cap_chan_destroy(struct l2cap_chan *chan)
 	list_del(&chan->global_l);
 	write_unlock_bh(&chan_list_lock);
 
-	kfree(chan);
+	chan_put(chan);
 }
 
 static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 {
-	struct sock *sk = chan->sk;
-
 	BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn,
 			chan->psm, chan->dcid);
 
@@ -320,7 +332,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 		chan->omtu = L2CAP_DEFAULT_MTU;
 	}
 
-	sock_hold(sk);
+	chan_hold(chan);
 
 	list_add(&chan->list, &conn->chan_l);
 }
@@ -342,7 +354,6 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 		write_lock_bh(&conn->chan_lock);
 		list_del(&chan->list);
 		write_unlock_bh(&conn->chan_lock);
-		__sock_put(sk);
 
 		chan->conn = NULL;
 		hci_conn_put(conn->hcon);
-- 
1.7.5.1


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

* [PATCH 5/6] Bluetooth: Make timer functions generic
  2011-06-02 16:15     ` [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan Gustavo F. Padovan
@ 2011-06-02 16:15       ` Gustavo F. Padovan
  2011-06-02 16:15         ` [PATCH 6/6] Bluetooth: keep reference if any ERTM timer is enabled Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

We now plan to use l2cap_set_timer and l2cap_clear_timer in ERTM timers.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |    2 +
 net/bluetooth/l2cap_core.c    |   59 +++++++++++++++++++++--------------------
 2 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index c620a92..e3e58f9 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -435,6 +435,8 @@ struct l2cap_pinfo {
 #define L2CAP_CONN_RNR_SENT        0x0200
 #define L2CAP_CONN_SAR_RETRY       0x0400
 
+#define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
+#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
 #define __mod_retrans_timer() mod_timer(&chan->retrans_timer, \
 		jiffies +  msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
 #define __mod_monitor_timer() mod_timer(&chan->monitor_timer, \
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index dd8da43..8586077 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -220,19 +220,19 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn)
 	return 0;
 }
 
-static void l2cap_chan_set_timer(struct l2cap_chan *chan, long timeout)
+static void l2cap_set_timer(struct l2cap_chan *chan, struct timer_list *timer, long timeout)
 {
        BT_DBG("chan %p state %d timeout %ld", chan->sk, chan->sk->sk_state,
 								 timeout);
-       if (!mod_timer(&chan->chan_timer, jiffies + timeout))
+       if (!mod_timer(timer, jiffies + timeout))
 	       chan_hold(chan);
 }
 
-static void l2cap_chan_clear_timer(struct l2cap_chan *chan)
+static void l2cap_clear_timer(struct l2cap_chan *chan, struct timer_list *timer)
 {
        BT_DBG("chan %p state %d", chan, chan->sk->sk_state);
 
-       if (timer_pending(&chan->chan_timer) && del_timer(&chan->chan_timer))
+       if (timer_pending(timer) && del_timer(timer))
 	       chan_put(chan);
 }
 
@@ -248,7 +248,7 @@ static void l2cap_chan_timeout(unsigned long arg)
 
 	if (sock_owned_by_user(sk)) {
 		/* sk is owned by user. Try again later */
-		l2cap_chan_set_timer(chan, HZ / 5);
+		__set_chan_timer(chan, HZ / 5);
 		bh_unlock_sock(sk);
 		chan_put(chan);
 		return;
@@ -345,7 +345,7 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	struct l2cap_conn *conn = chan->conn;
 	struct sock *parent = bt_sk(sk)->parent;
 
-	l2cap_chan_clear_timer(chan);
+	__clear_chan_timer(chan);
 
 	BT_DBG("chan %p, conn %p, err %d", chan, conn, err);
 
@@ -403,7 +403,8 @@ static void l2cap_chan_cleanup_listen(struct sock *parent)
 	/* Close not yet accepted channels */
 	while ((sk = bt_accept_dequeue(parent, NULL))) {
 		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
-		l2cap_chan_clear_timer(chan);
+
+		__clear_chan_timer(chan);
 		lock_sock(sk);
 		l2cap_chan_close(chan, ECONNRESET);
 		release_sock(sk);
@@ -430,8 +431,8 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 	case BT_CONFIG:
 		if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED &&
 					conn->hcon->type == ACL_LINK) {
-			l2cap_chan_clear_timer(chan);
-			l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+			__clear_chan_timer(chan);
+			__set_chan_timer(chan, sk->sk_sndtimeo);
 			l2cap_send_disconn_req(conn, chan, reason);
 		} else
 			l2cap_chan_del(chan, reason);
@@ -866,7 +867,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn)
 
 	__l2cap_chan_add(conn, chan);
 
-	l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+	__set_chan_timer(chan, sk->sk_sndtimeo);
 
 	sk->sk_state = BT_CONNECTED;
 	parent->sk_data_ready(parent, 0);
@@ -894,13 +895,13 @@ static void l2cap_conn_ready(struct l2cap_conn *conn)
 		bh_lock_sock(sk);
 
 		if (conn->hcon->type == LE_LINK) {
-			l2cap_chan_clear_timer(chan);
+			__clear_chan_timer(chan);
 			sk->sk_state = BT_CONNECTED;
 			sk->sk_state_change(sk);
 		}
 
 		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
-			l2cap_chan_clear_timer(chan);
+			__clear_chan_timer(chan);
 			sk->sk_state = BT_CONNECTED;
 			sk->sk_state_change(sk);
 		} else if (sk->sk_state == BT_CONNECT)
@@ -1099,11 +1100,11 @@ int l2cap_chan_connect(struct l2cap_chan *chan)
 	l2cap_chan_add(conn, chan);
 
 	sk->sk_state = BT_CONNECT;
-	l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+	__set_chan_timer(chan, sk->sk_sndtimeo);
 
 	if (hcon->state == BT_CONNECTED) {
 		if (chan->chan_type != L2CAP_CHAN_CONN_ORIENTED) {
-			l2cap_chan_clear_timer(chan);
+			__clear_chan_timer(chan);
 			if (l2cap_check_security(chan))
 				sk->sk_state = BT_CONNECTED;
 		} else
@@ -1667,7 +1668,7 @@ static void l2cap_chan_ready(struct sock *sk)
 	BT_DBG("sk %p, parent %p", sk, parent);
 
 	chan->conf_state = 0;
-	l2cap_chan_clear_timer(chan);
+	__clear_chan_timer(chan);
 
 	if (!parent) {
 		/* Outgoing channel.
@@ -2362,7 +2363,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
 
 	dcid = chan->scid;
 
-	l2cap_chan_set_timer(chan, sk->sk_sndtimeo);
+	__set_chan_timer(chan, sk->sk_sndtimeo);
 
 	chan->ident = cmd->ident;
 
@@ -2479,8 +2480,8 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
 		/* don't delete l2cap channel if sk is owned by user */
 		if (sock_owned_by_user(sk)) {
 			sk->sk_state = BT_DISCONN;
-			l2cap_chan_clear_timer(chan);
-			l2cap_chan_set_timer(chan, HZ / 5);
+			__clear_chan_timer(chan);
+			__set_chan_timer(chan, HZ / 5);
 			break;
 		}
 
@@ -2653,7 +2654,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 
 	default:
 		sk->sk_err = ECONNRESET;
-		l2cap_chan_set_timer(chan, HZ * 5);
+		__set_chan_timer(chan, HZ * 5);
 		l2cap_send_disconn_req(conn, chan, ECONNRESET);
 		goto done;
 	}
@@ -2709,8 +2710,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
 	/* don't delete l2cap channel if sk is owned by user */
 	if (sock_owned_by_user(sk)) {
 		sk->sk_state = BT_DISCONN;
-		l2cap_chan_clear_timer(chan);
-		l2cap_chan_set_timer(chan, HZ / 5);
+		__clear_chan_timer(chan);
+		__set_chan_timer(chan, HZ / 5);
 		bh_unlock_sock(sk);
 		return 0;
 	}
@@ -2743,8 +2744,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
 	/* don't delete l2cap channel if sk is owned by user */
 	if (sock_owned_by_user(sk)) {
 		sk->sk_state = BT_DISCONN;
-		l2cap_chan_clear_timer(chan);
-		l2cap_chan_set_timer(chan, HZ / 5);
+		__clear_chan_timer(chan);
+		__set_chan_timer(chan, HZ / 5);
 		bh_unlock_sock(sk);
 		return 0;
 	}
@@ -4140,13 +4141,13 @@ static inline void l2cap_check_encryption(struct l2cap_chan *chan, u8 encrypt)
 
 	if (encrypt == 0x00) {
 		if (chan->sec_level == BT_SECURITY_MEDIUM) {
-			l2cap_chan_clear_timer(chan);
-			l2cap_chan_set_timer(chan, HZ * 5);
+			__clear_chan_timer(chan);
+			__set_chan_timer(chan, HZ * 5);
 		} else if (chan->sec_level == BT_SECURITY_HIGH)
 			l2cap_chan_close(chan, ECONNREFUSED);
 	} else {
 		if (chan->sec_level == BT_SECURITY_MEDIUM)
-			l2cap_chan_clear_timer(chan);
+			__clear_chan_timer(chan);
 	}
 }
 
@@ -4191,8 +4192,8 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 				l2cap_send_cmd(conn, chan->ident,
 					L2CAP_CONN_REQ, sizeof(req), &req);
 			} else {
-				l2cap_chan_clear_timer(chan);
-				l2cap_chan_set_timer(chan, HZ / 10);
+				__clear_chan_timer(chan);
+				__set_chan_timer(chan, HZ / 10);
 			}
 		} else if (sk->sk_state == BT_CONNECT2) {
 			struct l2cap_conn_rsp rsp;
@@ -4203,7 +4204,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 				result = L2CAP_CR_SUCCESS;
 			} else {
 				sk->sk_state = BT_DISCONN;
-				l2cap_chan_set_timer(chan, HZ / 10);
+				__set_chan_timer(chan, HZ / 10);
 				result = L2CAP_CR_SEC_BLOCK;
 			}
 
-- 
1.7.5.1


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

* [PATCH 6/6] Bluetooth: keep reference if any ERTM timer is enabled
  2011-06-02 16:15       ` [PATCH 5/6] Bluetooth: Make timer functions generic Gustavo F. Padovan
@ 2011-06-02 16:15         ` Gustavo F. Padovan
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 16:15 UTC (permalink / raw)
  To: linux-bluetooth

ERTM use the generic L2CAP timer functions to keep a reference to the
channel. This is useful for avoiding crashes.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 include/net/bluetooth/l2cap.h |   15 ++++++++-----
 net/bluetooth/l2cap_core.c    |   44 ++++++++++++++++++++--------------------
 2 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index e3e58f9..c4799da2 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -437,12 +437,15 @@ struct l2cap_pinfo {
 
 #define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
 #define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
-#define __mod_retrans_timer() mod_timer(&chan->retrans_timer, \
-		jiffies +  msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
-#define __mod_monitor_timer() mod_timer(&chan->monitor_timer, \
-		jiffies + msecs_to_jiffies(L2CAP_DEFAULT_MONITOR_TO));
-#define __mod_ack_timer() mod_timer(&chan->ack_timer, \
-		jiffies + msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
+#define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \
+		L2CAP_DEFAULT_RETRANS_TO);
+#define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer)
+#define __set_monitor_timer(c) l2cap_set_timer(c, &c->monitor_timer, \
+		L2CAP_DEFAULT_MONITOR_TO);
+#define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer)
+#define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \
+		L2CAP_DEFAULT_ACK_TO);
+#define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)
 
 static inline int l2cap_tx_window_full(struct l2cap_chan *ch)
 {
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 8586077..7042f6e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -380,9 +380,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	if (chan->mode == L2CAP_MODE_ERTM) {
 		struct srej_list *l, *tmp;
 
-		del_timer(&chan->retrans_timer);
-		del_timer(&chan->monitor_timer);
-		del_timer(&chan->ack_timer);
+		__clear_retrans_timer(chan);
+		__clear_monitor_timer(chan);
+		__clear_ack_timer(chan);
 
 		skb_queue_purge(&chan->srej_q);
 		skb_queue_purge(&chan->busy_q);
@@ -686,9 +686,9 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c
 	sk = chan->sk;
 
 	if (chan->mode == L2CAP_MODE_ERTM) {
-		del_timer(&chan->retrans_timer);
-		del_timer(&chan->monitor_timer);
-		del_timer(&chan->ack_timer);
+		__clear_retrans_timer(chan);
+		__clear_monitor_timer(chan);
+		__clear_ack_timer(chan);
 	}
 
 	req.dcid = cpu_to_le16(chan->dcid);
@@ -1166,7 +1166,7 @@ static void l2cap_monitor_timeout(unsigned long arg)
 	}
 
 	chan->retry_count++;
-	__mod_monitor_timer();
+	__set_monitor_timer(chan);
 
 	l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL);
 	bh_unlock_sock(sk);
@@ -1181,7 +1181,7 @@ static void l2cap_retrans_timeout(unsigned long arg)
 
 	bh_lock_sock(sk);
 	chan->retry_count = 1;
-	__mod_monitor_timer();
+	__set_monitor_timer(chan);
 
 	chan->conn_state |= L2CAP_CONN_WAIT_F;
 
@@ -1205,7 +1205,7 @@ static void l2cap_drop_acked_frames(struct l2cap_chan *chan)
 	}
 
 	if (!chan->unacked_frames)
-		del_timer(&chan->retrans_timer);
+		__clear_retrans_timer(chan);
 }
 
 void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
@@ -1332,7 +1332,7 @@ int l2cap_ertm_send(struct l2cap_chan *chan)
 
 		l2cap_do_send(chan, tx_skb);
 
-		__mod_retrans_timer();
+		__set_retrans_timer(chan);
 
 		bt_cb(skb)->tx_seq = chan->next_tx_seq;
 		chan->next_tx_seq = (chan->next_tx_seq + 1) % 64;
@@ -3249,8 +3249,8 @@ static int l2cap_try_push_rx_skb(struct l2cap_chan *chan)
 	l2cap_send_sframe(chan, control);
 	chan->retry_count = 1;
 
-	del_timer(&chan->retrans_timer);
-	__mod_monitor_timer();
+	__clear_retrans_timer(chan);
+	__set_monitor_timer(chan);
 
 	chan->conn_state |= L2CAP_CONN_WAIT_F;
 
@@ -3341,7 +3341,7 @@ static int l2cap_push_rx_skb(struct l2cap_chan *chan, struct sk_buff *skb, u16 c
 
 	chan->conn_state |= L2CAP_CONN_RNR_SENT;
 
-	del_timer(&chan->ack_timer);
+	__clear_ack_timer(chan);
 
 	queue_work(_busy_wq, &chan->busy_work);
 
@@ -3510,9 +3510,9 @@ static inline int l2cap_data_channel_iframe(struct l2cap_chan *chan, u16 rx_cont
 
 	if (L2CAP_CTRL_FINAL & rx_control &&
 			chan->conn_state & L2CAP_CONN_WAIT_F) {
-		del_timer(&chan->monitor_timer);
+		__clear_monitor_timer(chan);
 		if (chan->unacked_frames > 0)
-			__mod_retrans_timer();
+			__set_retrans_timer(chan);
 		chan->conn_state &= ~L2CAP_CONN_WAIT_F;
 	}
 
@@ -3593,7 +3593,7 @@ static inline int l2cap_data_channel_iframe(struct l2cap_chan *chan, u16 rx_cont
 
 		l2cap_send_srejframe(chan, tx_seq);
 
-		del_timer(&chan->ack_timer);
+		__clear_ack_timer(chan);
 	}
 	return 0;
 
@@ -3618,7 +3618,7 @@ expected:
 			l2cap_retransmit_frames(chan);
 	}
 
-	__mod_ack_timer();
+	__set_ack_timer(chan);
 
 	chan->num_acked = (chan->num_acked + 1) % num_to_ack;
 	if (chan->num_acked == num_to_ack - 1)
@@ -3644,7 +3644,7 @@ static inline void l2cap_data_channel_rrframe(struct l2cap_chan *chan, u16 rx_co
 		if (chan->conn_state & L2CAP_CONN_SREJ_SENT) {
 			if ((chan->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
 					(chan->unacked_frames > 0))
-				__mod_retrans_timer();
+				__set_retrans_timer(chan);
 
 			chan->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
 			l2cap_send_srejtail(chan);
@@ -3663,7 +3663,7 @@ static inline void l2cap_data_channel_rrframe(struct l2cap_chan *chan, u16 rx_co
 	} else {
 		if ((chan->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
 				(chan->unacked_frames > 0))
-			__mod_retrans_timer();
+			__set_retrans_timer(chan);
 
 		chan->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
 		if (chan->conn_state & L2CAP_CONN_SREJ_SENT)
@@ -3746,7 +3746,7 @@ static inline void l2cap_data_channel_rnrframe(struct l2cap_chan *chan, u16 rx_c
 		chan->conn_state |= L2CAP_CONN_SEND_FBIT;
 
 	if (!(chan->conn_state & L2CAP_CONN_SREJ_SENT)) {
-		del_timer(&chan->retrans_timer);
+		__clear_retrans_timer(chan);
 		if (rx_control & L2CAP_CTRL_POLL)
 			l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_FINAL);
 		return;
@@ -3764,9 +3764,9 @@ static inline int l2cap_data_channel_sframe(struct l2cap_chan *chan, u16 rx_cont
 
 	if (L2CAP_CTRL_FINAL & rx_control &&
 			chan->conn_state & L2CAP_CONN_WAIT_F) {
-		del_timer(&chan->monitor_timer);
+		__clear_monitor_timer(chan);
 		if (chan->unacked_frames > 0)
-			__mod_retrans_timer();
+			__set_retrans_timer(chan);
 		chan->conn_state &= ~L2CAP_CONN_WAIT_F;
 	}
 
-- 
1.7.5.1


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

* Re: [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops
  2011-06-02 16:15   ` [PATCH 3/6] Bluetooth: add close() " Gustavo F. Padovan
  2011-06-02 16:15     ` [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan Gustavo F. Padovan
@ 2011-06-02 21:30     ` Mat Martineau
  2011-06-02 21:50       ` Gustavo F. Padovan
  1 sibling, 1 reply; 8+ messages in thread
From: Mat Martineau @ 2011-06-02 21:30 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth


Hi Gustavo -


On Thu, 2 Jun 2011, Gustavo F. Padovan wrote:

> close() calls l2cap_sock_kill() on l2cap_sock.c
>
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
> include/net/bluetooth/l2cap.h |    3 +--
> net/bluetooth/l2cap_core.c    |   17 +++++++++--------
> net/bluetooth/l2cap_sock.c    |   10 +++++++++-
> 3 files changed, 19 insertions(+), 11 deletions(-)

<snip>

> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
> /* Kill socket (only if zapped and orphan)
>  * Must be called on unlocked socket.
>  */
> -void l2cap_sock_kill(struct sock *sk)
> +static void l2cap_sock_kill(struct sock *sk)
> {
> 	if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> 		return;
> @@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
> 	return sock_queue_rcv_skb(sk, skb);
> }
>
> +static void l2cap_chan_close_cb(void *data)
> +{
> +	struct sock *sk = data;
> +
> +	l2cap_sock_kill(sk);
> +}
> +
> static struct l2cap_ops l2cap_chan_ops = {
> 	.name		= "L2CAP Socket Interface",
> 	.new_connection	= l2cap_chan_new_connection_cb,
> 	.recv		= l2cap_chan_recv_cb,
> +	.close		= l2cap_chan_close_cb,
> };
>
> static void l2cap_sock_destruct(struct sock *sk)


It looks like we'll have l2cap_ops for sock, rfcomm, and AMP's A2MP 
channel -- so I would suggest using unique names for the callbacks for 
each of those modules:

l2cap_sock_new_connection_cb
l2cap_sock_recv_cb
l2cap_sock_close_cb

l2cap_a2mp_new_connection_cb
l2cap_a2mp_recv_cb
l2cap_a2mp_close_cb

etc., instead of "l2cap_chan_*_cb"

Even though the functions are (or will be) static within their 
respective files, it will be less confusing if their names tie them
to the specific module.

I hope you can check these in to bluetooth-next soon, it would help 
with the patch set I'm working on.

Regards,

--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

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

* Re: [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops
  2011-06-02 21:30     ` [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops Mat Martineau
@ 2011-06-02 21:50       ` Gustavo F. Padovan
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2011-06-02 21:50 UTC (permalink / raw)
  To: Mat Martineau; +Cc: linux-bluetooth

Hi Mat,

* Mat Martineau <mathewm@codeaurora.org> [2011-06-02 14:30:41 -0700]:

> 
> Hi Gustavo -
> 
> 
> On Thu, 2 Jun 2011, Gustavo F. Padovan wrote:
> 
> >close() calls l2cap_sock_kill() on l2cap_sock.c
> >
> >Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> >---
> >include/net/bluetooth/l2cap.h |    3 +--
> >net/bluetooth/l2cap_core.c    |   17 +++++++++--------
> >net/bluetooth/l2cap_sock.c    |   10 +++++++++-
> >3 files changed, 19 insertions(+), 11 deletions(-)
> 
> <snip>
> 
> >--- a/net/bluetooth/l2cap_sock.c
> >+++ b/net/bluetooth/l2cap_sock.c
> >@@ -679,7 +679,7 @@ static int l2cap_sock_recvmsg(struct kiocb *iocb, struct socket *sock, struct ms
> >/* Kill socket (only if zapped and orphan)
> > * Must be called on unlocked socket.
> > */
> >-void l2cap_sock_kill(struct sock *sk)
> >+static void l2cap_sock_kill(struct sock *sk)
> >{
> >	if (!sock_flag(sk, SOCK_ZAPPED) || sk->sk_socket)
> >		return;
> >@@ -762,10 +762,18 @@ static int l2cap_chan_recv_cb(void *data, struct sk_buff *skb)
> >	return sock_queue_rcv_skb(sk, skb);
> >}
> >
> >+static void l2cap_chan_close_cb(void *data)
> >+{
> >+	struct sock *sk = data;
> >+
> >+	l2cap_sock_kill(sk);
> >+}
> >+
> >static struct l2cap_ops l2cap_chan_ops = {
> >	.name		= "L2CAP Socket Interface",
> >	.new_connection	= l2cap_chan_new_connection_cb,
> >	.recv		= l2cap_chan_recv_cb,
> >+	.close		= l2cap_chan_close_cb,
> >};
> >
> >static void l2cap_sock_destruct(struct sock *sk)
> 
> 
> It looks like we'll have l2cap_ops for sock, rfcomm, and AMP's A2MP
> channel -- so I would suggest using unique names for the callbacks
> for each of those modules:
> 
> l2cap_sock_new_connection_cb
> l2cap_sock_recv_cb
> l2cap_sock_close_cb
> 
> l2cap_a2mp_new_connection_cb
> l2cap_a2mp_recv_cb
> l2cap_a2mp_close_cb
> 
> etc., instead of "l2cap_chan_*_cb"
> 
> Even though the functions are (or will be) static within their
> respective files, it will be less confusing if their names tie them
> to the specific module.

I'm not sure, what I'm trying to avoid here is create a false relation between
l2cap_sock_close and l2cap_sock_close_cb for example, thus I called it
l2cap_chan_close_cb. But unique names are indeed a good idea.

Maybe, l2cap_sock_chan_new_cb, l2cap_sock_chan_close_cb,
l2cap_sock_chan_recv_cb. But those are a bit big.

> 
> I hope you can check these in to bluetooth-next soon, it would help
> with the patch set I'm working on.

Btw, I really want to fix this L2CAP mess before push any A2MP stuff into the
tree. A2MP code shouldn't touch sockets at all. I don't want to bring more
issues to the stack like the many we have for RFCOMM.
I intend to finish this soon. Patches are welcome. :)

-- 
Gustavo F. Padovan
http://profusion.mobi

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

end of thread, other threads:[~2011-06-02 21:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-02 16:15 [PATCH 1/6] Bluetooth: Add l2cap_chan_ops abstraction Gustavo F. Padovan
2011-06-02 16:15 ` [PATCH 2/6] Bluetooth: add recv() callback to l2cap_chan_ops Gustavo F. Padovan
2011-06-02 16:15   ` [PATCH 3/6] Bluetooth: add close() " Gustavo F. Padovan
2011-06-02 16:15     ` [PATCH 4/6] Bluetooth: Add refcnt to struct l2cap_chan Gustavo F. Padovan
2011-06-02 16:15       ` [PATCH 5/6] Bluetooth: Make timer functions generic Gustavo F. Padovan
2011-06-02 16:15         ` [PATCH 6/6] Bluetooth: keep reference if any ERTM timer is enabled Gustavo F. Padovan
2011-06-02 21:30     ` [PATCH 3/6] Bluetooth: add close() callback to l2cap_chan_ops Mat Martineau
2011-06-02 21:50       ` Gustavo F. 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).