[RFC 0/5 v2] prioritizing data over HCI

[RFC 0/5 v2] prioritizing data over HCI

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This incorporate some suggestions like removal of fixed amount of queues in
favor of one per L2CAP channel using HCI Channel abstraction (hci_chan) so
packet order is maintained, SCO/ESCO packets are no longer affected and
some other minor things like promoting starving channels directly to
maximum possible priority (6).

Priorities are unchanged, so anything bigger than 6 requires extra
capabilities and are meant for guaranteed channels or time critical packets
e.g. RFCOMM commands.

In addition to that I decide to maintain the queue per hci_conn so hci_chan
is only created when L2CAP connection completes.

Luiz Augusto von Dentz (5):
  Bluetooth: make use of connection number to optimize the scheduler
  Bluetooth: set skbuffer priority based on L2CAP socket priority
  Bluetooth: make use sk_priority to priritize RFCOMM packets
  Bluetooth: prioritizing data over HCI
  Bluetooth: recalculate priorities when channels are starving

 include/net/bluetooth/hci_core.h |   61 +++++++++
 include/net/bluetooth/l2cap.h    |    4 +-
 net/bluetooth/hci_conn.c         |   59 +++++++++
 net/bluetooth/hci_core.c         |  256 +++++++++++++++++++++++++++++++++++---
 net/bluetooth/l2cap_core.c       |   52 ++++++--
 net/bluetooth/l2cap_sock.c       |    2 +-
 net/bluetooth/rfcomm/core.c      |   51 +++++---
 net/bluetooth/rfcomm/sock.c      |    2 +
 8 files changed, 437 insertions(+), 50 deletions(-)

-- 
1.7.6

[RFC 1/5 v2] Bluetooth: make use of connection number to optimize the scheduler

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This checks if there is any existing connection according to its type
before start iterating in the list and immediately stop iterating when
reaching the number of connections.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci_core.h |   16 ++++++++++++++++
 net/bluetooth/hci_core.c         |   15 +++++++++++++++
 2 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8f441b8..e2ba4d6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -395,6 +395,22 @@ static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c)
 	}
 }
 
+static inline unsigned int hci_conn_num(struct hci_dev *hdev, __u8 type)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	switch (type) {
+	case ACL_LINK:
+		return h->acl_num;
+	case LE_LINK:
+		return h->le_num;
+	case SCO_LINK:
+	case ESCO_LINK:
+		return h->sco_num;
+	default:
+		return 0;
+	}
+}
+
 static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
 								__u16 handle)
 {
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index ec0bc3f..9574752 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2075,6 +2075,9 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
 			min  = c->sent;
 			conn = c;
 		}
+
+		if (hci_conn_num(hdev, type) == num)
+			break;
 	}
 
 	if (conn) {
@@ -2132,6 +2135,9 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
 
 	BT_DBG("%s", hdev->name);
 
+	if (!hci_conn_num(hdev, ACL_LINK))
+		return;
+
 	if (!test_bit(HCI_RAW, &hdev->flags)) {
 		/* ACL tx timeout must be longer than maximum
 		 * link supervision timeout (40.9 seconds) */
@@ -2163,6 +2169,9 @@ static inline void hci_sched_sco(struct hci_dev *hdev)
 
 	BT_DBG("%s", hdev->name);
 
+	if (!hci_conn_num(hdev, SCO_LINK))
+		return;
+
 	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
 			BT_DBG("skb %p len %d", skb, skb->len);
@@ -2183,6 +2192,9 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
 
 	BT_DBG("%s", hdev->name);
 
+	if (!hci_conn_num(hdev, ESCO_LINK))
+		return;
+
 	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
 			BT_DBG("skb %p len %d", skb, skb->len);
@@ -2203,6 +2215,9 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 
 	BT_DBG("%s", hdev->name);
 
+	if (!hci_conn_num(hdev, LE_LINK))
+		return;
+
 	if (!test_bit(HCI_RAW, &hdev->flags)) {
 		/* LE tx timeout must be longer than maximum
 		 * link supervision timeout (40.9 seconds) */
-- 
1.7.6

[RFC 2/5 v2] Bluetooth: set skbuffer priority based on L2CAP socket priority

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This uses SO_PRIORITY to set the skbuffer priority field

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci_core.h |    3 ++
 include/net/bluetooth/l2cap.h    |    3 +-
 net/bluetooth/l2cap_core.c       |   45 +++++++++++++++++++++++++++-----------
 net/bluetooth/l2cap_sock.c       |    2 +-
 4 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index e2ba4d6..0742828 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -32,6 +32,9 @@
 #define HCI_PROTO_L2CAP	0
 #define HCI_PROTO_SCO	1
 
+/* HCI priority */
+#define HCI_PRIO_MAX	7
+
 /* HCI Core structures */
 struct inquiry_data {
 	bdaddr_t	bdaddr;
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 4f34ad2..f018e5d 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -511,7 +511,8 @@ 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);
 int l2cap_chan_connect(struct l2cap_chan *chan);
-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len);
+int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
+								u32 priority);
 void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
 
 #endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3204ba8..3af0569 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1245,7 +1245,8 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 	struct hci_conn *hcon = chan->conn->hcon;
 	u16 flags;
 
-	BT_DBG("chan %p, skb %p len %d", chan, skb, skb->len);
+	BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
+							skb->priority);
 
 	if (!chan->flushable && lmp_no_flush_capable(hcon->hdev))
 		flags = ACL_START_NO_FLUSH;
@@ -1451,6 +1452,8 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
 		if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
 			return -EFAULT;
 
+		(*frag)->priority = skb->priority;
+
 		sent += count;
 		len  -= count;
 
@@ -1460,7 +1463,9 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
 	return sent;
 }
 
-struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
+static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
+						struct msghdr *msg, size_t len,
+						u32 priority)
 {
 	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
@@ -1468,7 +1473,7 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
 	int err, count, hlen = L2CAP_HDR_SIZE + 2;
 	struct l2cap_hdr *lh;
 
-	BT_DBG("sk %p len %d", sk, (int)len);
+	BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
 
 	count = min_t(unsigned int, (conn->mtu - hlen), len);
 	skb = bt_skb_send_alloc(sk, count + hlen,
@@ -1476,6 +1481,8 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
 	if (!skb)
 		return ERR_PTR(err);
 
+	skb->priority = priority;
+
 	/* Create L2CAP header */
 	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
 	lh->cid = cpu_to_le16(chan->dcid);
@@ -1490,7 +1497,9 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
 	return skb;
 }
 
-struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
+static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
+						struct msghdr *msg, size_t len,
+						u32 priority)
 {
 	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
@@ -1506,6 +1515,8 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
 	if (!skb)
 		return ERR_PTR(err);
 
+	skb->priority = priority;
+
 	/* Create L2CAP header */
 	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
 	lh->cid = cpu_to_le16(chan->dcid);
@@ -1519,7 +1530,10 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
 	return skb;
 }
 
-struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len, u16 control, u16 sdulen)
+static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
+						struct msghdr *msg, size_t len,
+						u32 priority, u16 control,
+						u16 sdulen)
 {
 	struct sock *sk = chan->sk;
 	struct l2cap_conn *conn = chan->conn;
@@ -1527,7 +1541,7 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
 	int err, count, hlen = L2CAP_HDR_SIZE + 2;
 	struct l2cap_hdr *lh;
 
-	BT_DBG("sk %p len %d", sk, (int)len);
+	BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
 
 	if (!conn)
 		return ERR_PTR(-ENOTCONN);
@@ -1544,6 +1558,8 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
 	if (!skb)
 		return ERR_PTR(err);
 
+	skb->priority = priority;
+
 	/* Create L2CAP header */
 	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
 	lh->cid = cpu_to_le16(chan->dcid);
@@ -1574,7 +1590,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
 
 	skb_queue_head_init(&sar_queue);
 	control = L2CAP_SDU_START;
-	skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps, control, len);
+	skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps,
+						HCI_PRIO_MAX, control, len);
 	if (IS_ERR(skb))
 		return PTR_ERR(skb);
 
@@ -1593,7 +1610,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
 			buflen = len;
 		}
 
-		skb = l2cap_create_iframe_pdu(chan, msg, buflen, control, 0);
+		skb = l2cap_create_iframe_pdu(chan, msg, buflen, HCI_PRIO_MAX,
+								control, 0);
 		if (IS_ERR(skb)) {
 			skb_queue_purge(&sar_queue);
 			return PTR_ERR(skb);
@@ -1610,7 +1628,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
 	return size;
 }
 
-int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
+int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
+								u32 priority)
 {
 	struct sk_buff *skb;
 	u16 control;
@@ -1618,7 +1637,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
 
 	/* Connectionless channel */
 	if (chan->chan_type == L2CAP_CHAN_CONN_LESS) {
-		skb = l2cap_create_connless_pdu(chan, msg, len);
+		skb = l2cap_create_connless_pdu(chan, msg, len, priority);
 		if (IS_ERR(skb))
 			return PTR_ERR(skb);
 
@@ -1633,7 +1652,7 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
 			return -EMSGSIZE;
 
 		/* Create a basic PDU */
-		skb = l2cap_create_basic_pdu(chan, msg, len);
+		skb = l2cap_create_basic_pdu(chan, msg, len, priority);
 		if (IS_ERR(skb))
 			return PTR_ERR(skb);
 
@@ -1646,8 +1665,8 @@ int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
 		/* Entire SDU fits into one PDU */
 		if (len <= chan->remote_mps) {
 			control = L2CAP_SDU_UNSEGMENTED;
-			skb = l2cap_create_iframe_pdu(chan, msg, len, control,
-									0);
+			skb = l2cap_create_iframe_pdu(chan, msg, len, priority,
+								control, 0);
 			if (IS_ERR(skb))
 				return PTR_ERR(skb);
 
diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 5c36b3e..abe302b 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -706,7 +706,7 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
 		return -ENOTCONN;
 	}
 
-	err = l2cap_chan_send(chan, msg, len);
+	err = l2cap_chan_send(chan, msg, len, sk->sk_priority);
 
 	release_sock(sk);
 	return err;
-- 
1.7.6

[RFC 3/5 v2] Bluetooth: make use sk_priority to priritize RFCOMM packets

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/rfcomm/core.c |   51 +++++++++++++++++++++++++++++-------------
 net/bluetooth/rfcomm/sock.c |    2 +
 2 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 5759bb7..3ec98bd 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -66,7 +66,8 @@ static unsigned long rfcomm_event;
 
 static LIST_HEAD(session_list);
 
-static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len);
+static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len,
+							u32 priority);
 static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci);
 static int rfcomm_send_disc(struct rfcomm_session *s, u8 dlci);
 static int rfcomm_queue_disc(struct rfcomm_dlc *d);
@@ -751,19 +752,34 @@ void rfcomm_session_getaddr(struct rfcomm_session *s, bdaddr_t *src, bdaddr_t *d
 }
 
 /* ---- RFCOMM frame sending ---- */
