Re: [RFC PATCH 2/2] Don't continue reclaim if the system have plenty free memory

Minchan Kim
2009-07-07T13:20:27Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 7592 bytes --]

Hi, Kosaki.
On Tue, Jul 7, 2009 at 6:48 PM, KOSAKIMotohiro<kosaki.motohiro@jp.fujitsu.com> wrote:> Subject: [PATCH] Don't continue reclaim if the system have plenty free memory>> On concurrent reclaim situation, if one reclaimer makes OOM, maybe other> reclaimer can stop reclaim because OOM killer makes enough free memory.>> But current kernel doesn't have its logic. Then, we can face following accidental> 2nd OOM scenario.>> 1. System memory is used by only one big process.> 2. memory shortage occur and concurrent reclaim start.> 3. One reclaimer makes OOM and OOM killer kill above big process.> 4. Almost reclaimable page will be freed.> 5. Another reclaimer can't find any reclaimable page because those pages are> Â  already freed.> 6. Then, system makes accidental and unnecessary 2nd OOM killer.>
Did you see the this situation ?Why I ask is that we have already a routine for preventing parallelOOM killing in __alloc_pages_may_oom.
Couldn't it protect your scenario ?If it can't, Could you explain the scenario in more detail ?
I think first we try to modify old routine with efficient.
>> Plus, nowaday datacenter system have badboy process monitoring system and> it kill too much memory consumption process.> But it don't stop other reclaimer and it makes accidental 2nd OOM by the> same reason.>>> This patch have one good side effect. it increase reclaim depended benchmark> performance.>> e.g.> =====> % ./hackbench 140 process 100>> before:> Â  Â  Â  Â Time: 93.361> after:> Â  Â  Â  Â Time: 28.799>>>> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>> ---> Â fs/buffer.c Â  Â  Â  Â  Â | Â  Â 2 +-> Â include/linux/swap.h | Â  Â 3 ++-> Â mm/page_alloc.c Â  Â  Â | Â  Â 3 ++-> Â mm/vmscan.c Â  Â  Â  Â  Â | Â  29 ++++++++++++++++++++++++++++-> Â 4 files changed, 33 insertions(+), 4 deletions(-)>> Index: b/mm/vmscan.c> ===================================================================> --- a/mm/vmscan.c> +++ b/mm/vmscan.c> @@ -87,6 +87,9 @@ struct scan_control {> Â  Â  Â  Â  */> Â  Â  Â  Â nodemask_t Â  Â  Â *nodemask;>> + Â  Â  Â  /* Caller's preferred zone. */> + Â  Â  Â  struct zone Â  Â  *preferred_zone;> +> Â  Â  Â  Â /* Pluggable isolate pages callback */> Â  Â  Â  Â unsigned long (*isolate_pages)(unsigned long nr, struct list_head *dst,> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â unsigned long *scanned, int order, int mode,> @@ -1535,6 +1538,10 @@ static void shrink_zone(int priority, st> Â  Â  Â  Â unsigned long nr_reclaimed = sc->nr_reclaimed;> Â  Â  Â  Â unsigned long swap_cluster_max = sc->swap_cluster_max;> Â  Â  Â  Â int noswap = 0;> + Â  Â  Â  int classzone_idx = 0;> +> + Â  Â  Â  if (sc->preferred_zone)> + Â  Â  Â  Â  Â  Â  Â  classzone_idx = zone_idx(sc->preferred_zone);>> Â  Â  Â  Â /* If we have no swap space, do not bother scanning anon pages. */> Â  Â  Â  Â if (!sc->may_swap || (nr_swap_pages <= 0)) {> @@ -1583,6 +1590,20 @@ static void shrink_zone(int priority, st> Â  Â  Â  Â  Â  Â  Â  Â if (nr_reclaimed > swap_cluster_max &&> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â priority < DEF_PRIORITY && !current_is_kswapd())> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â break;> +> + Â  Â  Â  Â  Â  Â  Â  /*> + Â  Â  Â  Â  Â  Â  Â  Â * Now, we have plenty free memory.> + Â  Â  Â  Â  Â  Â  Â  Â * Perhaps, big processes exited or they killed by OOM killer.> + Â  Â  Â  Â  Â  Â  Â  Â * To continue reclaim doesn't make any sense.> + Â  Â  Â  Â  Â  Â  Â  Â */> + Â  Â  Â  Â  Â  Â  Â  if (zone_page_state(zone, NR_FREE_PAGES) >> + Â  Â  Â  Â  Â  Â  Â  Â  Â  zone_lru_pages(zone) &&> + Â  Â  Â  Â  Â  Â  Â  Â  Â  zone_watermark_ok(zone, sc->order, high_wmark_pages(zone),> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  classzone_idx, 0)) {> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  /* fake result for reclaim stop */> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  nr_reclaimed += swap_cluster_max;> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  break;> + Â  Â  Â  Â  Â  Â  Â  }> Â  Â  Â  Â }>> Â  Â  Â  Â sc->nr_reclaimed = nr_reclaimed;> @@ -1767,7 +1788,8 @@ out:> Â }>> Â unsigned long try_to_free_pages(struct zonelist *zonelist, int order,> - Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  gfp_t gfp_mask, nodemask_t *nodemask)> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  gfp_t gfp_mask, nodemask_t *nodemask,> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  struct zone *preferred_zone)> Â {> Â  Â  Â  Â struct scan_control sc = {> Â  Â  Â  Â  Â  Â  Â  Â .gfp_mask = gfp_mask,> @@ -1780,6 +1802,7 @@ unsigned long try_to_free_pages(struct z> Â  Â  Â  Â  Â  Â  Â  Â .mem_cgroup = NULL,> Â  Â  Â  Â  Â  Â  Â  Â .isolate_pages = isolate_pages_global,> Â  Â  Â  Â  Â  Â  Â  Â .nodemask = nodemask,> + Â  Â  Â  Â  Â  Â  Â  .preferred_zone = preferred_zone,> Â  Â  Â  Â };>> Â  Â  Â  Â return do_try_to_free_pages(zonelist, &sc);> @@ -1808,6 +1831,10 @@ unsigned long try_to_free_mem_cgroup_pag> Â  Â  Â  Â sc.gfp_mask = (gfp_mask & GFP_RECLAIM_MASK) |> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â (GFP_HIGHUSER_MOVABLE & ~GFP_RECLAIM_MASK);> Â  Â  Â  Â zonelist = NODE_DATA(numa_node_id())->node_zonelists;> + Â  Â  Â  first_zones_zonelist(zonelist,> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â gfp_zone(sc.gfp_mask), NULL,> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â &sc.preferred_zone);> +> Â  Â  Â  Â return do_try_to_free_pages(zonelist, &sc);> Â }> Â #endif> Index: b/fs/buffer.c> ===================================================================> --- a/fs/buffer.c> +++ b/fs/buffer.c> @@ -290,7 +290,7 @@ static void free_more_memory(void)> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â &zone);> Â  Â  Â  Â  Â  Â  Â  Â if (zone)> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â try_to_free_pages(node_zonelist(nid, GFP_NOFS), 0,> - Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  GFP_NOFS, NULL);> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  GFP_NOFS, NULL, zone);> Â  Â  Â  Â }> Â }>> Index: b/include/linux/swap.h> ===================================================================> --- a/include/linux/swap.h> +++ b/include/linux/swap.h> @@ -213,7 +213,8 @@ static inline void lru_cache_add_active_>> Â /* linux/mm/vmscan.c */> Â extern unsigned long try_to_free_pages(struct zonelist *zonelist, int order,> - Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  gfp_t gfp_mask, nodemask_t *mask);> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â gfp_t gfp_mask, nodemask_t *mask,> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â struct zone *preferred_zone);> Â extern unsigned long try_to_free_mem_cgroup_pages(struct mem_cgroup *mem,> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â gfp_t gfp_mask, bool noswap,> Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â unsigned int swappiness);> Index: b/mm/page_alloc.c> ===================================================================> --- a/mm/page_alloc.c> +++ b/mm/page_alloc.c> @@ -1629,7 +1629,8 @@ __alloc_pages_direct_reclaim(gfp_t gfp_m> Â  Â  Â  Â reclaim_state.reclaimed_slab = 0;> Â  Â  Â  Â p->reclaim_state = &reclaim_state;>> - Â  Â  Â  *did_some_progress = try_to_free_pages(zonelist, order, gfp_mask, nodemask);> + Â  Â  Â  *did_some_progress = try_to_free_pages(zonelist, order, gfp_mask,> + Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â  Â nodemask, preferred_zone);>> Â  Â  Â  Â p->reclaim_state = NULL;> Â  Â  Â  Â lockdep_clear_current_reclaim_state();>>>

-- Kind regards,Minchan Kimÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éÝ¶\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dÊ‡Ú™ë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