[RFC 2/3] memcg: Use mc.moving_task as the indication for charge moving

[Michal Hocko]

move_charge_struct::to not being NULL has been used to indicate whether
the currently ongoing move operation should migrate the charges. The follow up
patch will require mc.to being initialized even when we do not migrate
charges so replace the check by checking mc.moving_task which is set
only when the migration is requested. Also replace the open coded check
by a helper function (mc_move_charge).

mem_cgroup_clear_mc has to be called unconditionally now because it
has to clean up from and to pointers. __mem_cgroup_clear_mc does the
migration specific cleanup so it still checks for mc_move_charge.

Signed-off-by: Michal Hocko <mhocko@xxxxxxx>
---
 mm/memcontrol.c | 63 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f3d92cf0caf4..4d905209f00f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4984,14 +4984,22 @@ static void __mem_cgroup_clear_mc(void)
 	wake_up_all(&mc.waitq);
 }
 
+static bool mc_move_charge(void)
+{
+	/* moving_task is configured only if the charge is really moved */
+	return mc.moving_task != NULL;
+}
+
 static void mem_cgroup_clear_mc(void)
 {
+	bool move_charge = mc_move_charge();
 	/*
 	 * we must clear moving_task before waking up waiters at the end of
 	 * task migration.
 	 */
 	mc.moving_task = NULL;
-	__mem_cgroup_clear_mc();
+	if (move_charge)
+		__mem_cgroup_clear_mc();
 	spin_lock(&mc.lock);
 	mc.from = NULL;
 	mc.to = NULL;
@@ -5008,15 +5016,6 @@ static int mem_cgroup_can_attach(struct cgroup_subsys_state *css,
 	unsigned long move_flags;
 	int ret = 0;
 
-	/*
-	 * We are now commited to this value whatever it is. Changes in this
-	 * tunable will only affect upcoming migrations, not the current one.
-	 * So we need to save it, and keep it going.
-	 */
-	move_flags = READ_ONCE(memcg->move_charge_at_immigrate);
-	if (!move_flags)
-		return 0;
-
 	p = cgroup_taskset_first(tset);
 	from = mem_cgroup_from_task(p);
 
@@ -5025,21 +5024,29 @@ static int mem_cgroup_can_attach(struct cgroup_subsys_state *css,
 	mm = get_task_mm(p);
 	if (!mm)
 		return 0;
-	/* We move charges only when we move a owner of the mm */
-	if (mm->owner == p) {
-		VM_BUG_ON(mc.from);
-		VM_BUG_ON(mc.to);
-		VM_BUG_ON(mc.precharge);
-		VM_BUG_ON(mc.moved_charge);
-		VM_BUG_ON(mc.moved_swap);
-
-		spin_lock(&mc.lock);
-		mc.from = from;
-		mc.to = memcg;
-		mc.flags = move_flags;
-		spin_unlock(&mc.lock);
-		/* We set mc.moving_task later */
 
+	VM_BUG_ON(mc.from);
+	VM_BUG_ON(mc.to);
+	VM_BUG_ON(mc.precharge);
+	VM_BUG_ON(mc.moved_charge);
+	VM_BUG_ON(mc.moved_swap);
+
+	spin_lock(&mc.lock);
+	mc.from = from;
+	mc.to = memcg;
+	mc.flags = move_flags;
+	spin_unlock(&mc.lock);
+	/* We set mc.moving_task later */
+
+	/*
+	 * We are now commited to this value whatever it is. Changes in this
+	 * tunable will only affect upcoming migrations, not the current one.
+	 * So we need to save it, and keep it going.
+	 */
+	move_flags = READ_ONCE(memcg->move_charge_at_immigrate);
+
+	/* We move charges only when we move a owner of the mm */
+	if (move_flags && mm->owner == p) {
 		ret = mem_cgroup_precharge_mc(mm);
 		if (ret)
 			mem_cgroup_clear_mc();
@@ -5051,8 +5058,7 @@ static int mem_cgroup_can_attach(struct cgroup_subsys_state *css,
 static void mem_cgroup_cancel_attach(struct cgroup_subsys_state *css,
 				     struct cgroup_taskset *tset)
 {
-	if (mc.to)
-		mem_cgroup_clear_mc();
+	mem_cgroup_clear_mc();
 }
 
 static int mem_cgroup_move_charge_pte_range(pmd_t *pmd,
@@ -5198,12 +5204,11 @@ static void mem_cgroup_move_task(struct cgroup_subsys_state *css,
 	struct mm_struct *mm = get_task_mm(p);
 
 	if (mm) {
-		if (mc.to)
+		if (mc_move_charge())
 			mem_cgroup_move_charge(mm);
 		mmput(mm);
 	}
-	if (mc.to)
-		mem_cgroup_clear_mc();
+	mem_cgroup_clear_mc();
 }
 #else	/* !CONFIG_MMU */
 static int mem_cgroup_can_attach(struct cgroup_subsys_state *css,
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/