[PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-28 Thread Rakib Mullick 
On Sat, Nov 26, 2011 at 10:53 PM, Chris Wilson  
wrote:
> On Sat, 26 Nov 2011 22:29:12 +0600, Rakib Mullick  gmail.com> wrote:
>> Yes, no real problem with current code. I was just thinking from code
>> cleanup's pov. Is BUG_ON really needed in i915_add_request() ?
>
> No, just documentation as a reminder that the request should be
> preallocated, ideally so that we can fail gracefully without touching
> hardware and leaving it in an inconsistent state wrt to our bookkeeping.
> (This is more apparent in the overlay code which could hang the
> chip/driver if we hit a malloc error too late.)
>
> The BUG_ON has certainly outlived its usefulness.

Actually, I'm not seeing how BUG_ON could trigger (though, I've
wrongly mentioned in previous thread, if request == NULL, BUG_ON could
trigger), it's usefulness will never come into action. Other callers
of i915_add_request also makes sure that, it gets called only if
(request).

Although, kfree(NULL) is permitted, we shouldn't use it unnecessarily.
Anyway, since the issue is not a big deal and no real bug, it could be
droped.

Thanks,
Rakib

Re: [PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-27 Thread Rakib Mullick 
On Sat, Nov 26, 2011 at 10:53 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Sat, 26 Nov 2011 22:29:12 +0600, Rakib Mullick rakib.mull...@gmail.com 
 wrote:
 Yes, no real problem with current code. I was just thinking from code
 cleanup's pov. Is BUG_ON really needed in i915_add_request() ?

 No, just documentation as a reminder that the request should be
 preallocated, ideally so that we can fail gracefully without touching
 hardware and leaving it in an inconsistent state wrt to our bookkeeping.
 (This is more apparent in the overlay code which could hang the
 chip/driver if we hit a malloc error too late.)

 The BUG_ON has certainly outlived its usefulness.

Actually, I'm not seeing how BUG_ON could trigger (though, I've
wrongly mentioned in previous thread, if request == NULL, BUG_ON could
trigger), it's usefulness will never come into action. Other callers
of i915_add_request also makes sure that, it gets called only if
(request).

Although, kfree(NULL) is permitted, we shouldn't use it unnecessarily.
Anyway, since the issue is not a big deal and no real bug, it could be
droped.

Thanks,
Rakib
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-26 Thread Rakib Mullick 
On Sat, Nov 26, 2011 at 4:57 PM, Chris Wilson  
wrote:
> On Sat, 26 Nov 2011 10:44:17 +0600, Rakib Mullick  gmail.com> wrote:
>> On Mon, Nov 21, 2011 at 11:16 PM, Keith Packard  wrote:
>> > On Mon, 21 Nov 2011 17:23:06 +0100, Daniel Vetter  
>> > wrote:
>> >
>> >> Indeed, nice catch (albeit totally unlikely to be hit, because the error
>> >> only happens when the gpu ceases to progress in the ring, so imo not
>> >> stable material). Keith, please pick this up for fixes, thanks.
>> >
>> > It's already there and queued for airlied :-)
>> >
>> Thank you guys for reviewing and taking the patch.
>>
>> Now, while I was looking at the uses of i915_add_request(), I found
>> the following code :
>>
>> ? ? ? ? ? ? ? ? ? ? ? ? ret = i915_gem_flush_ring(ring, 0,
>> I915_GEM_GPU_DOMAINS);
>> ? ? ? ? ? ? ? ? ? ? ? ? request = kzalloc(sizeof(*request), GFP_KERNEL);
>> ? ? ? ? ? ? ? ? ? ? ? ? if (ret || request == NULL ||
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? i915_add_request(ring, NULL, request))
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? kfree(request);
>>
>> From above code, we might ended up by calling kfree(request) without
>> allocating request. Though, it's unlikely cause if request is NULL
>> then BUG_ON will be hit in i915_add_request(). So, to unify the callee
>> uses of i915_add_request(), I'm proposing the following patch. Please
>> let me know what do you guys think. If you guys agree, I can sent a
>> formal patch.
>
> kfree(NULL) is permitted and taken advantage of here along with the
> short-circuiting behaviour of '||'.

