Messages in this thread Patch in this message |  | | | Subject | Re: [PATCH 02/22] Do not sanity check order in the fast path | | From | Dave Hansen <> | | Date | Thu, 23 Apr 2009 10:36:50 -0700 |
| |
On Thu, 2009-04-23 at 10:58 +0100, Mel Gorman wrote: > > How about this: I'll go and audit the use of order in page_alloc.c to > > make sure that having an order>MAX_ORDER-1 floating around is OK and > > won't break anything. > > Great. Right now, I think it's ok but I haven't audited for this > explicily and a second set of eyes never hurts.
OK, after looking through this, I have a couple of ideas. One is that we do the MAX_ORDER check in __alloc_pages_internal(), but *after* the first call to get_page_from_freelist(). That's because I'm worried if we ever got into the reclaim code with a >MAX_ORDER 'order'. Such as:
void wakeup_kswapd(struct zone *zone, int order) { ... if (pgdat->kswapd_max_order < order) pgdat->kswapd_max_order = order; if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL)) return; if (!waitqueue_active(&pgdat->kswapd_wait)) return; wake_up_interruptible(&pgdat->kswapd_wait); } unsigned long try_to_free_pages(struct zonelist *zonelist, int order, gfp_t gfp_mask, nodemask_t *nodemask) { struct scan_control sc = { ... .order = order, .mem_cgroup = NULL, .isolate_pages = isolate_pages_global, .nodemask = nodemask, }; return do_try_to_free_pages(zonelist, &sc); } This will keep us only checking 'order' once for each alloc_pages_internal() call. It is an extra branch, but it is out of the really, really hot path since we're about to start reclaim here anyway.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index e2f2699..1e3a01e 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1498,6 +1498,13 @@ restart: zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET); if (page) goto got_pg; + /* + * We're out of the rocket-hot area above, so do a quick sanity + * check. We do this here to avoid ever trying to do any reclaim + * of >=MAX_ORDER areas which can never succeed, of course. + */ + if (order >= MAX_ORDER) + goto nopage; /* * GFP_THISNODE (meaning __GFP_THISNODE, __GFP_NORETRY and
-- Dave
|  |