-static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len)
+static int rfcomm_send_frame(struct rfcomm_session *s, u8 *data, int len,
+							u32 priority)
 {
 	struct socket *sock = s->sock;
+	struct sock *sk = sock->sk;
 	struct kvec iv = { data, len };
 	struct msghdr msg;
 
-	BT_DBG("session %p len %d", s, len);
+	BT_DBG("session %p len %d priority %u", s, len, priority);
+
+	if (sk->sk_priority != priority) {
+		lock_sock(sk);
+		sk->sk_priority = priority;
+		release_sock(sk);
+	}
 
 	memset(&msg, 0, sizeof(msg));
 
 	return kernel_sendmsg(sock, &msg, &iv, 1, len);
 }
 
+static int rfcomm_send_cmd(struct rfcomm_session *s, struct rfcomm_cmd *cmd)
+{
+	BT_DBG("%p cmd %u", s, cmd->ctrl);
+
+	return rfcomm_send_frame(s, (void *) cmd, sizeof(*cmd), HCI_PRIO_MAX);
+}
+
 static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci)
 {
 	struct rfcomm_cmd cmd;
@@ -775,7 +791,7 @@ static int rfcomm_send_sabm(struct rfcomm_session *s, u8 dlci)
 	cmd.len  = __len8(0);
 	cmd.fcs  = __fcs2((u8 *) &cmd);
 
-	return rfcomm_send_frame(s, (void *) &cmd, sizeof(cmd));
+	return rfcomm_send_cmd(s, &cmd);
 }
 
 static int rfcomm_send_ua(struct rfcomm_session *s, u8 dlci)
@@ -789,7 +805,7 @@ static int rfcomm_send_ua(struct rfcomm_session *s, u8 dlci)
 	cmd.len  = __len8(0);
 	cmd.fcs  = __fcs2((u8 *) &cmd);
 
-	return rfcomm_send_frame(s, (void *) &cmd, sizeof(cmd));
+	return rfcomm_send_cmd(s, &cmd);
 }
 
 static int rfcomm_send_disc(struct rfcomm_session *s, u8 dlci)
@@ -803,7 +819,7 @@ static int rfcomm_send_disc(struct rfcomm_session *s, u8 dlci)
 	cmd.len  = __len8(0);
 	cmd.fcs  = __fcs2((u8 *) &cmd);
 
-	return rfcomm_send_frame(s, (void *) &cmd, sizeof(cmd));
+	return rfcomm_send_cmd(s, &cmd);
 }
 
 static int rfcomm_queue_disc(struct rfcomm_dlc *d)
@@ -817,6 +833,8 @@ static int rfcomm_queue_disc(struct rfcomm_dlc *d)
 	if (!skb)
 		return -ENOMEM;
 
+	skb->priority = HCI_PRIO_MAX;
+
 	cmd = (void *) __skb_put(skb, sizeof(*cmd));
 	cmd->addr = d->addr;
 	cmd->ctrl = __ctrl(RFCOMM_DISC, 1);
@@ -839,7 +857,7 @@ static int rfcomm_send_dm(struct rfcomm_session *s, u8 dlci)
 	cmd.len  = __len8(0);
 	cmd.fcs  = __fcs2((u8 *) &cmd);
 
-	return rfcomm_send_frame(s, (void *) &cmd, sizeof(cmd));
+	return rfcomm_send_cmd(s, &cmd);
 }
 
 static int rfcomm_send_nsc(struct rfcomm_session *s, int cr, u8 type)
@@ -864,7 +882,7 @@ static int rfcomm_send_nsc(struct rfcomm_session *s, int cr, u8 type)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_pn(struct rfcomm_session *s, int cr, struct rfcomm_dlc *d)
@@ -906,7 +924,7 @@ static int rfcomm_send_pn(struct rfcomm_session *s, int cr, struct rfcomm_dlc *d
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
@@ -944,7 +962,7 @@ int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_rls(struct rfcomm_session *s, int cr, u8 dlci, u8 status)
@@ -971,7 +989,7 @@ static int rfcomm_send_rls(struct rfcomm_session *s, int cr, u8 dlci, u8 status)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_msc(struct rfcomm_session *s, int cr, u8 dlci, u8 v24_sig)
@@ -998,7 +1016,7 @@ static int rfcomm_send_msc(struct rfcomm_session *s, int cr, u8 dlci, u8 v24_sig
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_fcoff(struct rfcomm_session *s, int cr)
@@ -1020,7 +1038,7 @@ static int rfcomm_send_fcoff(struct rfcomm_session *s, int cr)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_fcon(struct rfcomm_session *s, int cr)
@@ -1042,7 +1060,7 @@ static int rfcomm_send_fcon(struct rfcomm_session *s, int cr)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static int rfcomm_send_test(struct rfcomm_session *s, int cr, u8 *pattern, int len)
@@ -1093,7 +1111,7 @@ static int rfcomm_send_credits(struct rfcomm_session *s, u8 addr, u8 credits)
 
 	*ptr = __fcs(buf); ptr++;
 
-	return rfcomm_send_frame(s, buf, ptr - buf);
+	return rfcomm_send_frame(s, buf, ptr - buf, HCI_PRIO_MAX);
 }
 
 static void rfcomm_make_uih(struct sk_buff *skb, u8 addr)
@@ -1771,7 +1789,8 @@ static inline int rfcomm_process_tx(struct rfcomm_dlc *d)
 		return skb_queue_len(&d->tx_queue);
 
 	while (d->tx_credits && (skb = skb_dequeue(&d->tx_queue))) {
-		err = rfcomm_send_frame(d->session, skb->data, skb->len);
+		err = rfcomm_send_frame(d->session, skb->data, skb->len,
+							skb->priority);
 		if (err < 0) {
 			skb_queue_head(&d->tx_queue, skb);
 			break;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 8f01e6b..de8c386 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -597,6 +597,8 @@ static int rfcomm_sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 			break;
 		}
 
+		skb->priority = sk->sk_priority;
+
 		err = rfcomm_dlc_send(d, skb);
 		if (err < 0) {
 			kfree_skb(skb);
-- 
1.7.6

[RFC 4/5 v2] Bluetooth: prioritizing data over HCI

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This implement priority based scheduler using skbuffer priority set via
SO_PRIORITY socket option.

It introduces hci_chan_hash (list of HCI Channel/hci_chan) per connection,
each item in this list refer to a L2CAP connection and it is used to
queue the data for transmission.

Each connection continue to have a queue but it is only used for time
critical packets such the L2CAP commands and it is always processed
before the channels thus it can be considered the highest priority.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 include/net/bluetooth/hci_core.h |   42 +++++++++
 include/net/bluetooth/l2cap.h    |    1 +
 net/bluetooth/hci_conn.c         |   59 ++++++++++++
 net/bluetooth/hci_core.c         |  180 ++++++++++++++++++++++++++++++++++----
 net/bluetooth/l2cap_core.c       |    5 +-
 5 files changed, 269 insertions(+), 18 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 0742828..e0d29eb 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -67,6 +67,12 @@ struct hci_conn_hash {
 	unsigned int     le_num;
 };
 
+struct hci_chan_hash {
+	struct list_head list;
+	spinlock_t       lock;
+	unsigned int     num;
+};
+
 struct bdaddr_list {
 	struct list_head list;
 	bdaddr_t bdaddr;
@@ -278,6 +284,7 @@ struct hci_conn {
 	unsigned int	sent;
 
 	struct sk_buff_head data_q;
+	struct hci_chan_hash chan_hash;
 
 	struct timer_list disc_timer;
 	struct timer_list idle_timer;
@@ -300,6 +307,14 @@ struct hci_conn {
 	void (*disconn_cfm_cb)	(struct hci_conn *conn, u8 reason);
 };
 
+struct hci_chan {
+	struct list_head list;
+
+	struct hci_conn *conn;
+	struct sk_buff_head data_q;
+	unsigned int	sent;
+};
+
 extern struct hci_proto *hci_proto[];
 extern struct list_head hci_dev_list;
 extern struct list_head hci_cb_list;
@@ -459,6 +474,28 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
 	return NULL;
 }
 
+static inline void hci_chan_hash_init(struct hci_conn *c)
+{
+	struct hci_chan_hash *h = &c->chan_hash;
+	INIT_LIST_HEAD(&h->list);
+	spin_lock_init(&h->lock);
+	h->num = 0;
+}
+
+static inline void hci_chan_hash_add(struct hci_conn *c, struct hci_chan *chan)
+{
+	struct hci_chan_hash *h = &c->chan_hash;
+	list_add(&chan->list, &h->list);
+	h->num++;
+}
+
+static inline void hci_chan_hash_del(struct hci_conn *c, struct hci_chan *chan)
+{
+	struct hci_chan_hash *h = &c->chan_hash;
+	list_del(&chan->list);
+	h->num--;
+}
+
 void hci_acl_connect(struct hci_conn *conn);
 void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
 void hci_add_sco(struct hci_conn *conn, __u16 handle);
@@ -470,6 +507,10 @@ int hci_conn_del(struct hci_conn *conn);
 void hci_conn_hash_flush(struct hci_dev *hdev);
 void hci_conn_check_pending(struct hci_dev *hdev);
 
+struct hci_chan *hci_chan_create(struct hci_conn *conn);
+int hci_chan_del(struct hci_chan *chan);
+void hci_chan_hash_flush(struct hci_conn *conn);
+
 struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
 						__u8 sec_level, __u8 auth_type);
 int hci_conn_check_link_mode(struct hci_conn *conn);
@@ -839,6 +880,7 @@ int hci_unregister_notifier(struct notifier_block *nb);
 
 int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
 void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags);
+void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
 void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
 
 void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f018e5d..1e4dd6b 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -301,6 +301,7 @@ struct srej_list {
 struct l2cap_chan {
 	struct sock *sk;
 
+	struct hci_chan		*hchan;
 	struct l2cap_conn	*conn;
 
 	__u8		state;
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index ea7f031..19ae6b4 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -374,6 +374,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
 
 	skb_queue_head_init(&conn->data_q);
 
+	hci_chan_hash_init(conn);
+
 	setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn);
 	setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn);
 	setup_timer(&conn->auto_accept_timer, hci_conn_auto_accept,
@@ -432,6 +434,8 @@ int hci_conn_del(struct hci_conn *conn)
 
 	tasklet_disable(&hdev->tx_task);
 
+	hci_chan_hash_flush(conn);
+
 	hci_conn_hash_del(hdev, conn);
 	if (hdev->notify)
 		hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
@@ -956,3 +960,58 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
 
 	return copy_to_user(arg, &req, sizeof(req)) ? -EFAULT : 0;
 }
+
+struct hci_chan *hci_chan_create(struct hci_conn *conn)
+{
+	struct hci_dev *hdev = conn->hdev;
+	struct hci_chan *chan;
+
+	BT_DBG("%s conn %p", hdev->name, conn);
+
+	chan = kzalloc(sizeof(struct hci_chan), GFP_ATOMIC);
+	if (!chan)
+		return NULL;
+
+	chan->conn = conn;
+	skb_queue_head_init(&chan->data_q);
+
+	tasklet_disable(&hdev->tx_task);
+	hci_chan_hash_add(conn, chan);
+	tasklet_enable(&hdev->tx_task);
+
+	return chan;
+}
+
+int hci_chan_del(struct hci_chan *chan)
+{
+	struct hci_conn *conn = chan->conn;
+	struct hci_dev *hdev = conn->hdev;
+
+	BT_DBG("%s conn %p chan %p", hdev->name, conn, chan);
+
+	tasklet_disable(&hdev->tx_task);
+	hci_chan_hash_del(conn, chan);
+	tasklet_enable(&hdev->tx_task);
+
+	skb_queue_purge(&chan->data_q);
+	kfree(chan);
+
+	return 0;
+}
+
+void hci_chan_hash_flush(struct hci_conn *conn)
+{
+	struct hci_chan_hash *h = &conn->chan_hash;
+	struct list_head *p;
+
+	BT_DBG("conn %p", conn);
+	p = h->list.next;
+	while (p != &h->list) {
+		struct hci_chan *chan;
+
+		chan = list_entry(p, struct hci_chan, list);
+		p = p->next;
+
+		hci_chan_del(chan);
+	}
+}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9574752..3a4c3a2 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1975,23 +1975,18 @@ static void hci_add_acl_hdr(struct sk_buff *skb, __u16 handle, __u16 flags)
 	hdr->dlen   = cpu_to_le16(len);
 }
 
-void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
+static void hci_queue_acl(struct hci_conn *conn, struct sk_buff_head *queue,
+				struct sk_buff *skb, __u16 flags)
 {
 	struct hci_dev *hdev = conn->hdev;
 	struct sk_buff *list;
 
-	BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
-
-	skb->dev = (void *) hdev;
-	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
-	hci_add_acl_hdr(skb, conn->handle, flags);
-
 	list = skb_shinfo(skb)->frag_list;
 	if (!list) {
 		/* Non fragmented */
 		BT_DBG("%s nonfrag skb %p len %d", hdev->name, skb, skb->len);
 
-		skb_queue_tail(&conn->data_q, skb);
+		skb_queue_tail(queue, skb);
 	} else {
 		/* Fragmented */
 		BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
@@ -1999,9 +1994,9 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
 		skb_shinfo(skb)->frag_list = NULL;
 
 		/* Queue all fragments atomically */
-		spin_lock_bh(&conn->data_q.lock);
+		spin_lock_bh(&queue->lock);
 
-		__skb_queue_tail(&conn->data_q, skb);
+		__skb_queue_tail(queue, skb);
 
 		flags &= ~ACL_START;
 		flags |= ACL_CONT;
@@ -2014,16 +2009,46 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
 
 			BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
 
-			__skb_queue_tail(&conn->data_q, skb);
+			__skb_queue_tail(queue, skb);
 		} while (list);
 
-		spin_unlock_bh(&conn->data_q.lock);
+		spin_unlock_bh(&queue->lock);
 	}
+}
+
+void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
+{
+	struct hci_dev *hdev = conn->hdev;
+
+	BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
+
+	skb->dev = (void *) hdev;
+	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
+	hci_add_acl_hdr(skb, conn->handle, flags);
+
+	hci_queue_acl(conn, &conn->data_q, skb, flags);
 
 	tasklet_schedule(&hdev->tx_task);
 }
 EXPORT_SYMBOL(hci_send_acl);
 