Yes, no real problem with current code. I was just thinking from code
cleanup's pov. Is BUG_ON really needed in i915_add_request() ?

thanks,
Rakib

[PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-26 Thread Rakib Mullick 
On Mon, Nov 21, 2011 at 11:16 PM, Keith Packard  wrote:
> On Mon, 21 Nov 2011 17:23:06 +0100, Daniel Vetter  wrote:
>
>> Indeed, nice catch (albeit totally unlikely to be hit, because the error
>> only happens when the gpu ceases to progress in the ring, so imo not
>> stable material). Keith, please pick this up for fixes, thanks.
>
> It's already there and queued for airlied :-)
>
Thank you guys for reviewing and taking the patch.

Now, while I was looking at the uses of i915_add_request(), I found
the following code :

ret = i915_gem_flush_ring(ring, 0,
I915_GEM_GPU_DOMAINS);
request = kzalloc(sizeof(*request), GFP_KERNEL);
if (ret || request == NULL ||
i915_add_request(ring, NULL, request))
kfree(request);

Re: [PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-26 Thread Rakib Mullick 
On Sat, Nov 26, 2011 at 4:57 PM, Chris Wilson ch...@chris-wilson.co.uk wrote:
 On Sat, 26 Nov 2011 10:44:17 +0600, Rakib Mullick rakib.mull...@gmail.com 
 wrote:
 On Mon, Nov 21, 2011 at 11:16 PM, Keith Packard kei...@keithp.com wrote:
  On Mon, 21 Nov 2011 17:23:06 +0100, Daniel Vetter dan...@ffwll.ch wrote:
 
  Indeed, nice catch (albeit totally unlikely to be hit, because the error
  only happens when the gpu ceases to progress in the ring, so imo not
  stable material). Keith, please pick this up for fixes, thanks.
 
  It's already there and queued for airlied :-)
 
 Thank you guys for reviewing and taking the patch.

 Now, while I was looking at the uses of i915_add_request(), I found
 the following code :

                         ret = i915_gem_flush_ring(ring, 0,
 I915_GEM_GPU_DOMAINS);
                         request = kzalloc(sizeof(*request), GFP_KERNEL);
                         if (ret || request == NULL ||
                             i915_add_request(ring, NULL, request))
                             kfree(request);

 From above code, we might ended up by calling kfree(request) without
 allocating request. Though, it's unlikely cause if request is NULL
 then BUG_ON will be hit in i915_add_request(). So, to unify the callee
 uses of i915_add_request(), I'm proposing the following patch. Please
 let me know what do you guys think. If you guys agree, I can sent a
 formal patch.

 kfree(NULL) is permitted and taken advantage of here along with the
 short-circuiting behaviour of '||'.

Yes, no real problem with current code. I was just thinking from code
cleanup's pov. Is BUG_ON really needed in i915_add_request() ?

thanks,
Rakib
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-25 Thread Rakib Mullick 
On Mon, Nov 21, 2011 at 11:16 PM, Keith Packard kei...@keithp.com wrote:
 On Mon, 21 Nov 2011 17:23:06 +0100, Daniel Vetter dan...@ffwll.ch wrote:

 Indeed, nice catch (albeit totally unlikely to be hit, because the error
 only happens when the gpu ceases to progress in the ring, so imo not
 stable material). Keith, please pick this up for fixes, thanks.

 It's already there and queued for airlied :-)

Thank you guys for reviewing and taking the patch.

Now, while I was looking at the uses of i915_add_request(), I found
the following code :

ret = i915_gem_flush_ring(ring, 0,
I915_GEM_GPU_DOMAINS);
request = kzalloc(sizeof(*request), GFP_KERNEL);
if (ret || request == NULL ||
i915_add_request(ring, NULL, request))
kfree(request);

From above code, we might ended up by calling kfree(request) without
allocating request. Though, it's unlikely cause if request is NULL
then BUG_ON will be hit in i915_add_request(). So, to unify the callee
uses of i915_add_request(), I'm proposing the following patch. Please
let me know what do you guys think. If you guys agree, I can sent a
formal patch.

Index: work/modi.linux-3.1.1/drivers/gpu/drm/i915/i915_gem.c
===
--- work.orig/modi.linux-3.1.1/drivers/gpu/drm/i915/i915_gem.c
+++ work/modi.linux-3.1.1/drivers/gpu/drm/i915/i915_gem.c
@@ -1736,8 +1736,6 @@ i915_add_request(struct intel_ring_buffe
int was_empty;
int ret;

-   BUG_ON(request == NULL);
-
ret = ring-add_request(ring, seqno);
if (ret)
return ret;
@@ -2002,9 +2000,11 @@ i915_gem_retire_work_handler(struct work
ret = i915_gem_flush_ring(ring,
  0, I915_GEM_GPU_DOMAINS);
request = kzalloc(sizeof(*request), GFP_KERNEL);
-   if (ret || request == NULL ||
-   i915_add_request(ring, NULL, request))
-   kfree(request);
+   if (request) {
+   ret = i915_add_request(ring, NULL, request);
+   if (ret)
+   kfree(request);
+   }
}

idle = list_empty(ring-request_list);


Thanks,
Rakib
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-16 Thread Rakib Mullick 

A call to i915_add_request() has been made in function i915_gem_busy_ioctl(). 
i915_add_request can fail,
so in it's exit path previously allocated memory needs to be freed.

Signed-off-by: Rakib Mullick 
---

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d18b07a..2dc24ab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3512,9 +3512,11 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 * so emit a request to do so.
 */
request = kzalloc(sizeof(*request), GFP_KERNEL);
-   if (request)
+   if (request) {
ret = i915_add_request(obj->ring, NULL, 
request);
-   else
+   if (ret)
+   kfree(request);
+   } else
ret = -ENOMEM;
}

[PATCH] drm, i915: Fix memory leak in i915_gem_busy_ioctl().

2011-11-15 Thread Rakib Mullick 

A call to i915_add_request() has been made in function i915_gem_busy_ioctl(). 
i915_add_request can fail,
so in it's exit path previously allocated memory needs to be freed.

Signed-off-by: Rakib Mullick rakib.mull...@gmail.com
---

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d18b07a..2dc24ab 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3512,9 +3512,11 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
 * so emit a request to do so.
 */
request = kzalloc(sizeof(*request), GFP_KERNEL);
-   if (request)
+   if (request) {
ret = i915_add_request(obj-ring, NULL, 
request);
-   else
+   if (ret)
+   kfree(request);
+   } else
ret = -ENOMEM;
}
 


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v1] drivers: Use kzalloc instead of 'kmalloc+memset', where possible

