2006-09-18 00:54:59

by Om Narasimhan

[permalink] [raw]
Subject: bluetooth drivers : kmalloc to kzalloc conversion

Tested by compiling with $make allmodconfig; make

Signed off by Om Narasimhan <[email protected]>

drivers/bluetooth/bfusb.c | 2 +-
drivers/bluetooth/hci_usb.c | 11 +++++------
2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index 23f9621..9772d8c 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -588,7 +588,7 @@ static int bfusb_load_firmware(struct bf

bfusb->udev->toggle[0] = bfusb->udev->toggle[1] = 0;

- buf = kmalloc(BFUSB_MAX_BLOCK_SIZE + 3, GFP_ATOMIC);
+ buf = kzalloc(BFUSB_MAX_BLOCK_SIZE + 3, GFP_ATOMIC);
if (!buf) {
BT_ERR("Can't allocate memory chunk for firmware");
return -ENOMEM;
diff --git a/drivers/bluetooth/hci_usb.c b/drivers/bluetooth/hci_usb.c
index e2d4bea..2274248 100644
--- a/drivers/bluetooth/hci_usb.c
+++ b/drivers/bluetooth/hci_usb.c
@@ -147,10 +147,9 @@ static struct usb_device_id blacklist_id

static struct _urb *_urb_alloc(int isoc, gfp_t gfp)
{
- struct _urb *_urb = kmalloc(sizeof(struct _urb) +
+ struct _urb *_urb = kzalloc(sizeof(struct _urb) +
sizeof(struct usb_iso_packet_descriptor) * isoc, gfp);
if (_urb) {
- memset(_urb, 0, sizeof(*_urb));
usb_init_urb(&_urb->urb);
}
return _urb;
@@ -220,7 +219,7 @@ static int hci_usb_intr_rx_submit(struct

size = le16_to_cpu(husb->intr_in_ep->desc.wMaxPacketSize);

- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kzalloc(size, GFP_ATOMIC);
if (!buf)
return -ENOMEM;

@@ -255,7 +254,7 @@ static int hci_usb_bulk_rx_submit(struct
int err, pipe, size = HCI_MAX_FRAME_SIZE;
void *buf;

- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kzalloc(size, GFP_ATOMIC);
if (!buf)
return -ENOMEM;

@@ -296,7 +295,7 @@ static int hci_usb_isoc_rx_submit(struct
mtu = le16_to_cpu(husb->isoc_in_ep->desc.wMaxPacketSize);
size = mtu * HCI_MAX_ISOC_FRAMES;

- buf = kmalloc(size, GFP_ATOMIC);
+ buf = kzalloc(size, GFP_ATOMIC);
if (!buf)
return -ENOMEM;

@@ -471,7 +470,7 @@ static inline int hci_usb_send_ctrl(stru
return -ENOMEM;
_urb->type = bt_cb(skb)->pkt_type;

- dr = kmalloc(sizeof(*dr), GFP_ATOMIC);
+ dr = kzalloc(sizeof(*dr), GFP_ATOMIC);
if (!dr) {
_urb_free(_urb);
return -ENOMEM;


2006-09-18 02:24:42

by Om Narasimhan

[permalink] [raw]
Subject: Re: bluetooth drivers : kmalloc to kzalloc conversion

On 9/17/06, Dmitry Torokhov <[email protected]> wrote:
> On Sunday 17 September 2006 20:54, Om Narasimhan wrote:
> > --- a/drivers/bluetooth/hci_usb.c
> > +++ b/drivers/bluetooth/hci_usb.c
> > @@ -147,10 +147,9 @@ static struct usb_device_id blacklist_id
> >
> > static struct _urb *_urb_alloc(int isoc, gfp_t gfp)
> > {
> > -struct _urb *_urb = kmalloc(sizeof(struct _urb) +
> > +struct _urb *_urb = kzalloc(sizeof(struct _urb) +
> > sizeof(struct usb_iso_packet_descriptor) * isoc, gfp);
> > if (_urb) {
> > -memset(_urb, 0, sizeof(*_urb));
> > usb_init_urb(&_urb->urb);
> > }
> > return _urb;
> >
>
> Note that only beginning if the aloocated memory was zeroed in original
> code; your patch may introduce slowdowns.
Would it? I thought memset() uses block move operation which doesn't
scale linearly.
And, usb_init_urb() calls memset() anyway, so the previously existed
memset() was superfluous.

Thanks for the comment.
Regards,
Om.

2006-09-18 02:38:30

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: bluetooth drivers : kmalloc to kzalloc conversion

On Sunday 17 September 2006 22:24, Om Narasimhan wrote:
> On 9/17/06, Dmitry Torokhov <[email protected]> wrote:
> > On Sunday 17 September 2006 20:54, Om Narasimhan wrote:
> > > --- a/drivers/bluetooth/hci_usb.c
> > > +++ b/drivers/bluetooth/hci_usb.c
> > > @@ -147,10 +147,9 @@ static struct usb_device_id blacklist_id
> > >
> > > static struct _urb *_urb_alloc(int isoc, gfp_t gfp)
> > > {
> > > -struct _urb *_urb = kmalloc(sizeof(struct _urb) +
> > > +struct _urb *_urb = kzalloc(sizeof(struct _urb) +
> > > sizeof(struct usb_iso_packet_descriptor) * isoc, gfp);
> > > if (_urb) {
> > > -memset(_urb, 0, sizeof(*_urb));
> > > usb_init_urb(&_urb->urb);
> > > }
> > > return _urb;
> > >
> >
> > Note that only beginning if the aloocated memory was zeroed in original
> > code; your patch may introduce slowdowns.
> Would it? I thought memset() uses block move operation which doesn't
> scale linearly.

Well, the old code was zeroing sizeof(struct _urb) bytes while yours is
zeroing sizeof(struct _urb) + X so it will definitely take more time.

> And, usb_init_urb() calls memset() anyway, so the previously existed
> memset() was superfluous.
>

It only clears part of struct _urb. Note that "stuct _urb" and "struct urb"
are 2 different structures.

--
Dmitry

2006-09-18 06:13:26

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] bluetooth drivers : kmalloc to kzalloc conversion

Hi Om,

> Tested by compiling with $make allmodconfig; make
>
> Signed off by Om Narasimhan <[email protected]>
>
> drivers/bluetooth/bfusb.c | 2 +-
> drivers/bluetooth/hci_usb.c | 11 +++++------
> 2 files changed, 6 insertions(+), 7 deletions(-)

the kzalloc is suppose to replace a kmalloc followed by a memset
operation. It is not the case for these changes and so these changes
make no sense.

Regards

Marcel