2009-06-20 06:12:08

by Vikram

[permalink] [raw]
Subject: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.

This patch increases the receive buffer size to HCI_MAX_FRAME_SIZE
which improves the Rx throughput considerably.

Signed-off-by: Vikram Kandukuri <[email protected]>
---
drivers/bluetooth/btusb.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index e70c57e..124db8c 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -301,7 +301,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
struct urb *urb;
unsigned char *buf;
unsigned int pipe;
- int err, size;
+ int err, size = HCI_MAX_FRAME_SIZE;

BT_DBG("%s", hdev->name);

@@ -312,8 +312,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
if (!urb)
return -ENOMEM;

- size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
-
buf = kmalloc(size, mem_flags);
if (!buf) {
usb_free_urb(urb);
--
1.6.0.4


2009-06-19 09:29:51

by Marcel Holtmann

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.

Hi Vikram,

> > This patch increases the receive buffer size to HCI_MAX_FRAME_SIZE
> > which improves the Rx throughput considerably.
> >
> > Signed-off-by: Vikram Kandukuri <[email protected]>
> > ---
> > drivers/bluetooth/btusb.c | 4 +---
> > 1 files changed, 1 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index e70c57e..124db8c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -301,7 +301,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> > struct urb *urb;
> > unsigned char *buf;
> > unsigned int pipe;
> > - int err, size;
> > + int err, size = HCI_MAX_FRAME_SIZE;
> >
> > BT_DBG("%s", hdev->name);
> >
> > @@ -312,8 +312,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> > if (!urb)
> > return -ENOMEM;
> >
> > - size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
> > -
>
> and we can ignore the wMaxPacketSize because. It would be nice to get a
> reason here and that this has been tested at least with a handful of
>
> When we are testing PAN profile with throughput run, we observed low rx throughput. We have referred hci_usb.c and found they have used HCI_MAX_FRAME_SIZE. This increases the bulk endpoint size to 1028 bytes.
>
> This change gave significant (around 40%) increase in performance (increase from .8 to 1.5 Mb/s in shield room)
>
> We have tested with BRM/Atheros/CSR USB dongles with PAN profile.
> We have tested with iperf and Chariot scripts. Let me know if you need more information.

I was more looking into the argument that the btusb driver doesn't have
to handle wMaxPacketSize, because the USB host controller will split
URBs accordingly anyway. Is this true for only bulk URBs? Or also for
interrupt and isoc URBs?

Regards

Marcel



2009-06-19 08:39:27

by Vikram Kandukuri

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.



-----Original Message-----
From: Marcel Holtmann [mailto:[email protected]]
Sent: Friday, June 19, 2009 1:51 PM
To: Vikram Kandukuri
Cc: [email protected]
Subject: Re: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.

Hi Vikram,

> This patch increases the receive buffer size to HCI_MAX_FRAME_SIZE
> which improves the Rx throughput considerably.
>
> Signed-off-by: Vikram Kandukuri <[email protected]>
> ---
> drivers/bluetooth/btusb.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e70c57e..124db8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -301,7 +301,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> struct urb *urb;
> unsigned char *buf;
> unsigned int pipe;
> - int err, size;
> + int err, size = HCI_MAX_FRAME_SIZE;
>
> BT_DBG("%s", hdev->name);
>
> @@ -312,8 +312,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> if (!urb)
> return -ENOMEM;
>
> - size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
> -

and we can ignore the wMaxPacketSize because. It would be nice to get a
reason here and that this has been tested at least with a handful of
devices.

Regards

Marcel

Hi Marcel,
When we are testing PAN profile with throughput run, we observed low rx throughput. We have referred hci_usb.c and found they have used HCI_MAX_FRAME_SIZE. This increases the bulk endpoint size to 1028 bytes.

This change gave significant (around 40%) increase in performance (increase from .8 to 1.5 Mb/s in shield room)

We have tested with BRM/Atheros/CSR USB dongles with PAN profile.
We have tested with iperf and Chariot scripts. Let me know if you need more information.

Thanks,
Vikram


2009-06-19 08:21:05

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.

Hi Vikram,

> This patch increases the receive buffer size to HCI_MAX_FRAME_SIZE
> which improves the Rx throughput considerably.
>
> Signed-off-by: Vikram Kandukuri <[email protected]>
> ---
> drivers/bluetooth/btusb.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index e70c57e..124db8c 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -301,7 +301,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> struct urb *urb;
> unsigned char *buf;
> unsigned int pipe;
> - int err, size;
> + int err, size = HCI_MAX_FRAME_SIZE;
>
> BT_DBG("%s", hdev->name);
>
> @@ -312,8 +312,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hdev, gfp_t mem_flags)
> if (!urb)
> return -ENOMEM;
>
> - size = le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
> -

and we can ignore the wMaxPacketSize because. It would be nice to get a
reason here and that this has been tested at least with a handful of
devices.

Regards

Marcel



2009-07-01 09:06:11

by Vikram Kandukuri

[permalink] [raw]
Subject: RE: [PATCH] Bluetooth: Improve the throughput by increasing the frame size.



-----Original Message-----
From: Marcel Holtmann [mailto:[email protected]]
Sent: Friday, June 19, 2009 3:00 PM
To: Vikram Kandukuri
Cc: [email protected]
Subject: RE: [PATCH] Bluetooth: Improve the throughput by increasing the fr=
ame size.

Hi Vikram,

> > This patch increases the receive buffer size to HCI_MAX_FRAME_SIZE
> > which improves the Rx throughput considerably.
> >
> > Signed-off-by: Vikram Kandukuri <[email protected]>
> > ---
> > drivers/bluetooth/btusb.c | 4 +---
> > 1 files changed, 1 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index e70c57e..124db8c 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -301,7 +301,7 @@ static int btusb_submit_bulk_urb(struct hci_dev *hd=
ev, gfp_t mem_flags)
> > struct urb *urb;
> > unsigned char *buf;
> > unsigned int pipe;
> > - int err, size;
> > + int err, size =3D HCI_MAX_FRAME_SIZE;
> >
> > BT_DBG("%s", hdev->name);
> >
> > @@ -312,8 +312,6 @@ static int btusb_submit_bulk_urb(struct hci_dev *hd=
ev, gfp_t mem_flags)
> > if (!urb)
> > return -ENOMEM;
> >
> > - size =3D le16_to_cpu(data->bulk_rx_ep->wMaxPacketSize);
> > -
>
> and we can ignore the wMaxPacketSize because. It would be nice to get a
> reason here and that this has been tested at least with a handful of
>
> When we are testing PAN profile with throughput run, we observed low rx t=
hroughput. We have referred hci_usb.c and found they have used HCI_MAX_FRAM=
E_SIZE. This increases the bulk endpoint size to 1028 bytes.
>
> This change gave significant (around 40%) increase in performance (increa=
se from .8 to 1.5 Mb/s in shield room)
>
> We have tested with BRM/Atheros/CSR USB dongles with PAN profile.
> We have tested with iperf and Chariot scripts. Let me know if you need mo=
re information.

I was more looking into the argument that the btusb driver doesn't have
to handle wMaxPacketSize, because the USB host controller will split
URBs accordingly anyway. Is this true for only bulk URBs? Or also for
interrupt and isoc URBs?

Regards

Marcel

Hi Marcel,
I did not get a chance to test interrupt and isoc URBs.
I feel this fix is needed for bulk. I have sent the patch again.

Thanks
Vikram