[v2 PATCH 0/4] Make deferred split shrinker memcg aware

[v2 PATCH 0/4] Make deferred split shrinker memcg aware

[Yang Shi]

Currently THP deferred split shrinker is not memcg aware, this may cause
premature OOM with some configuration. For example the below test would
run into premature OOM easily:

$ cgcreate -g memory:thp
$ echo 4G > /sys/fs/cgroup/memory/thp/memory/limit_in_bytes
$ cgexec -g memory:thp transhuge-stress 4000

transhuge-stress comes from kernel selftest.

It is easy to hit OOM, but there are still a lot THP on the deferred
split queue, memcg direct reclaim can't touch them since the deferred
split shrinker is not memcg aware.

Convert deferred split shrinker memcg aware by introducing per memcg
deferred split queue.  The THP should be on either per node or per memcg
deferred split queue if it belongs to a memcg.  When the page is
immigrated to the other memcg, it will be immigrated to the target
memcg's deferred split queue too.

And, move deleting THP from deferred split queue in page free before
memcg uncharge so that the page's memcg information is available.

Reuse the second tail page's deferred_list for per memcg list since the
same THP can't be on multiple deferred split queues.

Remove THP specific destructor since it is not used anymore with memcg
aware THP shrinker (Please see the commit log of patch 3/4 for the details).

Make deferred split shrinker not depend on memcg kmem since it is not slab.
It doesn't make sense to not shrink THP even though memcg kmem is disabled.

With the above change the test demonstrated above doesn't trigger OOM even
though with cgroup.memory=nokmem.


Changelog:
v2: * Adopted the suggestion from Krill Shutemov to extract deferred split
      fields into a struct to reduce code duplication (patch 1/4).  With this
      change, the lines of change is shrunk down to 198 from 278.
    * Removed memcg_deferred_list. Use deferred_list for both global and memcg.
      With the code deduplication, it doesn't make too much sense to keep it.
      Kirill Tkhai also suggested so.
    * Fixed typo for SHRINKER_NONSLAB.


Yang Shi (4):
      mm: thp: extract split_queue_* into a struct
      mm: thp: make deferred split shrinker memcg aware
      mm: thp: remove THP destructor
      mm: shrinker: make shrinker not depend on memcg kmem

 include/linux/huge_mm.h    | 15 +++++++++++
 include/linux/memcontrol.h |  4 +++
 include/linux/mm.h         |  3 ---
 include/linux/mm_types.h   |  1 +
 include/linux/mmzone.h     | 12 ++++++---
 include/linux/shrinker.h   |  3 +--
 mm/huge_memory.c           | 99 ++++++++++++++++++++++++++++++++++++++++++++------------------------
 mm/memcontrol.c            | 19 +++++++++++++
 mm/page_alloc.c            | 11 ++++----
 mm/swap.c                  |  4 +++
 mm/vmscan.c                | 27 +++++--------------
 11 files changed, 129 insertions(+), 69 deletions(-)

[PATCH 1/4] mm: thp: extract split_queue_* into a struct

[Yang Shi]

Put split_queue, split_queue_lock and split_queue_len into a struct in
order to reduce code duplication when we convert deferred_split to memcg
aware in the later patches.