+void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags)
+{
+	struct hci_conn *conn = chan->conn;
+	struct hci_dev *hdev = conn->hdev;
+
+	BT_DBG("%s chan %p flags 0x%x", hdev->name, chan, flags);
+
+	skb->dev = (void *) hdev;
+	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
+	hci_add_acl_hdr(skb, conn->handle, flags);
+
+	hci_queue_acl(conn, &chan->data_q, skb, flags);
+
+	tasklet_schedule(&hdev->tx_task);
+}
+EXPORT_SYMBOL(hci_chan_send_acl);
+
 /* Send SCO data */
 void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb)
 {
@@ -2127,11 +2152,95 @@ static inline void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
 	}
 }
 
+static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
+						int *quote)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	struct hci_chan *chan = NULL;
+	int num = 0, min = ~0, cur_prio = 0;
+	struct list_head *p, *c;
+	int cnt, q, conn_num = 0;
+
+	BT_DBG("%s", hdev->name);
+
+	list_for_each(p, &h->list) {
+		struct hci_conn *conn;
+		struct hci_chan_hash *ch;
+
+		conn = list_entry(p, struct hci_conn, list);
+		ch = &conn->chan_hash;
+
+		if (conn->type != type)
+			continue;
+
+		if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG)
+			continue;
+
+		conn_num++;
+
+		list_for_each(c, &ch->list) {
+			struct hci_chan *tmp;
+			struct sk_buff *skb;
+
+			tmp = list_entry(c, struct hci_chan, list);
+
+			if (skb_queue_empty(&tmp->data_q))
+				continue;
+
+			skb = skb_peek(&tmp->data_q);
+			if (skb->priority < cur_prio)
+				continue;
+
+			if (skb->priority > cur_prio) {
+				num = 0;
+				min = ~0;
+				cur_prio = skb->priority;
+			}
+
+			num++;
+
+			if (conn->sent < min) {
+				min  = conn->sent;
+				chan = tmp;
+			}
+		}
+
+		if (hci_conn_num(hdev, type) == conn_num)
+			break;
+	}
+
+	if (!chan)
+		return NULL;
+
+	switch (chan->conn->type) {
+	case ACL_LINK:
+		cnt = hdev->acl_cnt;
+		break;
+	case SCO_LINK:
+	case ESCO_LINK:
+		cnt = hdev->sco_cnt;
+		break;
+	case LE_LINK:
+		cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
+		break;
+	default:
+		cnt = 0;
+		BT_ERR("Unknown link type");
+	}
+
+	q = cnt / num;
+	*quote = q ? q : 1;
+	BT_DBG("chan %p quote %d", chan, *quote);
+	return chan;
+}
+
 static inline void hci_sched_acl(struct hci_dev *hdev)
 {
 	struct hci_conn *conn;
+	struct hci_chan *chan;
 	struct sk_buff *skb;
 	int quote;
+	unsigned int cnt;
 
 	BT_DBG("%s", hdev->name);
 
@@ -2145,9 +2254,10 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
 			hci_link_tx_to(hdev, ACL_LINK);
 	}
 
-	while (hdev->acl_cnt && (conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
+	while (hdev->acl_cnt &&
+			(conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
-			BT_DBG("skb %p len %d", skb, skb->len);
+			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
 
 			hci_conn_enter_active_mode(conn, bt_cb(skb)->force_active);
 
@@ -2158,6 +2268,26 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
 			conn->sent++;
 		}
 	}
+
+	cnt = hdev->acl_cnt;
+
+	while (hdev->acl_cnt &&
+			(chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
+		while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
+			BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
+					skb->len, skb->priority);
+
+			hci_conn_enter_active_mode(chan->conn,
+						bt_cb(skb)->force_active);
+
+			hci_send_frame(skb);
+			hdev->acl_last_tx = jiffies;
+
+			hdev->acl_cnt--;
+			chan->sent++;
+			chan->conn->sent++;
+		}
+	}
 }
 
 /* Schedule SCO */
@@ -2174,7 +2304,7 @@ static inline void hci_sched_sco(struct hci_dev *hdev)
 
 	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
-			BT_DBG("skb %p len %d", skb, skb->len);
+			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
 			hci_send_frame(skb);
 
 			conn->sent++;
@@ -2197,7 +2327,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
 
 	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
-			BT_DBG("skb %p len %d", skb, skb->len);
+			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
 			hci_send_frame(skb);
 
 			conn->sent++;
@@ -2210,6 +2340,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
 static inline void hci_sched_le(struct hci_dev *hdev)
 {
 	struct hci_conn *conn;
+	struct hci_chan *chan;
 	struct sk_buff *skb;
 	int quote, cnt;
 
@@ -2229,7 +2360,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 	cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
 	while (cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
-			BT_DBG("skb %p len %d", skb, skb->len);
+			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
 
 			hci_send_frame(skb);
 			hdev->le_last_tx = jiffies;
@@ -2238,6 +2369,21 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 			conn->sent++;
 		}
 	}
+
+	while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, &quote))) {
+		while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
+			BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
+					skb->len, skb->priority);
+
+			hci_send_frame(skb);
+			hdev->le_last_tx = jiffies;
+
+			cnt--;
+			chan->sent++;
+			chan->conn->sent++;
+		}
+	}
+
 	if (hdev->le_pkts)
 		hdev->le_cnt = cnt;
 	else
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3af0569..d30cb36 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -313,6 +313,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 	conn->disc_reason = 0x13;
 
 	chan->conn = conn;
+	chan->hchan = hci_chan_create(conn->hcon);
 
 	if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED) {
 		if (conn->hcon->type == LE_LINK) {
@@ -357,6 +358,8 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err)
 	if (conn) {
 		/* Delete from channel list */
 		write_lock_bh(&conn->chan_lock);
+		hci_chan_del(chan->hchan);
+		chan->hchan = NULL;
 		list_del(&chan->list);
 		write_unlock_bh(&conn->chan_lock);
 		chan_put(chan);
@@ -1254,7 +1257,7 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
 		flags = ACL_START;
 
 	bt_cb(skb)->force_active = chan->force_active;
-	hci_send_acl(hcon, skb, flags);
+	hci_chan_send_acl(chan->hchan, skb, flags);
 }
 
 void l2cap_streaming_send(struct l2cap_chan *chan)
-- 
1.7.6

[RFC 5/5 v2] Bluetooth: recalculate priorities when channels are starving

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

To avoid starvation the priority is recalculated so that the starving
channels are promoted to HCI_PRIO_MAX - 1 (6).

HCI_PRIO_MAX (7) is considered special, because it requires CAP_NET_ADMIN
capability which can be used to provide more guaranties, so it is not used
when promoting.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/hci_core.c |   63 ++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3a4c3a2..eabe84e 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2184,7 +2184,7 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
 
 			tmp = list_entry(c, struct hci_chan, list);
 
-			if (skb_queue_empty(&tmp->data_q))
+			if (tmp->sent || skb_queue_empty(&tmp->data_q))
 				continue;
 
 			skb = skb_peek(&tmp->data_q);
@@ -2234,6 +2234,58 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
 	return chan;
 }
 
+static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type)
+{
+	struct hci_conn_hash *h = &hdev->conn_hash;
+	struct list_head *p, *c;
+	int num = 0;
+
+	BT_DBG("%s", hdev->name);
+
+	list_for_each(p, &h->list) {
+		struct hci_conn *conn;
+		struct hci_chan_hash *ch;
+
+		conn = list_entry(p, struct hci_conn, list);
+		ch = &conn->chan_hash;
+
+		if (conn->type != type)
+			continue;
+
+		if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG)
+			continue;
+
+		num++;
+
+		list_for_each(c, &ch->list) {
+			struct hci_chan *chan;
+			struct sk_buff *skb;
+
+			chan = list_entry(c, struct hci_chan, list);
+
+			if (chan->sent) {
+				chan->sent = 0;
+				continue;
+			}
+
+			if (skb_queue_empty(&chan->data_q))
+				continue;
+
+			skb = skb_peek(&chan->data_q);
+			if (skb->priority >= HCI_PRIO_MAX - 1)
+				continue;
+
+			skb->priority = HCI_PRIO_MAX - 1;
+
+			BT_DBG("chan %p skb %p promoted to %d", chan, skb,
+								skb->priority);
+		}
+
+		if (hci_conn_num(hdev, type) == num)
+			break;
+	}
+}
+
 static inline void hci_sched_acl(struct hci_dev *hdev)
 {
 	struct hci_conn *conn;
@@ -2288,6 +2340,9 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
 			chan->conn->sent++;
 		}
 	}
