linux-kernel - Re: [PATCH 1/4] memcg, mm: introduce lowlimit reclaim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 11 Jun 2014 18:08:05 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Greg Thelen <gthelen@...gle.com>,
	Michel Lespinasse <walken@...gle.com>,
	Tejun Heo <tj@...nel.org>, Hugh Dickins <hughd@...gle.com>,
	Roman Gushchin <klamm@...dex-team.ru>,
	LKML <linux-kernel@...r.kernel.org>, linux-mm@...ck.org
Subject: Re: [PATCH 1/4] memcg, mm: introduce lowlimit reclaim

On Wed 11-06-14 11:15:44, Johannes Weiner wrote:
> On Mon, May 05, 2014 at 04:21:00PM +0200, Michal Hocko wrote:
[...]
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -2293,13 +2293,20 @@ static unsigned __shrink_zone(struct zone *zone, struct scan_control *sc,
> >  
> >  static void shrink_zone(struct zone *zone, struct scan_control *sc)
> >  {
> > -	if (!__shrink_zone(zone, sc, true)) {
> > +	bool honor_guarantee = true;
> > +
> > +	while (!__shrink_zone(zone, sc, honor_guarantee)) {
> >  		/*
> > -		 * First round of reclaim didn't find anything to reclaim
> > -		 * because of the memory guantees for all memcgs in the
> > -		 * reclaim target so try again and ignore guarantees this time.
> > +		 * The previous round of reclaim didn't find anything to scan
> > +		 * because
> > +		 * a) the whole reclaimed hierarchy is within guarantee so
> > +		 *    we fallback to ignore the guarantee because other option
> > +		 *    would be the OOM
> > +		 * b) multiple reclaimers are racing and so the first round
> > +		 *    should be retried
> >  		 */
> > -		__shrink_zone(zone, sc, false);
> > +		if (mem_cgroup_all_within_guarantee(sc->target_mem_cgroup))
> > +			honor_guarantee = false;
> >  	}
> 
> I don't like that this adds a non-chalant `for each memcg' here, we
> can have a lot of memcgs.  Sooner or later we'll have to break up that
> full hierarchy iteration in shrink_zone() because of scalability, I
> want to avoid adding more of them.

mem_cgroup_all_within_guarantee can be simply optimized to exclude whole
subtrees of each memcg which is mem_cgroup_within_guarantee. cgroups
iterator are easy and quite optimal to skip the whole subtree AFAIR so I
do not see this as a bottleneck here.

> How about these changes on top of what we currently have?

I really do not like how you got back to priority based break out.
We were discussing that 2 or so years ago and the main objection was
that this is really not useful. You do not want to scan/reclaim so far
"priviledged" memcgs at high priority all of the sudden.

> Sure it's not as accurate, but it should be good start, and it's a
> *lot* less overhead.
> 
> mem_cgroup_watermark() is also a more fitting name, given that this
> has nothing to do with a guarantee for now.

mem_cgroup_watermark sounds like a better name indeed.

> It can also be easily extended to support the MIN watermark while the
> code in vmscan.c remains readable.
 
-- 
Michal Hocko
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/