On Thu, 20 Jan 2005 14:31:34 +0000, 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<<order)-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.000000000 +0800
+++ linux-2.6.10/mm/page_alloc.c        2005-01-21 11:34:57.000000000 +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/

Reply via email to