On 09/23/2016 10:23 AM, Michal Hocko wrote: > On Fri 23-09-16 08:55:33, Vlastimil Babka wrote: > [...] >> >From 1623d5bd441160569ffad3808aeeec852048e558 Mon Sep 17 00:00:00 2001 >> From: Vlastimil Babka <vbabka@xxxxxxx> >> Date: Thu, 22 Sep 2016 17:02:37 +0200 >> Subject: [PATCH] mm, page_alloc: pull no_progress_loops update to >> should_reclaim_retry() >> >> The should_reclaim_retry() makes decisions based on no_progress_loops, so it >> makes sense to also update the counter there. It will be also consistent with >> should_compact_retry() and compaction_retries. No functional change. >> >> [hillf.zj@xxxxxxxxxxxxxxx: fix missing pointer dereferences] >> Signed-off-by: Vlastimil Babka <vbabka@xxxxxxx> >> Acked-by: Hillf Danton <hillf.zj@xxxxxxxxxxxxxxx> > > OK, this looks reasonable to me. Could you post both patches in a Both? I would argue that [1] might be relevant because it resets the number of retries. Only the should_reclaim_retry() cleanup is not stricly needed. [1] http://lkml.kernel.org/r/<deec7319-2976-6d34-ab7b-afbb3f6c32f8@xxxxxxx> > separate thread please? They shouldn't be really needed to mitigate the > pre-mature oom killer issues. Feel free to add > Acked-by: Michal Hocko <mhocko@xxxxxxxx> > > Thanks! > >> --- >> mm/page_alloc.c | 28 ++++++++++++++-------------- >> 1 file changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 582820080601..6039ff40452c 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -3401,16 +3401,26 @@ bool gfp_pfmemalloc_allowed(gfp_t gfp_mask) >> static inline bool >> should_reclaim_retry(gfp_t gfp_mask, unsigned order, >> struct alloc_context *ac, int alloc_flags, >> - bool did_some_progress, int no_progress_loops) >> + bool did_some_progress, int *no_progress_loops) >> { >> struct zone *zone; >> struct zoneref *z; >> >> /* >> + * Costly allocations might have made a progress but this doesn't mean >> + * their order will become available due to high fragmentation so >> + * always increment the no progress counter for them >> + */ >> + if (did_some_progress && order <= PAGE_ALLOC_COSTLY_ORDER) >> + *no_progress_loops = 0; >> + else >> + (*no_progress_loops)++; >> + >> + /* >> * Make sure we converge to OOM if we cannot make any progress >> * several times in the row. >> */ >> - if (no_progress_loops > MAX_RECLAIM_RETRIES) >> + if (*no_progress_loops > MAX_RECLAIM_RETRIES) >> return false; >> >> /* >> @@ -3425,7 +3435,7 @@ should_reclaim_retry(gfp_t gfp_mask, unsigned order, >> unsigned long reclaimable; >> >> available = reclaimable = zone_reclaimable_pages(zone); >> - available -= DIV_ROUND_UP(no_progress_loops * available, >> + available -= DIV_ROUND_UP((*no_progress_loops) * available, >> MAX_RECLAIM_RETRIES); >> available += zone_page_state_snapshot(zone, NR_FREE_PAGES); >> >> @@ -3641,18 +3651,8 @@ __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order, >> if (order > PAGE_ALLOC_COSTLY_ORDER && !(gfp_mask & __GFP_REPEAT)) >> goto nopage; >> >> - /* >> - * Costly allocations might have made a progress but this doesn't mean >> - * their order will become available due to high fragmentation so >> - * always increment the no progress counter for them >> - */ >> - if (did_some_progress && order <= PAGE_ALLOC_COSTLY_ORDER) >> - no_progress_loops = 0; >> - else >> - no_progress_loops++; >> - >> if (should_reclaim_retry(gfp_mask, order, ac, alloc_flags, >> - did_some_progress > 0, no_progress_loops)) >> + did_some_progress > 0, &no_progress_loops)) >> goto retry; >> >> /* >> -- >> 2.10.0 >> >> >> -- >> To unsubscribe, send a message with 'unsubscribe linux-mm' in >> the body to majordomo@xxxxxxxxx. For more info on Linux MM, >> see: http://www.linux-mm.org/ . >> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a> > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>