lkml.org 
[lkml]   [2008]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -mm] relayfs: support larger relay buffer take 3
Hi Andrew,

Andrew Morton wrote:
> On Wed, 16 Apr 2008 15:51:56 -0400
> Masami Hiramatsu <mhiramat@redhat.com> wrote:
>
>> +static struct page **relay_alloc_page_array(unsigned int n_pages)
>> +{
>> + struct page **array;
>> + unsigned int pa_size = n_pages * sizeof(struct page *);
>> +
>> + if (pa_size > PAGE_SIZE) {
>> + array = vmalloc(pa_size);
>> + if (array)
>> + memset(array, 0, pa_size);
>> + } else {
>> + array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
>> + }
>> + return array;
>> +}
>
> It's a bit odd to multiply n_pages*sizeof() and to then call kcalloc(),
> which needs to do the same multiplication.
>
> The compiler will presumably optimise that away, but still, how about this?

Sure, it looks good to me.
Thank you so much,

Acked-by: Masami Hiramatsu <mhiramat@redhat.com>

>
> --- a/kernel/relay.c~relayfs-support-larger-relay-buffer-take-3-cleanup
> +++ a/kernel/relay.c
> @@ -71,14 +71,14 @@ static struct vm_operations_struct relay
> static struct page **relay_alloc_page_array(unsigned int n_pages)
> {
> struct page **array;
> - unsigned int pa_size = n_pages * sizeof(struct page *);
> + size_t pa_size = n_pages * sizeof(struct page *);
>
> if (pa_size > PAGE_SIZE) {
> array = vmalloc(pa_size);
> if (array)
> memset(array, 0, pa_size);
> } else {
> - array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
> + array = kzalloc(pa_size, GFP_KERNEL);
> }
> return array;
> }
> _
>
>
> size_t is strictly the correct type for pa_size here. Even though
> vmalloc() takes a ulong.

Thanks for the advice,


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com



\
 
 \ /
  Last update: 2008-04-16 23:05    [W:0.125 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site