2011-05-27 Thread Rakib Mullick 
 A previous patch was sent to address this issue at, 
https://lkml.org/lkml/2011/5/23/305. Joe Perches suggest that, its best to use 
kcalloc for array allocation instead of kzalloc. This version addresses this 
issue.

Changes since V0:
   Use kcalloc instead of kzalloc for array allocation.


Signed-off-by: Rakib Mullick rakib.mull...@gmail.com
---

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 1c4b3aa..168b78f 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1638,13 +1638,12 @@ static int sata_dwc_probe(struct platform_device *ofdev)
const struct ata_port_info *ppi[] = { pi, NULL };
 
/* Allocate DWC SATA device */
-   hsdev = kmalloc(sizeof(*hsdev), GFP_KERNEL);
+   hsdev = kzalloc(sizeof(*hsdev), GFP_KERNEL);
if (hsdev == NULL) {
dev_err(ofdev-dev, kmalloc failed for hsdev\n);
err = -ENOMEM;
goto error_out;
}
-   memset(hsdev, 0, sizeof(*hsdev));
 
/* Ioremap SATA registers */
base = of_iomap(ofdev-dev.of_node, 0);
diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
index d15e09b..7525e03 100644
--- a/drivers/gpu/drm/drm_scatter.c
+++ b/drivers/gpu/drm/drm_scatter.c
@@ -83,30 +83,26 @@ int drm_sg_alloc(struct drm_device *dev, struct 
drm_scatter_gather * request)
if (dev-sg)
return -EINVAL;
 
-   entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return -ENOMEM;
 
-   memset(entry, 0, sizeof(*entry));
pages = (request-size + PAGE_SIZE - 1) / PAGE_SIZE;
DRM_DEBUG(size=%ld pages=%ld\n, request-size, pages);
 
