From: Tejun Heo <tj@kernel.org> To: Michal Hocko <mhocko@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Johannes Weiner <hannes@cmpxchg.org>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>, Ying Han <yinghan@google.com>, Hugh Dickins <hughd@google.com>, Glauber Costa <glommer@parallels.com>, Michel Lespinasse <walken@google.com>, Greg Thelen <gthelen@google.com>, Balbir Singh <bsingharora@gmail.com> Subject: Re: [patch v3 -mm 3/3] vmscan, memcg: Do softlimit reclaim also for targeted reclaim Date: Thu, 16 May 2013 16:12:38 -0700 [thread overview] Message-ID: <20130516231238.GA15025@mtj.dyndns.org> (raw) In-Reply-To: <1368431172-6844-4-git-send-email-mhocko@suse.cz> On Mon, May 13, 2013 at 09:46:12AM +0200, Michal Hocko wrote: > Soft reclaim has been done only for the global reclaim (both background > and direct). Since "memcg: integrate soft reclaim tighter with zone > shrinking code" there is no reason for this limitation anymore as the > soft limit reclaim doesn't use any special code paths and it is a > part of the zone shrinking code which is used by both global and > targeted reclaims. ... > Signed-off-by: Michal Hocko <mhocko@suse.cz> Reviewed-by: Tejun Heo <tj@kernel.org> Some nitpicks follow. > /* > - * A group is eligible for the soft limit reclaim if it is > - * a) is over its soft limit > + * A group is eligible for the soft limit reclaim under the given root > + * hierarchy if > + * a) it is over its soft limit > * b) any parent up the hierarchy is over its soft limit This was added before but in general I think the use of parent for ancestor is a bit confusing. Not a big deal but no reason to continue it. > /* > - * If any parent up the hierarchy is over its soft limit then we > - * have to obey and reclaim from this group as well. > + * If any parent up to the root in the hierarchy is over its soft limit > + * then we have to obey and reclaim from this group as well. Prolly using terms ancestors and subtree would make the explanation clearer? > static bool mem_cgroup_should_soft_reclaim(struct scan_control *sc) > { > - return global_reclaim(sc); > + return true; Kinda silly after this change, maybe just modify shrink_zone() like the following? if (IS_ENABLED(CONFIG_MEMCG)) { __shrink_zone(zone, sc, true); if (sc->nr_scanned == nr_scanned) __shrink_zone(zone, sc, false); } else { __shrink_zone(zone, sc, false); } > @@ -1974,7 +1974,7 @@ __shrink_zone(struct zone *zone, struct scan_control *sc, bool soft_reclaim) > struct lruvec *lruvec; > > if (soft_reclaim && > - !mem_cgroup_soft_reclaim_eligible(memcg)) { > + !mem_cgroup_soft_reclaim_eligible(memcg, root)) { Weird indentation which breaks line and goes over 80 col, why not do the following? if (soft_reclaim && !mem_cgroup_soft_reclaim_eligible(memcg, root)) { memcg = mem_cgroup_iter(root, memcg, &reclaim); continue; } Thanks. -- tejun
WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org> To: Michal Hocko <mhocko@suse.cz> Cc: Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org, Johannes Weiner <hannes@cmpxchg.org>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>, Ying Han <yinghan@google.com>, Hugh Dickins <hughd@google.com>, Glauber Costa <glommer@parallels.com>, Michel Lespinasse <walken@google.com>, Greg Thelen <gthelen@google.com>, Balbir Singh <bsingharora@gmail.com> Subject: Re: [patch v3 -mm 3/3] vmscan, memcg: Do softlimit reclaim also for targeted reclaim Date: Thu, 16 May 2013 16:12:38 -0700 [thread overview] Message-ID: <20130516231238.GA15025@mtj.dyndns.org> (raw) In-Reply-To: <1368431172-6844-4-git-send-email-mhocko@suse.cz> On Mon, May 13, 2013 at 09:46:12AM +0200, Michal Hocko wrote: > Soft reclaim has been done only for the global reclaim (both background > and direct). Since "memcg: integrate soft reclaim tighter with zone > shrinking code" there is no reason for this limitation anymore as the > soft limit reclaim doesn't use any special code paths and it is a > part of the zone shrinking code which is used by both global and > targeted reclaims. ... > Signed-off-by: Michal Hocko <mhocko@suse.cz> Reviewed-by: Tejun Heo <tj@kernel.org> Some nitpicks follow. > /* > - * A group is eligible for the soft limit reclaim if it is > - * a) is over its soft limit > + * A group is eligible for the soft limit reclaim under the given root > + * hierarchy if > + * a) it is over its soft limit > * b) any parent up the hierarchy is over its soft limit This was added before but in general I think the use of parent for ancestor is a bit confusing. Not a big deal but no reason to continue it. > /* > - * If any parent up the hierarchy is over its soft limit then we > - * have to obey and reclaim from this group as well. > + * If any parent up to the root in the hierarchy is over its soft limit > + * then we have to obey and reclaim from this group as well. Prolly using terms ancestors and subtree would make the explanation clearer? > static bool mem_cgroup_should_soft_reclaim(struct scan_control *sc) > { > - return global_reclaim(sc); > + return true; Kinda silly after this change, maybe just modify shrink_zone() like the following? if (IS_ENABLED(CONFIG_MEMCG)) { __shrink_zone(zone, sc, true); if (sc->nr_scanned == nr_scanned) __shrink_zone(zone, sc, false); } else { __shrink_zone(zone, sc, false); } > @@ -1974,7 +1974,7 @@ __shrink_zone(struct zone *zone, struct scan_control *sc, bool soft_reclaim) > struct lruvec *lruvec; > > if (soft_reclaim && > - !mem_cgroup_soft_reclaim_eligible(memcg)) { > + !mem_cgroup_soft_reclaim_eligible(memcg, root)) { Weird indentation which breaks line and goes over 80 col, why not do the following? if (soft_reclaim && !mem_cgroup_soft_reclaim_eligible(memcg, root)) { memcg = mem_cgroup_iter(root, memcg, &reclaim); continue; } Thanks. -- tejun -- 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:[~2013-05-16 23:12 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-05-13 7:46 [patch v3 0/3 -mm] Soft limit rework Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-13 7:46 ` [patch v3 -mm 1/3] memcg: integrate soft reclaim tighter with zone shrinking code Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-15 8:34 ` Glauber Costa 2013-05-15 8:34 ` Glauber Costa 2013-05-16 22:12 ` Tejun Heo 2013-05-16 22:12 ` Tejun Heo 2013-05-16 22:12 ` Tejun Heo 2013-05-16 22:15 ` Tejun Heo 2013-05-16 22:15 ` Tejun Heo 2013-05-17 7:16 ` Michal Hocko 2013-05-17 7:16 ` Michal Hocko 2013-05-17 7:16 ` Michal Hocko 2013-05-17 7:12 ` Michal Hocko 2013-05-17 7:12 ` Michal Hocko 2013-05-17 16:02 ` Johannes Weiner 2013-05-17 16:02 ` Johannes Weiner 2013-05-17 16:57 ` Tejun Heo 2013-05-17 16:57 ` Tejun Heo 2013-05-17 17:27 ` Johannes Weiner 2013-05-17 17:27 ` Johannes Weiner 2013-05-17 17:45 ` Tejun Heo 2013-05-17 17:45 ` Tejun Heo 2013-05-17 17:45 ` Tejun Heo 2013-05-20 14:44 ` Michal Hocko 2013-05-20 14:44 ` Michal Hocko 2013-05-20 14:44 ` Michal Hocko 2013-05-21 6:53 ` Michal Hocko 2013-05-21 6:53 ` Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:13 ` [PATCH 1/3] memcg: track children in soft limit excess to improve soft limit Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:13 ` [PATCH 2/3] memcg, vmscan: Do not attempt soft limit reclaim if it would not scan anything Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:13 ` [PATCH 3/3] memcg: Track all children over limit in the root Michal Hocko 2013-05-27 17:13 ` Michal Hocko 2013-05-27 17:20 ` [PATCH] memcg: enhance memcg iterator to support predicates Michal Hocko 2013-05-27 17:20 ` Michal Hocko 2013-05-27 17:20 ` Michal Hocko 2013-05-29 13:05 ` [patch v3 -mm 1/3] memcg: integrate soft reclaim tighter with zone shrinking code Michal Hocko 2013-05-29 13:05 ` Michal Hocko 2013-05-29 13:05 ` Michal Hocko 2013-05-29 15:57 ` Michal Hocko 2013-05-29 15:57 ` Michal Hocko 2013-05-29 20:01 ` Johannes Weiner 2013-05-29 20:01 ` Johannes Weiner 2013-05-30 8:45 ` Michal Hocko 2013-05-30 8:45 ` Michal Hocko 2013-05-29 14:54 ` Michal Hocko 2013-05-29 14:54 ` Michal Hocko 2013-05-30 8:36 ` Michal Hocko 2013-05-30 8:36 ` Michal Hocko 2013-05-13 7:46 ` [patch v3 -mm 2/3] memcg: Get rid of soft-limit tree infrastructure Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-15 8:38 ` Glauber Costa 2013-05-15 8:38 ` Glauber Costa 2013-05-16 22:16 ` Tejun Heo 2013-05-16 22:16 ` Tejun Heo 2013-05-13 7:46 ` [patch v3 -mm 3/3] vmscan, memcg: Do softlimit reclaim also for targeted reclaim Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-13 7:46 ` Michal Hocko 2013-05-15 8:42 ` Glauber Costa 2013-05-15 8:42 ` Glauber Costa 2013-05-17 7:50 ` Michal Hocko 2013-05-17 7:50 ` Michal Hocko 2013-05-17 7:50 ` Michal Hocko 2013-05-16 23:12 ` Tejun Heo [this message] 2013-05-16 23:12 ` Tejun Heo 2013-05-17 7:34 ` Michal Hocko 2013-05-17 7:34 ` Michal Hocko
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=20130516231238.GA15025@mtj.dyndns.org \ --to=tj@kernel.org \ --cc=akpm@linux-foundation.org \ --cc=bsingharora@gmail.com \ --cc=cgroups@vger.kernel.org \ --cc=glommer@parallels.com \ --cc=gthelen@google.com \ --cc=hannes@cmpxchg.org \ --cc=hughd@google.com \ --cc=kamezawa.hiroyu@jp.fujitsu.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@suse.cz \ --cc=walken@google.com \ --cc=yinghan@google.com \ /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.