From: Minchan Kim <minchan.kim@gmail.com> To: Andrew Morton <akpm@linux-foundation.org> Cc: Johannes Weiner <hannes@cmpxchg.org>, linux-mm <linux-mm@kvack.org>, LKML <linux-kernel@vger.kernel.org>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, Rik van Riel <riel@redhat.com>, "Rafael J. Wysocki" <rjw@sisk.pl>, "M. Vefa Bicakci" <bicave@superonline.com>, stable@kernel.org Subject: Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path Date: Mon, 13 Sep 2010 01:20:36 +0900 [thread overview] Message-ID: <AANLkTi=jmV=x2rJ=G4iicYFO6UqPbfob_VnkY7VNbP3X@mail.gmail.com> (raw) In-Reply-To: <20100908151929.2586ace5.akpm@linux-foundation.org> On Thu, Sep 9, 2010 at 7:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 9 Sep 2010 00:45:27 +0900 > Minchan Kim <minchan.kim@gmail.com> wrote: > >> +static inline bool zone_reclaimable(struct zone *zone) >> +{ >> + return zone->pages_scanned < zone_reclaimable_pages(zone) * 6; >> +} >> + >> +static inline bool all_unreclaimable(struct zonelist *zonelist, >> + struct scan_control *sc) >> +{ >> + struct zoneref *z; >> + struct zone *zone; >> + bool all_unreclaimable = true; >> + >> + if (!scanning_global_lru(sc)) >> + return false; >> + >> + for_each_zone_zonelist_nodemask(zone, z, zonelist, >> + gfp_zone(sc->gfp_mask), sc->nodemask) { >> + if (!populated_zone(zone)) >> + continue; >> + if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL)) >> + continue; >> + if (zone_reclaimable(zone)) { >> + all_unreclaimable = false; >> + break; >> + } >> + } >> + >> return all_unreclaimable; >> } > > Could we have some comments over these functions please? Why they > exist, what problem they solve, how they solve them, etc. Stuff which > will be needed for maintaining this code three years from now. > > We may as well remove the `inline's too. gcc will tkae care of that. Okay. I will resend. > >> - if (nr_slab == 0 && >> - zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6)) >> + if (nr_slab == 0 && !zone_reclaimable(zone)) > > Extra marks for working out and documenting how we decided on the value > of "6". Sigh. It's hopefully in the git record somewhere. > Originally it is just following as. if (zone->pages_scanned > zone->present_pages * 2) zone->all_unreclaimable = 1; Nick change it with remained lru * 4 [1] and increased 6 [2]. But the description doesn't have why we determine it by "4". So I can't handle it in my patch. I don't like undocumented magic value. :( [1] commit 9d0aa0f7a99c88dd20bc188756b892f174d93fc1 Author: nickpiggin <nickpiggin> Date: Sun Oct 17 16:20:56 2004 +0000 [PATCH] kswapd lockup fix Fix some bugs in the kswapd logic which can cause kswapd lockups. [2] commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077 Author: Nick Piggin <npiggin@suse.de> Date: Mon Sep 25 23:31:28 2006 -0700 [PATCH] oom: reclaim_mapped on oom -- Kind regards, Minchan Kim
WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan.kim@gmail.com> To: Andrew Morton <akpm@linux-foundation.org> Cc: Johannes Weiner <hannes@cmpxchg.org>, linux-mm <linux-mm@kvack.org>, LKML <linux-kernel@vger.kernel.org>, KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>, Rik van Riel <riel@redhat.com>, "Rafael J. Wysocki" <rjw@sisk.pl>, "M. Vefa Bicakci" <bicave@superonline.com>, stable@kernel.org Subject: Re: [PATCH] vmscan: check all_unreclaimable in direct reclaim path Date: Mon, 13 Sep 2010 01:20:36 +0900 [thread overview] Message-ID: <AANLkTi=jmV=x2rJ=G4iicYFO6UqPbfob_VnkY7VNbP3X@mail.gmail.com> (raw) In-Reply-To: <20100908151929.2586ace5.akpm@linux-foundation.org> On Thu, Sep 9, 2010 at 7:19 AM, Andrew Morton <akpm@linux-foundation.org> wrote: > On Thu, 9 Sep 2010 00:45:27 +0900 > Minchan Kim <minchan.kim@gmail.com> wrote: > >> +static inline bool zone_reclaimable(struct zone *zone) >> +{ >> + return zone->pages_scanned < zone_reclaimable_pages(zone) * 6; >> +} >> + >> +static inline bool all_unreclaimable(struct zonelist *zonelist, >> + struct scan_control *sc) >> +{ >> + struct zoneref *z; >> + struct zone *zone; >> + bool all_unreclaimable = true; >> + >> + if (!scanning_global_lru(sc)) >> + return false; >> + >> + for_each_zone_zonelist_nodemask(zone, z, zonelist, >> + gfp_zone(sc->gfp_mask), sc->nodemask) { >> + if (!populated_zone(zone)) >> + continue; >> + if (!cpuset_zone_allowed_hardwall(zone, GFP_KERNEL)) >> + continue; >> + if (zone_reclaimable(zone)) { >> + all_unreclaimable = false; >> + break; >> + } >> + } >> + >> return all_unreclaimable; >> } > > Could we have some comments over these functions please? Why they > exist, what problem they solve, how they solve them, etc. Stuff which > will be needed for maintaining this code three years from now. > > We may as well remove the `inline's too. gcc will tkae care of that. Okay. I will resend. > >> - if (nr_slab == 0 && >> - zone->pages_scanned >= (zone_reclaimable_pages(zone) * 6)) >> + if (nr_slab == 0 && !zone_reclaimable(zone)) > > Extra marks for working out and documenting how we decided on the value > of "6". Sigh. It's hopefully in the git record somewhere. > Originally it is just following as. if (zone->pages_scanned > zone->present_pages * 2) zone->all_unreclaimable = 1; Nick change it with remained lru * 4 [1] and increased 6 [2]. But the description doesn't have why we determine it by "4". So I can't handle it in my patch. I don't like undocumented magic value. :( [1] commit 9d0aa0f7a99c88dd20bc188756b892f174d93fc1 Author: nickpiggin <nickpiggin> Date: Sun Oct 17 16:20:56 2004 +0000 [PATCH] kswapd lockup fix Fix some bugs in the kswapd logic which can cause kswapd lockups. [2] commit 4ff1ffb4870b007b86f21e5f27eeb11498c4c077 Author: Nick Piggin <npiggin@suse.de> Date: Mon Sep 25 23:31:28 2006 -0700 [PATCH] oom: reclaim_mapped on oom -- Kind regards, Minchan Kim -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2010-09-12 16:20 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-09-05 14:40 [PATCH] vmscan: check all_unreclaimable in direct reclaim path Minchan Kim 2010-09-05 14:40 ` Minchan Kim 2010-09-05 22:30 ` Rafael J. Wysocki 2010-09-05 22:30 ` Rafael J. Wysocki 2010-09-08 5:48 ` Johannes Weiner 2010-09-08 5:48 ` Johannes Weiner 2010-09-08 15:45 ` Minchan Kim 2010-09-08 15:45 ` Minchan Kim 2010-09-08 22:19 ` Andrew Morton 2010-09-08 22:19 ` Andrew Morton 2010-09-10 8:24 ` Dave Young 2010-09-10 8:24 ` Dave Young 2010-09-12 16:20 ` Minchan Kim 2010-09-12 16:20 ` Minchan Kim 2010-09-12 16:20 ` Minchan Kim [this message] 2010-09-12 16:20 ` Minchan Kim
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='AANLkTi=jmV=x2rJ=G4iicYFO6UqPbfob_VnkY7VNbP3X@mail.gmail.com' \ --to=minchan.kim@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=bicave@superonline.com \ --cc=hannes@cmpxchg.org \ --cc=kosaki.motohiro@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=riel@redhat.com \ --cc=rjw@sisk.pl \ --cc=stable@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.