2017-03-13 10:58:34

by Daeseok Youn

[permalink] [raw]
Subject: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

If the atomisp_kernel_zalloc() has "true" as a second parameter, it
tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
But using kzalloc is rather than kmalloc followed by memset with 0.
(vzalloc is for same reason with kzalloc)

And also atomisp_kernel_malloc() can be used with
atomisp_kernel_zalloc(<size>, false);

Signed-off-by: Daeseok Youn <[email protected]>
---
I think kvmalloc() or kvzalloc() can be used to allocate memory if there is
no reason to use vmalloc() when the requested bytes is over PAGE_SIZE.

.../media/atomisp/pci/atomisp2/atomisp_cmd.c | 25 ++++++++++++----------
1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index d9a5c24..44b2244 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -86,32 +86,35 @@
};

/*
- * atomisp_kernel_malloc: chooses whether kmalloc() or vmalloc() is preferable.
+ * atomisp_kernel_malloc:
+ * allocating memory from atomisp_kernel_zalloc() without zeroing memory.
*
* It is also a wrap functions to pass into css framework.
*/
void *atomisp_kernel_malloc(size_t bytes)
{
- /* vmalloc() is preferable if allocating more than 1 page */
- if (bytes > PAGE_SIZE)
- return vmalloc(bytes);
-
- return kmalloc(bytes, GFP_KERNEL);
+ return atomisp_kernel_zalloc(bytes, false);
}

/*
- * atomisp_kernel_zalloc: chooses whether set 0 to the allocated memory.
+ * atomisp_kernel_zalloc: chooses whether set 0 to the allocated memory
+ * with k{z,m}alloc or v{z,m}alloc
*
* It is also a wrap functions to pass into css framework.
*/
void *atomisp_kernel_zalloc(size_t bytes, bool zero_mem)
{
- void *ptr = atomisp_kernel_malloc(bytes);
+ /* vmalloc() is preferable if allocating more than 1 page */
+ if (bytes > PAGE_SIZE) {
+ if (zero_mem)
+ return vzalloc(bytes);
+ return vmalloc(bytes);
+ }

- if (ptr && zero_mem)
- memset(ptr, 0, bytes);
+ if (zero_mem)
+ return kzalloc(bytes, GFP_KERNEL);

- return ptr;
+ return kmalloc(bytes, GFP_KERNEL);
}

/*
--
1.9.1


2017-03-13 11:52:23

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

On Mon, Mar 13, 2017 at 07:54:21PM +0900, Daeseok Youn wrote:
> If the atomisp_kernel_zalloc() has "true" as a second parameter, it
> tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
> But using kzalloc is rather than kmalloc followed by memset with 0.
> (vzalloc is for same reason with kzalloc)
>
> And also atomisp_kernel_malloc() can be used with
> atomisp_kernel_zalloc(<size>, false);
>

We should just change all the callers to kvmalloc() and kvzmalloc().

regards,
dan carpenter

2017-03-13 14:07:34

by Daeseok Youn

[permalink] [raw]
Subject: Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

2017-03-13 20:51 GMT+09:00 Dan Carpenter <[email protected]>:
> On Mon, Mar 13, 2017 at 07:54:21PM +0900, Daeseok Youn wrote:
>> If the atomisp_kernel_zalloc() has "true" as a second parameter, it
>> tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
>> But using kzalloc is rather than kmalloc followed by memset with 0.
>> (vzalloc is for same reason with kzalloc)
>>
>> And also atomisp_kernel_malloc() can be used with
>> atomisp_kernel_zalloc(<size>, false);
>>
>
> We should just change all the callers to kvmalloc() and kvzmalloc().
ok. I will try to change all the callers to kvmalloc() and kvzalloc().

Thanks.
Regards,
Daeseok Youn
>
> regards,
> dan carpenter
>

2017-03-13 15:40:14

by Daeseok Youn

[permalink] [raw]
Subject: Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

2017-03-13 23:07 GMT+09:00 DaeSeok Youn <[email protected]>:
> 2017-03-13 20:51 GMT+09:00 Dan Carpenter <[email protected]>:
>> On Mon, Mar 13, 2017 at 07:54:21PM +0900, Daeseok Youn wrote:
>>> If the atomisp_kernel_zalloc() has "true" as a second parameter, it
>>> tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
>>> But using kzalloc is rather than kmalloc followed by memset with 0.
>>> (vzalloc is for same reason with kzalloc)
>>>
>>> And also atomisp_kernel_malloc() can be used with
>>> atomisp_kernel_zalloc(<size>, false);
>>>
>>
>> We should just change all the callers to kvmalloc() and kvzmalloc().
> ok. I will try to change all the callers to kvmalloc() and kvzalloc().

The kvmalloc() and kvzalloc() are not ready to use in staging-testing
branch on staging tree.
If the kvmalloc and kvzalloc are available to use, I will replace
atomisp_kernel_malloc() and atomisp_kernel_zalloc() with kvmalloc()
and kvzalloc().

Thanks.
Regards,
Daeseok Youn.


>
> Thanks.
> Regards,
> Daeseok Youn
>>
>> regards,
>> dan carpenter
>>

2017-03-13 17:54:58

by Alan

[permalink] [raw]
Subject: Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

On Mon, 2017-03-13 at 19:54 +0900, Daeseok Youn wrote:
> If the atomisp_kernel_zalloc() has "true" as a second parameter, it
> tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
> But using kzalloc is rather than kmalloc followed by memset with 0.
> (vzalloc is for same reason with kzalloc)

This is true but please don't apply this. There are about five other
layers of indirection for memory allocators that want removing first so
that the driver just uses the correct kmalloc/kzalloc/kv* functions in
the right places.

Alan

2017-03-14 01:15:59

by Daeseok Youn

[permalink] [raw]
Subject: Re: [PATCH] staging: atomisp: use k{v}zalloc instead of k{v}alloc and memset

2017-03-14 2:54 GMT+09:00 Alan Cox <[email protected]>:
>
> On Mon, 2017-03-13 at 19:54 +0900, Daeseok Youn wrote:
> > If the atomisp_kernel_zalloc() has "true" as a second parameter, it
> > tries to allocate zeroing memory from kmalloc(vmalloc) and memset.
> > But using kzalloc is rather than kmalloc followed by memset with 0.
> > (vzalloc is for same reason with kzalloc)
>
> This is true but please don't apply this. There are about five other
> layers of indirection for memory allocators that want removing first so
> that the driver just uses the correct kmalloc/kzalloc/kv* functions in
> the right places.
right. kvmalloc/kvzalloc would be used after preparing those
interfaces in staging tree.
I will try to change all the atomisp_kernel_m{z}alloc() callers to
correct functions to allocate memory.

Thanks.
Regards,
Jake.

>
> Alan
>