entry-pages = pages;
-   entry-pagelist = kmalloc(pages * sizeof(*entry-pagelist), GFP_KERNEL);
+   entry-pagelist = kcalloc(pages, sizeof(*entry-pagelist), GFP_KERNEL);
if (!entry-pagelist) {
kfree(entry);
return -ENOMEM;
}
 
-   memset(entry-pagelist, 0, pages * sizeof(*entry-pagelist));
-
-   entry-busaddr = kmalloc(pages * sizeof(*entry-busaddr), GFP_KERNEL);
+   entry-busaddr = kcalloc(pages, sizeof(*entry-busaddr), GFP_KERNEL);
if (!entry-busaddr) {
kfree(entry-pagelist);
kfree(entry);
return -ENOMEM;
}
-   memset((void *)entry-busaddr, 0, pages * sizeof(*entry-busaddr));
 
entry-virtual = drm_vmalloc_dma(pages  PAGE_SHIFT);
if (!entry-virtual) {
diff --git a/drivers/gpu/drm/radeon/radeon_mem.c 
b/drivers/gpu/drm/radeon/radeon_mem.c
index ed95155..988548e 100644
--- a/drivers/gpu/drm/radeon/radeon_mem.c
+++ b/drivers/gpu/drm/radeon/radeon_mem.c
@@ -139,7 +139,7 @@ static int init_heap(struct mem_block **heap, int start, 
int size)
if (!blocks)
return -ENOMEM;
 
-   *heap = kmalloc(sizeof(**heap), GFP_KERNEL);
+   *heap = kzalloc(sizeof(**heap), GFP_KERNEL);
if (!*heap) {
kfree(blocks);
return -ENOMEM;
@@ -150,7 +150,6 @@ static int init_heap(struct mem_block **heap, int start, 
int size)
blocks-file_priv = NULL;
blocks-next = blocks-prev = *heap;
 
-   memset(*heap, 0, sizeof(**heap));
(*heap)-file_priv = (struct drm_file *) - 1;
(*heap)-next = (*heap)-prev = blocks;
return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
index f1a52f9..07ce02d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
@@ -585,11 +585,10 @@ int vmw_overlay_init(struct vmw_private *dev_priv)
return -ENOSYS;
}
 
-   overlay = kmalloc(sizeof(*overlay), GFP_KERNEL);
+   overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);
if (!overlay)
return -ENOMEM;
 
