2011-08-18 00:37:37

by Peter Hurley

[permalink] [raw]
Subject: [RFC v2 3/5] Bluetooth: Add buffer count to tx scheduler parameters

VXRpbGl6ZSBhbHJlYWR5IGtub3duIHR4IGJ1ZmZlciBjb3VudCB2YWx1ZXMgYXMgcGFyYW1ldGVy
cyB0bw0KdGhlIHR4IHNjaGVkdWxlciwgcmF0aGVyIHRoYW4gZGV0ZXJtaW5pbmcgdGhlIHZhbHVl
cyBvbiBlYWNoDQppdGVyYXRpb24gb2YgdGhlIHR4IHNjaGVkdWxlci4NCg0KVGhpcyBpcyBwcmVw
YXJhdG9yeSBmb3IgbWVyZ2luZyB0cmFuc21pc3Npb24gdHlwZXMgKHN1Y2ggYXMgU0NPDQphbmQg
RVNDTykgYW5kIGFsc28gZm9yIGhhbmRsaW5nIHR4IGJ1ZmZlciBjb3VudGVycyBhdG9taWNhbGx5
Lg0KDQpTaWduZWQtb2ZmLWJ5OiBQZXRlciBIdXJsZXkgPHBldGVyQGh1cmxleXNvZnR3YXJlLmNv
bT4NCi0tLQ0KIG5ldC9ibHVldG9vdGgvaGNpX2NvcmUuYyB8ICAgNTEgKysrKysrKysrKysrKysr
Ky0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tDQogMSBmaWxlcyBjaGFuZ2VkLCAxOCBpbnNl
cnRpb25zKCspLCAzMyBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL25ldC9ibHVldG9vdGgv
aGNpX2NvcmUuYyBiL25ldC9ibHVldG9vdGgvaGNpX2NvcmUuYw0KaW5kZXggMGRlZmE4My4uYjJh
MWI5YSAxMDA2NDQNCi0tLSBhL25ldC9ibHVldG9vdGgvaGNpX2NvcmUuYw0KKysrIGIvbmV0L2Js
dWV0b290aC9oY2lfY29yZS5jDQpAQCAtMTc5OCw3ICsxNzk4LDggQEAgRVhQT1JUX1NZTUJPTCho
Y2lfc2VuZF9zY28pOw0KIC8qIC0tLS0gSENJIFRYIHRhc2sgKG91dGdvaW5nIGRhdGEpIC0tLS0g
Ki8NCiANCiAvKiBIQ0kgQ29ubmVjdGlvbiBzY2hlZHVsZXIgKi8NCi1zdGF0aWMgaW5saW5lIHN0
cnVjdCBoY2lfY29ubiAqaGNpX2xvd19zZW50KHN0cnVjdCBoY2lfZGV2ICpoZGV2LCBfX3U4IHR5
cGUsIGludCAqcXVvdGUpDQorc3RhdGljIGlubGluZSBzdHJ1Y3QgaGNpX2Nvbm4gKmhjaV9sb3df
c2VudChzdHJ1Y3QgaGNpX2RldiAqaGRldiwgX191OCB0eXBlLA0KKwkJCQkJCQlpbnQgY250LCBp
bnQgKnF1b3RlKQ0KIHsNCiAJc3RydWN0IGhjaV9jb25uX2hhc2ggKmggPSAmaGRldi0+Y29ubl9o
YXNoOw0KIAlzdHJ1Y3QgaGNpX2Nvbm4gKmNvbm4gPSBOVUxMOw0KQEAgLTE4MjUsMjggKzE4MjYs
OCBAQCBzdGF0aWMgaW5saW5lIHN0cnVjdCBoY2lfY29ubiAqaGNpX2xvd19zZW50KHN0cnVjdCBo
Y2lfZGV2ICpoZGV2LCBfX3U4IHR5cGUsIGludA0KIAkJfQ0KIAl9DQogDQotCWlmIChjb25uKSB7
DQotCQlpbnQgY250Ow0KLQ0KLQkJc3dpdGNoIChjb25uLT50eXBlKSB7DQotCQljYXNlIEFDTF9M
SU5LOg0KLQkJCWNudCA9IGhkZXYtPmFjbF9jbnQ7DQotCQkJYnJlYWs7DQotCQljYXNlIFNDT19M
SU5LOg0KLQkJY2FzZSBFU0NPX0xJTks6DQotCQkJY250ID0gaGRldi0+c2NvX2NudDsNCi0JCQli
cmVhazsNCi0JCWNhc2UgTEVfTElOSzoNCi0JCQljbnQgPSBoZGV2LT5sZV9tdHUgPyBoZGV2LT5s
ZV9jbnQgOiBoZGV2LT5hY2xfY250Ow0KLQkJCWJyZWFrOw0KLQkJZGVmYXVsdDoNCi0JCQljbnQg
PSAwOw0KLQkJCUJUX0VSUigiVW5rbm93biBsaW5rIHR5cGUiKTsNCi0JCX0NCi0NCisJaWYgKGNv
bm4pDQogCQkqcXVvdGUgPSAoY250ICsgKG51bSAtIDEpKSAvIG51bTsNCi0JfSBlbHNlDQotCQkq
cXVvdGUgPSAwOw0KIA0KIAlCVF9EQkcoImNvbm4gJXAgcXVvdGUgJWQiLCBjb25uLCAqcXVvdGUp
Ow0KIAlyZXR1cm4gY29ubjsNCkBAIC0xODc1LDE4ICsxODU2LDE5IEBAIHN0YXRpYyBpbmxpbmUg
dm9pZCBoY2lfc2NoZWRfYWNsKHN0cnVjdCBoY2lfZGV2ICpoZGV2KQ0KIHsNCiAJc3RydWN0IGhj
aV9jb25uICpjb25uOw0KIAlzdHJ1Y3Qgc2tfYnVmZiAqc2tiOw0KLQlpbnQgcXVvdGU7DQorCWlu
dCBxdW90ZSA9IDA7DQorCWludCBjbnQgPSBoZGV2LT5hY2xfY250Ow0KIA0KIAlCVF9EQkcoIiVz
IiwgaGRldi0+bmFtZSk7DQogDQogCWlmICghdGVzdF9iaXQoSENJX1JBVywgJmhkZXYtPmZsYWdz
KSkgew0KIAkJLyogQUNMIHR4IHRpbWVvdXQgbXVzdCBiZSBsb25nZXIgdGhhbiBtYXhpbXVtDQog
CQkgKiBsaW5rIHN1cGVydmlzaW9uIHRpbWVvdXQgKDQwLjkgc2Vjb25kcykgKi8NCi0JCWlmICgh
aGRldi0+YWNsX2NudCAmJiB0aW1lX2FmdGVyKGppZmZpZXMsIGhkZXYtPmFjbF9sYXN0X3R4ICsg
SFogKiA0NSkpDQorCQlpZiAoIWNudCAmJiB0aW1lX2FmdGVyKGppZmZpZXMsIGhkZXYtPmFjbF9s
YXN0X3R4ICsgSFogKiA0NSkpDQogCQkJaGNpX2xpbmtfdHhfdG8oaGRldiwgQUNMX0xJTkspOw0K
IAl9DQogDQotCXdoaWxlIChoZGV2LT5hY2xfY250ICYmIChjb25uID0gaGNpX2xvd19zZW50KGhk
ZXYsIEFDTF9MSU5LLCAmcXVvdGUpKSkgew0KKwl3aGlsZSAoY250ICYmIChjb25uID0gaGNpX2xv
d19zZW50KGhkZXYsIEFDTF9MSU5LLCBjbnQsICZxdW90ZSkpKSB7DQogCQl3aGlsZSAocXVvdGUt
LSAmJiAoc2tiID0gc2tiX2RlcXVldWUoJmNvbm4tPmRhdGFfcSkpKSB7DQogCQkJQlRfREJHKCJz
a2IgJXAgbGVuICVkIiwgc2tiLCBza2ItPmxlbik7DQogDQpAQCAtMTg5NSwxMCArMTg3NywxMSBA
QCBzdGF0aWMgaW5saW5lIHZvaWQgaGNpX3NjaGVkX2FjbChzdHJ1Y3QgaGNpX2RldiAqaGRldikN
CiAJCQloY2lfc2VuZF9mcmFtZShza2IpOw0KIAkJCWhkZXYtPmFjbF9sYXN0X3R4ID0gamlmZmll
czsNCiANCi0JCQloZGV2LT5hY2xfY250LS07DQorCQkJY250LS07DQogCQkJY29ubi0+c2VudCsr
Ow0KIAkJfQ0KIAl9DQorCWhkZXYtPmFjbF9jbnQgPSBjbnQ7DQogfQ0KIA0KIC8qIFNjaGVkdWxl
IFNDTyAqLw0KQEAgLTE5MDYsMTEgKzE4ODksMTIgQEAgc3RhdGljIGlubGluZSB2b2lkIGhjaV9z
Y2hlZF9zY28oc3RydWN0IGhjaV9kZXYgKmhkZXYpDQogew0KIAlzdHJ1Y3QgaGNpX2Nvbm4gKmNv
bm47DQogCXN0cnVjdCBza19idWZmICpza2I7DQotCWludCBxdW90ZTsNCisJaW50IHF1b3RlID0g
MDsNCisJaW50IGNudCA9IGhkZXYtPnNjb19jbnQ7DQogDQogCUJUX0RCRygiJXMiLCBoZGV2LT5u
YW1lKTsNCiANCi0Jd2hpbGUgKGhkZXYtPnNjb19jbnQgJiYgKGNvbm4gPSBoY2lfbG93X3NlbnQo
aGRldiwgU0NPX0xJTkssICZxdW90ZSkpKSB7DQorCXdoaWxlIChjbnQgJiYgKGNvbm4gPSBoY2lf
bG93X3NlbnQoaGRldiwgU0NPX0xJTkssIGNudCwgJnF1b3RlKSkpIHsNCiAJCXdoaWxlIChxdW90
ZS0tICYmIChza2IgPSBza2JfZGVxdWV1ZSgmY29ubi0+ZGF0YV9xKSkpIHsNCiAJCQlCVF9EQkco
InNrYiAlcCBsZW4gJWQiLCBza2IsIHNrYi0+bGVuKTsNCiAJCQloY2lfc2VuZF9mcmFtZShza2Ip
Ow0KQEAgLTE5MjYsMTEgKzE5MTAsMTIgQEAgc3RhdGljIGlubGluZSB2b2lkIGhjaV9zY2hlZF9l
c2NvKHN0cnVjdCBoY2lfZGV2ICpoZGV2KQ0KIHsNCiAJc3RydWN0IGhjaV9jb25uICpjb25uOw0K
IAlzdHJ1Y3Qgc2tfYnVmZiAqc2tiOw0KLQlpbnQgcXVvdGU7DQorCWludCBxdW90ZSA9IDA7DQor
CWludCBjbnQgPSBoZGV2LT5zY29fY250Ow0KIA0KIAlCVF9EQkcoIiVzIiwgaGRldi0+bmFtZSk7
DQogDQotCXdoaWxlIChoZGV2LT5zY29fY250ICYmIChjb25uID0gaGNpX2xvd19zZW50KGhkZXYs
IEVTQ09fTElOSywgJnF1b3RlKSkpIHsNCisJd2hpbGUgKGNudCAmJiAoY29ubiA9IGhjaV9sb3df
c2VudChoZGV2LCBFU0NPX0xJTkssIGNudCwgJnF1b3RlKSkpIHsNCiAJCXdoaWxlIChxdW90ZS0t
ICYmIChza2IgPSBza2JfZGVxdWV1ZSgmY29ubi0+ZGF0YV9xKSkpIHsNCiAJCQlCVF9EQkcoInNr
YiAlcCBsZW4gJWQiLCBza2IsIHNrYi0+bGVuKTsNCiAJCQloY2lfc2VuZF9mcmFtZShza2IpOw0K
QEAgLTE5NDYsNyArMTkzMSw4IEBAIHN0YXRpYyBpbmxpbmUgdm9pZCBoY2lfc2NoZWRfbGUoc3Ry
dWN0IGhjaV9kZXYgKmhkZXYpDQogew0KIAlzdHJ1Y3QgaGNpX2Nvbm4gKmNvbm47DQogCXN0cnVj
dCBza19idWZmICpza2I7DQotCWludCBxdW90ZSwgY250Ow0KKwlpbnQgcXVvdGUgPSAwOw0KKwlp
bnQgY250ID0gaGRldi0+bGVfcGt0cyA/IGhkZXYtPmxlX2NudCA6IGhkZXYtPmFjbF9jbnQ7DQog
DQogCUJUX0RCRygiJXMiLCBoZGV2LT5uYW1lKTsNCiANCkBAIC0xOTU4LDggKzE5NDQsNyBAQCBz
dGF0aWMgaW5saW5lIHZvaWQgaGNpX3NjaGVkX2xlKHN0cnVjdCBoY2lfZGV2ICpoZGV2KQ0KIAkJ
CWhjaV9saW5rX3R4X3RvKGhkZXYsIExFX0xJTkspOw0KIAl9DQogDQotCWNudCA9IGhkZXYtPmxl
X3BrdHMgPyBoZGV2LT5sZV9jbnQgOiBoZGV2LT5hY2xfY250Ow0KLQl3aGlsZSAoY250ICYmIChj
b25uID0gaGNpX2xvd19zZW50KGhkZXYsIExFX0xJTkssICZxdW90ZSkpKSB7DQorCXdoaWxlIChj
bnQgJiYgKGNvbm4gPSBoY2lfbG93X3NlbnQoaGRldiwgTEVfTElOSywgY250LCAmcXVvdGUpKSkg
ew0KIAkJd2hpbGUgKHF1b3RlLS0gJiYgKHNrYiA9IHNrYl9kZXF1ZXVlKCZjb25uLT5kYXRhX3Ep
KSkgew0KIAkJCUJUX0RCRygic2tiICVwIGxlbiAlZCIsIHNrYiwgc2tiLT5sZW4pOw0KIA0KLS0g
DQoxLjcuNC4xDQoNCg==


