[patch, 2.5] __vmalloc allocates spurious page?

[patch, 2.5] __vmalloc allocates spurious page?

[Marcus Alanen]

Hi,

I think __vmalloc allocates an unnecessary page. While the virtual 
address space should have a one-page hole in it, that is taken care of 
by get_vm_area. All other routines (particularly map_vm_area)
subtract PAGE_SIZE from area->size before usage, so the last page 
table entry isn't even set up.

The unnecessary page is allocated only if size is initially a multiple 
of PAGE_SIZE, which sounds like a common case.

Marcus

diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude linus-2.5.42/mm/vmalloc.c msa-2.5.42/mm/vmalloc.c
--- linus-2.5.42/mm/vmalloc.c	Sat Oct 12 16:42:57 2002
+++ msa-2.5.42/mm/vmalloc.c	Tue Oct 15 21:53:10 2002
@@ -387,7 +387,7 @@
 	if (!area)
 		return NULL;
 
-	nr_pages = (size+PAGE_SIZE) >> PAGE_SHIFT;
+	nr_pages = (size+PAGE_SIZE-1) >> PAGE_SHIFT;
 	array_size = (nr_pages * sizeof(struct page *));
 
 	area->nr_pages = nr_pages;

-- 
Marcus Alanen                       * Software Construction Laboratory *
marcus.alanen@abo.fi   * http://www.tucs.fi/Research/labs/softcons.php *

Re: [patch, 2.5] __vmalloc allocates spurious page?

[Marcus Alanen]

>The unnecessary page is allocated only if size is initially a multiple 
>of PAGE_SIZE, which sounds like a common case.

Actually, size is already PAGE_ALIGNed, so we get the amount of pages
even easier.

Marcus


diff -Naurd --exclude-from=/home/maalanen/linux/base/diff_exclude linus-2.5.42/mm/vmalloc.c msa-2.5.42/mm/vmalloc.c
--- linus-2.5.42/mm/vmalloc.c	Sat Oct 12 16:42:57 2002
+++ msa-2.5.42/mm/vmalloc.c	Wed Oct 16 00:52:33 2002
@@ -387,7 +387,7 @@
 	if (!area)
 		return NULL;
 
-	nr_pages = (size+PAGE_SIZE) >> PAGE_SHIFT;
+	nr_pages = size >> PAGE_SHIFT;
 	array_size = (nr_pages * sizeof(struct page *));
 
 	area->nr_pages = nr_pages;

Re: [patch, 2.5] __vmalloc allocates spurious page?

[Russell King]

On Wed, Oct 16, 2002 at 12:58:12AM +0300, Marcus Alanen wrote:
> >The unnecessary page is allocated only if size is initially a multiple 
> >of PAGE_SIZE, which sounds like a common case.
> 
> Actually, size is already PAGE_ALIGNed, so we get the amount of pages
> even easier.

IIRC, back in the dim and distant past, the extra page was originally to
catch things running off the end of their space (eg, modules).  The
idea was that modules (and other vmalloc'd areas) would be separated
by one unmapped page.

It looks like this got broken recently though.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: [patch, 2.5] __vmalloc allocates spurious page?

[William Lee Irwin III]

On Wed, Oct 16, 2002 at 12:58:12AM +0300, Marcus Alanen wrote:
>> Actually, size is already PAGE_ALIGNed, so we get the amount of pages
>> even easier.

On Tue, Oct 15, 2002 at 11:05:06PM +0100, Russell King wrote:
> IIRC, back in the dim and distant past, the extra page was originally to
> catch things running off the end of their space (eg, modules).  The
> idea was that modules (and other vmalloc'd areas) would be separated
> by one unmapped page.
> It looks like this got broken recently though.

Hmm? I looked briefly to check the patch and the guard page was added
onto the thing elsewhere in vmalloc.c


Bill

Re: [patch, 2.5] __vmalloc allocates spurious page?

[Andrew Morton]

Russell King wrote:
> 
> On Wed, Oct 16, 2002 at 12:58:12AM +0300, Marcus Alanen wrote:
> > >The unnecessary page is allocated only if size is initially a multiple
> > >of PAGE_SIZE, which sounds like a common case.
> >
> > Actually, size is already PAGE_ALIGNed, so we get the amount of pages
> > even easier.
> 
> IIRC, back in the dim and distant past, the extra page was originally to
> catch things running off the end of their space (eg, modules).  The
> idea was that modules (and other vmalloc'd areas) would be separated
> by one unmapped page.
> 
> It looks like this got broken recently though.

Still there I think.

struct vm_struct *get_vm_area(unsigned long size, unsigned long flags)
{
	...

        /*
         * We always allocate a guard page.
         */
        size += PAGE_SIZE;


Marcus's patch looks reasonable.