Re: [patch 2/2] mm: memcg: hierarchical soft limit reclaim

[Ying Han <yinghan@google.com>]

On Wed, Jan 18, 2012 at 1:45 AM, Johannes Weiner <hannes@cmpxchg.org> wrote:
> On Fri, Jan 13, 2012 at 01:45:30PM -0800, Ying Han wrote:
>> On Fri, Jan 13, 2012 at 8:34 AM, Michal Hocko <mhocko@suse.cz> wrote:
>> > On Fri 13-01-12 16:50:01, Johannes Weiner wrote:
>> >> On Fri, Jan 13, 2012 at 01:04:06PM +0100, Michal Hocko wrote:
>> >> > On Tue 10-01-12 16:02:52, Johannes Weiner wrote:
>> > [...]
>> >> > > +bool mem_cgroup_over_softlimit(struct mem_cgroup *root,
>> >> > > +                        struct mem_cgroup *memcg)
>> >> > > +{
>> >> > > + if (mem_cgroup_disabled())
>> >> > > +         return false;
>> >> > > +
>> >> > > + if (!root)
>> >> > > +         root = root_mem_cgroup;
>> >> > > +
>> >> > > + for (; memcg; memcg = parent_mem_cgroup(memcg)) {
>> >> > > +         /* root_mem_cgroup does not have a soft limit */
>> >> > > +         if (memcg == root_mem_cgroup)
>> >> > > +                 break;
>> >> > > +         if (res_counter_soft_limit_excess(&memcg->res))
>> >> > > +                 return true;
>> >> > > +         if (memcg == root)
>> >> > > +                 break;
>> >> > > + }
>> >> > > + return false;
>> >> > > +}
>> >> >
>> >> > Well, this might be little bit tricky. We do not check whether memcg and
>> >> > root are in a hierarchy (in terms of use_hierarchy) relation.
>> >> >
>> >> > If we are under global reclaim then we iterate over all memcgs and so
>> >> > there is no guarantee that there is a hierarchical relation between the
>> >> > given memcg and its parent. While, on the other hand, if we are doing
>> >> > memcg reclaim then we have this guarantee.
>> >> >
>> >> > Why should we punish a group (subtree) which is perfectly under its soft
>> >> > limit just because some other subtree contributes to the common parent's
>> >> > usage and makes it over its limit?
>> >> > Should we check memcg->use_hierarchy here?
>> >>
>> >> We do, actually.  parent_mem_cgroup() checks the res_counter parent,
>> >> which is only set when ->use_hierarchy is also set.
>> >
>> > Of course I am blind.. We do not setup res_counter parent for
>> > !use_hierarchy case. Sorry for noise...
>> > Now it makes much better sense. I was wondering how !use_hierarchy could
>> > ever work, this should be a signal that I am overlooking something
>> > terribly.
>> >
>> > [...]
>> >> > > @@ -2121,8 +2121,16 @@ static void shrink_zone(int priority, struct zone *zone,
>> >> > >                   .mem_cgroup = memcg,
>> >> > >                   .zone = zone,
>> >> > >           };
>> >> > > +         int epriority = priority;
>> >> > > +         /*
>> >> > > +          * Put more pressure on hierarchies that exceed their
>> >> > > +          * soft limit, to push them back harder than their
>> >> > > +          * well-behaving siblings.
>> >> > > +          */
>> >> > > +         if (mem_cgroup_over_softlimit(root, memcg))
>> >> > > +                 epriority = 0;
>> >> >
>> >> > This sounds too aggressive to me. Shouldn't we just double the pressure
>> >> > or something like that?
>> >>
>> >> That's the historical value.  When I tried priority - 1, it was not
>> >> aggressive enough.
>> >
>> > Probably because we want to reclaim too much. Maybe we should do
>> > reduce nr_to_reclaim (ugly) or reclaim only overlimit groups until certain
>> > priority level as Ying suggested in her patchset.
>>
>> I plan to post that change on top of this, and this patch set does the
>> basic stuff to allow us doing further improvement.
>>
>> I still like the design to skip over_soft_limit cgroups until certain
>> priority. One way to set up the soft limit for each cgroup is to base
>> on its actual working set size, and we prefer to punish A first with
>> lots of page cache ( cold file pages above soft limit) than reclaiming
>> anon pages from B ( below soft limit ). Unless we can not get enough
>> pages reclaimed from A, we will start reclaiming from B.
>>
>> This might not be the ideal solution, but should be a good start. Thoughts?
>
> I don't like this design at all because unless you add weird code to
> detect if soft limits apply to any memcgs on the reclaimed hierarchy
> you may iterate over the same bunch of memcgs doing nothing for
> several times.  For example in the default case of no softlimits set
> anywhere and you repeatedly walk ALL memcgs in the system doing jack
> until you reach your threshold priority level.  Elegant is something
> else in my book.

Agree that change isn't ready until the default soft limit is changed to "0".

> Once we invert soft limits to mean guarantees and make the default
> soft limit not infinity but zero, then we can ignore memcgs below
> their soft limit for a few priority levels just fine because being
> below the soft limit is the exception.  But I don't really want to
> make this quite invasive behavioural change a requirement for a
> refactoring patch if possible.

Sounds reasonable to me.

--Ying

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>