Re: Possible regression with cgroups in 3.11

[Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>]

On Mon 18-11-13 15:31:26, Markus Blank-Burian wrote:
> > There is one more issue which is discussed in another thread
> > (https://lkml.org/lkml/2013/11/15/31) and Tejun has posted a patch (and
> > Hugh followed up on it https://lkml.org/lkml/2013/11/17/166) to fix
> > the cgroup destruction path which may get stuck.
> >
> 
> Tried out the patches from Johannes, Tejun+Hugh and Michal all

You do not have to apply my patch as Johannes' one should be sufficient.

> together and my problem still persists :-(
> Anything I can try besides bisectioning (which I still could not get
> to work yet)?

Add more debugging information I guess. Let's see if something like the
following helps
---
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3d69a3fe4c55..602db53c690d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3928,6 +3928,8 @@ static int mem_cgroup_move_parent(struct page *page,
 put:
 	put_page(page);
 out:
+	if (ret)
+		trace_printk("%s failed with %d for memcg:%p\n", __FUNCTION__, ret, child);
 	return ret;
 }
 
@@ -4946,7 +4948,7 @@ static void mem_cgroup_force_empty_list(struct mem_cgroup *memcg,
 static void mem_cgroup_reparent_charges(struct mem_cgroup *memcg)
 {
 	int node, zid;
-	u64 usage;
+	u64 usage, u, k;
 
 	do {
 		/* This is for making all *used* pages to be on LRU. */
@@ -4978,8 +4980,11 @@ static void mem_cgroup_reparent_charges(struct mem_cgroup *memcg)
 		 * right after the check. RES_USAGE should be safe as we always
 		 * charge before adding to the LRU.
 		 */
-		usage = res_counter_read_u64(&memcg->res, RES_USAGE) -
-			res_counter_read_u64(&memcg->kmem, RES_USAGE);
+		u = res_counter_read_u64(&memcg->res, RES_USAGE);
+		k = res_counter_read_u64(&memcg->kmem, RES_USAGE);
+		usage = u - k;
+		if (usage > 0)
+			trace_printk("memcg:%p u:%lu k:%lu tasks:%d\n", memcg, u, k, cgroup_task_count(memcg->css.cgrp));
 	} while (usage > 0);
 }
 
@@ -6358,12 +6363,14 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 
+	trace_printk("memcg:%p is going offline now\n", memcg);
 	kmem_cgroup_css_offline(memcg);
 
 	mem_cgroup_invalidate_reclaim_iterators(memcg);
 	mem_cgroup_reparent_charges(memcg);
 	mem_cgroup_destroy_all_caches(memcg);
 	vmpressure_cleanup(&memcg->vmpressure);
+	trace_printk("memcg:%p is offline now\n", memcg);
 }
 
 static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
@@ -6746,6 +6753,10 @@ static int mem_cgroup_can_attach(struct cgroup_subsys_state *css,
 	struct mem_cgroup *memcg = mem_cgroup_from_css(css);
 	unsigned long move_charge_at_immigrate;
 
+	if (!css_tryget(css))
+		trace_printk("Target memcg %p is dead!\n", memcg);
+	else
+		css_put(css);
 	/*
 	 * We are now commited to this value whatever it is. Changes in this
 	 * tunable will only affect upcoming migrations, not the current one.

-- 
Michal Hocko
SUSE Labs