Re: [PATCH 05/25] Break up the allocator entry point into fast and slow paths

KOSAKI Motohiro
2009-04-21T06:34:43Z

> The core of the page allocator is one giant function which allocates memory
> on the stack and makes calculations that may not be needed for every
> allocation. This patch breaks up the allocator path into fast and slow
> paths for clarity. Note the slow paths are still inlined but the entry is
> marked unlikely. If they were not inlined, it actally increases text size
> to generate the as there is only one call site.

hmm..

this patch have few behavior change.
please separate big cleanup patch and behavior patch.

I hope to make this patch non functional change. I'm not sure about these
are your intentional change or not. it cause harder reviewing...

>
> Signed-off-by: Mel Gorman <mel@csn.ul.ie>
> Reviewed-by: Christoph Lameter <cl@linux-foundation.org>
> ---
> mm/page_alloc.c | 356 ++++++++++++++++++++++++++++++++++---------------------
> 1 files changed, 222 insertions(+), 134 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 3bed856..13b4d11 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1460,47 +1460,172 @@ try_next_zone:
> return page;
> }
>
> -/*
> - * This is the 'heart' of the zoned buddy allocator.
> - */
> -struct page *
> -__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> - struct zonelist *zonelist, nodemask_t *nodemask)
> +static inline int
> +should_alloc_retry(gfp_t gfp_mask, unsigned int order,
> + unsigned long pages_reclaimed)
> {
> - const gfp_t wait = gfp_mask & __GFP_WAIT;
> - enum zone_type high_zoneidx = gfp_zone(gfp_mask);
> - struct zoneref *z;
> - struct zone *zone;
> - struct page *page;
> - struct reclaim_state reclaim_state;
> - struct task_struct *p = current;
> - int do_retry;
> - int alloc_flags;
> - unsigned long did_some_progress;
> - unsigned long pages_reclaimed = 0;
> + /* Do not loop if specifically requested */
> + if (gfp_mask & __GFP_NORETRY)
> + return 0;
>
> - lockdep_trace_alloc(gfp_mask);
> + /*
> + * In this implementation, order <= PAGE_ALLOC_COSTLY_ORDER
> + * means __GFP_NOFAIL, but that may not be true in other
> + * implementations.
> + */
> + if (order <= PAGE_ALLOC_COSTLY_ORDER)
> + return 1;
> +
> + /*
> + * For order > PAGE_ALLOC_COSTLY_ORDER, if __GFP_REPEAT is
> + * specified, then we retry until we no longer reclaim any pages
> + * (above), or we've reclaimed an order of pages at least as
> + * large as the allocation's order. In both cases, if the
> + * allocation still fails, we stop retrying.
> + */
> + if (gfp_mask & __GFP_REPEAT && pages_reclaimed < (1 << order))
> + return 1;
>
> - might_sleep_if(wait);
> + /*
> + * Don't let big-order allocations loop unless the caller
> + * explicitly requests that.
> + */
> + if (gfp_mask & __GFP_NOFAIL)
> + return 1;
>
> - if (should_fail_alloc_page(gfp_mask, order))
> - return NULL;
> + return 0;
> +}
>
> - /* the list of zones suitable for gfp_mask */
> - z = zonelist->_zonerefs;
> - if (unlikely(!z->zone)) {
> - /*
> - * Happens if we have an empty zonelist as a result of
> - * GFP_THISNODE being used on a memoryless node
> - */
> +static inline struct page *
> +__alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
> + struct zonelist *zonelist, enum zone_type high_zoneidx,
> + nodemask_t *nodemask)
> +{
> + struct page *page;
> +
> + /* Acquire the OOM killer lock for the zones in zonelist */
> + if (!try_set_zone_oom(zonelist, gfp_mask)) {
> + schedule_timeout_uninterruptible(1);
> return NULL;
> }
>
> -restart:
> - page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
> - zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET);
> + /*
> + * Go through the zonelist yet one more time, keep very high watermark
> + * here, this is only to catch a parallel oom killing, we must fail if
> + * we're still under heavy pressure.
> + */
> + page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask,
> + order, zonelist, high_zoneidx,
> + ALLOC_WMARK_HIGH|ALLOC_CPUSET);
> if (page)
> - goto got_pg;
> + goto out;
> +
> + /* The OOM killer will not help higher order allocs */
> + if (order > PAGE_ALLOC_COSTLY_ORDER)
> + goto out;
> +
> + /* Exhausted what can be done so it's blamo time */
> + out_of_memory(zonelist, gfp_mask, order);
> +
> +out:
> + clear_zonelist_oom(zonelist, gfp_mask);
> + return page;
> +}
> +
> +/* The really slow allocator path where we enter direct reclaim */
> +static inline struct page *
> +__alloc_pages_direct_reclaim(gfp_t gfp_mask, unsigned int order,
> + struct zonelist *zonelist, enum zone_type high_zoneidx,
> + nodemask_t *nodemask, int alloc_flags, unsigned long *did_some_progress)
> +{
> + struct page *page = NULL;
> + struct reclaim_state reclaim_state;
> + struct task_struct *p = current;
> +
> + cond_resched();
> +
> + /* We now go into synchronous reclaim */
> + cpuset_memory_pressure_bump();
> +
> + /*
> + * The task's cpuset might have expanded its set of allowable nodes
> + */
> + p->flags |= PF_MEMALLOC;
> + lockdep_set_current_reclaim_state(gfp_mask);
> + reclaim_state.reclaimed_slab = 0;
> + p->reclaim_state = &reclaim_state;
> +
> + *did_some_progress = try_to_free_pages(zonelist, order, gfp_mask, nodemask);
> +
> + p->reclaim_state = NULL;
> + lockdep_clear_current_reclaim_state();
> + p->flags &= ~PF_MEMALLOC;
> +
> + cond_resched();
> +
> + if (order != 0)
> + drain_all_pages();
> +
> + if (likely(*did_some_progress))
> + page = get_page_from_freelist(gfp_mask, nodemask, order,
> + zonelist, high_zoneidx, alloc_flags);
> + return page;
> +}
> +
> +static inline int
> +is_allocation_high_priority(struct task_struct *p, gfp_t gfp_mask)
> +{
> + if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> + && !in_interrupt())
> + if (!(gfp_mask & __GFP_NOMEMALLOC))
> + return 1;
> + return 0;
> +}
> +
> +/*
> + * This is called in the allocator slow-path if the allocation request is of
> + * sufficient urgency to ignore watermarks and take other desperate measures
> + */
> +static inline struct page *
> +__alloc_pages_high_priority(gfp_t gfp_mask, unsigned int order,
> + struct zonelist *zonelist, enum zone_type high_zoneidx,
> + nodemask_t *nodemask)
> +{
> + struct page *page;
> +
> + do {
> + page = get_page_from_freelist(gfp_mask, nodemask, order,
> + zonelist, high_zoneidx, ALLOC_NO_WATERMARKS);
> +
> + if (!page && gfp_mask & __GFP_NOFAIL)
> + congestion_wait(WRITE, HZ/50);
> + } while (!page && (gfp_mask & __GFP_NOFAIL));
> +
> + return page;
> +}
> +
> +static inline
> +void wake_all_kswapd(unsigned int order, struct zonelist *zonelist,
> + enum zone_type high_zoneidx)
> +{
> + struct zoneref *z;
> + struct zone *zone;
> +
> + for_each_zone_zonelist(zone, z, zonelist, high_zoneidx)
> + wakeup_kswapd(zone, order);
> +}
> +
> +static inline struct page *
> +__alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
> + struct zonelist *zonelist, enum zone_type high_zoneidx,
> + nodemask_t *nodemask)
> +{
> + const gfp_t wait = gfp_mask & __GFP_WAIT;
> + struct page *page = NULL;
> + int alloc_flags;
> + unsigned long pages_reclaimed = 0;
> + unsigned long did_some_progress;
> + struct task_struct *p = current;
>
> /*
> * GFP_THISNODE (meaning __GFP_THISNODE, __GFP_NORETRY and
> @@ -1513,8 +1638,7 @@ restart:
> if (NUMA_BUILD && (gfp_mask & GFP_THISNODE) == GFP_THISNODE)
> goto nopage;
>
> - for_each_zone_zonelist(zone, z, zonelist, high_zoneidx)
> - wakeup_kswapd(zone, order);
> + wake_all_kswapd(order, zonelist, high_zoneidx);
>
> /*
> * OK, we're below the kswapd watermark and have kicked background
> @@ -1534,6 +1658,7 @@ restart:
> if (wait)
> alloc_flags |= ALLOC_CPUSET;
>
> +restart:
> /*
> * Go through the zonelist again. Let __GFP_HIGH and allocations
> * coming from realtime tasks go deeper into reserves.
> @@ -1547,119 +1672,47 @@ restart:
> if (page)
> goto got_pg;
>
> - /* This allocation should allow future memory freeing. */
> -
> -rebalance:
> - if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
> - && !in_interrupt()) {
> - if (!(gfp_mask & __GFP_NOMEMALLOC)) {
> -nofail_alloc:
> - /* go through the zonelist yet again, ignoring mins */
> - page = get_page_from_freelist(gfp_mask, nodemask, order,
> - zonelist, high_zoneidx, ALLOC_NO_WATERMARKS);
> - if (page)
> - goto got_pg;
> - if (gfp_mask & __GFP_NOFAIL) {
> - congestion_wait(WRITE, HZ/50);
> - goto nofail_alloc;
> - }
> - }
> - goto nopage;
> - }
> + /* Allocate without watermarks if the context allows */
> + if (is_allocation_high_priority(p, gfp_mask))
> + page = __alloc_pages_high_priority(gfp_mask, order,
> + zonelist, high_zoneidx, nodemask);
> + if (page)
> + goto got_pg;
>
> /* Atomic allocations - we can't balance anything */
> if (!wait)
> goto nopage;
>

