| Subject | Re: [PATCH 08/25] Calculate the preferred zone for allocation only once | From | Pekka Enberg <> | Date | Tue, 21 Apr 2009 10:37:37 +0300 |
| |
Hi Mel,
On Mon, 2009-04-20 at 23:19 +0100, Mel Gorman wrote: > get_page_from_freelist() can be called multiple times for an > allocation. > Part of this calculates the preferred_zone which is the first usable > zone in the zonelist. This patch calculates preferred_zone once. > > Signed-off-by: Mel Gorman <mel@csn.ul.ie>
Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> @@ -1772,11 +1774,20 @@ __alloc_pages_nodemask(gfp_t gfp_mask, > unsigned int order, > if (unlikely(!zonelist->_zonerefs->zone)) > return NULL; > > + /* The preferred zone is used for statistics later */ > + (void)first_zones_zonelist(zonelist, high_zoneidx, nodemask, > + &preferred_zone); > + if (!preferred_zone) > + return NULL;
You might want to add an explanation to the changelog why this change is safe. It looked like a functional change at first glance and it was pretty difficult to convince myself that __alloc_pages_slowpath() will always return NULL when there's no preferred zone because of the other cleanups in this patch series.
> + > + /* First allocation attempt */ > page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order, > - zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET); > + zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET, > + preferred_zone); > if (unlikely(!page)) > page = __alloc_pages_slowpath(gfp_mask, order, > - zonelist, high_zoneidx, nodemask); > + zonelist, high_zoneidx, nodemask, > + preferred_zone); > > return page; > }
|