* [PATCH -mm] relayfs: support larger relay buffer
@ 2008-04-15 21:47 Masami Hiramatsu
2008-04-16 7:27 ` Tom Zanussi
2008-04-16 9:34 ` [PATCH -mm] relayfs: support larger relay buffer Pekka Enberg
0 siblings, 2 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-15 21:47 UTC (permalink / raw)
To: David Wilder, Tom Zanussi, Andrew Morton; +Cc: systemtap-ml, LKML
Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
when the array size is bigger than one page. This enables relayfs to support
bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
---
This is useful for a 64-bit system which has a plenty of memory (tens of
giga bytes) and a large kernel memory space.
I tested it on x86-64 and ia64.
kernel/relay.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
Index: 2.6.25-rc8-mm2/kernel/relay.c
===================================================================
--- 2.6.25-rc8-mm2.orig/kernel/relay.c
+++ 2.6.25-rc8-mm2/kernel/relay.c
@@ -104,12 +104,20 @@ static int relay_mmap_buf(struct rchan_b
static void *relay_alloc_buf(struct rchan_buf *buf, size_t *size)
{
void *mem;
- unsigned int i, j, n_pages;
+ unsigned int i, j, n_pages, pa_size;
*size = PAGE_ALIGN(*size);
n_pages = *size >> PAGE_SHIFT;
+ pa_size = n_pages * sizeof(struct page *);
- buf->page_array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
+ if (pa_size > PAGE_SIZE) {
+ buf->page_array = vmalloc(pa_size);
+ if (buf->page_array)
+ memset(buf->page_array, 0, pa_size);
+ } else {
+ buf->page_array = kcalloc(n_pages, sizeof(struct page *),
+ GFP_KERNEL);
+ }
if (!buf->page_array)
return NULL;
@@ -130,7 +138,10 @@ static void *relay_alloc_buf(struct rcha
depopulate:
for (j = 0; j < i; j++)
__free_page(buf->page_array[j]);
- kfree(buf->page_array);
+ if (pa_size > PAGE_SIZE)
+ vfree(buf->page_array);
+ else
+ kfree(buf->page_array);
return NULL;
}
@@ -189,7 +200,10 @@ static void relay_destroy_buf(struct rch
vunmap(buf->start);
for (i = 0; i < buf->page_count; i++)
__free_page(buf->page_array[i]);
- kfree(buf->page_array);
+ if (buf->page_count * sizeof(struct page *) > PAGE_SIZE)
+ vfree(buf->page_array);
+ else
+ kfree(buf->page_array);
}
chan->buf[buf->cpu] = NULL;
kfree(buf->padding);
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer
2008-04-15 21:47 [PATCH -mm] relayfs: support larger relay buffer Masami Hiramatsu
@ 2008-04-16 7:27 ` Tom Zanussi
2008-04-16 18:21 ` Masami Hiramatsu
2008-04-16 18:34 ` [PATCH -mm] relayfs: support larger relay buffer take 2 Masami Hiramatsu
2008-04-16 9:34 ` [PATCH -mm] relayfs: support larger relay buffer Pekka Enberg
1 sibling, 2 replies; 13+ messages in thread
From: Tom Zanussi @ 2008-04-16 7:27 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: David Wilder, Andrew Morton, systemtap-ml, LKML, tzanussi
On Tue, 2008-04-15 at 11:27 -0400, Masami Hiramatsu wrote:
> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
> when the array size is bigger than one page. This enables relayfs to support
> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
> This is useful for a 64-bit system which has a plenty of memory (tens of
> giga bytes) and a large kernel memory space.
>
> I tested it on x86-64 and ia64.
>
Hi,
It looks ok to me, but it might be a little cleaner and avoid some
duplication if you add the new code as a couple of functions instead.
Just a suggestion...
Tom
> kernel/relay.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> Index: 2.6.25-rc8-mm2/kernel/relay.c
> ===================================================================
> --- 2.6.25-rc8-mm2.orig/kernel/relay.c
> +++ 2.6.25-rc8-mm2/kernel/relay.c
> @@ -104,12 +104,20 @@ static int relay_mmap_buf(struct rchan_b
> static void *relay_alloc_buf(struct rchan_buf *buf, size_t *size)
> {
> void *mem;
> - unsigned int i, j, n_pages;
> + unsigned int i, j, n_pages, pa_size;
>
> *size = PAGE_ALIGN(*size);
> n_pages = *size >> PAGE_SHIFT;
> + pa_size = n_pages * sizeof(struct page *);
>
> - buf->page_array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
> + if (pa_size > PAGE_SIZE) {
> + buf->page_array = vmalloc(pa_size);
> + if (buf->page_array)
> + memset(buf->page_array, 0, pa_size);
> + } else {
> + buf->page_array = kcalloc(n_pages, sizeof(struct page *),
> + GFP_KERNEL);
> + }
> if (!buf->page_array)
> return NULL;
>
> @@ -130,7 +138,10 @@ static void *relay_alloc_buf(struct rcha
> depopulate:
> for (j = 0; j < i; j++)
> __free_page(buf->page_array[j]);
> - kfree(buf->page_array);
> + if (pa_size > PAGE_SIZE)
> + vfree(buf->page_array);
> + else
> + kfree(buf->page_array);
> return NULL;
> }
>
> @@ -189,7 +200,10 @@ static void relay_destroy_buf(struct rch
> vunmap(buf->start);
> for (i = 0; i < buf->page_count; i++)
> __free_page(buf->page_array[i]);
> - kfree(buf->page_array);
> + if (buf->page_count * sizeof(struct page *) > PAGE_SIZE)
> + vfree(buf->page_array);
> + else
> + kfree(buf->page_array);
> }
> chan->buf[buf->cpu] = NULL;
> kfree(buf->padding);
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer
2008-04-15 21:47 [PATCH -mm] relayfs: support larger relay buffer Masami Hiramatsu
2008-04-16 7:27 ` Tom Zanussi
@ 2008-04-16 9:34 ` Pekka Enberg
2008-04-16 15:23 ` Masami Hiramatsu
1 sibling, 1 reply; 13+ messages in thread
From: Pekka Enberg @ 2008-04-16 9:34 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: David Wilder, Tom Zanussi, Andrew Morton, systemtap-ml, LKML
On Tue, Apr 15, 2008 at 6:27 PM, Masami Hiramatsu <mhiramat@redhat.com> wrote:
> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
> when the array size is bigger than one page. This enables relayfs to support
> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
> @@ -130,7 +138,10 @@ static void *relay_alloc_buf(struct rcha
> depopulate:
> for (j = 0; j < i; j++)
> __free_page(buf->page_array[j]);
> - kfree(buf->page_array);
> + if (pa_size > PAGE_SIZE)
You can use is_vmalloc_addr() here.
> + vfree(buf->page_array);
> + else
> + kfree(buf->page_array);
> return NULL;
> }
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer
2008-04-16 9:34 ` [PATCH -mm] relayfs: support larger relay buffer Pekka Enberg
@ 2008-04-16 15:23 ` Masami Hiramatsu
0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-16 15:23 UTC (permalink / raw)
To: Pekka Enberg; +Cc: David Wilder, Tom Zanussi, Andrew Morton, systemtap-ml, LKML
Hi Pekka,
Pekka Enberg wrote:
> On Tue, Apr 15, 2008 at 6:27 PM, Masami Hiramatsu <mhiramat@redhat.com> wrote:
>> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
>> when the array size is bigger than one page. This enables relayfs to support
>> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>>
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> ---
>> @@ -130,7 +138,10 @@ static void *relay_alloc_buf(struct rcha
>> depopulate:
>> for (j = 0; j < i; j++)
>> __free_page(buf->page_array[j]);
>> - kfree(buf->page_array);
>> + if (pa_size > PAGE_SIZE)
>
> You can use is_vmalloc_addr() here.
Thank you for your good advice!
I'll use that.
>
>> + vfree(buf->page_array);
>> + else
>> + kfree(buf->page_array);
>> return NULL;
>> }
>>
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer
2008-04-16 7:27 ` Tom Zanussi
@ 2008-04-16 18:21 ` Masami Hiramatsu
2008-04-16 18:34 ` [PATCH -mm] relayfs: support larger relay buffer take 2 Masami Hiramatsu
1 sibling, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-16 18:21 UTC (permalink / raw)
To: Tom Zanussi; +Cc: David Wilder, Andrew Morton, systemtap-ml, LKML, tzanussi
Hi Tom,
Tom Zanussi wrote:
> On Tue, 2008-04-15 at 11:27 -0400, Masami Hiramatsu wrote:
>> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
>> when the array size is bigger than one page. This enables relayfs to support
>> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>>
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> ---
>> This is useful for a 64-bit system which has a plenty of memory (tens of
>> giga bytes) and a large kernel memory space.
>>
>> I tested it on x86-64 and ia64.
>>
>
> Hi,
>
> It looks ok to me, but it might be a little cleaner and avoid some
> duplication if you add the new code as a couple of functions instead.
> Just a suggestion...
Sure, that is a good idea, I'll renew my patch.
Thank you,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH -mm] relayfs: support larger relay buffer take 2
2008-04-16 7:27 ` Tom Zanussi
2008-04-16 18:21 ` Masami Hiramatsu
@ 2008-04-16 18:34 ` Masami Hiramatsu
2008-04-16 19:52 ` Pekka J Enberg
1 sibling, 1 reply; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-16 18:34 UTC (permalink / raw)
To: Tom Zanussi, David Wilder, Andrew Morton, Pekka Enberg
Cc: systemtap-ml, LKML, tzanussi
Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
when the array size is bigger than one page. This enables relayfs to support
bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
---
Changes from take1 to take2:
- add relay_alloc_page_array() and relay_free_page_array()
- use is_vmalloc_addr() instead of checking array size.
This is useful for a 64-bit system which has a plenty of memory (tens of
giga bytes) and a large kernel memory space.
I tested it on x86-64 and ia64.
kernel/relay.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
Index: 2.6.25-rc8-mm2/kernel/relay.c
===================================================================
--- 2.6.25-rc8-mm2.orig/kernel/relay.c
+++ 2.6.25-rc8-mm2/kernel/relay.c
@@ -27,6 +27,29 @@
static DEFINE_MUTEX(relay_channels_mutex);
static LIST_HEAD(relay_channels);
+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;
+}
+
+static void relay_free_page_array(struct page *array)
+{
+ if (is_vmalloc_addr(array))
+ vfree(array);
+ else
+ kfree(array);
+}
+
/*
* close() vm_op implementation for relay file mapping.
*/
@@ -109,7 +132,7 @@ static void *relay_alloc_buf(struct rcha
*size = PAGE_ALIGN(*size);
n_pages = *size >> PAGE_SHIFT;
- buf->page_array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
+ buf->page_array = relay_alloc_page_array(n_pages);
if (!buf->page_array)
return NULL;
@@ -130,7 +153,7 @@ static void *relay_alloc_buf(struct rcha
depopulate:
for (j = 0; j < i; j++)
__free_page(buf->page_array[j]);
- kfree(buf->page_array);
+ relay_free_page_array(buf->page_array);
return NULL;
}
@@ -189,7 +212,7 @@ static void relay_destroy_buf(struct rch
vunmap(buf->start);
for (i = 0; i < buf->page_count; i++)
__free_page(buf->page_array[i]);
- kfree(buf->page_array);
+ relay_free_page_array(buf->page_array);
}
chan->buf[buf->cpu] = NULL;
kfree(buf->padding);
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 2
2008-04-16 18:34 ` [PATCH -mm] relayfs: support larger relay buffer take 2 Masami Hiramatsu
@ 2008-04-16 19:52 ` Pekka J Enberg
2008-04-16 19:56 ` Masami Hiramatsu
0 siblings, 1 reply; 13+ messages in thread
From: Pekka J Enberg @ 2008-04-16 19:52 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Tom Zanussi, David Wilder, Andrew Morton, systemtap-ml, LKML, tzanussi
Hi Masami,
On Wed, 16 Apr 2008, Masami Hiramatsu 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;
> +}
I think it's bit confusing to have relay_alloc_page_array() return a
pointer to struct page as it's really allocating an _array_ of pointers to
struct page. So why not just use void * here as the kernel memory
allocators do?
> +static void relay_free_page_array(struct page *array)
> +{
> + if (is_vmalloc_addr(array))
> + vfree(array);
> + else
> + kfree(array);
> +}
Here as well.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 2
2008-04-16 19:52 ` Pekka J Enberg
@ 2008-04-16 19:56 ` Masami Hiramatsu
2008-04-16 20:49 ` [PATCH -mm] relayfs: support larger relay buffer take 3 Masami Hiramatsu
0 siblings, 1 reply; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-16 19:56 UTC (permalink / raw)
To: Pekka J Enberg
Cc: Tom Zanussi, David Wilder, Andrew Morton, systemtap-ml, LKML, tzanussi
Hi,
Pekka J Enberg wrote:
> Hi Masami,
>
> On Wed, 16 Apr 2008, Masami Hiramatsu 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;
>> +}
>
> I think it's bit confusing to have relay_alloc_page_array() return a
> pointer to struct page as it's really allocating an _array_ of pointers to
> struct page. So why not just use void * here as the kernel memory
> allocators do?
Thank you very much, it was my mistake.
It should return struct page **.
>> +static void relay_free_page_array(struct page *array)
>> +{
>> + if (is_vmalloc_addr(array))
>> + vfree(array);
>> + else
>> + kfree(array);
>> +}
>
> Here as well.
Thanks,
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH -mm] relayfs: support larger relay buffer take 3
2008-04-16 19:56 ` Masami Hiramatsu
@ 2008-04-16 20:49 ` Masami Hiramatsu
2008-04-16 21:00 ` Pekka Enberg
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-16 20:49 UTC (permalink / raw)
To: Pekka J Enberg, Tom Zanussi, David Wilder, Andrew Morton
Cc: systemtap-ml, LKML, tzanussi
Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
when the array size is bigger than one page. This enables relayfs to support
bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
---
Changes from take2 to take3:
- Use struct page ** instead of struct page *.
- move functions to the place before relay_mmap_buf.
- add comments.
This is useful for a 64-bit system which has a plenty of memory (tens of
giga bytes) and a large kernel memory space.
I tested it on x86-64 and ia64.
kernel/relay.c | 35 ++++++++++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 3 deletions(-)
Index: 2.6.25-rc8-mm2/kernel/relay.c
===================================================================
--- 2.6.25-rc8-mm2.orig/kernel/relay.c
+++ 2.6.25-rc8-mm2/kernel/relay.c
@@ -65,6 +65,35 @@ static struct vm_operations_struct relay
.close = relay_file_mmap_close,
};
+/*
+ * allocate an array of pointers of struct page
+ */
+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;
+}
+
+/*
+ * free an array of pointers of struct page
+ */
+static void relay_free_page_array(struct page **array)
+{
+ if (is_vmalloc_addr(array))
+ vfree(array);
+ else
+ kfree(array);
+}
+
/**
* relay_mmap_buf: - mmap channel buffer to process address space
* @buf: relay channel buffer
@@ -109,7 +138,7 @@ static void *relay_alloc_buf(struct rcha
*size = PAGE_ALIGN(*size);
n_pages = *size >> PAGE_SHIFT;
- buf->page_array = kcalloc(n_pages, sizeof(struct page *), GFP_KERNEL);
+ buf->page_array = relay_alloc_page_array(n_pages);
if (!buf->page_array)
return NULL;
@@ -130,7 +159,7 @@ static void *relay_alloc_buf(struct rcha
depopulate:
for (j = 0; j < i; j++)
__free_page(buf->page_array[j]);
- kfree(buf->page_array);
+ relay_free_page_array(buf->page_array);
return NULL;
}
@@ -189,7 +218,7 @@ static void relay_destroy_buf(struct rch
vunmap(buf->start);
for (i = 0; i < buf->page_count; i++)
__free_page(buf->page_array[i]);
- kfree(buf->page_array);
+ relay_free_page_array(buf->page_array);
}
chan->buf[buf->cpu] = NULL;
kfree(buf->padding);
--
Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division
e-mail: mhiramat@redhat.com
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 3
2008-04-16 20:49 ` [PATCH -mm] relayfs: support larger relay buffer take 3 Masami Hiramatsu
@ 2008-04-16 21:00 ` Pekka Enberg
2008-04-16 23:01 ` Andrew Morton
2008-04-17 8:14 ` Tom Zanussi
2 siblings, 0 replies; 13+ messages in thread
From: Pekka Enberg @ 2008-04-16 21:00 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Tom Zanussi, David Wilder, Andrew Morton, systemtap-ml, LKML, tzanussi
Masami Hiramatsu wrote:
> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
> when the array size is bigger than one page. This enables relayfs to support
> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Looks good to me!
Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 3
2008-04-16 20:49 ` [PATCH -mm] relayfs: support larger relay buffer take 3 Masami Hiramatsu
2008-04-16 21:00 ` Pekka Enberg
@ 2008-04-16 23:01 ` Andrew Morton
2008-04-17 1:29 ` Masami Hiramatsu
2008-04-17 8:14 ` Tom Zanussi
2 siblings, 1 reply; 13+ messages in thread
From: Andrew Morton @ 2008-04-16 23:01 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: penberg, zanussi, dwilder, systemtap, linux-kernel, tzanussi
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?
--- 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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 3
2008-04-16 23:01 ` Andrew Morton
@ 2008-04-17 1:29 ` Masami Hiramatsu
0 siblings, 0 replies; 13+ messages in thread
From: Masami Hiramatsu @ 2008-04-17 1:29 UTC (permalink / raw)
To: Andrew Morton
Cc: penberg, zanussi, dwilder, systemtap, linux-kernel, tzanussi
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -mm] relayfs: support larger relay buffer take 3
2008-04-16 20:49 ` [PATCH -mm] relayfs: support larger relay buffer take 3 Masami Hiramatsu
2008-04-16 21:00 ` Pekka Enberg
2008-04-16 23:01 ` Andrew Morton
@ 2008-04-17 8:14 ` Tom Zanussi
2 siblings, 0 replies; 13+ messages in thread
From: Tom Zanussi @ 2008-04-17 8:14 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Pekka J Enberg, David Wilder, Andrew Morton, systemtap-ml, LKML
On Wed, 2008-04-16 at 15:51 -0400, Masami Hiramatsu wrote:
> Use vmalloc() and memset() instead of kcalloc() to allocate a page* array
> when the array size is bigger than one page. This enables relayfs to support
> bigger relay buffers than 64MB on 4k-page system, 512MB on 16k-page system.
>
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> ---
> Changes from take2 to take3:
> - Use struct page ** instead of struct page *.
> - move functions to the place before relay_mmap_buf.
> - add comments.
>
> This is useful for a 64-bit system which has a plenty of memory (tens of
> giga bytes) and a large kernel memory space.
>
> I tested it on x86-64 and ia64.
>
Hi,
Looks fine to me.
Reviewed-by: Tom Zanussi <tzanussi@gmail.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2008-04-17 4:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-04-15 21:47 [PATCH -mm] relayfs: support larger relay buffer Masami Hiramatsu
2008-04-16 7:27 ` Tom Zanussi
2008-04-16 18:21 ` Masami Hiramatsu
2008-04-16 18:34 ` [PATCH -mm] relayfs: support larger relay buffer take 2 Masami Hiramatsu
2008-04-16 19:52 ` Pekka J Enberg
2008-04-16 19:56 ` Masami Hiramatsu
2008-04-16 20:49 ` [PATCH -mm] relayfs: support larger relay buffer take 3 Masami Hiramatsu
2008-04-16 21:00 ` Pekka Enberg
2008-04-16 23:01 ` Andrew Morton
2008-04-17 1:29 ` Masami Hiramatsu
2008-04-17 8:14 ` Tom Zanussi
2008-04-16 9:34 ` [PATCH -mm] relayfs: support larger relay buffer Pekka Enberg
2008-04-16 15:23 ` Masami Hiramatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).