2006-03-30 03:05:20

by Luke Yang

[permalink] [raw]
Subject: [PATCH] nommu page refcount bug fixing

Hi all,

The previous "nommu use compound pages" patch has a problem: when
the pages allocated is not compound page (eg: slab allocator), the
refcount value of every page still need to be set, otherwise the
get/put_page() would free a single page improperly, such as in
access_process_vm().

Signed-off-by: Luke Yang <[email protected]>

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b7f14a4..fc8b544 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -436,6 +436,14 @@ static void __free_pages_ok(struct page
mutex_debug_check_no_locks_freed(page_address(page),
PAGE_SIZE<<order);

+#ifndef CONFIG_MMU
+ if (!PageCompound(page)) {
+ for (i = 1 ; i < (1 << order) ; ++i) {
+ __put_page(page + i);
+ }
+ }
+#endif
+
for (i = 0 ; i < (1 << order) ; ++i)
reserved += free_pages_check(page + i);
if (reserved)
@@ -453,6 +461,8 @@ static void __free_pages_ok(struct page
*/
void fastcall __init __free_pages_bootmem(struct page *page, unsigned
int order)
{
+ int i;
+
if (order == 0) {
__ClearPageReserved(page);
set_page_count(page, 0);
@@ -472,6 +482,11 @@ void fastcall __init __free_pages_bootme
}

set_page_refcounted(page);
+
+#ifndef CONFIG_MMU
+ for (i = 1; i < (1 << order); i++)
+ set_page_refcounted(page + i);
+#endif
__free_pages(page, order);
}
}
@@ -512,6 +527,8 @@ static inline void expand(struct zone *z
*/
static int prep_new_page(struct page *page, int order, gfp_t gfp_flags)
{
+ int i;
+
if (unlikely(page_mapcount(page) |
(page->mapping != NULL) |
(page_count(page) != 0) |
@@ -539,7 +556,21 @@ static int prep_new_page(struct page *pa
1 << PG_referenced | 1 << PG_arch_1 |
1 << PG_checked | 1 << PG_mappedtodisk);
set_page_private(page, 0);
+
set_page_refcounted(page);
+
+#ifndef CONFIG_MMU
+ if (!(gfp_flags & __GFP_COMP)) {
+ /*
+ * Reference all the pages for this order, otherwise if
+ * anyone accesses one of the pages with (get/put) it
+ * will be freed. - eg: access_process_vm()
+ */
+ for (i = 1; i < (1 << order); i++)
+ set_page_refcounted(page + i);
+ }
+#endif
+
kernel_map_pages(page, 1 << order, 1);

if (gfp_flags & __GFP_ZERO)

--
Best regards,
Luke Yang
[email protected]


Attachments:
(No filename) (2.62 kB)
nommu_page_count_fix.patch (1.79 kB)
Download all attachments

2006-03-30 06:30:25

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH] nommu page refcount bug fixing

Luke Yang wrote:

>Hi all,
>
> The previous "nommu use compound pages" patch has a problem: when
>the pages allocated is not compound page (eg: slab allocator), the
>refcount value of every page still need to be set, otherwise the
>get/put_page() would free a single page improperly, such as in
>access_process_vm().
>
>

Yep, sorry this slipped into the kernel. It's my fault for not giving
Andrew a fix for it.

As you might know, page refcounting in nommu was already broken, so
I'm working on a proper solution to fix it.

In the meantime though, this is a step backwards and reintroduces
NOMMU special-casing in page refcounting. As a temporary fix, what I
think should happen is simply for all slab allocations to ask for
__GFP_COMP pages.

Could you check that fixes your problem?

Thanks,
Nick

--

Send instant messages to your online friends http://au.messenger.yahoo.com

2006-03-30 08:56:35

by Luke Yang

[permalink] [raw]
Subject: Re: [PATCH] nommu page refcount bug fixing

On 3/30/06, Nick Piggin <[email protected]> wrote:
> Luke Yang wrote:
>
> Yep, sorry this slipped into the kernel. It's my fault for not giving
> Andrew a fix for it.
>
> As you might know, page refcounting in nommu was already broken, so
> I'm working on a proper solution to fix it.
>
> In the meantime though, this is a step backwards and reintroduces
> NOMMU special-casing in page refcounting. As a temporary fix, what I
> think should happen is simply for all slab allocations to ask for
> __GFP_COMP pages.
>
> Could you check that fixes your problem?
It works. What's your plan to modify nommu mm? I would like to
help. And I am also interested in implementing the "non-power-of-2"
allocator in 2.6.

New patch:
Signed-off-by: Luke Yang <[email protected]>

diff --git a/mm/slab.c b/mm/slab.c
index 4cbf8bb..f93d3d5 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1454,7 +1454,7 @@ static void *kmem_getpages(struct kmem_c
int i;

flags |= cachep->gfpflags;
- page = alloc_pages_node(nodeid, flags, cachep->gfporder);
+ page = alloc_pages_node(nodeid, (flags | __GFP_COMP), cachep->gfporder);
if (!page)
return NULL;
addr = page_address(page);

--
Best regards,
Luke Yang (Kernel for Blackfin maintainer)
[email protected]


Attachments:
(No filename) (1.27 kB)
nommu_use_compound_page_in_slab.patch (440.00 B)
Download all attachments

2006-03-30 09:00:11

by Luke Yang

[permalink] [raw]
Subject: Re: [PATCH] nommu page refcount bug fixing

> > NOMMU special-casing in page refcounting. As a temporary fix, what I
> > think should happen is simply for all slab allocations to ask for
> > __GFP_COMP pages.
> >
> > Could you check that fixes your problem?
> It works. What's your plan to modify nommu mm? I would like to
> help. And I am also interested in implementing the "non-power-of-2"
> allocator in 2.6.
>
> New patch:
Sorry! Previous patch only works for nommu... Here's the correct one:
Signed-off-by: Luke Yang <[email protected]>

diff --git a/mm/slab.c b/mm/slab.c
index 4cbf8bb..388a6a9 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1454,7 +1454,11 @@ static void *kmem_getpages(struct kmem_c
int i;

flags |= cachep->gfpflags;
+#ifdef CONFIG_MMU
page = alloc_pages_node(nodeid, flags, cachep->gfporder);
+#else
+ page = alloc_pages_node(nodeid, (flags | __GFP_COMP), cachep->gfporder);
+#endif
if (!page)
return NULL;
addr = page_address(page);

> --
> Best regards,
> Luke Yang (Kernel for Blackfin maintainer)
> [email protected]
>
>
>


--
Best regards,
Luke Yang
[email protected]


Attachments:
(No filename) (1.10 kB)
nommu_use_compound_page_in_slab.patch (475.00 B)
Download all attachments

2006-03-30 19:05:34

by Nick Piggin

[permalink] [raw]
Subject: Re: [PATCH] nommu page refcount bug fixing

Luke Yang wrote:
>>>NOMMU special-casing in page refcounting. As a temporary fix, what I
>>>think should happen is simply for all slab allocations to ask for
>>>__GFP_COMP pages.
>>>
>>>Could you check that fixes your problem?
>>
>> It works. What's your plan to modify nommu mm? I would like to
>>help. And I am also interested in implementing the "non-power-of-2"
>>allocator in 2.6.
>>

I'll get it up to date and send it over to you, offline. It
would be great if you could help.

>> New patch:
>

Acked-by: Nick Piggin <[email protected]>

I'll write a changelog for you:

***
The earlier patch to consolidate mmu and nommu page allocation
and refcounting by using compound pages for nommu allocations
had a bug: kmalloc slabs who's pages were initially allocated
by a non-__GFP_COMP allocator could be passed into mm/nommu.c
kmalloc allocations which really wanted __GFP_COMP underlying
pages.

Fix that by having nommu pass __GFP_COMP to all higher order
slab allocations
***

Sound OK? Can you do it next time? ;)

--
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com