+
+	if (cnt != hdev->acl_cnt)
+		hci_prio_recalculate(hdev, ACL_LINK);
 }
 
 /* Schedule SCO */
@@ -2342,7 +2397,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 	struct hci_conn *conn;
 	struct hci_chan *chan;
 	struct sk_buff *skb;
-	int quote, cnt;
+	int quote, cnt, tmp;
 
 	BT_DBG("%s", hdev->name);
 
@@ -2370,6 +2425,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 		}
 	}
 
+	tmp = cnt;
 	while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
 			BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
@@ -2388,6 +2444,9 @@ static inline void hci_sched_le(struct hci_dev *hdev)
 		hdev->le_cnt = cnt;
 	else
 		hdev->acl_cnt = cnt;
+
+	if (cnt != tmp)
+		hci_prio_recalculate(hdev, LE_LINK);
 }
 
 static void hci_tx_task(unsigned long arg)
-- 
1.7.6

Re: [RFC 2/5 v2] Bluetooth: set skbuffer priority based on L2CAP socket priority

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:01 +0300]:

> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This uses SO_PRIORITY to set the skbuffer priority field
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |    3 ++
>  include/net/bluetooth/l2cap.h    |    3 +-
>  net/bluetooth/l2cap_core.c       |   45 +++++++++++++++++++++++++++-----------
>  net/bluetooth/l2cap_sock.c       |    2 +-
>  4 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e2ba4d6..0742828 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -32,6 +32,9 @@
>  #define HCI_PROTO_L2CAP	0
>  #define HCI_PROTO_SCO	1
>  
> +/* HCI priority */
> +#define HCI_PRIO_MAX	7
> +
>  /* HCI Core structures */
>  struct inquiry_data {
>  	bdaddr_t	bdaddr;
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 4f34ad2..f018e5d 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -511,7 +511,8 @@ 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);
>  int l2cap_chan_connect(struct l2cap_chan *chan);
> -int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len);
> +int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> +								u32 priority);

I think priority could be part of struct l2cap_chan, so we avoid an extra
parameter in some calls here. Priority is also accessible from chan->sk, but I
would like to avoid more references to sk in l2cap core.

>  void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
>  
>  #endif /* __L2CAP_H */
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 3204ba8..3af0569 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -1245,7 +1245,8 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
>  	struct hci_conn *hcon = chan->conn->hcon;
>  	u16 flags;
>  
> -	BT_DBG("chan %p, skb %p len %d", chan, skb, skb->len);
> +	BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
> +							skb->priority);
>  
>  	if (!chan->flushable && lmp_no_flush_capable(hcon->hdev))
>  		flags = ACL_START_NO_FLUSH;
> @@ -1451,6 +1452,8 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>  		if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
>  			return -EFAULT;
>  
> +		(*frag)->priority = skb->priority;
> +
>  		sent += count;
>  		len  -= count;
>  
> @@ -1460,7 +1463,9 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>  	return sent;
>  }
>  
> -struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
> +static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
> +						struct msghdr *msg, size_t len,
> +						u32 priority)
>  {
>  	struct sock *sk = chan->sk;
>  	struct l2cap_conn *conn = chan->conn;
> @@ -1468,7 +1473,7 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>  	int err, count, hlen = L2CAP_HDR_SIZE + 2;
>  	struct l2cap_hdr *lh;
>  
> -	BT_DBG("sk %p len %d", sk, (int)len);
> +	BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
>  
>  	count = min_t(unsigned int, (conn->mtu - hlen), len);
>  	skb = bt_skb_send_alloc(sk, count + hlen,
> @@ -1476,6 +1481,8 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>  	if (!skb)
>  		return ERR_PTR(err);
>  
> +	skb->priority = priority;
> +
>  	/* Create L2CAP header */
>  	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>  	lh->cid = cpu_to_le16(chan->dcid);
> @@ -1490,7 +1497,9 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>  	return skb;
>  }
>  
> -struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
> +static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
> +						struct msghdr *msg, size_t len,
> +						u32 priority)
>  {
>  	struct sock *sk = chan->sk;
>  	struct l2cap_conn *conn = chan->conn;
> @@ -1506,6 +1515,8 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
>  	if (!skb)
>  		return ERR_PTR(err);
>  
> +	skb->priority = priority;
> +
>  	/* Create L2CAP header */
>  	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>  	lh->cid = cpu_to_le16(chan->dcid);
> @@ -1519,7 +1530,10 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
>  	return skb;
>  }
>  
> -struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len, u16 control, u16 sdulen)
> +static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
> +						struct msghdr *msg, size_t len,
> +						u32 priority, u16 control,
> +						u16 sdulen)
>  {
>  	struct sock *sk = chan->sk;
>  	struct l2cap_conn *conn = chan->conn;
> @@ -1527,7 +1541,7 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
>  	int err, count, hlen = L2CAP_HDR_SIZE + 2;
>  	struct l2cap_hdr *lh;
>  
> -	BT_DBG("sk %p len %d", sk, (int)len);
> +	BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
>  
>  	if (!conn)
>  		return ERR_PTR(-ENOTCONN);
> @@ -1544,6 +1558,8 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
>  	if (!skb)
>  		return ERR_PTR(err);
>  
> +	skb->priority = priority;
> +
>  	/* Create L2CAP header */
>  	lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>  	lh->cid = cpu_to_le16(chan->dcid);
> @@ -1574,7 +1590,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
>  
>  	skb_queue_head_init(&sar_queue);
>  	control = L2CAP_SDU_START;
> -	skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps, control, len);
> +	skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps,
> +						HCI_PRIO_MAX, control, len);


HCI_PRIO_MAX, why?


>  	if (IS_ERR(skb))
>  		return PTR_ERR(skb);
>  
> @@ -1593,7 +1610,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
>  			buflen = len;
>  		}
>  
> -		skb = l2cap_create_iframe_pdu(chan, msg, buflen, control, 0);
> +		skb = l2cap_create_iframe_pdu(chan, msg, buflen, HCI_PRIO_MAX,
> +								control, 0);

same here.

	Gustavo

Re: [RFC 4/5 v2] Bluetooth: prioritizing data over HCI

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:03 +0300]:

> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This implement priority based scheduler using skbuffer priority set via
> SO_PRIORITY socket option.
> 
> It introduces hci_chan_hash (list of HCI Channel/hci_chan) per connection,
> each item in this list refer to a L2CAP connection and it is used to
> queue the data for transmission.
> 
> Each connection continue to have a queue but it is only used for time
> critical packets such the L2CAP commands and it is always processed
> before the channels thus it can be considered the highest priority.

I think we can drop the connection and queue create a channel by default for
the special L2CAP channels, BR/EDR and LE signalling, SMP and AMP Manager.
Those will have high priority of course. Standardize this in just one way to
send packets would be better. It simplifies the sending logic and reduces the
code.

> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |   42 +++++++++
>  include/net/bluetooth/l2cap.h    |    1 +
>  net/bluetooth/hci_conn.c         |   59 ++++++++++++
>  net/bluetooth/hci_core.c         |  180 ++++++++++++++++++++++++++++++++++----
>  net/bluetooth/l2cap_core.c       |    5 +-
>  5 files changed, 269 insertions(+), 18 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 0742828..e0d29eb 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -67,6 +67,12 @@ struct hci_conn_hash {
>  	unsigned int     le_num;
>  };
>  
> +struct hci_chan_hash {
> +	struct list_head list;
> +	spinlock_t       lock;
> +	unsigned int     num;
> +};
> +
>  struct bdaddr_list {
>  	struct list_head list;
>  	bdaddr_t bdaddr;
> @@ -278,6 +284,7 @@ struct hci_conn {
>  	unsigned int	sent;
>  
>  	struct sk_buff_head data_q;
> +	struct hci_chan_hash chan_hash;
>  
>  	struct timer_list disc_timer;
>  	struct timer_list idle_timer;
> @@ -300,6 +307,14 @@ struct hci_conn {
>  	void (*disconn_cfm_cb)	(struct hci_conn *conn, u8 reason);
>  };
>  
> +struct hci_chan {
> +	struct list_head list;
> +
> +	struct hci_conn *conn;
> +	struct sk_buff_head data_q;
> +	unsigned int	sent;
> +};
> +
>  extern struct hci_proto *hci_proto[];
>  extern struct list_head hci_dev_list;
>  extern struct list_head hci_cb_list;
> @@ -459,6 +474,28 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
>  	return NULL;
>  }
>  
> +static inline void hci_chan_hash_init(struct hci_conn *c)
> +{
> +	struct hci_chan_hash *h = &c->chan_hash;
> +	INIT_LIST_HEAD(&h->list);
> +	spin_lock_init(&h->lock);
> +	h->num = 0;
> +}
> +
> +static inline void hci_chan_hash_add(struct hci_conn *c, struct hci_chan *chan)
> +{
> +	struct hci_chan_hash *h = &c->chan_hash;
> +	list_add(&chan->list, &h->list);
> +	h->num++;
> +}
> +
> +static inline void hci_chan_hash_del(struct hci_conn *c, struct hci_chan *chan)
> +{
> +	struct hci_chan_hash *h = &c->chan_hash;
> +	list_del(&chan->list);
> +	h->num--;
> +}
> +
>  void hci_acl_connect(struct hci_conn *conn);
>  void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
>  void hci_add_sco(struct hci_conn *conn, __u16 handle);
> @@ -470,6 +507,10 @@ int hci_conn_del(struct hci_conn *conn);
>  void hci_conn_hash_flush(struct hci_dev *hdev);
>  void hci_conn_check_pending(struct hci_dev *hdev);
>  
> +struct hci_chan *hci_chan_create(struct hci_conn *conn);
> +int hci_chan_del(struct hci_chan *chan);
> +void hci_chan_hash_flush(struct hci_conn *conn);
> +
>  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>  						__u8 sec_level, __u8 auth_type);
>  int hci_conn_check_link_mode(struct hci_conn *conn);
> @@ -839,6 +880,7 @@ int hci_unregister_notifier(struct notifier_block *nb);
>  
>  int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
>  void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags);
> +void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);

So we should have only one function to send data to HCI here, I propose keep
the hci_send_acl() name and just change its parameter to hci_chan.