2011-08-18 07:57:52

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [RFC v2 3/5] Bluetooth: Add buffer count to tx scheduler parameters

Hi Peter,

On Thu, Aug 18, 2011 at 3:37 AM, Peter Hurley <[email protected]> wrote:
> Utilize already known tx buffer count values as parameters to
> the tx scheduler, rather than determining the values on each
> iteration of the tx scheduler.
>
> This is preparatory for merging transmission types (such as SCO
> and ESCO) and also for handling tx buffer counters atomically.
>
> Signed-off-by: Peter Hurley <[email protected]>
> ---
> ?net/bluetooth/hci_core.c | ? 51 ++++++++++++++++-----------------------------
> ?1 files changed, 18 insertions(+), 33 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 0defa83..b2a1b9a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1798,7 +1798,8 @@ EXPORT_SYMBOL(hci_send_sco);
> ?/* ---- HCI TX task (outgoing data) ---- */
>
> ?/* HCI Connection scheduler */
> -static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int *quote)
> +static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int cnt, int *quote)
> ?{
> ? ? ? ?struct hci_conn_hash *h = &hdev->conn_hash;
> ? ? ? ?struct hci_conn *conn = NULL;
> @@ -1825,28 +1826,8 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> - ? ? ? if (conn) {
> - ? ? ? ? ? ? ? int cnt;
> -
> - ? ? ? ? ? ? ? switch (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");
> - ? ? ? ? ? ? ? }
> -
> + ? ? ? if (conn)
> ? ? ? ? ? ? ? ?*quote = (cnt + (num - 1)) / num;
> - ? ? ? } else
> - ? ? ? ? ? ? ? *quote = 0;
>
> ? ? ? ?BT_DBG("conn %p quote %d", conn, *quote);
> ? ? ? ?return conn;
> @@ -1875,18 +1856,19 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> ?{
> ? ? ? ?struct hci_conn *conn;
> ? ? ? ?struct sk_buff *skb;
> - ? ? ? int quote;
> + ? ? ? int quote = 0;
> + ? ? ? int cnt = hdev->acl_cnt;
>
> ? ? ? ?BT_DBG("%s", hdev->name);
>
> ? ? ? ?if (!test_bit(HCI_RAW, &hdev->flags)) {
> ? ? ? ? ? ? ? ?/* ACL tx timeout must be longer than maximum
> ? ? ? ? ? ? ? ? * link supervision timeout (40.9 seconds) */
> - ? ? ? ? ? ? ? if (!hdev->acl_cnt && time_after(jiffies, hdev->acl_last_tx + HZ * 45))
> + ? ? ? ? ? ? ? if (!cnt && time_after(jiffies, hdev->acl_last_tx + HZ * 45))
> ? ? ? ? ? ? ? ? ? ? ? ?hci_link_tx_to(hdev, ACL_LINK);
> ? ? ? ?}
>
> - ? ? ? while (hdev->acl_cnt && (conn = hci_low_sent(hdev, ACL_LINK, &quote))) {
> + ? ? ? while (cnt && (conn = hci_low_sent(hdev, ACL_LINK, cnt, &quote))) {
> ? ? ? ? ? ? ? ?while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> ? ? ? ? ? ? ? ? ? ? ? ?BT_DBG("skb %p len %d", skb, skb->len);
>
> @@ -1895,10 +1877,11 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
> ? ? ? ? ? ? ? ? ? ? ? ?hci_send_frame(skb);
> ? ? ? ? ? ? ? ? ? ? ? ?hdev->acl_last_tx = jiffies;
>
> - ? ? ? ? ? ? ? ? ? ? ? hdev->acl_cnt--;
> + ? ? ? ? ? ? ? ? ? ? ? cnt--;
> ? ? ? ? ? ? ? ? ? ? ? ?conn->sent++;
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
> + ? ? ? hdev->acl_cnt = cnt;
> ?}
>
> ?/* Schedule SCO */
> @@ -1906,11 +1889,12 @@ static inline void hci_sched_sco(struct hci_dev *hdev)
> ?{
> ? ? ? ?struct hci_conn *conn;
> ? ? ? ?struct sk_buff *skb;
> - ? ? ? int quote;
> + ? ? ? int quote = 0;
> + ? ? ? int cnt = hdev->sco_cnt;
>
> ? ? ? ?BT_DBG("%s", hdev->name);
>
> - ? ? ? while (hdev->sco_cnt && (conn = hci_low_sent(hdev, SCO_LINK, &quote))) {
> + ? ? ? while (cnt && (conn = hci_low_sent(hdev, SCO_LINK, cnt, &quote))) {
> ? ? ? ? ? ? ? ?while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> ? ? ? ? ? ? ? ? ? ? ? ?BT_DBG("skb %p len %d", skb, skb->len);
> ? ? ? ? ? ? ? ? ? ? ? ?hci_send_frame(skb);
> @@ -1926,11 +1910,12 @@ static inline void hci_sched_esco(struct hci_dev *hdev)
> ?{
> ? ? ? ?struct hci_conn *conn;
> ? ? ? ?struct sk_buff *skb;
> - ? ? ? int quote;
> + ? ? ? int quote = 0;
> + ? ? ? int cnt = hdev->sco_cnt;
>
> ? ? ? ?BT_DBG("%s", hdev->name);
>
> - ? ? ? while (hdev->sco_cnt && (conn = hci_low_sent(hdev, ESCO_LINK, &quote))) {
> + ? ? ? while (cnt && (conn = hci_low_sent(hdev, ESCO_LINK, cnt, &quote))) {
> ? ? ? ? ? ? ? ?while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> ? ? ? ? ? ? ? ? ? ? ? ?BT_DBG("skb %p len %d", skb, skb->len);
> ? ? ? ? ? ? ? ? ? ? ? ?hci_send_frame(skb);
> @@ -1946,7 +1931,8 @@ static inline void hci_sched_le(struct hci_dev *hdev)
> ?{
> ? ? ? ?struct hci_conn *conn;
> ? ? ? ?struct sk_buff *skb;
> - ? ? ? int quote, cnt;
> + ? ? ? int quote = 0;
> + ? ? ? int cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
>
> ? ? ? ?BT_DBG("%s", hdev->name);
>
> @@ -1958,8 +1944,7 @@ static inline void hci_sched_le(struct hci_dev *hdev)
> ? ? ? ? ? ? ? ? ? ? ? ?hci_link_tx_to(hdev, LE_LINK);
> ? ? ? ?}
>
> - ? ? ? cnt = hdev->le_pkts ? hdev->le_cnt : hdev->acl_cnt;
> - ? ? ? while (cnt && (conn = hci_low_sent(hdev, LE_LINK, &quote))) {
> + ? ? ? while (cnt && (conn = hci_low_sent(hdev, LE_LINK, cnt, &quote))) {
> ? ? ? ? ? ? ? ?while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
> ? ? ? ? ? ? ? ? ? ? ? ?BT_DBG("skb %p len %d", skb, skb->len);
>
> --
> 1.7.4.1
>
>

Nice work, it probably conflict with one of my patches to check if
there is any connection, but I guess it is easy to rebase, btw it is
probably a good idea to no initialize the variables in the declaration
since with hci_conn_num check we may bail out without using them.


--
Luiz Augusto von Dentz