From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx141.postini.com [74.125.245.141]) by kanga.kvack.org (Postfix) with SMTP id CA3606B0044 for ; Mon, 23 Apr 2012 19:18:03 -0400 (EDT) Received: by lagz14 with SMTP id z14so72753lag.14 for ; Mon, 23 Apr 2012 16:18:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1335214564-17619-1-git-send-email-yinghan@google.com> Date: Mon, 23 Apr 2012 16:18:01 -0700 Message-ID: Subject: Re: [RFC PATCH] do_try_to_free_pages() might enter infinite loop From: Ying Han Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Sender: owner-linux-mm@kvack.org List-ID: To: KOSAKI Motohiro Cc: Michal Hocko , Johannes Weiner , Mel Gorman , KAMEZAWA Hiroyuki , Rik van Riel , Minchan Kim , Hugh Dickins , Nick Piggin , Andrew Morton , linux-mm@kvack.org On Mon, Apr 23, 2012 at 3:20 PM, KOSAKI Motohiro wrote: > On Mon, Apr 23, 2012 at 4:56 PM, Ying Han wrote: >> This is not a patch targeted to be merged at all, but trying to understa= nd >> a logic in global direct reclaim. >> >> There is a logic in global direct reclaim where reclaim fails on priorit= y 0 >> and zone->all_unreclaimable is not set, it will cause the direct to star= t over >> from DEF_PRIORITY. In some extreme cases, we've seen the system hang whi= ch is >> very likely caused by direct reclaim enters infinite loop. >> >> There have been serious patches trying to fix similar issue and the late= st >> patch has good summary of all the efforts: >> >> commit 929bea7c714220fc76ce3f75bef9056477c28e74 >> Author: KOSAKI Motohiro >> Date: =A0 Thu Apr 14 15:22:12 2011 -0700 >> >> =A0 =A0vmscan: all_unreclaimable() use zone->all_unreclaimable as a name >> >> Kosaki explained the problem triggered by async zone->all_unreclaimable = and >> zone->pages_scanned where the later one was being checked by direct recl= aim. >> However, after the patch, the problem remains where the setting of >> zone->all_unreclaimable is asynchronous with zone is actually reclaimabl= e or not. >> >> The zone->all_unreclaimable flag is set by kswapd by checking zone->page= s_scanned in >> zone_reclaimable(). Is that possible to have zone->all_unreclaimable =3D= =3D false while >> the zone is actually unreclaimable? >> >> 1. while kswapd in reclaim priority loop, someone frees a page on the zo= ne. It >> will end up resetting the pages_scanned. >> >> 2. kswapd is frozen for whatever reason. I noticed Kosaki's covered the >> hibernation case by checking oom_killer_disabled, but not sure if that i= s >> everything we need to worry about. The key point here is that direct rec= laim >> relies on a flag which is set by kswapd asynchronously, that doesn't sou= nd safe. > > If kswapd was frozen except hibernation, why don't you add frozen > check instead of > hibernation check? And when and why is that happen? I haven't tried to reproduce the issue, so everything is based on eye-balling the code. The problem is that we have the potential infinite loop in direct reclaim where it keeps trying as long as !zone->all_unreclaimable. The flag is only set by kswapd and it will skip setting the flag if the following condition is true: zone->pages_scanned < zone_reclaimable_pages(zone) * 6; In a few-pages-on-lru condition, the zone->pages_scanned is easily remains 0 and also it is reset to 0 everytime a page being freed. Then, i will cause global direct reclaim entering infinite loop. > > >> >> Instead of keep fixing the problem, I am wondering why we have the logic >> "not oom but keep trying reclaim w/ priority 0 reclaim failure" at the f= irst place: >> >> Here is the patch introduced the logic initially: >> >> commit 408d85441cd5a9bd6bc851d677a10c605ed8db5f >> Author: Nick Piggin >> Date: =A0 Mon Sep 25 23:31:27 2006 -0700 >> >> =A0 =A0[PATCH] oom: use unreclaimable info >> >> However, I didn't find detailed description of what problem the commit t= rying >> to fix and wondering if the problem still exist after 5 years. I would b= e happy >> to see the later case where we can consider to revert the initial patch. > > This patch fixed one of false oom issue. Think, > > 1. thread-a reach priority-0. > 2. thread-b was exited and free a lot of pages. > 3. thread-a call out_of_memory(). > > This is not very good because we now have enough memory.... Isn't that being covered by the following in __alloc_pages_may_oom() ? >-------/* >------- * Go through the zonelist yet one more time, keep very high waterm= ark >------- * here, this is only to catch a parallel oom killing, we must fail= if >------- * we're still under heavy pressure. >------- */ >-------page =3D get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, >------->-------order, zonelist, high_zoneidx, >------->-------ALLOC_WMARK_HIGH|ALLOC_CPUSET, >------->-------preferred_zone, migratetype); Thanks --Ying -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org