>  void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
>  
>  void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index f018e5d..1e4dd6b 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -301,6 +301,7 @@ struct srej_list {
>  struct l2cap_chan {
>  	struct sock *sk;
>  
> +	struct hci_chan		*hchan;
>  	struct l2cap_conn	*conn;
>  
>  	__u8		state;
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index ea7f031..19ae6b4 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -374,6 +374,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
>  
>  	skb_queue_head_init(&conn->data_q);
>  
> +	hci_chan_hash_init(conn);
> +
>  	setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn);
>  	setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn);
>  	setup_timer(&conn->auto_accept_timer, hci_conn_auto_accept,
> @@ -432,6 +434,8 @@ int hci_conn_del(struct hci_conn *conn)
>  
>  	tasklet_disable(&hdev->tx_task);
>  
> +	hci_chan_hash_flush(conn);
> +
>  	hci_conn_hash_del(hdev, conn);
>  	if (hdev->notify)
>  		hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
> @@ -956,3 +960,58 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
>  
>  	return copy_to_user(arg, &req, sizeof(req)) ? -EFAULT : 0;
>  }
> +
> +struct hci_chan *hci_chan_create(struct hci_conn *conn)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +	struct hci_chan *chan;
> +
> +	BT_DBG("%s conn %p", hdev->name, conn);
> +
> +	chan = kzalloc(sizeof(struct hci_chan), GFP_ATOMIC);
> +	if (!chan)
> +		return NULL;
> +
> +	chan->conn = conn;
> +	skb_queue_head_init(&chan->data_q);
> +
> +	tasklet_disable(&hdev->tx_task);
> +	hci_chan_hash_add(conn, chan);
> +	tasklet_enable(&hdev->tx_task);
> +
> +	return chan;
> +}
> +
> +int hci_chan_del(struct hci_chan *chan)
> +{
> +	struct hci_conn *conn = chan->conn;
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	BT_DBG("%s conn %p chan %p", hdev->name, conn, chan);
> +
> +	tasklet_disable(&hdev->tx_task);
> +	hci_chan_hash_del(conn, chan);
> +	tasklet_enable(&hdev->tx_task);
> +
> +	skb_queue_purge(&chan->data_q);
> +	kfree(chan);
> +
> +	return 0;
> +}
> +
> +void hci_chan_hash_flush(struct hci_conn *conn)
> +{
> +	struct hci_chan_hash *h = &conn->chan_hash;
> +	struct list_head *p;
> +
> +	BT_DBG("conn %p", conn);
> +	p = h->list.next;
> +	while (p != &h->list) {
> +		struct hci_chan *chan;
> +
> +		chan = list_entry(p, struct hci_chan, list);

Use list_for_each_entry() here, instead while + list_entry


> +		p = p->next;
> +
> +		hci_chan_del(chan);
> +	}
> +}
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9574752..3a4c3a2 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1975,23 +1975,18 @@ static void hci_add_acl_hdr(struct sk_buff *skb, __u16 handle, __u16 flags)
>  	hdr->dlen   = cpu_to_le16(len);
>  }
>  
> -void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
> +static void hci_queue_acl(struct hci_conn *conn, struct sk_buff_head *queue,
> +				struct sk_buff *skb, __u16 flags)
>  {
>  	struct hci_dev *hdev = conn->hdev;
>  	struct sk_buff *list;
>  
> -	BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
> -
> -	skb->dev = (void *) hdev;
> -	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
> -	hci_add_acl_hdr(skb, conn->handle, flags);
> -
>  	list = skb_shinfo(skb)->frag_list;
>  	if (!list) {
>  		/* Non fragmented */
>  		BT_DBG("%s nonfrag skb %p len %d", hdev->name, skb, skb->len);
>  
> -		skb_queue_tail(&conn->data_q, skb);
> +		skb_queue_tail(queue, skb);
>  	} else {
>  		/* Fragmented */
>  		BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
> @@ -1999,9 +1994,9 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>  		skb_shinfo(skb)->frag_list = NULL;
>  
>  		/* Queue all fragments atomically */
> -		spin_lock_bh(&conn->data_q.lock);
> +		spin_lock_bh(&queue->lock);
>  
> -		__skb_queue_tail(&conn->data_q, skb);
> +		__skb_queue_tail(queue, skb);
>  
>  		flags &= ~ACL_START;
>  		flags |= ACL_CONT;
> @@ -2014,16 +2009,46 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>  
>  			BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
>  
> -			__skb_queue_tail(&conn->data_q, skb);
> +			__skb_queue_tail(queue, skb);
>  		} while (list);
>  
> -		spin_unlock_bh(&conn->data_q.lock);
> +		spin_unlock_bh(&queue->lock);
>  	}
> +}
> +
> +void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
> +{
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
> +
> +	skb->dev = (void *) hdev;
> +	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
> +	hci_add_acl_hdr(skb, conn->handle, flags);
> +
> +	hci_queue_acl(conn, &conn->data_q, skb, flags);
>  
>  	tasklet_schedule(&hdev->tx_task);
>  }
>  EXPORT_SYMBOL(hci_send_acl);
>  
> +void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags)
> +{
> +	struct hci_conn *conn = chan->conn;
> +	struct hci_dev *hdev = conn->hdev;
> +
> +	BT_DBG("%s chan %p flags 0x%x", hdev->name, chan, flags);
> +
> +	skb->dev = (void *) hdev;
> +	bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
> +	hci_add_acl_hdr(skb, conn->handle, flags);
> +
> +	hci_queue_acl(conn, &chan->data_q, skb, flags);
> +
> +	tasklet_schedule(&hdev->tx_task);
> +}
> +EXPORT_SYMBOL(hci_chan_send_acl);
> +
>  /* Send SCO data */
>  void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb)
>  {
> @@ -2127,11 +2152,95 @@ static inline void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
>  	}
>  }
>  
> +static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
> +						int *quote)
> +{
> +	struct hci_conn_hash *h = &hdev->conn_hash;
> +	struct hci_chan *chan = NULL;
> +	int num = 0, min = ~0, cur_prio = 0;
> +	struct list_head *p, *c;
> +	int cnt, q, conn_num = 0;
> +
> +	BT_DBG("%s", hdev->name);
> +
> +	list_for_each(p, &h->list) {
> +		struct hci_conn *conn;
> +		struct hci_chan_hash *ch;
> +
> +		conn = list_entry(p, struct hci_conn, list);
> +		ch = &conn->chan_hash;
> +
> +		if (conn->type != type)
> +			continue;
> +
> +		if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG)
> +			continue;
> +
> +		conn_num++;
> +
> +		list_for_each(c, &ch->list) {
> +			struct hci_chan *tmp;
> +			struct sk_buff *skb;
> +
> +			tmp = list_entry(c, struct hci_chan, list);
> +
> +			if (skb_queue_empty(&tmp->data_q))
> +				continue;
> +
> +			skb = skb_peek(&tmp->data_q);
> +			if (skb->priority < cur_prio)
> +				continue;
> +
> +			if (skb->priority > cur_prio) {
> +				num = 0;
> +				min = ~0;
> +				cur_prio = skb->priority;
> +			}
> +
> +			num++;
> +
> +			if (conn->sent < min) {
> +				min  = conn->sent;
> +				chan = tmp;
> +			}
> +		}
> +
> +		if (hci_conn_num(hdev, type) == conn_num)
> +			break;
> +	}
> +
> +	if (!chan)
> +		return NULL;
> +
> +	switch (chan->conn->type) {
> +	case ACL_LINK:
> +		cnt = hdev->acl_cnt;
> +		break;
> +	case SCO_LINK:
> +	case ESCO_LINK:
> +		cnt = hdev->sco_cnt;
> +		break;
> +	case LE_LINK:
> +		cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
> +		break;
> +	default:
> +		cnt = 0;
> +		BT_ERR("Unknown link type");
> +	}
> +
> +	q = cnt / num;
> +	*quote = q ? q : 1;
> +	BT_DBG("chan %p quote %d", chan, *quote);
> +	return chan;
> +}
> +
>  static inline void hci_sched_acl(struct hci_dev *hdev)
>  {
>  	struct hci_conn *conn;
> +	struct hci_chan *chan;
>  	struct sk_buff *skb;
>  	int quote;
> +	unsigned int cnt;
>  
>  	BT_DBG("%s", hdev->name);
>  
> @@ -2145,9 +2254,10 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>  			hci_link_tx_to(hdev, ACL_LINK);
>  	}
>  
> -	while (hdev->acl_cnt && (conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
> +	while (hdev->acl_cnt &&
> +			(conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
>  		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> -			BT_DBG("skb %p len %d", skb, skb->len);
> +			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>  
>  			hci_conn_enter_active_mode(conn, bt_cb(skb)->force_active);
>  
> @@ -2158,6 +2268,26 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>  			conn->sent++;
>  		}
>  	}
> +
> +	cnt = hdev->acl_cnt;
> +
> +	while (hdev->acl_cnt &&
> +			(chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
> +		while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
> +			BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
> +					skb->len, skb->priority);
> +
> +			hci_conn_enter_active_mode(chan->conn,
> +						bt_cb(skb)->force_active);
> +
> +			hci_send_frame(skb);
> +			hdev->acl_last_tx = jiffies;
> +
> +			hdev->acl_cnt--;
> +			chan->sent++;
> +			chan->conn->sent++;
> +		}
> +	}
>  }
>  
>  /* Schedule SCO */
> @@ -2174,7 +2304,7 @@ static inline void hci_sched_sco(struct hci_dev *hdev)
>  
>  	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, &quote))) {
>  		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> -			BT_DBG("skb %p len %d", skb, skb->len);
> +			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>  			hci_send_frame(skb);
>  
>  			conn->sent++;
> @@ -2197,7 +2327,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
>  
>  	while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, &quote))) {
>  		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> -			BT_DBG("skb %p len %d", skb, skb->len);
> +			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>  			hci_send_frame(skb);
>  
>  			conn->sent++;
> @@ -2210,6 +2340,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
>  static inline void hci_sched_le(struct hci_dev *hdev)
>  {
>  	struct hci_conn *conn;
> +	struct hci_chan *chan;
>  	struct sk_buff *skb;
>  	int quote, cnt;
>  
> @@ -2229,7 +2360,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
>  	cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
>  	while (cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
>  		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> -			BT_DBG("skb %p len %d", skb, skb->len);
> +			BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>  
>  			hci_send_frame(skb);
>  			hdev->le_last_tx = jiffies;
> @@ -2238,6 +2369,21 @@ static inline void hci_sched_le(struct hci_dev *hdev)
>  			conn->sent++;
>  		}
>  	}
> +
> +	while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, &quote))) {
> +		while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
> +			BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
> +					skb->len, skb->priority);
> +
> +			hci_send_frame(skb);
> +			hdev->le_last_tx = jiffies;
> +
> +			cnt--;
> +			chan->sent++;
> +			chan->conn->sent++;
> +		}
> +	}
> +

IMHO when le_pkts equals zero(i.e., ACL and LE use the same buffer) we should
handle LE sending together with the ACL sending. This way we are prioritizing
ACL data over LE if we call hci_sched_acl before hci_sched_le.

	Gustavo

Re: [RFC 1/5 v2] Bluetooth: make use of connection number to optimize the scheduler

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:00 +0300]:

> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> 
> This checks if there is any existing connection according to its type
> before start iterating in the list and immediately stop iterating when
> reaching the number of connections.
> 
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
>  include/net/bluetooth/hci_core.h |   16 ++++++++++++++++
>  net/bluetooth/hci_core.c         |   15 +++++++++++++++
>  2 files changed, 31 insertions(+), 0 deletions(-)

Applied, thanks.

	Gustavo

Re: [RFC 2/5 v2] Bluetooth: set skbuffer priority based on L2CAP socket priority

[Luiz Augusto von Dentz]

Hi Gustavo,

