[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git-commits-head
Subject:    memcg: use __this_cpu_sub() to dec stats to avoid incorrect subtrahend casting
From:       "Linux Kernel Mailing List" <linux-kernel () vger ! kernel ! org>
Date:       2013-10-30 21:29:22
Message-ID: 20131030212922.A4B66660911 () gitolite ! kernel ! org
[Download RAW message or body]

Gitweb:     http://git.kernel.org/linus/;a=commit;h=5e8cfc3c75b3e43497389896c0ecda62fc311ce9
Commit:     5e8cfc3c75b3e43497389896c0ecda62fc311ce9
Parent:     bd09d9a35111b6ffc0c7585d3853d0ec7f9f1eb4
Author:     Greg Thelen <gthelen@google.com>
AuthorDate: Wed Oct 30 13:56:21 2013 -0700
Committer:  Linus Torvalds <torvalds@linux-foundation.org>
CommitDate: Wed Oct 30 14:27:03 2013 -0700

    memcg: use __this_cpu_sub() to dec stats to avoid incorrect subtrahend casting
    
    As of commit 3ea67d06e467 ("memcg: add per cgroup writeback pages
    accounting") memcg counter errors are possible when moving charged
    memory to a different memcg.  Charge movement occurs when processing
    writes to memory.force_empty, moving tasks to a memcg with
    memcg.move_charge_at_immigrate=1, or memcg deletion.
    
    An example showing error after memory.force_empty:
    
      $ cd /sys/fs/cgroup/memory
      $ mkdir x
      $ rm /data/tmp/file
      $ (echo $BASHPID >> x/tasks && exec mmap_writer /data/tmp/file 1M) &
      [1] 13600
      $ grep ^mapped x/memory.stat
      mapped_file 1048576
      $ echo 13600 > tasks
      $ echo 1 > x/memory.force_empty
      $ grep ^mapped x/memory.stat
      mapped_file 4503599627370496
    
    mapped_file should end with 0.
      4503599627370496 == 0x10,0000,0000,0000 == 0x100,0000,0000 pages
      1048576          == 0x10,0000           == 0x100 pages
    
    This issue only affects the source memcg on 64 bit machines; the
    destination memcg counters are correct.  So the rmdir case is not too
    important because such counters are soon disappearing with the entire
    memcg.  But the memcg.force_empty and memory.move_charge_at_immigrate=1
    cases are larger problems as the bogus counters are visible for the
    (possibly long) remaining life of the source memcg.
    
    The problem is due to memcg use of __this_cpu_from(.., -nr_pages), which
    is subtly wrong because it subtracts the unsigned int nr_pages (either
    -1 or -512 for THP) from a signed long percpu counter.  When
    nr_pages=-1, -nr_pages=0xffffffff.  On 64 bit machines stat->count[idx]
    is signed 64 bit.  So memcg's attempt to simply decrement a count (e.g.
    from 1 to 0) boils down to:
    
      long count = 1
      unsigned int nr_pages = 1
      count += -nr_pages  /* -nr_pages == 0xffff,ffff */
      count is now 0x1,0000,0000 instead of 0
    
    The fix is to subtract the unsigned page count rather than adding its
    negation.  This only works once "percpu: fix this_cpu_sub() subtrahend
    casting for unsigneds" is applied to fix this_cpu_sub().
    
    Signed-off-by: Greg Thelen <gthelen@google.com>
    Acked-by: Tejun Heo <tj@kernel.org>
    Acked-by: Johannes Weiner <hannes@cmpxchg.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 mm/memcontrol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 34d3ca9..497ec33 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3774,7 +3774,7 @@ void mem_cgroup_move_account_page_stat(struct mem_cgroup *from,
 	/* Update stat data for mem_cgroup */
 	preempt_disable();
 	WARN_ON_ONCE(from->stat->count[idx] < nr_pages);
-	__this_cpu_add(from->stat->count[idx], -nr_pages);
+	__this_cpu_sub(from->stat->count[idx], nr_pages);
 	__this_cpu_add(to->stat->count[idx], nr_pages);
 	preempt_enable();
 }
--
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]