2012-02-03 14:27:53

by Andrei Emeltchenko

Subject: [PATCHv1 0/2] Adds support for block based flow ctrl

From: Andrei Emeltchenko <[email protected]>

Split HCI scheduling for block and packet flow control.

Changes:
No change from RFC I've sent some time ago since I've got
no comments.

Andrei Emeltchenko (2):
Bluetooth: Recalculate sched HCI blk/pkt flow ctrl
Bluetooth: Helper removes duplicated code

net/bluetooth/hci_core.c | 92 ++++++++++++++++++++++++++++++++++++++++------
1 files changed, 80 insertions(+), 12 deletions(-)

--
1.7.8.3

2012-02-03 18:52:40

by Johan Hedberg

Subject: Re: [PATCHv1 0/2] Adds support for block based flow ctrl

Hi Andrei,

On Fri, Feb 03, 2012, Emeltchenko Andrei wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Split HCI scheduling for block and packet flow control.
>
> Changes:
> No change from RFC I've sent some time ago since I've got
> no comments.
>
> Andrei Emeltchenko (2):
> Bluetooth: Recalculate sched HCI blk/pkt flow ctrl
> Bluetooth: Helper removes duplicated code
>
> net/bluetooth/hci_core.c | 92 ++++++++++++++++++++++++++++++++++++++++------
> 1 files changed, 80 insertions(+), 12 deletions(-)

Both patches have been applied to my bluetooth-next tree. Thanks.

Johan

2012-02-03 17:02:09

by Marcel Holtmann

Subject: Re: [PATCHv1 2/2] Bluetooth: Helper removes duplicated code

Hi Andrei,

> Use __check_timout helper to remove duplicated code
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_core.c | 31 +++++++++++++------------------
> 1 files changed, 13 insertions(+), 18 deletions(-)

Reviewed-by: Ulisses Furquim <[email protected]>
Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel

2012-02-03 17:00:54

by Ulisses Furquim

Subject: Re: [PATCHv1 2/2] Bluetooth: Helper removes duplicated code

Hi Marcel,

