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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