Suggested-by: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
 include/linux/mmzone.h | 12 +++++++++---
 mm/huge_memory.c       | 45 +++++++++++++++++++++++++--------------------
 mm/page_alloc.c        |  8 +++++---
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 70394ca..7799166 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -676,6 +676,14 @@ struct zonelist {
 extern struct page *mem_map;
 #endif
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+struct deferred_split {
+	spinlock_t split_queue_lock;
+	struct list_head split_queue;
+	unsigned long split_queue_len;
+};
+#endif
+
 /*
  * On NUMA machines, each NUMA node would have a pg_data_t to describe
  * it's memory layout. On UMA machines there is a single pglist_data which
@@ -755,9 +763,7 @@ struct zonelist {
 #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */
 
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	spinlock_t split_queue_lock;
-	struct list_head split_queue;
-	unsigned long split_queue_len;
+	struct deferred_split deferred_split_queue;
 #endif
 
 	/* Fields commonly accessed by the page reclaim scanner */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9f8bce9..81cf759 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2658,6 +2658,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 {
 	struct page *head = compound_head(page);
 	struct pglist_data *pgdata = NODE_DATA(page_to_nid(head));
+	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
 	struct anon_vma *anon_vma = NULL;
 	struct address_space *mapping = NULL;
 	int count, mapcount, extra_pins, ret;
@@ -2744,17 +2745,17 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 	}
 
 	/* Prevent deferred_split_scan() touching ->_refcount */
-	spin_lock(&pgdata->split_queue_lock);
+	spin_lock(&ds_queue->split_queue_lock);
 	count = page_count(head);
 	mapcount = total_mapcount(head);
 	if (!mapcount && page_ref_freeze(head, 1 + extra_pins)) {
 		if (!list_empty(page_deferred_list(head))) {
-			pgdata->split_queue_len--;
+			ds_queue->split_queue_len--;
 			list_del(page_deferred_list(head));
 		}
 		if (mapping)
 			__dec_node_page_state(page, NR_SHMEM_THPS);
-		spin_unlock(&pgdata->split_queue_lock);
+		spin_unlock(&ds_queue->split_queue_lock);
 		__split_huge_page(page, list, end, flags);
 		if (PageSwapCache(head)) {
 			swp_entry_t entry = { .val = page_private(head) };
@@ -2771,7 +2772,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 			dump_page(page, "total_mapcount(head) > 0");
 			BUG();
 		}
-		spin_unlock(&pgdata->split_queue_lock);
+		spin_unlock(&ds_queue->split_queue_lock);
 fail:		if (mapping)
 			xa_unlock(&mapping->i_pages);
 		spin_unlock_irqrestore(&pgdata->lru_lock, flags);
@@ -2794,52 +2795,56 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 void free_transhuge_page(struct page *page)
 {
 	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
+	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
 	unsigned long flags;
 
-	spin_lock_irqsave(&pgdata->split_queue_lock, flags);
+	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
 	if (!list_empty(page_deferred_list(page))) {
-		pgdata->split_queue_len--;
+		ds_queue->split_queue_len--;
 		list_del(page_deferred_list(page));
 	}
-	spin_unlock_irqrestore(&pgdata->split_queue_lock, flags);
+	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 	free_compound_page(page);
 }
 
 void deferred_split_huge_page(struct page *page)
 {
 	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
+	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
 	unsigned long flags;
 
 	VM_BUG_ON_PAGE(!PageTransHuge(page), page);
 
-	spin_lock_irqsave(&pgdata->split_queue_lock, flags);
+	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
 	if (list_empty(page_deferred_list(page))) {
 		count_vm_event(THP_DEFERRED_SPLIT_PAGE);
-		list_add_tail(page_deferred_list(page), &pgdata->split_queue);
-		pgdata->split_queue_len++;
+		list_add_tail(page_deferred_list(page), &ds_queue->split_queue);
+		ds_queue->split_queue_len++;
 	}
-	spin_unlock_irqrestore(&pgdata->split_queue_lock, flags);
+	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 }
 
 static unsigned long deferred_split_count(struct shrinker *shrink,
 		struct shrink_control *sc)
 {
 	struct pglist_data *pgdata = NODE_DATA(sc->nid);
-	return READ_ONCE(pgdata->split_queue_len);
+	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
+	return READ_ONCE(ds_queue->split_queue_len);
 }
 
 static unsigned long deferred_split_scan(struct shrinker *shrink,
 		struct shrink_control *sc)
 {
 	struct pglist_data *pgdata = NODE_DATA(sc->nid);
+	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
 	unsigned long flags;
 	LIST_HEAD(list), *pos, *next;
 	struct page *page;
 	int split = 0;
 
-	spin_lock_irqsave(&pgdata->split_queue_lock, flags);
+	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
 	/* Take pin on all head pages to avoid freeing them under us */
-	list_for_each_safe(pos, next, &pgdata->split_queue) {
+	list_for_each_safe(pos, next, &ds_queue->split_queue) {
 		page = list_entry((void *)pos, struct page, mapping);
 		page = compound_head(page);
 		if (get_page_unless_zero(page)) {
@@ -2847,12 +2852,12 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 		} else {
 			/* We lost race with put_compound_page() */
 			list_del_init(page_deferred_list(page));
-			pgdata->split_queue_len--;
+			ds_queue->split_queue_len--;
 		}
 		if (!--sc->nr_to_scan)
 			break;
 	}
-	spin_unlock_irqrestore(&pgdata->split_queue_lock, flags);
+	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 
 	list_for_each_safe(pos, next, &list) {
 		page = list_entry((void *)pos, struct page, mapping);
@@ -2866,15 +2871,15 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 		put_page(page);
 	}
 
-	spin_lock_irqsave(&pgdata->split_queue_lock, flags);
-	list_splice_tail(&list, &pgdata->split_queue);
-	spin_unlock_irqrestore(&pgdata->split_queue_lock, flags);
+	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
+	list_splice_tail(&list, &ds_queue->split_queue);
+	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 
 	/*
 	 * Stop shrinker if we didn't split any page, but the queue is empty.
 	 * This can happen if pages were freed under us.
 	 */
-	if (!split && list_empty(&pgdata->split_queue))
+	if (!split && list_empty(&ds_queue->split_queue))
 		return SHRINK_STOP;
 	return split;
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 3b13d39..a82104a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6581,9 +6581,11 @@ static unsigned long __init calc_memmap_size(unsigned long spanned_pages,
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void pgdat_init_split_queue(struct pglist_data *pgdat)
 {
-	spin_lock_init(&pgdat->split_queue_lock);
-	INIT_LIST_HEAD(&pgdat->split_queue);
-	pgdat->split_queue_len = 0;
+	struct deferred_split *ds_queue = &pgdat->deferred_split_queue;
+
+	spin_lock_init(&ds_queue->split_queue_lock);
+	INIT_LIST_HEAD(&ds_queue->split_queue);
+	ds_queue->split_queue_len = 0;
 }
 #else
 static void pgdat_init_split_queue(struct pglist_data *pgdat) {}
-- 
1.8.3.1

[PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Yang Shi]

Currently THP deferred split shrinker is not memcg aware, this may cause
premature OOM with some configuration. For example the below test would
run into premature OOM easily:

$ cgcreate -g memory:thp
$ echo 4G > /sys/fs/cgroup/memory/thp/memory/limit_in_bytes
$ cgexec -g memory:thp transhuge-stress 4000

transhuge-stress comes from kernel selftest.

It is easy to hit OOM, but there are still a lot THP on the deferred
split queue, memcg direct reclaim can't touch them since the deferred
split shrinker is not memcg aware.

Convert deferred split shrinker memcg aware by introducing per memcg
deferred split queue.  The THP should be on either per node or per memcg
deferred split queue if it belongs to a memcg.  When the page is
immigrated to the other memcg, it will be immigrated to the target
memcg's deferred split queue too.

And, move deleting THP from deferred split queue in page free before
memcg uncharge so that the page's memcg information is available.

Reuse the second tail page's deferred_list for per memcg list since the
same THP can't be on multiple deferred split queues.

Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
 include/linux/huge_mm.h    | 15 ++++++++++
 include/linux/memcontrol.h |  4 +++
 include/linux/mm_types.h   |  1 +
 mm/huge_memory.c           | 71 +++++++++++++++++++++++++++++++++-------------
 mm/memcontrol.c            | 19 +++++++++++++
 mm/swap.c                  |  4 +++
 6 files changed, 94 insertions(+), 20 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 7cd5c15..8137c3a 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -250,6 +250,17 @@ static inline bool thp_migration_supported(void)
 	return IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION);
 }
 
+static inline struct list_head *page_deferred_list(struct page *page)
+{
+	/*
+	 * Global or memcg deferred list in the second tail pages is
+	 * occupied by compound_head.
+	 */
+	return &page[2].deferred_list;
+}
+
+extern void del_thp_from_deferred_split_queue(struct page *);
+
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
 #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
 #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
@@ -368,6 +379,10 @@ static inline bool thp_migration_supported(void)
 {
 	return false;
 }
+
+static inline void del_thp_from_deferred_split_queue(struct page *page)
+{
+}
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
 #endif /* _LINUX_HUGE_MM_H */
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index bc74d6a..5d3c10c 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -316,6 +316,10 @@ struct mem_cgroup {
 	struct list_head event_list;
 	spinlock_t event_list_lock;
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	struct deferred_split deferred_split_queue;
+#endif
+
 	struct mem_cgroup_per_node *nodeinfo[0];
 	/* WARNING: nodeinfo must be the last member here */
 };
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 8ec38b1..4eabf80 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -139,6 +139,7 @@ struct page {
 		struct {	/* Second tail page of compound page */
 			unsigned long _compound_pad_1;	/* compound_head */
 			unsigned long _compound_pad_2;
+			/* For both global and memcg */
 			struct list_head deferred_list;
 		};
 		struct {	/* Page table pages */
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 81cf759..3307697 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -492,10 +492,15 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
 	return pmd;
 }
 
-static inline struct list_head *page_deferred_list(struct page *page)
+static inline struct deferred_split *get_deferred_split_queue(struct page *page)
 {
-	/* ->lru in the tail pages is occupied by compound_head. */
-	return &page[2].deferred_list;
+	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
+	struct pglist_data *pgdat = NODE_DATA(page_to_nid(page));
+
+	if (memcg)
+		return &memcg->deferred_split_queue;
+	else
+		return &pgdat->deferred_split_queue;
 }
 
 void prep_transhuge_page(struct page *page)
@@ -2658,7 +2663,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 {
 	struct page *head = compound_head(page);
 	struct pglist_data *pgdata = NODE_DATA(page_to_nid(head));
-	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
+	struct deferred_split *ds_queue = get_deferred_split_queue(page);
 	struct anon_vma *anon_vma = NULL;
 	struct address_space *mapping = NULL;
 	int count, mapcount, extra_pins, ret;
@@ -2792,25 +2797,36 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
 	return ret;
 }
 
-void free_transhuge_page(struct page *page)
+void del_thp_from_deferred_split_queue(struct page *page)
 {
-	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
-	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
-	unsigned long flags;
-
-	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
-	if (!list_empty(page_deferred_list(page))) {
-		ds_queue->split_queue_len--;
-		list_del(page_deferred_list(page));
+	/*
+	 * The THP may be not on LRU at this point, e.g. the old page of
+	 * NUMA migration.  And PageTransHuge is not enough to distinguish
+	 * with other compound page, e.g. skb, THP destructor is not used
+	 * anymore and will be removed, so the compound order sounds like
+	 * the only choice here.
+	 */
+	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
+		struct deferred_split *ds_queue = get_deferred_split_queue(page);
+		unsigned long flags;
+		spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
+			if (!list_empty(page_deferred_list(page))) {
+				ds_queue->split_queue_len--;
+				list_del(page_deferred_list(page));
+			}
+		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 	}
-	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
+}
+
+void free_transhuge_page(struct page *page)
+{
 	free_compound_page(page);
 }
 
 void deferred_split_huge_page(struct page *page)
 {
-	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
-	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
+	struct deferred_split *ds_queue = get_deferred_split_queue(page);
+	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
 	unsigned long flags;
 
 	VM_BUG_ON_PAGE(!PageTransHuge(page), page);
@@ -2820,6 +2836,9 @@ void deferred_split_huge_page(struct page *page)
 		count_vm_event(THP_DEFERRED_SPLIT_PAGE);
 		list_add_tail(page_deferred_list(page), &ds_queue->split_queue);
 		ds_queue->split_queue_len++;
+		if (memcg)
+			memcg_set_shrinker_bit(memcg, page_to_nid(page),
+					       deferred_split_shrinker.id);
 	}
 	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
 }
@@ -2827,8 +2846,15 @@ void deferred_split_huge_page(struct page *page)
 static unsigned long deferred_split_count(struct shrinker *shrink,
 		struct shrink_control *sc)
 {
-	struct pglist_data *pgdata = NODE_DATA(sc->nid);
-	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
+	struct deferred_split *ds_queue;
+
+	if (!sc->memcg) {
+		struct pglist_data *pgdata = NODE_DATA(sc->nid);
+		ds_queue = &pgdata->deferred_split_queue;
+		return READ_ONCE(ds_queue->split_queue_len);
+	}
+
+	ds_queue = &sc->memcg->deferred_split_queue;
 	return READ_ONCE(ds_queue->split_queue_len);
 }
 
@@ -2836,12 +2862,17 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 		struct shrink_control *sc)
 {
 	struct pglist_data *pgdata = NODE_DATA(sc->nid);
-	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
+	struct deferred_split *ds_queue;
 	unsigned long flags;
 	LIST_HEAD(list), *pos, *next;
 	struct page *page;
 	int split = 0;
 
+	if (sc->memcg)
+		ds_queue = &sc->memcg->deferred_split_queue;
+	else
+		ds_queue = &pgdata->deferred_split_queue;
+
 	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
 	/* Take pin on all head pages to avoid freeing them under us */
 	list_for_each_safe(pos, next, &ds_queue->split_queue) {
@@ -2888,7 +2919,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 	.count_objects = deferred_split_count,
 	.scan_objects = deferred_split_scan,
 	.seeks = DEFAULT_SEEKS,
-	.flags = SHRINKER_NUMA_AWARE,
+	.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE,
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index e50a2db..fe7e544 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4579,6 +4579,11 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
 #ifdef CONFIG_CGROUP_WRITEBACK
 	INIT_LIST_HEAD(&memcg->cgwb_list);
 #endif
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	spin_lock_init(&memcg->deferred_split_queue.split_queue_lock);
+	INIT_LIST_HEAD(&memcg->deferred_split_queue.split_queue);
+	memcg->deferred_split_queue.split_queue_len = 0;
+#endif
 	idr_replace(&mem_cgroup_idr, memcg, memcg->id.id);
 	return memcg;
 fail:
@@ -4949,6 +4954,20 @@ static int mem_cgroup_move_account(struct page *page,
 		__mod_memcg_state(to, NR_WRITEBACK, nr_pages);
 	}
 
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+	if (compound && !list_empty(page_deferred_list(page))) {
+		spin_lock(&from->deferred_split_queue.split_queue_lock);
+		list_del(page_deferred_list(page));
+		from->deferred_split_queue.split_queue_len--;
+		spin_unlock(&from->deferred_split_queue.split_queue_lock);
+
+		spin_lock(&to->deferred_split_queue.split_queue_lock);
+		list_add_tail(page_deferred_list(page),
+			      &to->deferred_split_queue.split_queue);
+		to->deferred_split_queue.split_queue_len++;
+		spin_unlock(&to->deferred_split_queue.split_queue_lock);
+	}
+#endif
 	/*
 	 * It is safe to change page->mem_cgroup here because the page
 	 * is referenced, charged, and isolated - we can't race with
diff --git a/mm/swap.c b/mm/swap.c
index 3a75722..3348295 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -69,6 +69,10 @@ static void __page_cache_release(struct page *page)
 		del_page_from_lru_list(page, lruvec, page_off_lru(page));
 		spin_unlock_irqrestore(&pgdat->lru_lock, flags);
 	}
+
+	/* Delete THP from deferred split queue before memcg uncharge */
+	del_thp_from_deferred_split_queue(page);
+
 	__ClearPageWaiters(page);
 	mem_cgroup_uncharge(page);
 }
-- 
1.8.3.1

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Kirill Tkhai]

Hi, Yang,

On 07.06.2019 09:07, Yang Shi wrote:
> Currently THP deferred split shrinker is not memcg aware, this may cause
> premature OOM with some configuration. For example the below test would
> run into premature OOM easily:
> 
> $ cgcreate -g memory:thp
> $ echo 4G > /sys/fs/cgroup/memory/thp/memory/limit_in_bytes
> $ cgexec -g memory:thp transhuge-stress 4000
> 
> transhuge-stress comes from kernel selftest.
> 
> It is easy to hit OOM, but there are still a lot THP on the deferred
> split queue, memcg direct reclaim can't touch them since the deferred
> split shrinker is not memcg aware.
> 
> Convert deferred split shrinker memcg aware by introducing per memcg
> deferred split queue.  The THP should be on either per node or per memcg
> deferred split queue if it belongs to a memcg.  When the page is
> immigrated to the other memcg, it will be immigrated to the target
> memcg's deferred split queue too.
> 
> And, move deleting THP from deferred split queue in page free before
> memcg uncharge so that the page's memcg information is available.
> 
> Reuse the second tail page's deferred_list for per memcg list since the
> same THP can't be on multiple deferred split queues.
> 
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Shakeel Butt <shakeelb@google.com>
> Cc: David Rientjes <rientjes@google.com>
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> ---
>  include/linux/huge_mm.h    | 15 ++++++++++
>  include/linux/memcontrol.h |  4 +++
>  include/linux/mm_types.h   |  1 +
>  mm/huge_memory.c           | 71 +++++++++++++++++++++++++++++++++-------------
>  mm/memcontrol.c            | 19 +++++++++++++
>  mm/swap.c                  |  4 +++
>  6 files changed, 94 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 7cd5c15..8137c3a 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -250,6 +250,17 @@ static inline bool thp_migration_supported(void)
>  	return IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION);
>  }
>  
> +static inline struct list_head *page_deferred_list(struct page *page)
> +{
> +	/*
> +	 * Global or memcg deferred list in the second tail pages is
> +	 * occupied by compound_head.
> +	 */
> +	return &page[2].deferred_list;
> +}
> +
> +extern void del_thp_from_deferred_split_queue(struct page *);
> +
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>  #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
>  #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
> @@ -368,6 +379,10 @@ static inline bool thp_migration_supported(void)
>  {
>  	return false;
>  }
> +
> +static inline void del_thp_from_deferred_split_queue(struct page *page)
> +{
> +}
>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>  
>  #endif /* _LINUX_HUGE_MM_H */
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index bc74d6a..5d3c10c 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -316,6 +316,10 @@ struct mem_cgroup {
>  	struct list_head event_list;
>  	spinlock_t event_list_lock;
>  
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	struct deferred_split deferred_split_queue;
> +#endif
> +
>  	struct mem_cgroup_per_node *nodeinfo[0];
>  	/* WARNING: nodeinfo must be the last member here */
>  };
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 8ec38b1..4eabf80 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -139,6 +139,7 @@ struct page {
>  		struct {	/* Second tail page of compound page */
>  			unsigned long _compound_pad_1;	/* compound_head */
>  			unsigned long _compound_pad_2;
> +			/* For both global and memcg */
>  			struct list_head deferred_list;
>  		};
>  		struct {	/* Page table pages */
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 81cf759..3307697 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -492,10 +492,15 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
>  	return pmd;
>  }
>  
> -static inline struct list_head *page_deferred_list(struct page *page)
> +static inline struct deferred_split *get_deferred_split_queue(struct page *page)
>  {
> -	/* ->lru in the tail pages is occupied by compound_head. */
> -	return &page[2].deferred_list;
> +	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
> +	struct pglist_data *pgdat = NODE_DATA(page_to_nid(page));
> +
> +	if (memcg)
> +		return &memcg->deferred_split_queue;
> +	else
> +		return &pgdat->deferred_split_queue;

memory_cgrp_subsys is not early initialized, so at the beginning of boot
root_mem_cgroup is NULL, and pages will use &pgdat->deferred_split_queue
list head. But after root_mem_cgroup is initialized, another list head
will be used, won't it?! So there will be two different list heads used
for same cgroup.

This may be a reason of a problem (I won't say you, where the problem will
occur).

>  }
>  
>  void prep_transhuge_page(struct page *page)
> @@ -2658,7 +2663,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  {
>  	struct page *head = compound_head(page);
>  	struct pglist_data *pgdata = NODE_DATA(page_to_nid(head));
> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
> +	struct deferred_split *ds_queue = get_deferred_split_queue(page);
>  	struct anon_vma *anon_vma = NULL;
>  	struct address_space *mapping = NULL;
>  	int count, mapcount, extra_pins, ret;
> @@ -2792,25 +2797,36 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>  	return ret;
>  }
>  
> -void free_transhuge_page(struct page *page)
> +void del_thp_from_deferred_split_queue(struct page *page)
>  {
> -	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
> -	if (!list_empty(page_deferred_list(page))) {
> -		ds_queue->split_queue_len--;
> -		list_del(page_deferred_list(page));
> +	/*
> +	 * The THP may be not on LRU at this point, e.g. the old page of
> +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
> +	 * with other compound page, e.g. skb, THP destructor is not used
> +	 * anymore and will be removed, so the compound order sounds like
> +	 * the only choice here.
> +	 */
> +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
> +		struct deferred_split *ds_queue = get_deferred_split_queue(page);
> +		unsigned long flags;
> +		spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
> +			if (!list_empty(page_deferred_list(page))) {
> +				ds_queue->split_queue_len--;
> +				list_del(page_deferred_list(page));
> +			}
> +		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>  	}
> -	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
> +}
> +
> +void free_transhuge_page(struct page *page)
> +{
>  	free_compound_page(page);
>  }
>  
>  void deferred_split_huge_page(struct page *page)
>  {
> -	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
> +	struct deferred_split *ds_queue = get_deferred_split_queue(page);
> +	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
>  	unsigned long flags;
>  
>  	VM_BUG_ON_PAGE(!PageTransHuge(page), page);
> @@ -2820,6 +2836,9 @@ void deferred_split_huge_page(struct page *page)
>  		count_vm_event(THP_DEFERRED_SPLIT_PAGE);
>  		list_add_tail(page_deferred_list(page), &ds_queue->split_queue);
>  		ds_queue->split_queue_len++;
> +		if (memcg)
> +			memcg_set_shrinker_bit(memcg, page_to_nid(page),
> +					       deferred_split_shrinker.id);
>  	}
>  	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>  }
> @@ -2827,8 +2846,15 @@ void deferred_split_huge_page(struct page *page)
>  static unsigned long deferred_split_count(struct shrinker *shrink,
>  		struct shrink_control *sc)
>  {
> -	struct pglist_data *pgdata = NODE_DATA(sc->nid);
> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
> +	struct deferred_split *ds_queue;
> +
> +	if (!sc->memcg) {
> +		struct pglist_data *pgdata = NODE_DATA(sc->nid);
> +		ds_queue = &pgdata->deferred_split_queue;
> +		return READ_ONCE(ds_queue->split_queue_len);
> +	}
> +
> +	ds_queue = &sc->memcg->deferred_split_queue;
>  	return READ_ONCE(ds_queue->split_queue_len);
>  }
>  
> @@ -2836,12 +2862,17 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>  		struct shrink_control *sc)
>  {
>  	struct pglist_data *pgdata = NODE_DATA(sc->nid);
> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
> +	struct deferred_split *ds_queue;
>  	unsigned long flags;
>  	LIST_HEAD(list), *pos, *next;
>  	struct page *page;
>  	int split = 0;
>  
> +	if (sc->memcg)
> +		ds_queue = &sc->memcg->deferred_split_queue;
> +	else
> +		ds_queue = &pgdata->deferred_split_queue;
> +
>  	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>  	/* Take pin on all head pages to avoid freeing them under us */
>  	list_for_each_safe(pos, next, &ds_queue->split_queue) {
> @@ -2888,7 +2919,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>  	.count_objects = deferred_split_count,
>  	.scan_objects = deferred_split_scan,
>  	.seeks = DEFAULT_SEEKS,
> -	.flags = SHRINKER_NUMA_AWARE,
> +	.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE,
>  };
>  
>  #ifdef CONFIG_DEBUG_FS
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e50a2db..fe7e544 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -4579,6 +4579,11 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
>  #ifdef CONFIG_CGROUP_WRITEBACK
>  	INIT_LIST_HEAD(&memcg->cgwb_list);
>  #endif
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	spin_lock_init(&memcg->deferred_split_queue.split_queue_lock);
> +	INIT_LIST_HEAD(&memcg->deferred_split_queue.split_queue);
> +	memcg->deferred_split_queue.split_queue_len = 0;
> +#endif
>  	idr_replace(&mem_cgroup_idr, memcg, memcg->id.id);
>  	return memcg;
>  fail:
> @@ -4949,6 +4954,20 @@ static int mem_cgroup_move_account(struct page *page,
>  		__mod_memcg_state(to, NR_WRITEBACK, nr_pages);
>  	}
>  
> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
> +	if (compound && !list_empty(page_deferred_list(page))) {
> +		spin_lock(&from->deferred_split_queue.split_queue_lock);
> +		list_del(page_deferred_list(page));
> +		from->deferred_split_queue.split_queue_len--;
> +		spin_unlock(&from->deferred_split_queue.split_queue_lock);

Won't be better to assign

page->mem_cgroup = to;

after removing from one list and before linking to another list?
There is possible no a problem, but another people writing code
on top of this may not expect such the behavior.
> +		spin_lock(&to->deferred_split_queue.split_queue_lock);
> +		list_add_tail(page_deferred_list(page),
> +			      &to->deferred_split_queue.split_queue);
> +		to->deferred_split_queue.split_queue_len++;
> +		spin_unlock(&to->deferred_split_queue.split_queue_lock);
> +	}
> +#endif
>  	/*
>  	 * It is safe to change page->mem_cgroup here because the page
>  	 * is referenced, charged, and isolated - we can't race with
> diff --git a/mm/swap.c b/mm/swap.c
> index 3a75722..3348295 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -69,6 +69,10 @@ static void __page_cache_release(struct page *page)
>  		del_page_from_lru_list(page, lruvec, page_off_lru(page));
>  		spin_unlock_irqrestore(&pgdat->lru_lock, flags);
>  	}
> +
> +	/* Delete THP from deferred split queue before memcg uncharge */
> +	del_thp_from_deferred_split_queue(page);
> +
>  	__ClearPageWaiters(page);
>  	mem_cgroup_uncharge(page);
>  }

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Yang Shi]

On 6/10/19 1:21 AM, Kirill Tkhai wrote:
> Hi, Yang,
>
> On 07.06.2019 09:07, Yang Shi wrote:
>> Currently THP deferred split shrinker is not memcg aware, this may cause
>> premature OOM with some configuration. For example the below test would
>> run into premature OOM easily:
>>
>> $ cgcreate -g memory:thp
>> $ echo 4G > /sys/fs/cgroup/memory/thp/memory/limit_in_bytes
>> $ cgexec -g memory:thp transhuge-stress 4000
>>
>> transhuge-stress comes from kernel selftest.
>>
>> It is easy to hit OOM, but there are still a lot THP on the deferred
>> split queue, memcg direct reclaim can't touch them since the deferred
>> split shrinker is not memcg aware.
>>
>> Convert deferred split shrinker memcg aware by introducing per memcg
>> deferred split queue.  The THP should be on either per node or per memcg
>> deferred split queue if it belongs to a memcg.  When the page is
>> immigrated to the other memcg, it will be immigrated to the target
>> memcg's deferred split queue too.
>>
>> And, move deleting THP from deferred split queue in page free before
>> memcg uncharge so that the page's memcg information is available.
>>
>> Reuse the second tail page's deferred_list for per memcg list since the
>> same THP can't be on multiple deferred split queues.
>>
>> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
>> Cc: Hugh Dickins <hughd@google.com>
>> Cc: Shakeel Butt <shakeelb@google.com>
>> Cc: David Rientjes <rientjes@google.com>
>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>> ---
>>   include/linux/huge_mm.h    | 15 ++++++++++
>>   include/linux/memcontrol.h |  4 +++
>>   include/linux/mm_types.h   |  1 +
>>   mm/huge_memory.c           | 71 +++++++++++++++++++++++++++++++++-------------
>>   mm/memcontrol.c            | 19 +++++++++++++
>>   mm/swap.c                  |  4 +++
>>   6 files changed, 94 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
>> index 7cd5c15..8137c3a 100644
>> --- a/include/linux/huge_mm.h
>> +++ b/include/linux/huge_mm.h
>> @@ -250,6 +250,17 @@ static inline bool thp_migration_supported(void)
>>   	return IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION);
>>   }
>>   
>> +static inline struct list_head *page_deferred_list(struct page *page)
>> +{
>> +	/*
>> +	 * Global or memcg deferred list in the second tail pages is
>> +	 * occupied by compound_head.
>> +	 */
>> +	return &page[2].deferred_list;
>> +}
>> +
>> +extern void del_thp_from_deferred_split_queue(struct page *);
>> +
>>   #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>>   #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
>>   #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
>> @@ -368,6 +379,10 @@ static inline bool thp_migration_supported(void)
>>   {
>>   	return false;
>>   }
>> +
>> +static inline void del_thp_from_deferred_split_queue(struct page *page)
>> +{
>> +}
>>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>>   
>>   #endif /* _LINUX_HUGE_MM_H */
>> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>> index bc74d6a..5d3c10c 100644
>> --- a/include/linux/memcontrol.h
>> +++ b/include/linux/memcontrol.h
>> @@ -316,6 +316,10 @@ struct mem_cgroup {
>>   	struct list_head event_list;
>>   	spinlock_t event_list_lock;
>>   
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +	struct deferred_split deferred_split_queue;
>> +#endif
>> +
>>   	struct mem_cgroup_per_node *nodeinfo[0];
>>   	/* WARNING: nodeinfo must be the last member here */
>>   };
>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>> index 8ec38b1..4eabf80 100644
>> --- a/include/linux/mm_types.h
>> +++ b/include/linux/mm_types.h
>> @@ -139,6 +139,7 @@ struct page {
>>   		struct {	/* Second tail page of compound page */
>>   			unsigned long _compound_pad_1;	/* compound_head */
>>   			unsigned long _compound_pad_2;
>> +			/* For both global and memcg */
>>   			struct list_head deferred_list;
>>   		};
>>   		struct {	/* Page table pages */
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 81cf759..3307697 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -492,10 +492,15 @@ pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
>>   	return pmd;
>>   }
>>   
>> -static inline struct list_head *page_deferred_list(struct page *page)
>> +static inline struct deferred_split *get_deferred_split_queue(struct page *page)
>>   {
>> -	/* ->lru in the tail pages is occupied by compound_head. */
>> -	return &page[2].deferred_list;
>> +	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
>> +	struct pglist_data *pgdat = NODE_DATA(page_to_nid(page));
>> +
>> +	if (memcg)
>> +		return &memcg->deferred_split_queue;
>> +	else
>> +		return &pgdat->deferred_split_queue;
> memory_cgrp_subsys is not early initialized, so at the beginning of boot
> root_mem_cgroup is NULL, and pages will use &pgdat->deferred_split_queue
> list head. But after root_mem_cgroup is initialized, another list head
> will be used, won't it?! So there will be two different list heads used
> for same cgroup.
>
> This may be a reason of a problem (I won't say you, where the problem will
> occur).

Actually I was wondering this too. But, it looks impossible. The memcg 
is initialized by subsys_initcall, there are still fs, device and late 
initcalls after it. The init process is run much later, so I can't think 
of any THP could be put on deferred split queue before memcg is 
initialized. There may be compound page allocation, i.e. skb, but we 
don't care them.

>
>>   }
>>   
>>   void prep_transhuge_page(struct page *page)
>> @@ -2658,7 +2663,7 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>>   {
>>   	struct page *head = compound_head(page);
>>   	struct pglist_data *pgdata = NODE_DATA(page_to_nid(head));
>> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>> +	struct deferred_split *ds_queue = get_deferred_split_queue(page);
>>   	struct anon_vma *anon_vma = NULL;
>>   	struct address_space *mapping = NULL;
>>   	int count, mapcount, extra_pins, ret;
>> @@ -2792,25 +2797,36 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>>   	return ret;
>>   }
>>   
>> -void free_transhuge_page(struct page *page)
>> +void del_thp_from_deferred_split_queue(struct page *page)
>>   {
>> -	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
>> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>> -	unsigned long flags;
>> -
>> -	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>> -	if (!list_empty(page_deferred_list(page))) {
>> -		ds_queue->split_queue_len--;
>> -		list_del(page_deferred_list(page));
>> +	/*
>> +	 * The THP may be not on LRU at this point, e.g. the old page of
>> +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
>> +	 * with other compound page, e.g. skb, THP destructor is not used
>> +	 * anymore and will be removed, so the compound order sounds like
>> +	 * the only choice here.
>> +	 */
>> +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
>> +		struct deferred_split *ds_queue = get_deferred_split_queue(page);
>> +		unsigned long flags;
>> +		spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>> +			if (!list_empty(page_deferred_list(page))) {
>> +				ds_queue->split_queue_len--;
>> +				list_del(page_deferred_list(page));
>> +			}
>> +		spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>>   	}
>> -	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>> +}
>> +
>> +void free_transhuge_page(struct page *page)
>> +{
>>   	free_compound_page(page);
>>   }
>>   
>>   void deferred_split_huge_page(struct page *page)
>>   {
>> -	struct pglist_data *pgdata = NODE_DATA(page_to_nid(page));
>> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>> +	struct deferred_split *ds_queue = get_deferred_split_queue(page);
>> +	struct mem_cgroup *memcg = compound_head(page)->mem_cgroup;
>>   	unsigned long flags;
>>   
>>   	VM_BUG_ON_PAGE(!PageTransHuge(page), page);
>> @@ -2820,6 +2836,9 @@ void deferred_split_huge_page(struct page *page)
>>   		count_vm_event(THP_DEFERRED_SPLIT_PAGE);
>>   		list_add_tail(page_deferred_list(page), &ds_queue->split_queue);
>>   		ds_queue->split_queue_len++;
>> +		if (memcg)
>> +			memcg_set_shrinker_bit(memcg, page_to_nid(page),
>> +					       deferred_split_shrinker.id);
>>   	}
>>   	spin_unlock_irqrestore(&ds_queue->split_queue_lock, flags);
>>   }
>> @@ -2827,8 +2846,15 @@ void deferred_split_huge_page(struct page *page)
>>   static unsigned long deferred_split_count(struct shrinker *shrink,
>>   		struct shrink_control *sc)
>>   {
>> -	struct pglist_data *pgdata = NODE_DATA(sc->nid);
>> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>> +	struct deferred_split *ds_queue;
>> +
>> +	if (!sc->memcg) {
>> +		struct pglist_data *pgdata = NODE_DATA(sc->nid);
>> +		ds_queue = &pgdata->deferred_split_queue;
>> +		return READ_ONCE(ds_queue->split_queue_len);
>> +	}
>> +
>> +	ds_queue = &sc->memcg->deferred_split_queue;
>>   	return READ_ONCE(ds_queue->split_queue_len);
>>   }
>>   
>> @@ -2836,12 +2862,17 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>>   		struct shrink_control *sc)
>>   {
>>   	struct pglist_data *pgdata = NODE_DATA(sc->nid);
>> -	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>> +	struct deferred_split *ds_queue;
>>   	unsigned long flags;
>>   	LIST_HEAD(list), *pos, *next;
>>   	struct page *page;
>>   	int split = 0;
>>   
>> +	if (sc->memcg)
>> +		ds_queue = &sc->memcg->deferred_split_queue;
>> +	else
>> +		ds_queue = &pgdata->deferred_split_queue;
>> +
>>   	spin_lock_irqsave(&ds_queue->split_queue_lock, flags);
>>   	/* Take pin on all head pages to avoid freeing them under us */
>>   	list_for_each_safe(pos, next, &ds_queue->split_queue) {
>> @@ -2888,7 +2919,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>>   	.count_objects = deferred_split_count,
>>   	.scan_objects = deferred_split_scan,
>>   	.seeks = DEFAULT_SEEKS,
>> -	.flags = SHRINKER_NUMA_AWARE,
>> +	.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE,
>>   };
>>   
>>   #ifdef CONFIG_DEBUG_FS
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index e50a2db..fe7e544 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4579,6 +4579,11 @@ static struct mem_cgroup *mem_cgroup_alloc(void)
>>   #ifdef CONFIG_CGROUP_WRITEBACK
>>   	INIT_LIST_HEAD(&memcg->cgwb_list);
>>   #endif
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +	spin_lock_init(&memcg->deferred_split_queue.split_queue_lock);
>> +	INIT_LIST_HEAD(&memcg->deferred_split_queue.split_queue);
>> +	memcg->deferred_split_queue.split_queue_len = 0;
>> +#endif
>>   	idr_replace(&mem_cgroup_idr, memcg, memcg->id.id);
>>   	return memcg;
>>   fail:
>> @@ -4949,6 +4954,20 @@ static int mem_cgroup_move_account(struct page *page,
>>   		__mod_memcg_state(to, NR_WRITEBACK, nr_pages);
>>   	}
>>   
>> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> +	if (compound && !list_empty(page_deferred_list(page))) {
>> +		spin_lock(&from->deferred_split_queue.split_queue_lock);
>> +		list_del(page_deferred_list(page));
>> +		from->deferred_split_queue.split_queue_len--;
>> +		spin_unlock(&from->deferred_split_queue.split_queue_lock);
> Won't be better to assign
>
> page->mem_cgroup = to;
>
> after removing from one list and before linking to another list?
> There is possible no a problem, but another people writing code
> on top of this may not expect such the behavior.

It should be fine too.

>> +		spin_lock(&to->deferred_split_queue.split_queue_lock);
>> +		list_add_tail(page_deferred_list(page),
>> +			      &to->deferred_split_queue.split_queue);
>> +		to->deferred_split_queue.split_queue_len++;
>> +		spin_unlock(&to->deferred_split_queue.split_queue_lock);
>> +	}
>> +#endif
>>   	/*
>>   	 * It is safe to change page->mem_cgroup here because the page
>>   	 * is referenced, charged, and isolated - we can't race with
>> diff --git a/mm/swap.c b/mm/swap.c
>> index 3a75722..3348295 100644
>> --- a/mm/swap.c
>> +++ b/mm/swap.c
>> @@ -69,6 +69,10 @@ static void __page_cache_release(struct page *page)
>>   		del_page_from_lru_list(page, lruvec, page_off_lru(page));
>>   		spin_unlock_irqrestore(&pgdat->lru_lock, flags);
>>   	}
>> +
>> +	/* Delete THP from deferred split queue before memcg uncharge */
>> +	del_thp_from_deferred_split_queue(page);
>> +
>>   	__ClearPageWaiters(page);
>>   	mem_cgroup_uncharge(page);
>>   }

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Kirill A. Shutemov]

On Fri, Jun 07, 2019 at 02:07:37PM +0800, Yang Shi wrote:
> +	/*
> +	 * The THP may be not on LRU at this point, e.g. the old page of
> +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
> +	 * with other compound page, e.g. skb, THP destructor is not used
> +	 * anymore and will be removed, so the compound order sounds like
> +	 * the only choice here.
> +	 */
> +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {

What happens if the page is the same order as THP is not THP? Why removing
of destructor is required?

-- 
 Kirill A. Shutemov

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Yang Shi]

On 6/11/19 7:47 PM, Kirill A. Shutemov wrote:
> On Fri, Jun 07, 2019 at 02:07:37PM +0800, Yang Shi wrote:
>> +	/*
>> +	 * The THP may be not on LRU at this point, e.g. the old page of
>> +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
>> +	 * with other compound page, e.g. skb, THP destructor is not used
>> +	 * anymore and will be removed, so the compound order sounds like
>> +	 * the only choice here.
>> +	 */
>> +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
> What happens if the page is the same order as THP is not THP? Why removing

It may corrupt the deferred split queue since it is never added into the 
list, but deleted here.

> of destructor is required?

Due to the change to free_transhuge_page() (extracted deferred split 
queue manipulation and moved before memcg uncharge since 
page->mem_cgroup is needed), it just calls free_compound_page(). So, it 
sounds pointless to still keep THP specific destructor.

It looks there is not a good way to tell if the compound page is THP in 
free_page path or not, we may keep the destructor just for this?

>

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Kirill A. Shutemov]

On Tue, Jun 11, 2019 at 10:06:36PM -0700, Yang Shi wrote:
> 
> 
> On 6/11/19 7:47 PM, Kirill A. Shutemov wrote:
> > On Fri, Jun 07, 2019 at 02:07:37PM +0800, Yang Shi wrote:
> > > +	/*
> > > +	 * The THP may be not on LRU at this point, e.g. the old page of
> > > +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
> > > +	 * with other compound page, e.g. skb, THP destructor is not used
> > > +	 * anymore and will be removed, so the compound order sounds like
> > > +	 * the only choice here.
> > > +	 */
> > > +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
> > What happens if the page is the same order as THP is not THP? Why removing
> 
> It may corrupt the deferred split queue since it is never added into the
> list, but deleted here.
> 
> > of destructor is required?
> 
> Due to the change to free_transhuge_page() (extracted deferred split queue
> manipulation and moved before memcg uncharge since page->mem_cgroup is
> needed), it just calls free_compound_page(). So, it sounds pointless to
> still keep THP specific destructor.
> 
> It looks there is not a good way to tell if the compound page is THP in
> free_page path or not, we may keep the destructor just for this?

Other option would be to move mem_cgroup_uncharge(page); from
__page_cache_release() to destructors. Destructors will be able to
call it as it fits.

-- 
 Kirill A. Shutemov

Re: [PATCH 2/4] mm: thp: make deferred split shrinker memcg aware

[Yang Shi]

On 6/12/19 3:09 AM, Kirill A. Shutemov wrote:
> On Tue, Jun 11, 2019 at 10:06:36PM -0700, Yang Shi wrote:
>>
>> On 6/11/19 7:47 PM, Kirill A. Shutemov wrote:
>>> On Fri, Jun 07, 2019 at 02:07:37PM +0800, Yang Shi wrote:
>>>> +	/*
>>>> +	 * The THP may be not on LRU at this point, e.g. the old page of
>>>> +	 * NUMA migration.  And PageTransHuge is not enough to distinguish
>>>> +	 * with other compound page, e.g. skb, THP destructor is not used
>>>> +	 * anymore and will be removed, so the compound order sounds like
>>>> +	 * the only choice here.
>>>> +	 */
>>>> +	if (PageTransHuge(page) && compound_order(page) == HPAGE_PMD_ORDER) {
>>> What happens if the page is the same order as THP is not THP? Why removing
>> It may corrupt the deferred split queue since it is never added into the
>> list, but deleted here.
>>
>>> of destructor is required?
>> Due to the change to free_transhuge_page() (extracted deferred split queue
>> manipulation and moved before memcg uncharge since page->mem_cgroup is
>> needed), it just calls free_compound_page(). So, it sounds pointless to
>> still keep THP specific destructor.
>>
>> It looks there is not a good way to tell if the compound page is THP in
>> free_page path or not, we may keep the destructor just for this?
> Other option would be to move mem_cgroup_uncharge(page); from
> __page_cache_release() to destructors. Destructors will be able to
> call it as it fits.

Yes, it is an option. Since __page_cache_release() is called by 
__put_single_page() too, so mem_cgroup_uncharge() has to be called in 
both __put_single_page() and the desctructor (free_compound_page() which 
is called by both THP and other compound page except HugeTLB). But, it 
sounds acceptable IMHO.

>

[PATCH 3/4] mm: thp: remove THP destructor

[Yang Shi]

The THP destructor is used to delete THP from per node deferred split
queue, now the operation is moved out of it, so the destructor is not
used anymore, remove it.

Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
 include/linux/mm.h | 3 ---
 mm/huge_memory.c   | 6 ------
 mm/page_alloc.c    | 3 ---
 3 files changed, 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834a..e543984 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -740,9 +740,6 @@ enum compound_dtor_id {
 #ifdef CONFIG_HUGETLB_PAGE
 	HUGETLB_PAGE_DTOR,
 #endif
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	TRANSHUGE_PAGE_DTOR,
-#endif
 	NR_COMPOUND_DTORS,
 };
 extern compound_page_dtor * const compound_page_dtors[];
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3307697..50f4720 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -511,7 +511,6 @@ void prep_transhuge_page(struct page *page)
 	 */
 
 	INIT_LIST_HEAD(page_deferred_list(page));
-	set_compound_page_dtor(page, TRANSHUGE_PAGE_DTOR);
 }
 
 static unsigned long __thp_get_unmapped_area(struct file *filp, unsigned long len,
@@ -2818,11 +2817,6 @@ void del_thp_from_deferred_split_queue(struct page *page)
 	}
 }
 
-void free_transhuge_page(struct page *page)
-{
-	free_compound_page(page);
-}
-
 void deferred_split_huge_page(struct page *page)
 {
 	struct deferred_split *ds_queue = get_deferred_split_queue(page);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a82104a..6009214 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -261,9 +261,6 @@ bool pm_suspended_storage(void)
 #ifdef CONFIG_HUGETLB_PAGE
 	free_huge_page,
 #endif
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
-	free_transhuge_page,
-#endif
 };
 
 int min_free_kbytes = 1024;
-- 
1.8.3.1

[PATCH 4/4] mm: shrinker: make shrinker not depend on memcg kmem

[Yang Shi]

Currently shrinker is just allocated and can work when memcg kmem is
enabled.  But, THP deferred split shrinker is not slab shrinker, it
doesn't make too much sense to have such shrinker depend on memcg kmem.
It should be able to reclaim THP even though memcg kmem is disabled.

Introduce a new shrinker flag, SHRINKER_NONSLAB, for non-slab shrinker,
i.e. THP deferred split shrinker.  When memcg kmem is disabled, just
such shrinkers can be called in shrinking memcg slab.

Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Shakeel Butt <shakeelb@google.com>
Cc: David Rientjes <rientjes@google.com>
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
 include/linux/shrinker.h |  3 +--
 mm/huge_memory.c         |  3 ++-
 mm/vmscan.c              | 27 ++++++---------------------
 3 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/include/linux/shrinker.h b/include/linux/shrinker.h
index 9443caf..e14f68e 100644
--- a/include/linux/shrinker.h
+++ b/include/linux/shrinker.h
@@ -69,10 +69,8 @@ struct shrinker {
 
 	/* These are for internal use */
 	struct list_head list;
-#ifdef CONFIG_MEMCG_KMEM
 	/* ID in shrinker_idr */
 	int id;
-#endif
 	/* objs pending delete, per node */
 	atomic_long_t *nr_deferred;
 };
@@ -81,6 +79,7 @@ struct shrinker {
 /* Flags */
 #define SHRINKER_NUMA_AWARE	(1 << 0)
 #define SHRINKER_MEMCG_AWARE	(1 << 1)
+#define SHRINKER_NONSLAB	(1 << 2)
 
 extern int prealloc_shrinker(struct shrinker *shrinker);
 extern void register_shrinker_prepared(struct shrinker *shrinker);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 50f4720..e77a9fc 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -2913,7 +2913,8 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
 	.count_objects = deferred_split_count,
 	.scan_objects = deferred_split_scan,
 	.seeks = DEFAULT_SEEKS,
-	.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE,
+	.flags = SHRINKER_NUMA_AWARE | SHRINKER_MEMCG_AWARE |
+		 SHRINKER_NONSLAB,
 };
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7acd0af..62000ae 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -174,8 +174,6 @@ struct scan_control {
 static LIST_HEAD(shrinker_list);
 static DECLARE_RWSEM(shrinker_rwsem);
 
-#ifdef CONFIG_MEMCG_KMEM
-
 /*
  * We allow subsystems to populate their shrinker-related
  * LRU lists before register_shrinker_prepared() is called
@@ -227,16 +225,6 @@ static void unregister_memcg_shrinker(struct shrinker *shrinker)
 	idr_remove(&shrinker_idr, id);
 	up_write(&shrinker_rwsem);
 }
-#else /* CONFIG_MEMCG_KMEM */
-static int prealloc_memcg_shrinker(struct shrinker *shrinker)
-{
-	return 0;
-}
-
-static void unregister_memcg_shrinker(struct shrinker *shrinker)
-{
-}
-#endif /* CONFIG_MEMCG_KMEM */
 
 #ifdef CONFIG_MEMCG
 static bool global_reclaim(struct scan_control *sc)
@@ -579,7 +567,6 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
 	return freed;
 }
 
-#ifdef CONFIG_MEMCG_KMEM
 static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 			struct mem_cgroup *memcg, int priority)
 {
@@ -587,7 +574,7 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 	unsigned long ret, freed = 0;
 	int i;
 
-	if (!memcg_kmem_enabled() || !mem_cgroup_online(memcg))
+	if (!mem_cgroup_online(memcg))
 		return 0;
 
 	if (!down_read_trylock(&shrinker_rwsem))
@@ -613,6 +600,11 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 			continue;
 		}
 
+		/* Call non-slab shrinkers even though kmem is disabled */
+		if (!memcg_kmem_enabled() &&
+		    !(shrinker->flags & SHRINKER_NONSLAB))
+			continue;
+
 		ret = do_shrink_slab(&sc, shrinker, priority);
 		if (ret == SHRINK_EMPTY) {
 			clear_bit(i, map->map);
@@ -649,13 +641,6 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
 	up_read(&shrinker_rwsem);
 	return freed;
 }
-#else /* CONFIG_MEMCG_KMEM */
-static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
-			struct mem_cgroup *memcg, int priority)
-{
-	return 0;
-}
-#endif /* CONFIG_MEMCG_KMEM */
 
 /**
  * shrink_slab - shrink slab caches
-- 
1.8.3.1

Re: [PATCH 4/4] mm: shrinker: make shrinker not depend on memcg kmem

[Kirill A. Shutemov]

On Fri, Jun 07, 2019 at 02:07:39PM +0800, Yang Shi wrote:
> Currently shrinker is just allocated and can work when memcg kmem is
> enabled.  But, THP deferred split shrinker is not slab shrinker, it
> doesn't make too much sense to have such shrinker depend on memcg kmem.
> It should be able to reclaim THP even though memcg kmem is disabled.
> 
> Introduce a new shrinker flag, SHRINKER_NONSLAB, for non-slab shrinker,
> i.e. THP deferred split shrinker.  When memcg kmem is disabled, just
> such shrinkers can be called in shrinking memcg slab.

Looks like it breaks bisectability. It has to be done before makeing
shrinker memcg-aware, hasn't it?

-- 
 Kirill A. Shutemov

Re: [PATCH 4/4] mm: shrinker: make shrinker not depend on memcg kmem

[Yang Shi]

On 6/11/19 7:52 PM, Kirill A. Shutemov wrote:
> On Fri, Jun 07, 2019 at 02:07:39PM +0800, Yang Shi wrote:
>> Currently shrinker is just allocated and can work when memcg kmem is
>> enabled.  But, THP deferred split shrinker is not slab shrinker, it
>> doesn't make too much sense to have such shrinker depend on memcg kmem.
>> It should be able to reclaim THP even though memcg kmem is disabled.
>>
>> Introduce a new shrinker flag, SHRINKER_NONSLAB, for non-slab shrinker,
>> i.e. THP deferred split shrinker.  When memcg kmem is disabled, just
>> such shrinkers can be called in shrinking memcg slab.
> Looks like it breaks bisectability. It has to be done before makeing
> shrinker memcg-aware, hasn't it?

No, it doesn't break bisectability. But, THP shrinker just can be called 
with kmem charge enabled without this patch.

>

Re: [PATCH 4/4] mm: shrinker: make shrinker not depend on memcg kmem

[Kirill A. Shutemov]

On Tue, Jun 11, 2019 at 10:07:54PM -0700, Yang Shi wrote:
> 
> 
> On 6/11/19 7:52 PM, Kirill A. Shutemov wrote:
> > On Fri, Jun 07, 2019 at 02:07:39PM +0800, Yang Shi wrote:
> > > Currently shrinker is just allocated and can work when memcg kmem is
> > > enabled.  But, THP deferred split shrinker is not slab shrinker, it
> > > doesn't make too much sense to have such shrinker depend on memcg kmem.
> > > It should be able to reclaim THP even though memcg kmem is disabled.
> > > 
> > > Introduce a new shrinker flag, SHRINKER_NONSLAB, for non-slab shrinker,
> > > i.e. THP deferred split shrinker.  When memcg kmem is disabled, just
> > > such shrinkers can be called in shrinking memcg slab.
> > Looks like it breaks bisectability. It has to be done before makeing
> > shrinker memcg-aware, hasn't it?
> 
> No, it doesn't break bisectability. But, THP shrinker just can be called
> with kmem charge enabled without this patch.

So, if kmem is disabled, it will not be called, right? Then it is
regression in my opinion. This patch has to go in before 2/4.

-- 
 Kirill A. Shutemov

Re: [PATCH 4/4] mm: shrinker: make shrinker not depend on memcg kmem

[Yang Shi]

On 6/12/19 3:11 AM, Kirill A. Shutemov wrote:
> On Tue, Jun 11, 2019 at 10:07:54PM -0700, Yang Shi wrote:
>>
>> On 6/11/19 7:52 PM, Kirill A. Shutemov wrote:
>>> On Fri, Jun 07, 2019 at 02:07:39PM +0800, Yang Shi wrote:
>>>> Currently shrinker is just allocated and can work when memcg kmem is
>>>> enabled.  But, THP deferred split shrinker is not slab shrinker, it
>>>> doesn't make too much sense to have such shrinker depend on memcg kmem.
>>>> It should be able to reclaim THP even though memcg kmem is disabled.
>>>>
>>>> Introduce a new shrinker flag, SHRINKER_NONSLAB, for non-slab shrinker,
>>>> i.e. THP deferred split shrinker.  When memcg kmem is disabled, just
>>>> such shrinkers can be called in shrinking memcg slab.
>>> Looks like it breaks bisectability. It has to be done before makeing
>>> shrinker memcg-aware, hasn't it?
>> No, it doesn't break bisectability. But, THP shrinker just can be called
>> with kmem charge enabled without this patch.
> So, if kmem is disabled, it will not be called, right? Then it is
> regression in my opinion. This patch has to go in before 2/4.

I don't think this is a regression. "regression" should mean something 
used to work, but it is broken now. Actually, deferred split shrinker 
never works with memcg.

Anyway, either before 2/4 or after 2/4 looks ok.

>