old code is below.
if PF_MEMALLOC and !in_interrupt() and __GFP_NOMEMALLOC case,
old code jump to nopage, your one call reclaim.

I think, if the task have PF_MEMALLOC, it shouldn't call reclaim.
if not, endless reclaim recursion happend.

--------------------------------------------------------------------
rebalance:
if (((p->flags & PF_MEMALLOC) || unlikely(test_thread_flag(TIF_MEMDIE)))
&& !in_interrupt()) {
if (!(gfp_mask & __GFP_NOMEMALLOC)) {
nofail_alloc:
/* go through the zonelist yet again, ignoring mins */
page = get_page_from_freelist(gfp_mask, nodemask, order,
zonelist, high_zoneidx, ALLOC_NO_WATERMARKS);
if (page)
goto got_pg;
if (gfp_mask & __GFP_NOFAIL) {
congestion_wait(WRITE, HZ/50);
goto nofail_alloc;
}
}
goto nopage;
}
--------------------------------------------------------------------

>
> /* Atomic allocations - we can't balance anything */
> if (!wait)
> goto nopage;
>
> - cond_resched();
> -
> - /* We now go into synchronous reclaim */
> - cpuset_memory_pressure_bump();
> -
> - p->flags |= PF_MEMALLOC;
> -
> - lockdep_set_current_reclaim_state(gfp_mask);
> - reclaim_state.reclaimed_slab = 0;
> - p->reclaim_state = &reclaim_state;
> -
> - did_some_progress = try_to_free_pages(zonelist, order,
> - gfp_mask, nodemask);
> -
> - p->reclaim_state = NULL;
> - lockdep_clear_current_reclaim_state();
> - p->flags &= ~PF_MEMALLOC;
> -
> - cond_resched();
> + /* Try direct reclaim and then allocating */
> + page = __alloc_pages_direct_reclaim(gfp_mask, order,
> + zonelist, high_zoneidx,
> + nodemask,
> + alloc_flags, &did_some_progress);
> + if (page)
> + goto got_pg;
>
> - if (order != 0)
> - drain_all_pages();
> + /*
> + * If we failed to make any progress reclaiming, then we are
> + * running out of options and have to consider going OOM
> + */
> + if (!did_some_progress) {
> + if ((gfp_mask & __GFP_FS) && !(gfp_mask & __GFP_NORETRY)) {
> + page = __alloc_pages_may_oom(gfp_mask, order,
> + zonelist, high_zoneidx,
> + nodemask);
> + if (page)
> + goto got_pg;

the old code here.

------------------------------------------------------------------------
} else if ((gfp_mask & __GFP_FS) && !(gfp_mask & __GFP_NORETRY)) {
if (!try_set_zone_oom(zonelist, gfp_mask)) {
schedule_timeout_uninterruptible(1);
goto restart;
}

/*
* Go through the zonelist yet one more time, keep
* very high watermark here, this is only to catch
* a parallel oom killing, we must fail if we're still
* under heavy pressure.
*/
page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask,
order, zonelist, high_zoneidx,
ALLOC_WMARK_HIGH|ALLOC_CPUSET);
if (page) {
clear_zonelist_oom(zonelist, gfp_mask);
goto got_pg;
}

/* The OOM killer will not help higher order allocs so fail */
if (order > PAGE_ALLOC_COSTLY_ORDER) {
clear_zonelist_oom(zonelist, gfp_mask);
goto nopage;
}

out_of_memory(zonelist, gfp_mask, order);
clear_zonelist_oom(zonelist, gfp_mask);
goto restart;
}
------------------------------------------------------------------------

if get_page_from_freelist() return NULL and order > PAGE_ALLOC_COSTLY_ORDER,
old code jump to nopage, your one jump to restart.

> - if (likely(did_some_progress)) {
> - page = get_page_from_freelist(gfp_mask, nodemask, order,
> - zonelist, high_zoneidx, alloc_flags);
> - if (page)
> - goto got_pg;
> - } else if ((gfp_mask & __GFP_FS) && !(gfp_mask & __GFP_NORETRY)) {
> - if (!try_set_zone_oom(zonelist, gfp_mask)) {
> - schedule_timeout_uninterruptible(1);
> goto restart;
> }
> -
> - /*
> - * Go through the zonelist yet one more time, keep
> - * very high watermark here, this is only to catch
> - * a parallel oom killing, we must fail if we're still
> - * under heavy pressure.
> - */
> - page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask,
> - order, zonelist, high_zoneidx,
> - ALLOC_WMARK_HIGH|ALLOC_CPUSET);
> - if (page) {
> - clear_zonelist_oom(zonelist, gfp_mask);
> - goto got_pg;
> - }
> -
> - /* The OOM killer will not help higher order allocs so fail */
> - if (order > PAGE_ALLOC_COSTLY_ORDER) {
> - clear_zonelist_oom(zonelist, gfp_mask);
> - goto nopage;
> - }
> -
> - out_of_memory(zonelist, gfp_mask, order);
> - clear_zonelist_oom(zonelist, gfp_mask);
> - goto restart;
> }
>
> - /*
> - * Don't let big-order allocations loop unless the caller explicitly
> - * requests that. Wait for some write requests to complete then retry.
> - *
> - * In this implementation, order <= PAGE_ALLOC_COSTLY_ORDER
> - * means __GFP_NOFAIL, but that may not be true in other
> - * implementations.
> - *
> - * For order > PAGE_ALLOC_COSTLY_ORDER, if __GFP_REPEAT is
> - * specified, then we retry until we no longer reclaim any pages
> - * (above), or we've reclaimed an order of pages at least as
> - * large as the allocation's order. In both cases, if the
> - * allocation still fails, we stop retrying.
> - */
> + /* Check if we should retry the allocation */
> pages_reclaimed += did_some_progress;
> - do_retry = 0;
> - if (!(gfp_mask & __GFP_NORETRY)) {
> - if (order <= PAGE_ALLOC_COSTLY_ORDER) {
> - do_retry = 1;
> - } else {
> - if (gfp_mask & __GFP_REPEAT &&
> - pages_reclaimed < (1 << order))
> - do_retry = 1;
> - }
> - if (gfp_mask & __GFP_NOFAIL)
> - do_retry = 1;
> - }
> - if (do_retry) {
> + if (should_alloc_retry(gfp_mask, order, pages_reclaimed)) {
> + /* Wait for some write requests to complete then retry */
> congestion_wait(WRITE, HZ/50);
> - goto rebalance;
> + goto restart;

this change rebalance to restart.

> }
>
> nopage:
> @@ -1672,6 +1725,41 @@ nopage:
> }
> got_pg:
> return page;
> +
> +}
> +
> +/*
> + * This is the 'heart' of the zoned buddy allocator.
> + */
> +struct page *
> +__alloc_pages_nodemask(gfp_t gfp_mask, unsigned int order,
> + struct zonelist *zonelist, nodemask_t *nodemask)
> +{
> + enum zone_type high_zoneidx = gfp_zone(gfp_mask);
> + struct page *page;
> +
> + lockdep_trace_alloc(gfp_mask);
> +
> + might_sleep_if(gfp_mask & __GFP_WAIT);
> +
> + if (should_fail_alloc_page(gfp_mask, order))
> + return NULL;
> +
> + /*
> + * Check the zones suitable for the gfp_mask contain at least one
> + * valid zone. It's possible to have an empty zonelist as a result
> + * of GFP_THISNODE and a memoryless node
> + */
> + if (unlikely(!zonelist->_zonerefs->zone))
> + return NULL;
> +
> + page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
> + zonelist, high_zoneidx, ALLOC_WMARK_LOW|ALLOC_CPUSET);
> + if (unlikely(!page))
> + page = __alloc_pages_slowpath(gfp_mask, order,
> + zonelist, high_zoneidx, nodemask);
> +
> + return page;
> }
> EXPORT_SYMBOL(__alloc_pages_nodemask);
>
> --
> 1.5.6.5