On Tue, 9 Aug 2011 17:37:32 +0200
Johannes Weiner [off-list ref] wrote:

On Tue, Aug 09, 2011 at 05:22:18PM +0200, Michal Hocko wrote:

quoted

On Tue 09-08-11 16:03:12, Johannes Weiner wrote:

quoted

 	struct mem_cgroup *iter, *failed = NULL;
 	bool cond = true;
 
 	for_each_mem_cgroup_tree_cond(iter, mem, cond) {
-		bool locked = iter->oom_lock;
-
-		iter->oom_lock = true;
-		if (lock_count == -1)
-			lock_count = iter->oom_lock;
-		else if (lock_count != locked) {
+		if (iter->oom_lock) {
 			/*
 			 * this subtree of our hierarchy is already locked
 			 * so we cannot give a lock.
 			 */
-			lock_count = 0;
 			failed = iter;
 			cond = false;
-		}
+		} else
+			iter->oom_lock = true;
 	}
 
 	if (!failed)

We can return here and get rid of done label.

Ah, right you are.  Here is an update.

---
From 86b36904033e6c6a1af4716e9deef13ebd31e64c Mon Sep 17 00:00:00 2001
From: Johannes Weiner <redacted>
Date: Tue, 9 Aug 2011 15:31:30 +0200
Subject: [patch] memcg: fix hierarchical oom locking

Commit "79dfdac memcg: make oom_lock 0 and 1 based rather than
counter" tried to oom lock the hierarchy and roll back upon
encountering an already locked memcg.

The code is confused when it comes to detecting a locked memcg,
though, so it would fail and rollback after locking one memcg and
encountering an unlocked second one.

The result is that oom-locking hierarchies fails unconditionally and
that every oom killer invocation simply goes to sleep on the oom
waitqueue forever.  The tasks practically hang forever without anyone
intervening, possibly holding locks that trip up unrelated tasks, too.

Signed-off-by: Johannes Weiner <redacted>
Acked-by: Michal Hocko <redacted>

Thanks,
Acked-by: KAMEZAWA Hiroyuki <redacted>


--
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>