On Wed, Oct 04, 2017 at 12:48:03PM -0700, Shakeel Butt wrote:
> > +
> > +static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
> > +{
> > + struct mem_cgroup *iter;
> > +
> > + oc->chosen_memcg = NULL;
> > + oc->chosen_points = 0;
> > +
> > + /*
> > + * The oom_score is calculated for leaf memory cgroups (including
> > + * the root memcg).
> > + */
> > + rcu_read_lock();
> > + for_each_mem_cgroup_tree(iter, root) {
> > + long score;
> > +
> > + if (memcg_has_children(iter))
> > + continue;
>
> && iter != root_mem_cgroup ?
Oh, sure. I had a stupid bug in my test script, which prevented me from
catching this. Thanks!
This should fix the problem.
--
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2e82625bd354..b3848bce4c86 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2807,7 +2807,8 @@ static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
* We don't consider non-leaf non-oom_group memory cgroups
* as OOM victims.
*/
- if (memcg_has_children(iter) && !mem_cgroup_oom_group(iter))
+ if (memcg_has_children(iter) && iter != root_mem_cgroup &&
+ !mem_cgroup_oom_group(iter))
continue;
/*
@@ -2820,7 +2821,7 @@ static void select_victim_memcg(struct mem_cgroup *root, struct oom_control *oc)
group_score = 0;
}
- if (memcg_has_children(iter))
+ if (memcg_has_children(iter) && iter != root_mem_cgroup)
continue;
score = oom_evaluate_memcg(iter, oc->nodemask, oc->totalpages);
--
>
> > +
> > + score = oom_evaluate_memcg(iter, oc->nodemask, oc->totalpages);
> > +
> > + /*
> > + * Ignore empty and non-eligible memory cgroups.
> > + */
> > + if (score == 0)
> > + continue;
> > +
> > + /*
> > + * If there are inflight OOM victims, we don't need
> > + * to look further for new victims.
> > + */
> > + if (score == -1) {
> > + oc->chosen_memcg = INFLIGHT_VICTIM;
> > + mem_cgroup_iter_break(root, iter);
> > + break;
> > + }
> > +
>
> Shouldn't there be a CSS_ONLINE check? Also instead of css_get at the
> end why not css_tryget_online() here and css_put for the previous
> selected one.
Hm, why do we need to check this? I do not see, how we can choose
an OFFLINE memcg as a victim, tbh. Please, explain the problem.
Thank you!
--
To unsubscribe from this list: send the line "unsubscribe cgroups" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