-   memset(overlay, 0, sizeof(*overlay));
mutex_init(overlay-mutex);
for (i = 0; i  VMW_MAX_NUM_STREAMS; i++) {
overlay-stream[i].buf = NULL;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 5408b1b..bfe1bcc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -612,11 +612,9 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
srf-sizes[0].height == 64 
srf-format == SVGA3D_A8R8G8B8) {
 
-   srf-snooper.image = kmalloc(64 * 64 * 4, GFP_KERNEL);
-   /* clear the image */
-   if (srf-snooper.image) {
-   memset(srf-snooper.image, 0x00, 64 * 64 * 4);
-   } else {
+   /* allocate image area and clear it */
+   srf-snooper.image = kzalloc(64 * 64 * 4, GFP_KERNEL);
+   if (!srf-snooper.image) {
DRM_ERROR(Failed to allocate

[PATCH v1] drivers: Use kzalloc instead of 'kmalloc+memset', where possible

2011-05-26 Thread Rakib Mullick 
 A previous patch was sent to address this issue at, 
https://lkml.org/lkml/2011/5/23/305. Joe Perches suggest that, its best to use 
kcalloc for array allocation instead of kzalloc. This version addresses this 
issue.

Changes since V0:
   Use kcalloc instead of kzalloc for array allocation.


Signed-off-by: Rakib Mullick 
---

diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
index 1c4b3aa..168b78f 100644
--- a/drivers/ata/sata_dwc_460ex.c
+++ b/drivers/ata/sata_dwc_460ex.c
@@ -1638,13 +1638,12 @@ static int sata_dwc_probe(struct platform_device *ofdev)
const struct ata_port_info *ppi[] = { , NULL };

/* Allocate DWC SATA device */
-   hsdev = kmalloc(sizeof(*hsdev), GFP_KERNEL);
+   hsdev = kzalloc(sizeof(*hsdev), GFP_KERNEL);
if (hsdev == NULL) {
dev_err(>dev, "kmalloc failed for hsdev\n");
err = -ENOMEM;
goto error_out;
}
-   memset(hsdev, 0, sizeof(*hsdev));

/* Ioremap SATA registers */
base = of_iomap(ofdev->dev.of_node, 0);
diff --git a/drivers/gpu/drm/drm_scatter.c b/drivers/gpu/drm/drm_scatter.c
index d15e09b..7525e03 100644
--- a/drivers/gpu/drm/drm_scatter.c
+++ b/drivers/gpu/drm/drm_scatter.c
@@ -83,30 +83,26 @@ int drm_sg_alloc(struct drm_device *dev, struct 
drm_scatter_gather * request)
if (dev->sg)
return -EINVAL;

-   entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+   entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return -ENOMEM;

-   memset(entry, 0, sizeof(*entry));
pages = (request->size + PAGE_SIZE - 1) / PAGE_SIZE;
DRM_DEBUG("size=%ld pages=%ld\n", request->size, pages);

entry->pages = pages;
-   entry->pagelist = kmalloc(pages * sizeof(*entry->pagelist), GFP_KERNEL);
+   entry->pagelist = kcalloc(pages, sizeof(*entry->pagelist), GFP_KERNEL);
if (!entry->pagelist) {
kfree(entry);
return -ENOMEM;
}

-   memset(entry->pagelist, 0, pages * sizeof(*entry->pagelist));
-
-   entry->busaddr = kmalloc(pages * sizeof(*entry->busaddr), GFP_KERNEL);
+   entry->busaddr = kcalloc(pages, sizeof(*entry->busaddr), GFP_KERNEL);
if (!entry->busaddr) {
kfree(entry->pagelist);
kfree(entry);
return -ENOMEM;
}
-   memset((void *)entry->busaddr, 0, pages * sizeof(*entry->busaddr));

entry->virtual = drm_vmalloc_dma(pages << PAGE_SHIFT);
if (!entry->virtual) {
diff --git a/drivers/gpu/drm/radeon/radeon_mem.c 
b/drivers/gpu/drm/radeon/radeon_mem.c
index ed95155..988548e 100644
--- a/drivers/gpu/drm/radeon/radeon_mem.c
+++ b/drivers/gpu/drm/radeon/radeon_mem.c
@@ -139,7 +139,7 @@ static int init_heap(struct mem_block **heap, int start, 
int size)
if (!blocks)
return -ENOMEM;

-   *heap = kmalloc(sizeof(**heap), GFP_KERNEL);
+   *heap = kzalloc(sizeof(**heap), GFP_KERNEL);
if (!*heap) {
kfree(blocks);
return -ENOMEM;
@@ -150,7 +150,6 @@ static int init_heap(struct mem_block **heap, int start, 
int size)
blocks->file_priv = NULL;
blocks->next = blocks->prev = *heap;

-   memset(*heap, 0, sizeof(**heap));
(*heap)->file_priv = (struct drm_file *) - 1;
(*heap)->next = (*heap)->prev = blocks;
return 0;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
index f1a52f9..07ce02d 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c
@@ -585,11 +585,10 @@ int vmw_overlay_init(struct vmw_private *dev_priv)
return -ENOSYS;
}

-   overlay = kmalloc(sizeof(*overlay), GFP_KERNEL);
+   overlay = kzalloc(sizeof(*overlay), GFP_KERNEL);
if (!overlay)
return -ENOMEM;

-   memset(overlay, 0, sizeof(*overlay));
mutex_init(>mutex);
for (i = 0; i < VMW_MAX_NUM_STREAMS; i++) {
overlay->stream[i].buf = NULL;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 5408b1b..bfe1bcc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -612,11 +612,9 @@ int vmw_surface_define_ioctl(struct drm_device *dev, void 
*data,
srf->sizes[0].height == 64 &&
srf->format == SVGA3D_A8R8G8B8) {

-   srf->snooper.image = kmalloc(64 * 64 * 4, GFP_KERNEL);
-   /* clear the image */
-   if (srf->snooper.image) {
-   memset(srf->snooper.image, 0x00, 64 * 64 * 4);
-   } else {
+   /* allocate image area and clear it */
+   srf->snooper.image

[PATCH] drivers: Use kzalloc instead of 'kmalloc+memset', where possible.

2011-05-25 Thread Rakib Mullick 
On Tue, May 24, 2011 at 11:09 PM, Joe Perches  wrote:

> On Tue, 2011-05-24 at 22:59 +0600, Rakib Mullick wrote:
> > On 5/23/11, Joe Perches  wrote:
> > > On Mon, 2011-05-23 at 23:40 +0600, Rakib Mullick wrote:
> > >> Following patch removes the uses of 'kmalloc+memset' from various
> > >> drivers subsystems, which is replaced by kzalloc. kzalloc take care of
> > >> setting allocated memory with null, so it helps to get rid from using
> > >> memset.
> > >> diff --git a/drivers/gpu/drm/drm_scatter.c
> b/drivers/gpu/drm/drm_scatter.c
> > > []
> > >> -  entry->pagelist = kmalloc(pages * sizeof(*entry->pagelist),
> GFP_KERNEL);
> > >> +  entry->pagelist = kzalloc(pages * sizeof(*entry->pagelist),
> GFP_KERNEL);
> > > Perhaps it's better to use:
> > > entry->pagelist =  kcalloc(pages, sizeof(*entry->pagelist),
> GFP_KERNEL);
> > >> -  entry->busaddr = kmalloc(pages * sizeof(*entry->busaddr),
> GFP_KERNEL);
> > >> +  entry->busaddr = kzalloc(pages * sizeof(*entry->busaddr),
> GFP_KERNEL);
> > > here too.
> > Is there any significant benefit of using kcalloc here?
>
> Overflow and tradition.
>
> static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> {
>if (size != 0 && n > ULONG_MAX / size)
>return NULL;
>return __kmalloc(n * size, flags | __GFP_ZERO);
> }
>
> It's been used for allocating memory for an array. Maybe, using kcalloc in
entry->pagelist could be useful. But, not that much sure though. There's no
problem with current implementation.

This patch touches few drivers subsystems, they are added to the CC list.

Thanks,
Rakib

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20110524/188e6fed/attachment.htm>

Re: [PATCH] drivers: Use kzalloc instead of 'kmalloc+memset', where possible.

2011-05-25 Thread Rakib Mullick 
On Tue, May 24, 2011 at 11:09 PM, Joe Perches j...@perches.com wrote:

 On Tue, 2011-05-24 at 22:59 +0600, Rakib Mullick wrote:
  On 5/23/11, Joe Perches j...@perches.com wrote:
   On Mon, 2011-05-23 at 23:40 +0600, Rakib Mullick wrote:
   Following patch removes the uses of 'kmalloc+memset' from various
   drivers subsystems, which is replaced by kzalloc. kzalloc take care of
   setting allocated memory with null, so it helps to get rid from using
   memset.
   diff --git a/drivers/gpu/drm/drm_scatter.c
 b/drivers/gpu/drm/drm_scatter.c
   []
   -  entry-pagelist = kmalloc(pages * sizeof(*entry-pagelist),
 GFP_KERNEL);
   +  entry-pagelist = kzalloc(pages * sizeof(*entry-pagelist),
 GFP_KERNEL);
   Perhaps it's better to use:
   entry-pagelist =  kcalloc(pages, sizeof(*entry-pagelist),
 GFP_KERNEL);
   -  entry-busaddr = kmalloc(pages * sizeof(*entry-busaddr),
 GFP_KERNEL);
   +  entry-busaddr = kzalloc(pages * sizeof(*entry-busaddr),
 GFP_KERNEL);
   here too.
  Is there any significant benefit of using kcalloc here?

 Overflow and tradition.

 static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
 {
if (size != 0  n  ULONG_MAX / size)
return NULL;
return __kmalloc(n * size, flags | __GFP_ZERO);
 }

 It's been used for allocating memory for an array. Maybe, using kcalloc in
entry-pagelist could be useful. But, not that much sure though. There's no
problem with current implementation.

This patch touches few drivers subsystems, they are added to the CC list.

Thanks,
Rakib


 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel