This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [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