memcg: fix USED bit handling at uncharge in THP

Now, under THP:

at charge:
  - PageCgroupUsed bit is set to all page_cgroup on a hugepage.
    ....set to 512 pages.
at uncharge
  - PageCgroupUsed bit is unset on the head page.

So, some pages will remain with "Used" bit.

This patch fixes that Used bit is set only to the head page.
Used bits for tail pages will be set at splitting if necessary.

This patch adds this lock order:
   compound_lock() -> page_cgroup_move_lock().

[akpm@linux-foundation.org: fix warning]
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Balbir Singh <balbir@linux.vnet.ibm.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6a576f989437115eff8840a234b6057471a96a50..f512e189be5a496b2aa1b555c1fd34e056fc81e6 100644 (file)
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -146,6 +146,10 @@ unsigned long mem_cgroup_soft_limit_reclaim(struct zone *zone, int order,
                                                gfp_t gfp_mask);
 u64 mem_cgroup_get_limit(struct mem_cgroup *mem);
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail);
+#endif
+
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;
 
@@ -335,6 +339,11 @@ u64 mem_cgroup_get_limit(struct mem_cgroup *mem)
        return 0;
 }
 
+static inline void mem_cgroup_split_huge_fixup(struct page *head,
+                                               struct page *tail)
+{
+}
+
 #endif /* CONFIG_CGROUP_MEM_CONT */
 
 #endif /* _LINUX_MEMCONTROL_H */

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index c4f634b3a48eb551af6ef8444f9edb8820a83209..e187454d82f666a70bab08762ca92cd60b8f848c 100644 (file)
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1203,6 +1203,8 @@ static void __split_huge_page_refcount(struct page *page)
                BUG_ON(!PageDirty(page_tail));
                BUG_ON(!PageSwapBacked(page_tail));
 
+               mem_cgroup_split_huge_fixup(page, page_tail);
+
                lru_add_page_tail(zone, page, page_tail);
        }
 

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6d59a2bd520a0fba52a47a1ceb7558252eb07ba7..848b42195e5b4a5f0efc10cb337036bda448c74b 100644 (file)
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1614,7 +1614,7 @@ void mem_cgroup_update_page_stat(struct page *page,
        if (unlikely(!mem || !PageCgroupUsed(pc)))
                goto out;
        /* pc->mem_cgroup is unstable ? */
-       if (unlikely(mem_cgroup_stealed(mem))) {
+       if (unlikely(mem_cgroup_stealed(mem)) || PageTransHuge(page)) {
                /* take a lock against to access pc->mem_cgroup */
                move_lock_page_cgroup(pc, &flags);
                need_unlock = true;
@@ -2083,14 +2083,27 @@ struct mem_cgroup *try_get_mem_cgroup_from_page(struct page *page)
        return mem;
 }
 
-/*
- * commit a charge got by __mem_cgroup_try_charge() and makes page_cgroup to be
- * USED state. If already USED, uncharge and return.
- */
-static void ____mem_cgroup_commit_charge(struct mem_cgroup *mem,
-                                        struct page_cgroup *pc,
-                                        enum charge_type ctype)
+static void __mem_cgroup_commit_charge(struct mem_cgroup *mem,
+                                      struct page_cgroup *pc,
+                                      enum charge_type ctype,
+                                      int page_size)
 {
+       int nr_pages = page_size >> PAGE_SHIFT;
+
+       /* try_charge() can return NULL to *memcg, taking care of it. */
+       if (!mem)
+               return;
+
+       lock_page_cgroup(pc);
+       if (unlikely(PageCgroupUsed(pc))) {
+               unlock_page_cgroup(pc);
+               mem_cgroup_cancel_charge(mem, page_size);
+               return;
+       }
+       /*
+        * we don't need page_cgroup_lock about tail pages, becase they are not
+        * accessed by any other context at this point.
+        */
        pc->mem_cgroup = mem;
        /*
         * We access a page_cgroup asynchronously without lock_page_cgroup().
@@ -2114,35 +2127,7 @@ static void ____mem_cgroup_commit_charge(struct mem_cgroup *mem,
                break;
        }
 
-       mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), 1);
-}
-
-static void __mem_cgroup_commit_charge(struct mem_cgroup *mem,
-                                      struct page_cgroup *pc,
-                                      enum charge_type ctype,
-                                      int page_size)
-{
-       int i;
-       int count = page_size >> PAGE_SHIFT;
-
-       /* try_charge() can return NULL to *memcg, taking care of it. */
-       if (!mem)
-               return;
-
-       lock_page_cgroup(pc);
-       if (unlikely(PageCgroupUsed(pc))) {
-               unlock_page_cgroup(pc);
-               mem_cgroup_cancel_charge(mem, page_size);
-               return;
-       }
-
-       /*
-        * we don't need page_cgroup_lock about tail pages, becase they are not
-        * accessed by any other context at this point.
-        */
-       for (i = 0; i < count; i++)
-               ____mem_cgroup_commit_charge(mem, pc + i, ctype);
-
+       mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), nr_pages);
        unlock_page_cgroup(pc);
        /*
         * "charge_statistics" updated event counter. Then, check it.
@@ -2152,6 +2137,34 @@ static void __mem_cgroup_commit_charge(struct mem_cgroup *mem,
        memcg_check_events(mem, pc->page);
 }
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+
+#define PCGF_NOCOPY_AT_SPLIT ((1 << PCG_LOCK) | (1 << PCG_MOVE_LOCK) |\
+                       (1 << PCG_ACCT_LRU) | (1 << PCG_MIGRATION))
+/*
+ * Because tail pages are not marked as "used", set it. We're under
+ * zone->lru_lock, 'splitting on pmd' and compund_lock.
+ */
+void mem_cgroup_split_huge_fixup(struct page *head, struct page *tail)
+{
+       struct page_cgroup *head_pc = lookup_page_cgroup(head);
+       struct page_cgroup *tail_pc = lookup_page_cgroup(tail);
+       unsigned long flags;
+
+       /*
+        * We have no races witch charge/uncharge but will have races with
+        * page state accounting.
+        */
+       move_lock_page_cgroup(head_pc, &flags);
+
+       tail_pc->mem_cgroup = head_pc->mem_cgroup;
+       smp_wmb(); /* see __commit_charge() */
+       /* we don't need to copy all flags...*/
+       tail_pc->flags = head_pc->flags & ~PCGF_NOCOPY_AT_SPLIT;
+       move_unlock_page_cgroup(head_pc, &flags);
+}
+#endif
+
 /**
  * __mem_cgroup_move_account - move account of the page
  * @pc:        page_cgroup of the page.
@@ -2545,7 +2558,6 @@ direct_uncharge:
 static struct mem_cgroup *
 __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
 {
-       int i;
        int count;
        struct page_cgroup *pc;
        struct mem_cgroup *mem = NULL;
@@ -2595,8 +2607,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
                break;
        }
 
-       for (i = 0; i < count; i++)
-               mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), -1);
+       mem_cgroup_charge_statistics(mem, PageCgroupCache(pc), -count);
 
        ClearPageCgroupUsed(pc);
        /*