On Fri, Feb 3, 2012 at 2:03 PM, Marcel Holtmann <[email protected]> wrote=
:
> Hi Andrei,
>
>> Use __check_timout helper to remove duplicated code
>>
>> Signed-off-by: Andrei Emeltchenko <[email protected]>
>> ---
>> =A0net/bluetooth/hci_core.c | =A0 31 +++++++++++++------------------
>> =A01 files changed, 13 insertions(+), 18 deletions(-)
>>
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 9a56a40..95eeae5 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -2390,22 +2390,25 @@ static inline int __get_blocks(struct hci_dev *h=
dev, struct sk_buff *skb)
>> =A0 =A0 =A0 return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block=
_len);
>> =A0}
>>
>> -static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
>> +static inline void __check_timeout(struct hci_dev *hdev, unsigned int c=
nt)
>> =A0{
>> - =A0 =A0 struct hci_chan *chan;
>> - =A0 =A0 struct sk_buff *skb;
>> - =A0 =A0 int quote;
>> - =A0 =A0 unsigned int cnt;
>> -
>> =A0 =A0 =A0 if (!test_bit(HCI_RAW, &hdev->flags)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* ACL tx timeout must be longer than maximu=
m
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* link supervision timeout (40.9 seconds)=
*/
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (!hdev->acl_cnt && time_after(jiffies, hdev=
->acl_last_tx +
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (!cnt && time_after(jiffies, hdev->acl_last=
_tx +
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_link_tx_to(hdev, ACL_LIN=
K);
>> =A0 =A0 =A0 }
>> +}
>>
>> - =A0 =A0 cnt =3D hdev->acl_cnt;
>> +static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
>> +{
>> + =A0 =A0 unsigned int cnt =3D hdev->acl_cnt;

cnt is initialized here.

>> + =A0 =A0 struct hci_chan *chan;
>> + =A0 =A0 struct sk_buff *skb;
>> + =A0 =A0 int quote;
>> +
>> + =A0 =A0 __check_timeout(hdev, cnt);
>>
>> =A0 =A0 =A0 while (hdev->acl_cnt &&
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (chan =3D hci_chan_sent(hdev=
, ACL_LINK, &quote))) {
>> @@ -2438,20 +2441,12 @@ static inline void hci_sched_acl_pkt(struct hci_=
dev *hdev)
>>
>> =A0static inline void hci_sched_acl_blk(struct hci_dev *hdev)
>> =A0{
>> + =A0 =A0 unsigned int cnt =3D hdev->block_cnt;

And cnt is initialized here, for block count.

>> =A0 =A0 =A0 struct hci_chan *chan;
>> =A0 =A0 =A0 struct sk_buff *skb;
>> =A0 =A0 =A0 int quote;
>> - =A0 =A0 unsigned int cnt;
>> -
>> - =A0 =A0 if (!test_bit(HCI_RAW, &hdev->flags)) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* ACL tx timeout must be longer than maximum
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0* link supervision timeout (40.9 seconds) *=
/
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (!hdev->block_cnt && time_after(jiffies, hd=
ev->acl_last_tx +
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 hci_link_tx_to(hdev, ACL_LINK)=
;
>> - =A0 =A0 }
>>
>> - =A0 =A0 cnt =3D hdev->block_cnt;
>
> maybe the patch is just funnily trying to be too smart, but I am missing
> the cnt init here.

And yes, the patch itself looks confusing but it was because of how
git created the changes. I think the patch looks good.

>> + =A0 =A0 __check_timeout(hdev, cnt);
>>
>> =A0 =A0 =A0 while (hdev->block_cnt > 0 &&
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (chan =3D hci_chan_sent(hdev=
, ACL_LINK, &quote))) {
>
> Regards
>
> Marcel

Regards,

--=20
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs

2012-02-03 16:52:14

by Vinicius Costa Gomes

Subject: Re: [PATCHv1 2/2] Bluetooth: Helper removes duplicated code

Hi Andrei,

On 16:27 Fri 03 Feb, Emeltchenko Andrei wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Use __check_timout helper to remove duplicated code

Typo here: __check_timeout

>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_core.c | 31 +++++++++++++------------------
> 1 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9a56a40..95eeae5 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2390,22 +2390,25 @@ static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
> return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);


[ snip ]


Cheers,
--
Vinicius

2012-02-03 16:03:11

by Marcel Holtmann

Subject: Re: [PATCHv1 2/2] Bluetooth: Helper removes duplicated code

Hi Andrei,

> Use __check_timout helper to remove duplicated code
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_core.c | 31 +++++++++++++------------------
> 1 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 9a56a40..95eeae5 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2390,22 +2390,25 @@ static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
> return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);
> }
>
> -static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
> +static inline void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
> {
> - struct hci_chan *chan;
> - struct sk_buff *skb;
> - int quote;
> - unsigned int cnt;
> -
> 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 +
> + if (!cnt && time_after(jiffies, hdev->acl_last_tx +
> msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
> hci_link_tx_to(hdev, ACL_LINK);
> }
> +}
>
> - cnt = hdev->acl_cnt;
> +static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
> +{
> + unsigned int cnt = hdev->acl_cnt;
> + struct hci_chan *chan;
> + struct sk_buff *skb;
> + int quote;
> +
> + __check_timeout(hdev, cnt);
>
> while (hdev->acl_cnt &&
> (chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
> @@ -2438,20 +2441,12 @@ static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
>
> static inline void hci_sched_acl_blk(struct hci_dev *hdev)
> {
> + unsigned int cnt = hdev->block_cnt;
> struct hci_chan *chan;
> struct sk_buff *skb;
> int quote;
> - unsigned int cnt;
> -
> - if (!test_bit(HCI_RAW, &hdev->flags)) {
> - /* ACL tx timeout must be longer than maximum
> - * link supervision timeout (40.9 seconds) */
> - if (!hdev->block_cnt && time_after(jiffies, hdev->acl_last_tx +
> - msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
> - hci_link_tx_to(hdev, ACL_LINK);
> - }
>
> - cnt = hdev->block_cnt;

maybe the patch is just funnily trying to be too smart, but I am missing
the cnt init here.

> + __check_timeout(hdev, cnt);
>
> while (hdev->block_cnt > 0 &&
> (chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {

Regards

Marcel

2012-02-03 16:01:33

by Marcel Holtmann

Subject: Re: [PATCHv1 1/2] Bluetooth: Recalculate sched HCI blk/pkt flow ctrl

Hi Andrei,

> Split HCI scheduling for block and packet flow control.
>
> Signed-off-by: Andrei Emeltchenko <[email protected]>
> ---
> net/bluetooth/hci_core.c | 85 ++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 79 insertions(+), 6 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel

2012-02-03 14:27:55

by Andrei Emeltchenko

Subject: [PATCHv1 2/2] Bluetooth: Helper removes duplicated code

From: Andrei Emeltchenko <[email protected]>

Use __check_timout helper to remove duplicated code

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_core.c | 31 +++++++++++++------------------
1 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9a56a40..95eeae5 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2390,22 +2390,25 @@ static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);
}

-static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
+static inline void __check_timeout(struct hci_dev *hdev, unsigned int cnt)
{
- struct hci_chan *chan;
- struct sk_buff *skb;
- int quote;
- unsigned int cnt;
-
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 +
+ if (!cnt && time_after(jiffies, hdev->acl_last_tx +
msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
hci_link_tx_to(hdev, ACL_LINK);
}
+}

- cnt = hdev->acl_cnt;
+static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
+{
+ unsigned int cnt = hdev->acl_cnt;
+ struct hci_chan *chan;
+ struct sk_buff *skb;
+ int quote;
+
+ __check_timeout(hdev, cnt);

while (hdev->acl_cnt &&
(chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
@@ -2438,20 +2441,12 @@ static inline void hci_sched_acl_pkt(struct hci_dev *hdev)

static inline void hci_sched_acl_blk(struct hci_dev *hdev)
{
+ unsigned int cnt = hdev->block_cnt;
struct hci_chan *chan;
struct sk_buff *skb;
int quote;
- unsigned int cnt;
-
- if (!test_bit(HCI_RAW, &hdev->flags)) {
- /* ACL tx timeout must be longer than maximum
- * link supervision timeout (40.9 seconds) */
- if (!hdev->block_cnt && time_after(jiffies, hdev->acl_last_tx +
- msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
- hci_link_tx_to(hdev, ACL_LINK);
- }

- cnt = hdev->block_cnt;
+ __check_timeout(hdev, cnt);

while (hdev->block_cnt > 0 &&
(chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
--
1.7.8.3

2012-02-03 14:27:54

by Andrei Emeltchenko

Subject: [PATCHv1 1/2] Bluetooth: Recalculate sched HCI blk/pkt flow ctrl

From: Andrei Emeltchenko <[email protected]>

Split HCI scheduling for block and packet flow control.

Signed-off-by: Andrei Emeltchenko <[email protected]>
---
net/bluetooth/hci_core.c | 85 ++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 76dc153..9a56a40 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2384,18 +2384,19 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type)

}

-static inline void hci_sched_acl(struct hci_dev *hdev)
+static inline int __get_blocks(struct hci_dev *hdev, struct sk_buff *skb)
+{
+ /* Calculate count of blocks used by this packet */
+ return DIV_ROUND_UP(skb->len - HCI_ACL_HDR_SIZE, hdev->block_len);
+}
+
+static inline void hci_sched_acl_pkt(struct hci_dev *hdev)
{
struct hci_chan *chan;
struct sk_buff *skb;
int quote;
unsigned int cnt;

- 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) */
@@ -2435,6 +2436,78 @@ static inline void hci_sched_acl(struct hci_dev *hdev)
hci_prio_recalculate(hdev, ACL_LINK);
}

+static inline void hci_sched_acl_blk(struct hci_dev *hdev)
+{
+ struct hci_chan *chan;
+ struct sk_buff *skb;
+ int quote;
+ unsigned int cnt;
+
+ if (!test_bit(HCI_RAW, &hdev->flags)) {
+ /* ACL tx timeout must be longer than maximum
+ * link supervision timeout (40.9 seconds) */
+ if (!hdev->block_cnt && time_after(jiffies, hdev->acl_last_tx +
+ msecs_to_jiffies(HCI_ACL_TX_TIMEOUT)))
+ hci_link_tx_to(hdev, ACL_LINK);
+ }
+
+ cnt = hdev->block_cnt;
+
+ while (hdev->block_cnt > 0 &&
+ (chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
+ u32 priority = (skb_peek(&chan->data_q))->priority;
+ while (quote > 0 && (skb = skb_peek(&chan->data_q))) {
+ int blocks;
+
+ BT_DBG("chan %p skb %p len %d priority %u", chan, skb,
+ skb->len, skb->priority);
+
+ /* Stop if priority has changed */
+ if (skb->priority < priority)
+ break;
+
+ skb = skb_dequeue(&chan->data_q);
+
+ blocks = __get_blocks(hdev, skb);
+ if (blocks > hdev->block_cnt)
+ return;
+
+ hci_conn_enter_active_mode(chan->conn,
+ bt_cb(skb)->force_active);
+
+ hci_send_frame(skb);
+ hdev->acl_last_tx = jiffies;
+
+ hdev->block_cnt -= blocks;
+ quote -= blocks;
+
+ chan->sent += blocks;
+ chan->conn->sent += blocks;
+ }
+ }
+
+ if (cnt != hdev->block_cnt)
+ hci_prio_recalculate(hdev, ACL_LINK);
+}
+
+static inline void hci_sched_acl(struct hci_dev *hdev)
+{
+ BT_DBG("%s", hdev->name);
+
+ if (!hci_conn_num(hdev, ACL_LINK))
+ return;
+
+ switch (hdev->flow_ctl_mode) {
+ case HCI_FLOW_CTL_MODE_PACKET_BASED:
+ hci_sched_acl_pkt(hdev);
+ break;
+
+ case HCI_FLOW_CTL_MODE_BLOCK_BASED:
+ hci_sched_acl_blk(hdev);
+ break;
+ }
+}
+
/* Schedule SCO */
static inline void hci_sched_sco(struct hci_dev *hdev)
{
--
1.7.8.3