From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> At accounting file events per memory cgroup, we need to find memory cgroup via page_cgroup->mem_cgroup. Now, we use lock_page_cgroup(). But, considering the context which page-cgroup for files are accessed, we can use alternative light-weight mutual execusion in the most case. At handling file-caches, the only race we have to take care of is "moving" account, IOW, overwriting page_cgroup->mem_cgroup. (See comment in the patch) Unlike charge/uncharge, "move" happens not so frequently. It happens only when rmdir() and task-moving (with a special settings.) This patch adds a race-checker for file-cache-status accounting v.s. account moving. The new per-cpu-per-memcg counter MEM_CGROUP_ON_MOVE is added. The routine for account move 1. Increment it before start moving 2. Call synchronize_rcu() 3. Decrement it after the end of moving. By this, file-status-counting routine can check it needs to call lock_page_cgroup(). In most case, I doesn't need to call it. Changelog: 20100901 - changes id_to_memcg(pc, true) to be id_to_memcg(pc, false) in update_file_mapped() - updated comments on lock rule of update_file_mapped() Changelog: 20100825 - added a comment about mc.lock - fixed bad lock. Changelog: 20100804 - added a comment for possible optimization hint. Changelog: 20100730 - some cleanup. Changelog: 20100729 - replaced __this_cpu_xxx() with this_cpu_xxx (because we don't call spinlock) - added VM_BUG_ON(). Reviewed-by: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx> --- mm/memcontrol.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 85 insertions(+), 13 deletions(-) Index: mmotm-0827/mm/memcontrol.c =================================================================== --- mmotm-0827.orig/mm/memcontrol.c +++ mmotm-0827/mm/memcontrol.c @@ -90,6 +90,7 @@ enum mem_cgroup_stat_index { MEM_CGROUP_STAT_PGPGOUT_COUNT, /* # of pages paged out */ MEM_CGROUP_STAT_SWAPOUT, /* # of pages, swapped out */ MEM_CGROUP_EVENTS, /* incremented at every pagein/pageout */ + MEM_CGROUP_ON_MOVE, /* A check for locking move account/status */ MEM_CGROUP_STAT_NSTATS, }; @@ -1092,7 +1093,52 @@ static unsigned int get_swappiness(struc return swappiness; } -/* A routine for testing mem is not under move_account */ +static void mem_cgroup_start_move(struct mem_cgroup *mem) +{ + int cpu; + /* + * reuse mc.lock. + */ + spin_lock(&mc.lock); + /* TODO: Can we optimize this by for_each_online_cpu() ? */ + for_each_possible_cpu(cpu) + per_cpu(mem->stat->count[MEM_CGROUP_ON_MOVE], cpu) += 1; + spin_unlock(&mc.lock); + + synchronize_rcu(); +} + +static void mem_cgroup_end_move(struct mem_cgroup *mem) +{ + int cpu; + + if (!mem) + return; + /* for fast checking in mem_cgroup_update_file_stat() etc..*/ + spin_lock(&mc.lock); + for_each_possible_cpu(cpu) + per_cpu(mem->stat->count[MEM_CGROUP_ON_MOVE], cpu) -= 1; + spin_unlock(&mc.lock); +} + +/* + * mem_cgroup_is_moved -- checking a cgroup is mc.from target or not. + * used for avoiding race. + * mem_cgroup_under_move -- checking a cgroup is mc.from or mc.to or + * under hierarchy of them. used for waiting at + * memory pressure. + * Result of is_moved can be trusted until the end of rcu_read_unlock(). + * The caller must do + * rcu_read_lock(); + * result = mem_cgroup_is_moved(); + * .....make use of result here.... + * rcu_read_unlock(); + */ +static bool mem_cgroup_is_moved(struct mem_cgroup *mem) +{ + VM_BUG_ON(!rcu_read_lock_held()); + return this_cpu_read(mem->stat->count[MEM_CGROUP_ON_MOVE]) > 0; +} static bool mem_cgroup_under_move(struct mem_cgroup *mem) { @@ -1503,34 +1549,56 @@ bool mem_cgroup_handle_oom(struct mem_cg /* * Currently used to update mapped file statistics, but the routine can be * generalized to update other statistics as well. + * + * Notes: How to consider race conditions. + * + * Lock against "charge" is not required. All operations happens after the + * page is attached to address_space->mapping, charge must be finished. + * + * Rquired lock against "move" is held if necessary and account move code + * does proper move of statistics data. + * + * Lock against "uncharge" is not required. Because uncharge() doesn't clear + * pc->mem_cgroup, we can catch proper memcg even under race with truncation. + * Another thinking, while we call this, we grab a page_count of the page. So, + * the page will be never reused for other purpose and page_cgroup->mem_cgroup + * will contain the last data it had. */ + void mem_cgroup_update_file_mapped(struct page *page, int val) { struct mem_cgroup *mem; struct page_cgroup *pc; + bool need_lock = false; pc = lookup_page_cgroup(page); if (unlikely(!pc)) return; - - lock_page_cgroup(pc); - mem = id_to_memcg(pc->mem_cgroup, true); - if (!mem || !PageCgroupUsed(pc)) + rcu_read_lock(); + mem = id_to_memcg(pc->mem_cgroup, false); + if (likely(mem)) { + if (mem_cgroup_is_moved(mem)) { + /* need to serialize with move_account */ + lock_page_cgroup(pc); + need_lock = true; + mem = id_to_memcg(pc->mem_cgroup, true); + if (unlikely(!mem)) + goto done; + } + } + if (unlikely(!PageCgroupUsed(pc))) goto done; - - /* - * Preemption is already disabled. We can use __this_cpu_xxx - */ if (val > 0) { - __this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); + this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); SetPageCgroupFileMapped(pc); } else { - __this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); + this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); ClearPageCgroupFileMapped(pc); } - done: - unlock_page_cgroup(pc); + if (need_lock) + unlock_page_cgroup(pc); + rcu_read_unlock(); } /* @@ -3070,6 +3138,7 @@ move_account: lru_add_drain_all(); drain_all_stock_sync(); ret = 0; + mem_cgroup_start_move(mem); for_each_node_state(node, N_HIGH_MEMORY) { for (zid = 0; !ret && zid < MAX_NR_ZONES; zid++) { enum lru_list l; @@ -3083,6 +3152,7 @@ move_account: if (ret) break; } + mem_cgroup_end_move(mem); memcg_oom_recover(mem); /* it seems parent cgroup doesn't have enough mem */ if (ret == -ENOMEM) @@ -4560,6 +4630,7 @@ static void mem_cgroup_clear_mc(void) mc.to = NULL; mc.moving_task = NULL; spin_unlock(&mc.lock); + mem_cgroup_end_move(from); memcg_oom_recover(from); memcg_oom_recover(to); wake_up_all(&mc.waitq); @@ -4590,6 +4661,7 @@ static int mem_cgroup_can_attach(struct VM_BUG_ON(mc.moved_charge); VM_BUG_ON(mc.moved_swap); VM_BUG_ON(mc.moving_task); + mem_cgroup_start_move(from); spin_lock(&mc.lock); mc.from = from; mc.to = mem; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx"> email@xxxxxxxxx </a>