memcg: add mem_cgroup_replace_page_cache() to fix LRU issue

Commit ef6a3c6311 ("mm: add replace_page_cache_page() function") added a
function replace_page_cache_page().  This function replaces a page in the
radix-tree with a new page.  WHen doing this, memory cgroup needs to fix
up the accounting information.  memcg need to check PCG_USED bit etc.

In some(many?) cases, 'newpage' is on LRU before calling
replace_page_cache().  So, memcg's LRU accounting information should be
fixed, too.

This patch adds mem_cgroup_replace_page_cache() and removes the old hooks.
 In that function, old pages will be unaccounted without touching
res_counter and new page will be accounted to the memcg (of old page).
WHen overwriting pc->mem_cgroup of newpage, take zone->lru_lock and avoid
races with LRU handling.

Background:
  replace_page_cache_page() is called by FUSE code in its splice() handling.
  Here, 'newpage' is replacing oldpage but this newpage is not a newly allocated
  page and may be on LRU. LRU mis-accounting will be critical for memory cgroup
  because rmdir() checks the whole LRU is empty and there is no account leak.
  If a page is on the other LRU than it should be, rmdir() will fail.

This bug was added in March 2011, but no bug report yet.  I guess there
are not many people who use memcg and FUSE at the same time with upstream
kernels.

The result of this bug is that admin cannot destroy a memcg because of
account leak.  So, no panic, no deadlock.  And, even if an active cgroup
exist, umount can succseed.  So no problem at shutdown.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Acked-by: Michal Hocko <mhocko@suse.cz>
Cc: Miklos Szeredi <mszeredi@suse.cz>
Cc: Hugh Dickins <hughd@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

2 files changed, 46 insertions, 16 deletions

diff --git a/mm/filemap.c b/mm/filemap.c
index c4ee2e918bea..97f49ed35bd2 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -393,24 +393,11 @@ EXPORT_SYMBOL(filemap_write_and_wait_range);
 int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
 {
         int error;
-        struct mem_cgroup *memcg = NULL;
 
         VM_BUG_ON(!PageLocked(old));
         VM_BUG_ON(!PageLocked(new));
         VM_BUG_ON(new->mapping);
 
-        /*
-         * This is not page migration, but prepare_migration and
-         * end_migration does enough work for charge replacement.
-         *
-         * In the longer term we probably want a specialized function
-         * for moving the charge from old to new in a more efficient
-         * manner.
-         */
-        error = mem_cgroup_prepare_migration(old, new, &memcg, gfp_mask);
-        if (error)
-                return error;
-
         error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM);
         if (!error) {
                 struct address_space *mapping = old->mapping;
@@ -432,13 +419,12 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
                 if (PageSwapBacked(new))
                         __inc_zone_page_state(new, NR_SHMEM);
                 spin_unlock_irq(&mapping->tree_lock);
+                /* mem_cgroup codes must not be called under tree_lock */
+                mem_cgroup_replace_page_cache(old, new);
                 radix_tree_preload_end();
                 if (freepage)
                         freepage(old);
                 page_cache_release(old);
-                mem_cgroup_end_migration(memcg, old, new, true);
-        } else {
-                mem_cgroup_end_migration(memcg, old, new, false);
         }
 
         return error;
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index d87aa3510c5e..0b2d4036f1cd 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3432,6 +3432,50 @@ void mem_cgroup_end_migration(struct mem_cgroup *memcg,
         cgroup_release_and_wakeup_rmdir(&memcg->css);
 }
 
+/*
+ * At replace page cache, newpage is not under any memcg but it's on
+ * LRU. So, this function doesn't touch res_counter but handles LRU
+ * in correct way. Both pages are locked so we cannot race with uncharge.
+ */
+void mem_cgroup_replace_page_cache(struct page *oldpage,
+                                  struct page *newpage)
+{
+        struct mem_cgroup *memcg;
+        struct page_cgroup *pc;
+        struct zone *zone;
+        enum charge_type type = MEM_CGROUP_CHARGE_TYPE_CACHE;
+        unsigned long flags;
+
+        if (mem_cgroup_disabled())
+                return;
+
+        pc = lookup_page_cgroup(oldpage);
+        /* fix accounting on old pages */
+        lock_page_cgroup(pc);
+        memcg = pc->mem_cgroup;
+        mem_cgroup_charge_statistics(memcg, PageCgroupCache(pc), -1);
+        ClearPageCgroupUsed(pc);
+        unlock_page_cgroup(pc);
+
+        if (PageSwapBacked(oldpage))
+                type = MEM_CGROUP_CHARGE_TYPE_SHMEM;
+
+        zone = page_zone(newpage);
+        pc = lookup_page_cgroup(newpage);
+        /*
+         * Even if newpage->mapping was NULL before starting replacement,
+         * the newpage may be on LRU(or pagevec for LRU) already. We lock
+         * LRU while we overwrite pc->mem_cgroup.
+         */
+        spin_lock_irqsave(&zone->lru_lock, flags);
+        if (PageLRU(newpage))
+                del_page_from_lru_list(zone, newpage, page_lru(newpage));
+        __mem_cgroup_commit_charge(memcg, newpage, 1, pc, type);
+        if (PageLRU(newpage))
+                add_page_to_lru_list(zone, newpage, page_lru(newpage));
+        spin_unlock_irqrestore(&zone->lru_lock, flags);
+}
+
 #ifdef CONFIG_DEBUG_VM
 static struct page_cgroup *lookup_page_cgroup_used(struct page *page)
 {