On Wed, Aug 24, 2011 at 10:37 PM, Gustavo Padovan
<padovan@profusion.mobi> wrote:
> Hi Luiz,
>
> * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:01 +0300]:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This uses SO_PRIORITY to set the skbuffer priority field
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> ---
>>  include/net/bluetooth/hci_core.h |    3 ++
>>  include/net/bluetooth/l2cap.h    |    3 +-
>>  net/bluetooth/l2cap_core.c       |   45 +++++++++++++++++++++++++++-----------
>>  net/bluetooth/l2cap_sock.c       |    2 +-
>>  4 files changed, 38 insertions(+), 15 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index e2ba4d6..0742828 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -32,6 +32,9 @@
>>  #define HCI_PROTO_L2CAP      0
>>  #define HCI_PROTO_SCO        1
>>
>> +/* HCI priority */
>> +#define HCI_PRIO_MAX 7
>> +
>>  /* HCI Core structures */
>>  struct inquiry_data {
>>       bdaddr_t        bdaddr;
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index 4f34ad2..f018e5d 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -511,7 +511,8 @@ 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);
>>  int l2cap_chan_connect(struct l2cap_chan *chan);
>> -int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len);
>> +int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
>> +                                                             u32 priority);
>
> I think priority could be part of struct l2cap_chan, so we avoid an extra
> parameter in some calls here. Priority is also accessible from chan->sk, but I
> would like to avoid more references to sk in l2cap core.

It would be great and simplify a lot the scheduler, but Im afraid have
to do it per skbuffer because of RFCOMM, besides it gives more
flexibility if we latter need to move to another queueing discipline.

>>  void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
>>
>>  #endif /* __L2CAP_H */
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 3204ba8..3af0569 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -1245,7 +1245,8 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
>>       struct hci_conn *hcon = chan->conn->hcon;
>>       u16 flags;
>>
>> -     BT_DBG("chan %p, skb %p len %d", chan, skb, skb->len);
>> +     BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
>> +                                                     skb->priority);
>>
>>       if (!chan->flushable && lmp_no_flush_capable(hcon->hdev))
>>               flags = ACL_START_NO_FLUSH;
>> @@ -1451,6 +1452,8 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>>               if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
>>                       return -EFAULT;
>>
>> +             (*frag)->priority = skb->priority;
>> +
>>               sent += count;
>>               len  -= count;
>>
>> @@ -1460,7 +1463,9 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
>>       return sent;
>>  }
>>
>> -struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
>> +static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
>> +                                             struct msghdr *msg, size_t len,
>> +                                             u32 priority)
>>  {
>>       struct sock *sk = chan->sk;
>>       struct l2cap_conn *conn = chan->conn;
>> @@ -1468,7 +1473,7 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>>       int err, count, hlen = L2CAP_HDR_SIZE + 2;
>>       struct l2cap_hdr *lh;
>>
>> -     BT_DBG("sk %p len %d", sk, (int)len);
>> +     BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
>>
>>       count = min_t(unsigned int, (conn->mtu - hlen), len);
>>       skb = bt_skb_send_alloc(sk, count + hlen,
>> @@ -1476,6 +1481,8 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>>       if (!skb)
>>               return ERR_PTR(err);
>>
>> +     skb->priority = priority;
>> +
>>       /* Create L2CAP header */
>>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>>       lh->cid = cpu_to_le16(chan->dcid);
>> @@ -1490,7 +1497,9 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
>>       return skb;
>>  }
>>
>> -struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
>> +static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
>> +                                             struct msghdr *msg, size_t len,
>> +                                             u32 priority)
>>  {
>>       struct sock *sk = chan->sk;
>>       struct l2cap_conn *conn = chan->conn;
>> @@ -1506,6 +1515,8 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
>>       if (!skb)
>>               return ERR_PTR(err);
>>
>> +     skb->priority = priority;
>> +
>>       /* Create L2CAP header */
>>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>>       lh->cid = cpu_to_le16(chan->dcid);
>> @@ -1519,7 +1530,10 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
>>       return skb;
>>  }
>>
>> -struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len, u16 control, u16 sdulen)
>> +static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
>> +                                             struct msghdr *msg, size_t len,
>> +                                             u32 priority, u16 control,
>> +                                             u16 sdulen)
>>  {
>>       struct sock *sk = chan->sk;
>>       struct l2cap_conn *conn = chan->conn;
>> @@ -1527,7 +1541,7 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
>>       int err, count, hlen = L2CAP_HDR_SIZE + 2;
>>       struct l2cap_hdr *lh;
>>
>> -     BT_DBG("sk %p len %d", sk, (int)len);
>> +     BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
>>
>>       if (!conn)
>>               return ERR_PTR(-ENOTCONN);
>> @@ -1544,6 +1558,8 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
>>       if (!skb)
>>               return ERR_PTR(err);
>>
>> +     skb->priority = priority;
>> +
>>       /* Create L2CAP header */
>>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
>>       lh->cid = cpu_to_le16(chan->dcid);
>> @@ -1574,7 +1590,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
>>
>>       skb_queue_head_init(&sar_queue);
>>       control = L2CAP_SDU_START;
>> -     skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps, control, len);
>> +     skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps,
>> +                                             HCI_PRIO_MAX, control, len);
>
>
> HCI_PRIO_MAX, why?

The idea is that everything that is time critical, has a timer/timeout
associated, should use HCI_PRIO_MAX, I thought this was the case here.

>
>>       if (IS_ERR(skb))
>>               return PTR_ERR(skb);
>>
>> @@ -1593,7 +1610,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
>>                       buflen = len;
>>               }
>>
>> -             skb = l2cap_create_iframe_pdu(chan, msg, buflen, control, 0);
>> +             skb = l2cap_create_iframe_pdu(chan, msg, buflen, HCI_PRIO_MAX,
>> +                                                             control, 0);
>
> same here.

Here too.

-- 
Luiz Augusto von Dentz

Re: [RFC 4/5 v2] Bluetooth: prioritizing data over HCI

[Luiz Augusto von Dentz]

Hi Gustavo,

On Wed, Aug 24, 2011 at 11:04 PM, Gustavo Padovan
<padovan@profusion.mobi> wrote:
> Hi Luiz,
>
> * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:03 +0300]:
>
>> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>>
>> This implement priority based scheduler using skbuffer priority set via
>> SO_PRIORITY socket option.
>>
>> It introduces hci_chan_hash (list of HCI Channel/hci_chan) per connection,
>> each item in this list refer to a L2CAP connection and it is used to
>> queue the data for transmission.
>>
>> Each connection continue to have a queue but it is only used for time
>> critical packets such the L2CAP commands and it is always processed
>> before the channels thus it can be considered the highest priority.
>
> I think we can drop the connection and queue create a channel by default for
> the special L2CAP channels, BR/EDR and LE signalling, SMP and AMP Manager.
> Those will have high priority of course. Standardize this in just one way to
> send packets would be better. It simplifies the sending logic and reduces the
> code.

Sure, but then we need something to identify this special hci_chan and
it needs to be added to the list to be processed together, not sure if
it will simplify that much in the end.

>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> ---
>>  include/net/bluetooth/hci_core.h |   42 +++++++++
>>  include/net/bluetooth/l2cap.h    |    1 +
>>  net/bluetooth/hci_conn.c         |   59 ++++++++++++
>>  net/bluetooth/hci_core.c         |  180 ++++++++++++++++++++++++++++++++++----
>>  net/bluetooth/l2cap_core.c       |    5 +-
>>  5 files changed, 269 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index 0742828..e0d29eb 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -67,6 +67,12 @@ struct hci_conn_hash {
>>       unsigned int     le_num;
>>  };
>>
>> +struct hci_chan_hash {
>> +     struct list_head list;
>> +     spinlock_t       lock;
>> +     unsigned int     num;
>> +};
>> +
>>  struct bdaddr_list {
>>       struct list_head list;
>>       bdaddr_t bdaddr;
>> @@ -278,6 +284,7 @@ struct hci_conn {
>>       unsigned int    sent;
>>
>>       struct sk_buff_head data_q;
>> +     struct hci_chan_hash chan_hash;
>>
>>       struct timer_list disc_timer;
>>       struct timer_list idle_timer;
>> @@ -300,6 +307,14 @@ struct hci_conn {
>>       void (*disconn_cfm_cb)  (struct hci_conn *conn, u8 reason);
>>  };
>>
>> +struct hci_chan {
>> +     struct list_head list;
>> +
>> +     struct hci_conn *conn;
>> +     struct sk_buff_head data_q;
>> +     unsigned int    sent;
>> +};
>> +
>>  extern struct hci_proto *hci_proto[];
>>  extern struct list_head hci_dev_list;
>>  extern struct list_head hci_cb_list;
>> @@ -459,6 +474,28 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev,
>>       return NULL;
>>  }
>>
>> +static inline void hci_chan_hash_init(struct hci_conn *c)
>> +{
>> +     struct hci_chan_hash *h = &c->chan_hash;
>> +     INIT_LIST_HEAD(&h->list);
>> +     spin_lock_init(&h->lock);
>> +     h->num = 0;
>> +}
>> +
>> +static inline void hci_chan_hash_add(struct hci_conn *c, struct hci_chan *chan)
>> +{
>> +     struct hci_chan_hash *h = &c->chan_hash;
>> +     list_add(&chan->list, &h->list);
>> +     h->num++;
>> +}
>> +
>> +static inline void hci_chan_hash_del(struct hci_conn *c, struct hci_chan *chan)
>> +{
>> +     struct hci_chan_hash *h = &c->chan_hash;
>> +     list_del(&chan->list);
>> +     h->num--;
>> +}
>> +
>>  void hci_acl_connect(struct hci_conn *conn);
>>  void hci_acl_disconn(struct hci_conn *conn, __u8 reason);
>>  void hci_add_sco(struct hci_conn *conn, __u16 handle);
>> @@ -470,6 +507,10 @@ int hci_conn_del(struct hci_conn *conn);
>>  void hci_conn_hash_flush(struct hci_dev *hdev);
>>  void hci_conn_check_pending(struct hci_dev *hdev);
>>
>> +struct hci_chan *hci_chan_create(struct hci_conn *conn);
>> +int hci_chan_del(struct hci_chan *chan);
>> +void hci_chan_hash_flush(struct hci_conn *conn);
>> +
>>  struct hci_conn *hci_connect(struct hci_dev *hdev, int type, bdaddr_t *dst,
>>                                               __u8 sec_level, __u8 auth_type);
>>  int hci_conn_check_link_mode(struct hci_conn *conn);
>> @@ -839,6 +880,7 @@ int hci_unregister_notifier(struct notifier_block *nb);
>>
>>  int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param);
>>  void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags);
>> +void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags);
>
> So we should have only one function to send data to HCI here, I propose keep
> the hci_send_acl() name and just change its parameter to hci_chan.
>
>>  void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb);
>>
>>  void *hci_sent_cmd_data(struct hci_dev *hdev, __u16 opcode);
>> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
>> index f018e5d..1e4dd6b 100644
>> --- a/include/net/bluetooth/l2cap.h
>> +++ b/include/net/bluetooth/l2cap.h
>> @@ -301,6 +301,7 @@ struct srej_list {
>>  struct l2cap_chan {
>>       struct sock *sk;
>>
>> +     struct hci_chan         *hchan;
>>       struct l2cap_conn       *conn;
>>
>>       __u8            state;
>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>> index ea7f031..19ae6b4 100644
>> --- a/net/bluetooth/hci_conn.c
>> +++ b/net/bluetooth/hci_conn.c
>> @@ -374,6 +374,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
>>
>>       skb_queue_head_init(&conn->data_q);
>>
>> +     hci_chan_hash_init(conn);
>> +
>>       setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn);
>>       setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn);
>>       setup_timer(&conn->auto_accept_timer, hci_conn_auto_accept,
>> @@ -432,6 +434,8 @@ int hci_conn_del(struct hci_conn *conn)
>>
>>       tasklet_disable(&hdev->tx_task);
>>
>> +     hci_chan_hash_flush(conn);
>> +
>>       hci_conn_hash_del(hdev, conn);
>>       if (hdev->notify)
>>               hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
>> @@ -956,3 +960,58 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg)
>>
>>       return copy_to_user(arg, &req, sizeof(req)) ? -EFAULT : 0;
>>  }
>> +
>> +struct hci_chan *hci_chan_create(struct hci_conn *conn)
>> +{
>> +     struct hci_dev *hdev = conn->hdev;
>> +     struct hci_chan *chan;
>> +
>> +     BT_DBG("%s conn %p", hdev->name, conn);
>> +
>> +     chan = kzalloc(sizeof(struct hci_chan), GFP_ATOMIC);
>> +     if (!chan)
>> +             return NULL;
>> +
>> +     chan->conn = conn;
>> +     skb_queue_head_init(&chan->data_q);
>> +
>> +     tasklet_disable(&hdev->tx_task);
>> +     hci_chan_hash_add(conn, chan);
>> +     tasklet_enable(&hdev->tx_task);
>> +
>> +     return chan;
>> +}
>> +
>> +int hci_chan_del(struct hci_chan *chan)
>> +{
>> +     struct hci_conn *conn = chan->conn;
>> +     struct hci_dev *hdev = conn->hdev;
>> +
>> +     BT_DBG("%s conn %p chan %p", hdev->name, conn, chan);
>> +
>> +     tasklet_disable(&hdev->tx_task);
>> +     hci_chan_hash_del(conn, chan);
>> +     tasklet_enable(&hdev->tx_task);
>> +
>> +     skb_queue_purge(&chan->data_q);
>> +     kfree(chan);
>> +
>> +     return 0;
>> +}
>> +
>> +void hci_chan_hash_flush(struct hci_conn *conn)
>> +{
>> +     struct hci_chan_hash *h = &conn->chan_hash;
>> +     struct list_head *p;
>> +
>> +     BT_DBG("conn %p", conn);
>> +     p = h->list.next;
>> +     while (p != &h->list) {
>> +             struct hci_chan *chan;
>> +
>> +             chan = list_entry(p, struct hci_chan, list);
>
> Use list_for_each_entry() here, instead while + list_entry

Sure.

>> +             p = p->next;
>> +
>> +             hci_chan_del(chan);
>> +     }
>> +}
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 9574752..3a4c3a2 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -1975,23 +1975,18 @@ static void hci_add_acl_hdr(struct sk_buff *skb, __u16 handle, __u16 flags)
>>       hdr->dlen   = cpu_to_le16(len);
>>  }
>>
>> -void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>> +static void hci_queue_acl(struct hci_conn *conn, struct sk_buff_head *queue,
>> +                             struct sk_buff *skb, __u16 flags)
>>  {
>>       struct hci_dev *hdev = conn->hdev;
>>       struct sk_buff *list;
>>
>> -     BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
>> -
>> -     skb->dev = (void *) hdev;
>> -     bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
>> -     hci_add_acl_hdr(skb, conn->handle, flags);
>> -
>>       list = skb_shinfo(skb)->frag_list;
>>       if (!list) {
>>               /* Non fragmented */
>>               BT_DBG("%s nonfrag skb %p len %d", hdev->name, skb, skb->len);
>>
>> -             skb_queue_tail(&conn->data_q, skb);
>> +             skb_queue_tail(queue, skb);
>>       } else {
>>               /* Fragmented */
>>               BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
>> @@ -1999,9 +1994,9 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>>               skb_shinfo(skb)->frag_list = NULL;
>>
>>               /* Queue all fragments atomically */
>> -             spin_lock_bh(&conn->data_q.lock);
>> +             spin_lock_bh(&queue->lock);
>>
>> -             __skb_queue_tail(&conn->data_q, skb);
>> +             __skb_queue_tail(queue, skb);
>>
>>               flags &= ~ACL_START;
>>               flags |= ACL_CONT;
>> @@ -2014,16 +2009,46 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>>
>>                       BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
>>
>> -                     __skb_queue_tail(&conn->data_q, skb);
>> +                     __skb_queue_tail(queue, skb);
>>               } while (list);
>>
>> -             spin_unlock_bh(&conn->data_q.lock);
>> +             spin_unlock_bh(&queue->lock);
>>       }
>> +}
>> +
>> +void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
>> +{
>> +     struct hci_dev *hdev = conn->hdev;
>> +
>> +     BT_DBG("%s conn %p flags 0x%x", hdev->name, conn, flags);
>> +
>> +     skb->dev = (void *) hdev;
>> +     bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
>> +     hci_add_acl_hdr(skb, conn->handle, flags);
>> +
>> +     hci_queue_acl(conn, &conn->data_q, skb, flags);
>>
>>       tasklet_schedule(&hdev->tx_task);
>>  }
>>  EXPORT_SYMBOL(hci_send_acl);
>>
>> +void hci_chan_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags)
>> +{
>> +     struct hci_conn *conn = chan->conn;
>> +     struct hci_dev *hdev = conn->hdev;
>> +
>> +     BT_DBG("%s chan %p flags 0x%x", hdev->name, chan, flags);
>> +
>> +     skb->dev = (void *) hdev;
>> +     bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
>> +     hci_add_acl_hdr(skb, conn->handle, flags);
>> +
>> +     hci_queue_acl(conn, &chan->data_q, skb, flags);
>> +
>> +     tasklet_schedule(&hdev->tx_task);
>> +}
>> +EXPORT_SYMBOL(hci_chan_send_acl);
>> +
>>  /* Send SCO data */
>>  void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb)
>>  {
>> @@ -2127,11 +2152,95 @@ static inline void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
>>       }
>>  }
>>
>> +static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type,
>> +                                             int *quote)
>> +{
>> +     struct hci_conn_hash *h = &hdev->conn_hash;
>> +     struct hci_chan *chan = NULL;
>> +     int num = 0, min = ~0, cur_prio = 0;
>> +     struct list_head *p, *c;
>> +     int cnt, q, conn_num = 0;
>> +
>> +     BT_DBG("%s", hdev->name);
>> +
>> +     list_for_each(p, &h->list) {
>> +             struct hci_conn *conn;
>> +             struct hci_chan_hash *ch;
>> +
>> +             conn = list_entry(p, struct hci_conn, list);
>> +             ch = &conn->chan_hash;
>> +
>> +             if (conn->type != type)
>> +                     continue;
>> +
>> +             if (conn->state != BT_CONNECTED && conn->state != BT_CONFIG)
>> +                     continue;
>> +
>> +             conn_num++;
>> +
>> +             list_for_each(c, &ch->list) {
>> +                     struct hci_chan *tmp;
>> +                     struct sk_buff *skb;
>> +
>> +                     tmp = list_entry(c, struct hci_chan, list);
>> +
>> +                     if (skb_queue_empty(&tmp->data_q))
>> +                             continue;
>> +
>> +                     skb = skb_peek(&tmp->data_q);
>> +                     if (skb->priority < cur_prio)
>> +                             continue;
>> +
>> +                     if (skb->priority > cur_prio) {
>> +                             num = 0;
>> +                             min = ~0;
>> +                             cur_prio = skb->priority;
>> +                     }
>> +
>> +                     num++;
>> +
>> +                     if (conn->sent < min) {
>> +                             min  = conn->sent;
>> +                             chan = tmp;
>> +                     }
>> +             }
>> +
>> +             if (hci_conn_num(hdev, type) == conn_num)
>> +                     break;
>> +     }
>> +
>> +     if (!chan)
>> +             return NULL;
>> +
>> +     switch (chan->conn->type) {
>> +     case ACL_LINK:
>> +             cnt = hdev->acl_cnt;
>> +             break;
>> +     case SCO_LINK:
>> +     case ESCO_LINK:
>> +             cnt = hdev->sco_cnt;
>> +             break;
>> +     case LE_LINK:
>> +             cnt = hdev->le_mtu ? hdev->le_cnt : hdev->acl_cnt;
>> +             break;
>> +     default:
>> +             cnt = 0;
>> +             BT_ERR("Unknown link type");
>> +     }
>> +
>> +     q = cnt / num;
>> +     *quote = q ? q : 1;
>> +     BT_DBG("chan %p quote %d", chan, *quote);
>> +     return chan;
>> +}
>> +
>>  static inline void hci_sched_acl(struct hci_dev *hdev)
>>  {
>>       struct hci_conn *conn;
>> +     struct hci_chan *chan;
>>       struct sk_buff *skb;
>>       int quote;
>> +     unsigned int cnt;
>>
>>       BT_DBG("%s", hdev->name);
>>
>> @@ -2145,9 +2254,10 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>>                       hci_link_tx_to(hdev, ACL_LINK);
>>       }
>>
>> -     while (hdev->acl_cnt && (conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
>> +     while (hdev->acl_cnt &&
>> +                     (conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
>>               while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
>> -                     BT_DBG("skb %p len %d", skb, skb->len);
>> +                     BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>>
>>                       hci_conn_enter_active_mode(conn, bt_cb(skb)->force_active);
>>
>> @@ -2158,6 +2268,26 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
>>                       conn->sent++;
>>               }
>>       }
>> +
>> +     cnt = hdev->acl_cnt;
>> +
>> +     while (hdev->acl_cnt &&
>> +                     (chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
>> +             while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
>> +                     BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
>> +                                     skb->len, skb->priority);
>> +
>> +                     hci_conn_enter_active_mode(chan->conn,
>> +                                             bt_cb(skb)->force_active);
>> +
>> +                     hci_send_frame(skb);
>> +                     hdev->acl_last_tx = jiffies;
>> +
>> +                     hdev->acl_cnt--;
>> +                     chan->sent++;
>> +                     chan->conn->sent++;
>> +             }
>> +     }
>>  }
>>
>>  /* Schedule SCO */
>> @@ -2174,7 +2304,7 @@ static inline void hci_sched_sco(struct hci_dev *hdev)
>>
>>       while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, &quote))) {
>>               while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
>> -                     BT_DBG("skb %p len %d", skb, skb->len);
>> +                     BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>>                       hci_send_frame(skb);
>>
>>                       conn->sent++;
>> @@ -2197,7 +2327,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
>>
>>       while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, &quote))) {
>>               while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
>> -                     BT_DBG("skb %p len %d", skb, skb->len);
>> +                     BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>>                       hci_send_frame(skb);
>>
>>                       conn->sent++;
>> @@ -2210,6 +2340,7 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
>>  static inline void hci_sched_le(struct hci_dev *hdev)
>>  {
>>       struct hci_conn *conn;
>> +     struct hci_chan *chan;
>>       struct sk_buff *skb;
>>       int quote, cnt;
>>
>> @@ -2229,7 +2360,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
>>       cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
>>       while (cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
>>               while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
>> -                     BT_DBG("skb %p len %d", skb, skb->len);
>> +                     BT_DBG("conn %p skb %p len %d", conn, skb, skb->len);
>>
>>                       hci_send_frame(skb);
>>                       hdev->le_last_tx = jiffies;
>> @@ -2238,6 +2369,21 @@ static inline void hci_sched_le(struct hci_dev *hdev)
>>                       conn->sent++;
>>               }
>>       }
>> +
>> +     while (cnt && (chan = hci_chan_sent(hdev, LE_LINK, &quote))) {
>> +             while (quote-- && (skb = skb_dequeue(&chan->data_q))) {
>> +                     BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
>> +                                     skb->len, skb->priority);
>> +
>> +                     hci_send_frame(skb);
>> +                     hdev->le_last_tx = jiffies;
>> +
>> +                     cnt--;
>> +                     chan->sent++;
>> +                     chan->conn->sent++;
>> +             }
>> +     }
>> +
>
> IMHO when le_pkts equals zero(i.e., ACL and LE use the same buffer) we should
> handle LE sending together with the ACL sending. This way we are prioritizing
> ACL data over LE if we call hci_sched_acl before hci_sched_le.

iirc Peter has some patch that merges them, the problem is that
conn->type cannot be set to ACL since it is used for other purposes,
but perhaps we gonna have a different type to identify the buffer type
the connection is using.


-- 
Luiz Augusto von Dentz

Re: [RFC 2/5 v2] Bluetooth: set skbuffer priority based on L2CAP socket priority

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-25 00:27:31 +0300]:

> Hi Gustavo,
> 
> On Wed, Aug 24, 2011 at 10:37 PM, Gustavo Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Luiz,
> >
> > * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:01 +0300]:
> >
> >> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>
> >> This uses SO_PRIORITY to set the skbuffer priority field
> >>
> >> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >> ---
> >>  include/net/bluetooth/hci_core.h |    3 ++
> >>  include/net/bluetooth/l2cap.h    |    3 +-
> >>  net/bluetooth/l2cap_core.c       |   45 +++++++++++++++++++++++++++-----------
> >>  net/bluetooth/l2cap_sock.c       |    2 +-
> >>  4 files changed, 38 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >> index e2ba4d6..0742828 100644
> >> --- a/include/net/bluetooth/hci_core.h
> >> +++ b/include/net/bluetooth/hci_core.h
> >> @@ -32,6 +32,9 @@
> >>  #define HCI_PROTO_L2CAP      0
> >>  #define HCI_PROTO_SCO        1
> >>
> >> +/* HCI priority */
> >> +#define HCI_PRIO_MAX 7
> >> +
> >>  /* HCI Core structures */
> >>  struct inquiry_data {
> >>       bdaddr_t        bdaddr;
> >> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> >> index 4f34ad2..f018e5d 100644
> >> --- a/include/net/bluetooth/l2cap.h
> >> +++ b/include/net/bluetooth/l2cap.h
> >> @@ -511,7 +511,8 @@ 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);
> >>  int l2cap_chan_connect(struct l2cap_chan *chan);
> >> -int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len);
> >> +int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len,
> >> +                                                             u32 priority);
> >
> > I think priority could be part of struct l2cap_chan, so we avoid an extra
> > parameter in some calls here. Priority is also accessible from chan->sk, but I
> > would like to avoid more references to sk in l2cap core.
> 
> It would be great and simplify a lot the scheduler, but Im afraid have
> to do it per skbuffer because of RFCOMM, besides it gives more
> flexibility if we latter need to move to another queueing discipline.

It looks we still can do that without adding a parameter to l2cap_chan_send(),
if needed we can step back on what I said before and use sk->priority inside
l2cap_core.c.

Actually we need to fix the mess that the interaction between L2CAP and
RFCOMM is, after that we will be able to handle rfcomm channels in the same
way we are going to do for ERTM channels.

> 
> >>  void l2cap_chan_busy(struct l2cap_chan *chan, int busy);
> >>
> >>  #endif /* __L2CAP_H */
> >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> >> index 3204ba8..3af0569 100644
> >> --- a/net/bluetooth/l2cap_core.c
> >> +++ b/net/bluetooth/l2cap_core.c
> >> @@ -1245,7 +1245,8 @@ void l2cap_do_send(struct l2cap_chan *chan, struct sk_buff *skb)
> >>       struct hci_conn *hcon = chan->conn->hcon;
> >>       u16 flags;
> >>
> >> -     BT_DBG("chan %p, skb %p len %d", chan, skb, skb->len);
> >> +     BT_DBG("chan %p, skb %p len %d priority %u", chan, skb, skb->len,
> >> +                                                     skb->priority);
> >>
> >>       if (!chan->flushable && lmp_no_flush_capable(hcon->hdev))
> >>               flags = ACL_START_NO_FLUSH;
> >> @@ -1451,6 +1452,8 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
> >>               if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count))
> >>                       return -EFAULT;
> >>
> >> +             (*frag)->priority = skb->priority;
> >> +
> >>               sent += count;
> >>               len  -= count;
> >>
> >> @@ -1460,7 +1463,9 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in
> >>       return sent;
> >>  }
> >>
> >> -struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
> >> +static struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan,
> >> +                                             struct msghdr *msg, size_t len,
> >> +                                             u32 priority)
> >>  {
> >>       struct sock *sk = chan->sk;
> >>       struct l2cap_conn *conn = chan->conn;
> >> @@ -1468,7 +1473,7 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
> >>       int err, count, hlen = L2CAP_HDR_SIZE + 2;
> >>       struct l2cap_hdr *lh;
> >>
> >> -     BT_DBG("sk %p len %d", sk, (int)len);
> >> +     BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
> >>
> >>       count = min_t(unsigned int, (conn->mtu - hlen), len);
> >>       skb = bt_skb_send_alloc(sk, count + hlen,
> >> @@ -1476,6 +1481,8 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
> >>       if (!skb)
> >>               return ERR_PTR(err);
> >>
> >> +     skb->priority = priority;
> >> +
> >>       /* Create L2CAP header */
> >>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> >>       lh->cid = cpu_to_le16(chan->dcid);
> >> @@ -1490,7 +1497,9 @@ struct sk_buff *l2cap_create_connless_pdu(struct l2cap_chan *chan, struct msghdr
> >>       return skb;
> >>  }
> >>
> >> -struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len)
> >> +static struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan,
> >> +                                             struct msghdr *msg, size_t len,
> >> +                                             u32 priority)
> >>  {
> >>       struct sock *sk = chan->sk;
> >>       struct l2cap_conn *conn = chan->conn;
> >> @@ -1506,6 +1515,8 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
> >>       if (!skb)
> >>               return ERR_PTR(err);
> >>
> >> +     skb->priority = priority;
> >> +
> >>       /* Create L2CAP header */
> >>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> >>       lh->cid = cpu_to_le16(chan->dcid);
> >> @@ -1519,7 +1530,10 @@ struct sk_buff *l2cap_create_basic_pdu(struct l2cap_chan *chan, struct msghdr *m
> >>       return skb;
> >>  }
> >>
> >> -struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *msg, size_t len, u16 control, u16 sdulen)
> >> +static struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan,
> >> +                                             struct msghdr *msg, size_t len,
> >> +                                             u32 priority, u16 control,
> >> +                                             u16 sdulen)
> >>  {
> >>       struct sock *sk = chan->sk;
> >>       struct l2cap_conn *conn = chan->conn;
> >> @@ -1527,7 +1541,7 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
> >>       int err, count, hlen = L2CAP_HDR_SIZE + 2;
> >>       struct l2cap_hdr *lh;
> >>
> >> -     BT_DBG("sk %p len %d", sk, (int)len);
> >> +     BT_DBG("sk %p len %d priority %u", sk, (int)len, priority);
> >>
> >>       if (!conn)
> >>               return ERR_PTR(-ENOTCONN);
> >> @@ -1544,6 +1558,8 @@ struct sk_buff *l2cap_create_iframe_pdu(struct l2cap_chan *chan, struct msghdr *
> >>       if (!skb)
> >>               return ERR_PTR(err);
> >>
> >> +     skb->priority = priority;
> >> +
> >>       /* Create L2CAP header */
> >>       lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
> >>       lh->cid = cpu_to_le16(chan->dcid);
> >> @@ -1574,7 +1590,8 @@ int l2cap_sar_segment_sdu(struct l2cap_chan *chan, struct msghdr *msg, size_t le
> >>
> >>       skb_queue_head_init(&sar_queue);
> >>       control = L2CAP_SDU_START;
> >> -     skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps, control, len);
> >> +     skb = l2cap_create_iframe_pdu(chan, msg, chan->remote_mps,
> >> +                                             HCI_PRIO_MAX, control, len);
> >
> >
> > HCI_PRIO_MAX, why?
> 
> The idea is that everything that is time critical, has a timer/timeout
> associated, should use HCI_PRIO_MAX, I thought this was the case here.

I got that, but this is not the case here. These a re normal ERTM/Streaming
packets ;)

	Gustavo

Re: [RFC 4/5 v2] Bluetooth: prioritizing data over HCI

[Gustavo Padovan]

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-25 00:53:04 +0300]:

> Hi Gustavo,
> 
> On Wed, Aug 24, 2011 at 11:04 PM, Gustavo Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Luiz,
> >
> > * Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-17 16:23:03 +0300]:
> >
> >> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >>
> >> This implement priority based scheduler using skbuffer priority set via
> >> SO_PRIORITY socket option.
> >>
> >> It introduces hci_chan_hash (list of HCI Channel/hci_chan) per connection,
> >> each item in this list refer to a L2CAP connection and it is used to
> >> queue the data for transmission.
> >>
> >> Each connection continue to have a queue but it is only used for time
> >> critical packets such the L2CAP commands and it is always processed
> >> before the channels thus it can be considered the highest priority.
> >
> > I think we can drop the connection and queue create a channel by default for
> > the special L2CAP channels, BR/EDR and LE signalling, SMP and AMP Manager.
> > Those will have high priority of course. Standardize this in just one way to
> > send packets would be better. It simplifies the sending logic and reduces the
> > code.
> 
> Sure, but then we need something to identify this special hci_chan and
> it needs to be added to the list to be processed together, not sure if
> it will simplify that much in the end.

It could be created and added to the connection hash when we create the
l2cap_conn and then have a l2cap_conn->hchan for it. IMHO this simplifies the
scheduler and the logic.

<...>

> >
> > IMHO when le_pkts equals zero(i.e., ACL and LE use the same buffer) we should
> > handle LE sending together with the ACL sending. This way we are prioritizing
> > ACL data over LE if we call hci_sched_acl before hci_sched_le.
> 
> iirc Peter has some patch that merges them, the problem is that
> conn->type cannot be set to ACL since it is used for other purposes,
> but perhaps we gonna have a different type to identify the buffer type
> the connection is using.

Yeah, I've read it just after reply to this email. ;)

	Gustavo