Re: [PATCH 2/4] memcg: prevent changes to move_charge_at_immigrate during task attach
From: Tejun Heo <tj@kernel.org>
Date: 2012-11-30 15:19:27
Also in:
linux-mm
Hello, Glauber. On Fri, Nov 30, 2012 at 05:31:24PM +0400, Glauber Costa wrote:
quoted hunk ↗ jump to hunk
--- mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 13 deletions(-)diff --git a/mm/memcontrol.c b/mm/memcontrol.c index feba87d..d80b6b5 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c@@ -311,7 +311,13 @@ struct mem_cgroup { * Should we move charges of a task when a task is moved into this * mem_cgroup ? And what type of charges should we move ? */ - unsigned long move_charge_at_immigrate; + unsigned long move_charge_at_immigrate; + /*
Weird indentation (maybe spaces instead of a tab?)
+ * Tasks are being attached to this memcg. Used mostly to prevent + * changes to move_charge_at_immigrate + */ + int attach_in_progress;
Ditto.
quoted hunk ↗ jump to hunk
/* * set > 0 if pages under this cgroup are moving to other cgroup. */@@ -4114,6 +4120,7 @@ static int mem_cgroup_move_charge_write(struct cgroup *cgrp, struct cftype *cft, u64 val) { struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp); + int ret = -EAGAIN; if (val >= (1 << NR_MOVE_TYPE)) return -EINVAL;@@ -4123,10 +4130,13 @@ static int mem_cgroup_move_charge_write(struct cgroup *cgrp, * inconsistent. */ cgroup_lock(); + if (memcg->attach_in_progress) + goto out; memcg->move_charge_at_immigrate = val; + ret = 0; +out: cgroup_unlock(); - - return 0; + return ret;
Unsure whether this is a good behavior. It's a bit nasty to fail for internal temporary reasons like this. If it ever causes a problem, the occurrences are likely to be far and between making it difficult to debug. Can't you determine to immigrate or not in ->can_attach(), record whether to do that or not on the css, and finish it in ->attach() according to that. There's no need to consult the config multiple times. Thanks. -- tejun -- 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/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>