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

[Minchan Kim <minchan.kim@gmail.com>]

[-- 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¥