Re: [patch 0/8] mm: memcg naturalization -rc2

[Johannes Weiner]

On Wed, Jun 08, 2011 at 08:52:03PM -0700, Ying Han wrote:
> On Wed, Jun 8, 2011 at 8:32 AM, Johannes Weiner <hannes@xxxxxxxxxxx> wrote:
> > On Tue, Jun 07, 2011 at 08:53:21PM -0700, Ying Han wrote:
> >> 2. The way we treat the per-memcg soft_limit is changed in this patch.
> >> The same comment I made on the following patch where we shouldn't
> >> change the definition of user API (soft_limit_in_bytes in this case).
> >> So I attached the patch to fix that where we should only go to the
> >> ones under their soft_limit above certain reclaim priority. Please
> >> consider.
> >
> > Here is your proposal from the other mail:
> >
> > : Basically, we shouldn't reclaim from a memcg under its soft_limit
> > : unless we have trouble reclaim pages from others. Something like the
> > : following makes better sense:
> > :
> > : diff --git a/mm/vmscan.c b/mm/vmscan.c
> > : index bdc2fd3..b82ba8c 100644
> > : --- a/mm/vmscan.c
> > : +++ b/mm/vmscan.c
> > : @@ -1989,6 +1989,8 @@ restart:
> > :         throttle_vm_writeout(sc->gfp_mask);
> > :  }
> > :
> > : +#define MEMCG_SOFTLIMIT_RECLAIM_PRIORITY       2
> > : +
> > :  static void shrink_zone(int priority, struct zone *zone,
> > :                                 struct scan_control *sc)
> > :  {
> > : @@ -2001,13 +2003,13 @@ static void shrink_zone(int priority, struct zone *zone,
> > :                 unsigned long reclaimed = sc->nr_reclaimed;
> > :                 unsigned long scanned = sc->nr_scanned;
> > :                 unsigned long nr_reclaimed;
> > : -               int epriority = priority;
> > :
> > : -               if (mem_cgroup_soft_limit_exceeded(root, mem))
> > : -                       epriority -= 1;
> > : +               if (!mem_cgroup_soft_limit_exceeded(root, mem) &&
> > : +                               priority > MEMCG_SOFTLIMIT_RECLAIM_PRIORITY)
> > : +                       continue;
> >
> > I am not sure if you are serious or playing devil's advocate here,
> > because it exacerbates the problem you are concerned about in 1. by
> > orders of magnitude.
> 
> No, the two are different issues. The first one is a performance
> concern of detailed implementation, while the second one is a design
> concern.

Got ya.

> > I guess it would make much more sense to evaluate if reclaiming from
> > memcgs while there are others exceeding their soft limit is even a
> > problem.  Otherwise this discussion is pretty pointless.
> 
> AFAIK it is a problem since it changes the spec of kernel API
> memory.soft_limit_in_bytes. That value is set per-memcg which all the
> pages allocated above that are best effort and targeted to reclaim
> prior to others.

That's not really true.  Quoting the documentation:

    When the system detects memory contention or low memory, control groups
    are pushed back to their soft limits. If the soft limit of each control
    group is very high, they are pushed back as much as possible to make
    sure that one control group does not starve the others of memory.

I am language lawyering here, but I don't think it says it won't touch
other memcgs at all while there are memcgs exceeding their soft limit.

It would be a lie about the current code in the first place, which
does soft limit reclaim and then regular reclaim, no matter the
outcome of the soft limit reclaim cycle.  It will go for the soft
limit first, but after an allocation under pressure the VM is likely
to have reclaimed from other memcgs as well.

I saw your patch to fix that and break out of reclaim if soft limit
reclaim did enough.  But this fix is not much newer than my changes.

The second part of this is:

    Please note that soft limits is a best effort feature, it comes with
    no guarantees, but it does its best to make sure that when memory is
    heavily contended for, memory is allocated based on the soft limit
    hints/setup. Currently soft limit based reclaim is setup such that
    it gets invoked from balance_pgdat (kswapd).

It's not the pages-over-soft-limit that are best effort.  It says that
it tries its best to take soft limits into account while reclaiming.

My code does that, so I don't think we are breaking any promises
currently made in the documentation.

But much more important than keeping documentation promises is not to
break actual users.  So if you are yourself a user of soft limits,
test the new code pretty please and complain if it breaks your setup!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/