Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:46:58.0 +0800
@@ -788,7 +788,22 @@
 
 fastcall void __free_pages(struct page *page, unsigned int order)
 {
-   if (!PageReserved(page) && put_page_testzero(page)) {
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+   for (i = 0; i < (1 << order); i++)
+   result &= put_page_testzero(page+i);
+   if (!result)
+   BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else


On Fri, 21 Jan 2005 11:40:52 +0800, zhan rongkai <[EMAIL PROTECTED]> wrote:
> --- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
> +++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:43:44.0 +0800
> @@ -788,7 +788,22 @@
> 
>  fastcall void __free_pages(struct page *page, unsigned int order)
>  {
> -   if (!PageReserved(page) && put_page_testzero(page)) {
> +   if (!PageReserved(page)) {
> +#ifdef CONFIG_MMU
> +   if (!put_page_testzero(page))
> +   return;
> +#else
> +   int i, result = 1;
> +
> +   /*
> +* We need to de-reference all the pages for this order -- see
> set_page_refs()
> +*/
> +for (i = 0; i < (1 << order); i++)
> +result &= put_page_testzero(page+i);
> +if (!result)
> +BUG();
> +#endif /* CONFIG_MMU */
> +
> if (order == 0)
> free_hot_page(page);
> else
> 
> --
> Rongkai Zhan
> 


-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:43:44.0 +0800
@@ -788,7 +788,22 @@
 
 fastcall void __free_pages(struct page *page, unsigned int order)
 {
-   if (!PageReserved(page) && put_page_testzero(page)) {
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+for (i = 0; i < (1 << order); i++)
+result &= put_page_testzero(page+i);
+if (!result)
+BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else


-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
On Thu, 20 Jan 2005 14:31:34 +, Russell King
<[EMAIL PROTECTED]> wrote:
> On Thu, Jan 20, 2005 at 09:34:17PM +0800, zhan rongkai wrote:
> > [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c
> > =
> >
> > The buddy allocator's __free_pages() function seems to be buggy.
> >
> > The following codes are from kernel 2.6.10:
> >
> > fastcall void __free_pages(struct page *page, unsigned int order)
> > {
> >   if (!PageReserved(page) && put_page_testzero(page)) {
> >   if (order == 0)
> >   free_hot_page(page);
> >   else
> >   __free_pages_ok(page, order);
> >   }
> > }
> >
> > As you know, before truely freeing all pages, this function calls
> > put_page_testzero(page) to
> > drop the refcount of the pages.
> >
> > But, in fact the macro put_page_testzero(page) **only** drops **one**
> > page's refcount.
> > Therefore, if (order > 0), the refcounts of (page+1) ..
> > (page+(1< > This will cause __free_pages_ok() to dump stack, because it finds some
> > pages' page_count()
> > are not zero!
> 
> When you allocate a page with order > 0, the first 0-order page has a
> refcount of 1, and the remaining 0-order pages have a refcount of 0.

Thank you for telling me this point.

> If you're triggering this check, I suspect you're fiddling about with
> the individual pages (using get_page on them individually?) which is
> a no-no.
> 
> --
> Russell King
> 

Oh, I forget to tell you that my CPU has no MMU, sorry:-)
Let's see the function set_page_refs() which is called by
prep_new_page() function:

static inline void set_page_refs(struct page *page, int order)
{
#ifdef CONFIG_MMU
set_page_count(page, 1);
#else
int i;

/*
 * We need to reference all the pages for this order, otherwise if
 * anyone accesses one of the pages with (get/put) it will be freed.
 */
for (i = 0; i < (1 << order); i++)
set_page_count(page+i, 1);
#endif /* CONFIG_MMU */
}

We can see that it sets all pages' refcount to 1 when there is no MMU.

My previous patch is wrong. Here is new one:


--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:34:57.0 +0800
@@ -787,8 +787,23 @@
 }
 
 fastcall void __free_pages(struct page *page, unsigned int order)
-{
-   if (!PageReserved(page) && put_page_testzero(page)) {
+{  
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+for (i = 0; i < (1 << order); i++)
+result &= put_page_testzero(page);
+if (!result)
+BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else

-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread Russell King
On Thu, Jan 20, 2005 at 09:34:17PM +0800, zhan rongkai wrote:
> [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c
> =
> 
> The buddy allocator's __free_pages() function seems to be buggy.
> 
> The following codes are from kernel 2.6.10:
> 
> fastcall void __free_pages(struct page *page, unsigned int order)
> {
>   if (!PageReserved(page) && put_page_testzero(page)) {
>   if (order == 0)
>   free_hot_page(page);
>   else
>   __free_pages_ok(page, order);
>   }
> }
> 
> As you know, before truely freeing all pages, this function calls
> put_page_testzero(page) to
> drop the refcount of the pages.
> 
> But, in fact the macro put_page_testzero(page) **only** drops **one**
> page's refcount.
> Therefore, if (order > 0), the refcounts of (page+1) ..
> (page+(1< This will cause __free_pages_ok() to dump stack, because it finds some
> pages' page_count()
> are not zero!

When you allocate a page with order > 0, the first 0-order page has a
refcount of 1, and the remaining 0-order pages have a refcount of 0.

If you're triggering this check, I suspect you're fiddling about with
the individual pages (using get_page on them individually?) which is
a no-no.

-- 
Russell King
 Linux kernel2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA  - http://pcmcia.arm.linux.org.uk/
 2.6 Serial core
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread Russell King
On Thu, Jan 20, 2005 at 09:34:17PM +0800, zhan rongkai wrote:
 [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c
 =
 
 The buddy allocator's __free_pages() function seems to be buggy.
 
 The following codes are from kernel 2.6.10:
 
 fastcall void __free_pages(struct page *page, unsigned int order)
 {
   if (!PageReserved(page)  put_page_testzero(page)) {
   if (order == 0)
   free_hot_page(page);
   else
   __free_pages_ok(page, order);
   }
 }
 
 As you know, before truely freeing all pages, this function calls
 put_page_testzero(page) to
 drop the refcount of the pages.
 
 But, in fact the macro put_page_testzero(page) **only** drops **one**
 page's refcount.
 Therefore, if (order  0), the refcounts of (page+1) ..
 (page+(1order)-1) are unchanged!
 This will cause __free_pages_ok() to dump stack, because it finds some
 pages' page_count()
 are not zero!

When you allocate a page with order  0, the first 0-order page has a
refcount of 1, and the remaining 0-order pages have a refcount of 0.

If you're triggering this check, I suspect you're fiddling about with
the individual pages (using get_page on them individually?) which is
a no-no.

-- 
Russell King
 Linux kernel2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 PCMCIA  - http://pcmcia.arm.linux.org.uk/
 2.6 Serial core
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
On Thu, 20 Jan 2005 14:31:34 +, Russell King
[EMAIL PROTECTED] wrote:
 On Thu, Jan 20, 2005 at 09:34:17PM +0800, zhan rongkai wrote:
  [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c
  =
 
  The buddy allocator's __free_pages() function seems to be buggy.
 
  The following codes are from kernel 2.6.10:
 
  fastcall void __free_pages(struct page *page, unsigned int order)
  {
if (!PageReserved(page)  put_page_testzero(page)) {
if (order == 0)
free_hot_page(page);
else
__free_pages_ok(page, order);
}
  }
 
  As you know, before truely freeing all pages, this function calls
  put_page_testzero(page) to
  drop the refcount of the pages.
 
  But, in fact the macro put_page_testzero(page) **only** drops **one**
  page's refcount.
  Therefore, if (order  0), the refcounts of (page+1) ..
  (page+(1order)-1) are unchanged!
  This will cause __free_pages_ok() to dump stack, because it finds some
  pages' page_count()
  are not zero!
 
 When you allocate a page with order  0, the first 0-order page has a
 refcount of 1, and the remaining 0-order pages have a refcount of 0.

Thank you for telling me this point.

 If you're triggering this check, I suspect you're fiddling about with
 the individual pages (using get_page on them individually?) which is
 a no-no.
 
 --
 Russell King
 

Oh, I forget to tell you that my CPU has no MMU, sorry:-)
Let's see the function set_page_refs() which is called by
prep_new_page() function:

static inline void set_page_refs(struct page *page, int order)
{
#ifdef CONFIG_MMU
set_page_count(page, 1);
#else
int i;

/*
 * We need to reference all the pages for this order, otherwise if
 * anyone accesses one of the pages with (get/put) it will be freed.
 */
for (i = 0; i  (1  order); i++)
set_page_count(page+i, 1);
#endif /* CONFIG_MMU */
}

We can see that it sets all pages' refcount to 1 when there is no MMU.

My previous patch is wrong. Here is new one:


--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:34:57.0 +0800
@@ -787,8 +787,23 @@
 }
 
 fastcall void __free_pages(struct page *page, unsigned int order)
-{
-   if (!PageReserved(page)  put_page_testzero(page)) {
+{  
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+for (i = 0; i  (1  order); i++)
+result = put_page_testzero(page);
+if (!result)
+BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else

-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:43:44.0 +0800
@@ -788,7 +788,22 @@
 
 fastcall void __free_pages(struct page *page, unsigned int order)
 {
-   if (!PageReserved(page)  put_page_testzero(page)) {
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+for (i = 0; i  (1  order); i++)
+result = put_page_testzero(page+i);
+if (!result)
+BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else


-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH]: fix the bug of __free_pages() of mm/page_alloc.c

2005-01-20 Thread zhan rongkai
--- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
+++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:46:58.0 +0800
@@ -788,7 +788,22 @@
 
 fastcall void __free_pages(struct page *page, unsigned int order)
 {
-   if (!PageReserved(page)  put_page_testzero(page)) {
+   if (!PageReserved(page)) {
+#ifdef CONFIG_MMU
+   if (!put_page_testzero(page))
+   return;
+#else
+   int i, result = 1;
+
+   /*
+* We need to de-reference all the pages for this order -- see
set_page_refs()
+*/
+   for (i = 0; i  (1  order); i++)
+   result = put_page_testzero(page+i);
+   if (!result)
+   BUG();
+#endif /* CONFIG_MMU */
+
if (order == 0)
free_hot_page(page);
else


On Fri, 21 Jan 2005 11:40:52 +0800, zhan rongkai [EMAIL PROTECTED] wrote:
 --- linux-2.6.10.orig/mm/page_alloc.c   2004-12-25 05:33:51.0 +0800
 +++ linux-2.6.10/mm/page_alloc.c2005-01-21 11:43:44.0 +0800
 @@ -788,7 +788,22 @@
 
  fastcall void __free_pages(struct page *page, unsigned int order)
  {
 -   if (!PageReserved(page)  put_page_testzero(page)) {
 +   if (!PageReserved(page)) {
 +#ifdef CONFIG_MMU
 +   if (!put_page_testzero(page))
 +   return;
 +#else
 +   int i, result = 1;
 +
 +   /*
 +* We need to de-reference all the pages for this order -- see
 set_page_refs()
 +*/
 +for (i = 0; i  (1  order); i++)
 +result = put_page_testzero(page+i);
 +if (!result)
 +BUG();
 +#endif /* CONFIG_MMU */
 +
 if (order == 0)
 free_hot_page(page);
 else
 
 --
 Rongkai Zhan
 


-- 
Rongkai Zhan
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/