Skip to contentSkip to conversation

memcg: fold mem_cgroup_stolen

PATCH In review Qiang Huang 2 participants 2 messages May 7, 2014 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 memcg: fold mem_cgroup_stolen

It is only used in __mem_cgroup_begin_update_page_stat(), the name is confusing and 2 routines for one thing also confuse people, so fold this function seems more clear.

---
 mm/memcontrol.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
mm/memcontrol.c +7 -18
index 4f97662..5804d71 100644
@@ -1568,23 +1568,12 @@ static void mem_cgroup_end_move(struct mem_cgroup *memcg)
 }
 
 /*
- * 2 routines for checking "mem" is under move_account() or not.
+ * A routine for checking "mem" is under move_account() or not.
  *
- * mem_cgroup_stolen() -  checking whether a cgroup is mc.from or not. This
- *			  is used for avoiding races in accounting.  If true,
- *			  pc->mem_cgroup may be overwritten.
- *
- * mem_cgroup_under_move() - checking a cgroup is mc.from or mc.to or
- *			  under hierarchy of moving cgroups. This is for
- *			  waiting at hith-memory prressure caused by "move".
+ * Checking a cgroup is mc.from or mc.to or under hierarchy of
+ * moving cgroups. This is for waiting at hith-memory pressure
+ * caused by "move".
  */
-
-static bool mem_cgroup_stolen(struct mem_cgroup *memcg)
-{
-	VM_BUG_ON(!rcu_read_lock_held());
-	return atomic_read(&memcg->moving_account) > 0;
-}
-
 static bool mem_cgroup_under_move(struct mem_cgroup *memcg)
 {
 	struct mem_cgroup *from;
@@ -1627,7 +1616,6 @@ static bool mem_cgroup_wait_acct_move(struct mem_cgroup *memcg)
  * Take this lock when
  * - a code tries to modify page's memcg while it's USED.
  * - a code tries to modify page state accounting in a memcg.
- * see mem_cgroup_stolen(), too.
  */
 static void move_lock_mem_cgroup(struct mem_cgroup *memcg,
 				  unsigned long *flags)
@@ -2301,9 +2289,10 @@ again:
 	 * If this memory cgroup is not under account moving, we don't
 	 * need to take move_lock_mem_cgroup(). Because we already hold
 	 * rcu_read_lock(), any calls to move_account will be delayed until
-	 * rcu_read_unlock() if mem_cgroup_stolen() == true.
+	 * rcu_read_unlock().
 	 */
-	if (!mem_cgroup_stolen(memcg))
+	VM_BUG_ON(!rcu_read_lock_held());
+	if (atomic_read(&memcg->moving_account) <= 0)
 		return;
 
 	move_lock_mem_cgroup(memcg, flags);
Signature · 1 line
1.8.3

A message that is not in the archive (sent off-list or not yet received): [email protected]

Acked-by
#2 lore raw
Quoted 3 lines · Qiang Huang

It is only used in __mem_cgroup_begin_update_page_stat(), the name is confusing and 2 routines for one thing also confuse people, so fold this function seems more clear.

Yes, the name was everything but self-explanatory.

The patch looks good to me. One nit below.

Quoted 1 line
Quoted 25 lines
---
 mm/memcontrol.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
mm/memcontrol.c +3 -8
index 4f97662..5804d71 100644
@@ -1568,23 +1568,12 @@ static void mem_cgroup_end_move(struct mem_cgroup *memcg)
 }
 
 /*
- * 2 routines for checking "mem" is under move_account() or not.
+ * A routine for checking "mem" is under move_account() or not.
  *
- * mem_cgroup_stolen() -  checking whether a cgroup is mc.from or not. This
- *			  is used for avoiding races in accounting.  If true,
- *			  pc->mem_cgroup may be overwritten.
- *
- * mem_cgroup_under_move() - checking a cgroup is mc.from or mc.to or
- *			  under hierarchy of moving cgroups. This is for
- *			  waiting at hith-memory prressure caused by "move".
+ * Checking a cgroup is mc.from or mc.to or under hierarchy of
+ * moving cgroups.
 
Quoted 1 line

This is for waiting at hith-memory pressure

s/hith/high/

but I guess that part can go away

[...]

Signature · 2 lines
Michal Hocko
SUSE Labs