lkml.org 
[lkml]   [2009]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 02/22] Do not sanity check order in the fast path
    Date
    > 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

    Good point.
    if (WARN_ON_ONCE(order >= MAX_ORDER)) is better?





    \
     
     \ /
      Last update: 2009-04-24 04:59    [W:0.027 / U:0.212 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site