memcg: fix race in file_mapped accouting flag management
authorKAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Wed, 27 Oct 2010 22:33:39 +0000 (15:33 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Thu, 28 Oct 2010 01:03:09 +0000 (18:03 -0700)
Presently memory cgroup accounts file-mapped by counter and flag.  counter
is working in the same way with zone_stat but FileMapped flag only exists
in memcg (for helping move_account).

This flag can be updated wrongly in a case.  Assume CPU0 and CPU1 and a
thread mapping a page on CPU0, another thread unmapping it on CPU1.

    CPU0                    CPU1
rmv rmap (mapcount 1->0)
   add rmap (mapcount 0->1)
   lock_page_cgroup()
   memcg counter+1 (some delay)
   set MAPPED FLAG.
   unlock_page_cgroup()
lock_page_cgroup()
memcg counter-1
clear MAPPED flag

In the above sequence counter is properly updated but FLAG is not.  This
means that representing a state by a flag which is maintained by counter
needs some special care.

To handle this, when clearing a flag, this patch check mapcount directly
and clear the flag only when mapcount == 0.  (if mapcount >0, someone will
make it to zero later and flag will be cleared.)

Reverse case, dec-after-inc cannot be a problem because page_table_lock()
works well for it.  (IOW, to make above sequence, 2 processes should touch
the same page at once with map/unmap.)

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Greg Thelen <gthelen@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
mm/memcontrol.c

index 9be3cf8a5da462d4b1b4103eef61f8d5a9a6e06c..0e3fdbd809c7dad28cc5c6558ad9d2ab23bf982f 100644 (file)
@@ -1485,7 +1485,8 @@ void mem_cgroup_update_file_mapped(struct page *page, int val)
                SetPageCgroupFileMapped(pc);
        } else {
                __this_cpu_dec(mem->stat->count[MEM_CGROUP_STAT_FILE_MAPPED]);
-               ClearPageCgroupFileMapped(pc);
+               if (!page_mapped(page)) /* for race between dec->inc counter */
+                       ClearPageCgroupFileMapped(pc);
        }
 
 done: