mm, vmscan: Update all zone LRU sizes before updating memcg

Minchan Kim reported setting the following warning on a 32-bit system
although it can affect 64-bit systems.

  WARNING: CPU: 4 PID: 1322 at mm/memcontrol.c:998 mem_cgroup_update_lru_size+0x103/0x110
  mem_cgroup_update_lru_size(f44b4000, 1, -7): zid 1 lru_size 1 but empty
  Modules linked in:
  CPU: 4 PID: 1322 Comm: cp Not tainted 4.7.0-rc4-mm1+ #143
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  Call Trace:
    dump_stack+0x76/0xaf
    __warn+0xea/0x110
    ? mem_cgroup_update_lru_size+0x103/0x110
    warn_slowpath_fmt+0x3b/0x40
    mem_cgroup_update_lru_size+0x103/0x110
    isolate_lru_pages.isra.61+0x2e2/0x360
    shrink_active_list+0xac/0x2a0
    ? __delay+0xe/0x10
    shrink_node_memcg+0x53c/0x7a0
    shrink_node+0xab/0x2a0
    do_try_to_free_pages+0xc6/0x390
    try_to_free_pages+0x245/0x590

LRU list contents and counts are updated separately.  Counts are updated
before pages are added to the LRU and updated after pages are removed.
The warning above is from a check in mem_cgroup_update_lru_size that
ensures that list sizes of zero are empty.

The problem is that node-lru needs to account for highmem pages if
CONFIG_HIGHMEM is set.  One impact of the implementation is that the
sizes are updated in multiple passes when pages from multiple zones were
isolated.  This happens whether HIGHMEM is set or not.  When multiple
zones are isolated, it's possible for a debugging check in memcg to be
tripped.

This patch forces all the zone counts to be updated before the memcg
function is called.

Link: http://lkml.kernel.org/r/1468588165-12461-6-git-send-email-mgorman@techsingularity.net
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
Tested-by: Minchan Kim <minchan@kernel.org>
Reported-by: Minchan Kim <minchan@kernel.org>
Acked-by: Minchan Kim <minchan@kernel.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 b759827b2f1e5d59f9414e712c7ab0837c1358b9..5147e650287ad5f7c9a72f479b834ab0f726cf86 100644 (file)
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -430,7 +430,7 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
 int mem_cgroup_select_victim_node(struct mem_cgroup *memcg);
 
 void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
-               enum zone_type zid, int nr_pages);
+               int nr_pages);
 
 unsigned long mem_cgroup_node_nr_lru_pages(struct mem_cgroup *memcg,
                                           int nid, unsigned int lru_mask);

diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index dd22b08c47be657dbf2ec5ddf18dd64f60227d95..bcc4ed07fa903e7e80381393df886de4092edd9c 100644 (file)
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -52,10 +52,9 @@ static __always_inline void update_lru_size(struct lruvec *lruvec,
                                enum lru_list lru, enum zone_type zid,
                                int nr_pages)
 {
-#ifdef CONFIG_MEMCG
-       mem_cgroup_update_lru_size(lruvec, lru, zid, nr_pages);
-#else
        __update_lru_size(lruvec, lru, zid, nr_pages);
+#ifdef CONFIG_MEMCG
+       mem_cgroup_update_lru_size(lruvec, lru, nr_pages);
 #endif
 }
 

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9cbd40ebccd1f77a4c81a97cd2253dd3ccdf2bbb..13be30c3ea7831e56e437380d7da3f6804c0dd5e 100644 (file)
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -965,7 +965,6 @@ out:
  * mem_cgroup_update_lru_size - account for adding or removing an lru page
  * @lruvec: mem_cgroup per zone lru vector
  * @lru: index of lru list the page is sitting on
- * @zid: Zone ID of the zone pages have been added to
  * @nr_pages: positive when adding or negative when removing
  *
  * This function must be called under lru_lock, just before a page is added
@@ -973,15 +972,13 @@ out:
  * so as to allow it to check that lru_size 0 is consistent with list_empty).
  */
 void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
-                               enum zone_type zid, int nr_pages)
+                               int nr_pages)
 {
        struct mem_cgroup_per_node *mz;
        unsigned long *lru_size;
        long size;
        bool empty;
 
-       __update_lru_size(lruvec, lru, zid, nr_pages);
-
        if (mem_cgroup_disabled())
                return;
 

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 5625eccc014002447dea8c79084f74c5d85f556b..b3f5b359280d9a200e23fd26a1c9a2c0d396e3f1 100644 (file)
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1350,6 +1350,38 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode)
        return ret;
 }
 
+
+/*
+ * Update LRU sizes after isolating pages. The LRU size updates must
+ * be complete before mem_cgroup_update_lru_size due to a santity check.
+ */
+static __always_inline void update_lru_sizes(struct lruvec *lruvec,
+                       enum lru_list lru, unsigned long *nr_zone_taken,
+                       unsigned long nr_taken)
+{
+#ifdef CONFIG_HIGHMEM
+       int zid;
+
+       /*
+        * Highmem has separate accounting for highmem pages so each zone
+        * is updated separately.
+        */
+       for (zid = 0; zid < MAX_NR_ZONES; zid++) {
+               if (!nr_zone_taken[zid])
+                       continue;
+
+               __update_lru_size(lruvec, lru, zid, -nr_zone_taken[zid]);
+       }
+#else
+       /* Zone ID does not matter on !HIGHMEM */
+       __update_lru_size(lruvec, lru, 0, -nr_taken);
+#endif
+
+#ifdef CONFIG_MEMCG
+       mem_cgroup_update_lru_size(lruvec, lru, -nr_taken);
+#endif
+}
+
 /*
  * zone_lru_lock is heavily contended.  Some of the functions that
  * shrink the lists perform better by taking out a batch of pages
@@ -1436,13 +1468,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
        *nr_scanned = scan;
        trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,
                                    nr_taken, mode, is_file_lru(lru));
-       for (scan = 0; scan < MAX_NR_ZONES; scan++) {
-               nr_pages = nr_zone_taken[scan];
-               if (!nr_pages)
-                       continue;
-
-               update_lru_size(lruvec, lru, scan, -nr_pages);
-       }
+       update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);
        return nr_taken;
 }