diff options
author | KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> | 2010-04-06 17:35:05 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2010-04-07 11:38:05 -0400 |
commit | 8725d5416213a145ccc9c236dbd26830ba409e00 (patch) | |
tree | aff10fc98874adfcb7755ec8bc1f2627036e8dc2 /mm | |
parent | 116354d177ba2da37e91cf884e3d11e67f825efd (diff) |
memcg: fix race in file_mapped accounting
Presently, memcg's FILE_MAPPED accounting has following race with
move_account (happens at rmdir()).
increment page->mapcount (rmap.c)
mem_cgroup_update_file_mapped() move_account()
lock_page_cgroup()
check page_mapped() if
page_mapped(page)>1 {
FILE_MAPPED -1 from old memcg
FILE_MAPPED +1 to old memcg
}
.....
overwrite pc->mem_cgroup
unlock_page_cgroup()
lock_page_cgroup()
FILE_MAPPED + 1 to pc->mem_cgroup
unlock_page_cgroup()
Then,
old memcg (-1 file mapped)
new memcg (+2 file mapped)
This happens because move_account see page_mapped() which is not guarded
by lock_page_cgroup(). This patch adds FILE_MAPPED flag to page_cgroup
and move account information based on it. Now, all checks are synchronous
with lock_page_cgroup().
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Reviewed-by: Balbir Singh <balbir@in.ibm.com>
Reviewed-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Andrea Righi <arighi@develer.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/memcontrol.c | 18 |
1 files changed, 9 insertions, 9 deletions
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 9ed760dc7448..f4ede99c8b9b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c | |||
@@ -1359,16 +1359,19 @@ void mem_cgroup_update_file_mapped(struct page *page, int val) | |||
1359 | 1359 | ||
1360 | lock_page_cgroup(pc); | 1360 | lock_page_cgroup(pc); |
1361 | mem = pc->mem_cgroup; | 1361 | mem = pc->mem_cgroup; |
1362 | if (!mem) | 1362 | if (!mem || !PageCgroupUsed(pc)) |
1363 | goto done; | ||
1364 | |||
1365 | if (!PageCgroupUsed(pc)) | ||
1366 | goto done; | 1363 | goto done; |
1367 | 1364 | ||
1368 | /* | 1365 | /* |
1369 | * Preemption is already disabled. We can use __this_cpu_xxx | 1366 | * Preemption is already disabled. We can use __this_cpu_xxx |
1370 | */ | 1367 | */ |
1371 | __this_cpu_add(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED], val); | 1368 | if (val > 0) { |
1369 | __this_cpu_inc(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); | ||
1370 | SetPageCgroupFileMapped(pc); | ||
1371 | } else { | ||
1372 | __this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); | ||
1373 | ClearPageCgroupFileMapped(pc); | ||
1374 | } | ||
1372 | 1375 | ||
1373 | done: | 1376 | done: |
1374 | unlock_page_cgroup(pc); | 1377 | unlock_page_cgroup(pc); |
@@ -1801,16 +1804,13 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *mem, | |||
1801 | static void __mem_cgroup_move_account(struct page_cgroup *pc, | 1804 | static void __mem_cgroup_move_account(struct page_cgroup *pc, |
1802 | struct mem_cgroup *from, struct mem_cgroup *to, bool uncharge) | 1805 | struct mem_cgroup *from, struct mem_cgroup *to, bool uncharge) |
1803 | { | 1806 | { |
1804 | struct page *page; | ||
1805 | |||
1806 | VM_BUG_ON(from == to); | 1807 | VM_BUG_ON(from == to); |
1807 | VM_BUG_ON(PageLRU(pc->page)); | 1808 | VM_BUG_ON(PageLRU(pc->page)); |
1808 | VM_BUG_ON(!PageCgroupLocked(pc)); | 1809 | VM_BUG_ON(!PageCgroupLocked(pc)); |
1809 | VM_BUG_ON(!PageCgroupUsed(pc)); | 1810 | VM_BUG_ON(!PageCgroupUsed(pc)); |
1810 | VM_BUG_ON(pc->mem_cgroup != from); | 1811 | VM_BUG_ON(pc->mem_cgroup != from); |
1811 | 1812 | ||
1812 | page = pc->page; | 1813 | if (PageCgroupFileMapped(pc)) { |
1813 | if (page_mapped(page) && !PageAnon(page)) { | ||
1814 | /* Update mapped_file data for mem_cgroup */ | 1814 | /* Update mapped_file data for mem_cgroup */ |
1815 | preempt_disable(); | 1815 | preempt_disable(); |
1816 | __this_cpu_dec(from->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); | 1816 | __this_cpu_dec(from->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]); |