Hi Linus and Rik,

The last for(;;) loop in mm/page_allo.c:__alloc_pages() looks strange to
me:

        for (;;) {
                struct page * page = NULL;

                ...

                if (direct_reclaim)
                        page = reclaim_page(z);
                if (page)
                        return page;
               
                ....
                if (!page)
                        page = rmqueue(z, order);
                if (page)
                        return page;
        }

a) if direct_reclaim == 0 then page will remain NULL so the
if(page) should be moved inside if(direct_reclaim) instead of being tested
on each iteration unnecessarily.

b) if (!page) is redundant as page==NULL at that point, if it wasn't then
it would have been returned earlier

Am I missing something obvious? Or perhaps one should remember the fact
that 'int x = 0' inside for(;;) means x is set to 0 on each for()
iteration.... :)

Here is the patch, tested under 2.4.0-test10-pre3 (also removed redundant
page = NULL at the beginning).

Regards,
Tigran


--- linux/mm/page_alloc.c       Sun Oct 15 20:40:38 2000
+++ work/mm/page_alloc.c        Sun Oct 15 20:53:53 2000
@@ -287,7 +287,7 @@
        zone_t **zone;
        int direct_reclaim = 0;
        unsigned int gfp_mask = zonelist->gfp_mask;
-       struct page * page = NULL;
+       struct page * page;
 
        /*
         * Allocations put pressure on the VM subsystem.
@@ -510,17 +510,17 @@
                 * happen when the OOM killer selects this task for
                 * instant execution...
                 */
-               if (direct_reclaim)
+               if (direct_reclaim) {
                        page = reclaim_page(z);
-               if (page)
-                       return page;
+                       if (page)
+                               return page;
+               }
 
                /* XXX: is pages_min/4 a good amount to reserve for this? */
                if (z->free_pages < z->pages_min / 4 &&
                                !(current->flags & PF_MEMALLOC))
                        continue;
-               if (!page)
-                       page = rmqueue(z, order);
+               page = rmqueue(z, order);
                if (page)
                        return page;
        }

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

Reply via email to