2002-10-15 19:26:31

by Marcus Alanen

[permalink] [raw]
Subject: [patch, 2.5] __vmalloc allocates spurious page?

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 *
[email protected] * http://www.tucs.fi/Research/labs/softcons.php *


2002-10-15 21:52:24

by Marcus Alanen

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

>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;



2002-10-15 22:01:13

by Russell King

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

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 ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2002-10-15 22:09:41

by William Lee Irwin III

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

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

2002-10-15 22:22:46

by Andrew Morton

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

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.