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

2019-08-20 Thread Yang Shi




On 8/20/19 4:06 AM, Kirill Tkhai wrote:

On 07.08.2019 05:17, 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.

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 
Cc: Johannes Weiner 
Cc: Michal Hocko 
Cc: "Kirill A . Shutemov" 
Cc: Hugh Dickins 
Cc: Shakeel Butt 
Cc: David Rientjes 
Cc: Qian Cai 
Acked-by: Kirill A. Shutemov 
Signed-off-by: Yang Shi 

Reviewed-by: Kirill Tkhai 

But, please, see below one small suggestion.


---
  include/linux/huge_mm.h|  9 ++
  include/linux/memcontrol.h |  4 +++
  include/linux/mm_types.h   |  1 +
  mm/huge_memory.c   | 76 +++---
  mm/memcontrol.c| 24 +++
  5 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 45ede62..61c9ffd 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -267,6 +267,15 @@ 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;
+}
+
  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
  #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
  #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 5771816..cace365 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -312,6 +312,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 3a37a89..156640c 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 e0d8e08..c9a596e 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -495,11 +495,25 @@ 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)

+#ifdef CONFIG_MEMCG
+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;
+}
+#else
+static inline struct deferred_split *get_deferred_split_queue(struct page 
*page)
+{
+   struct pglist_data *pgdat = NODE_DATA(page_to_nid(page));
+
+   return &pgdat->deferred_split_queue;
  }
+#endif
  
  void prep_transhuge_page(struct page *page)

  {
@@ -2658,7 +2672,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;
@@ -2794,8 +2808,7 @@ int split_huge_pag

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

2019-08-20 Thread Kirill Tkhai
On 07.08.2019 05:17, 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.
> 
> 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 
> Cc: Johannes Weiner 
> Cc: Michal Hocko 
> Cc: "Kirill A . Shutemov" 
> Cc: Hugh Dickins 
> Cc: Shakeel Butt 
> Cc: David Rientjes 
> Cc: Qian Cai 
> Acked-by: Kirill A. Shutemov 
> Signed-off-by: Yang Shi 

Reviewed-by: Kirill Tkhai 

But, please, see below one small suggestion.

> ---
>  include/linux/huge_mm.h|  9 ++
>  include/linux/memcontrol.h |  4 +++
>  include/linux/mm_types.h   |  1 +
>  mm/huge_memory.c   | 76 
> +++---
>  mm/memcontrol.c| 24 +++
>  5 files changed, 103 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 45ede62..61c9ffd 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -267,6 +267,15 @@ 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;
> +}
> +
>  #else /* CONFIG_TRANSPARENT_HUGEPAGE */
>  #define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
>  #define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 5771816..cace365 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -312,6 +312,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 3a37a89..156640c 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 e0d8e08..c9a596e 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -495,11 +495,25 @@ 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)
> +#ifdef CONFIG_MEMCG
> +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;
> +}
> +#else
> +static inline struct deferred_split *get_deferred_split_queue(struct page 
> *page)
> +{
> + struct pglist_data *pgdat = NODE_DATA(page_to_nid(page));
> +
> + return &pgdat->deferred_split_queue;
>  }
> +#endif
>  
>  void prep_transhuge_page(struct page *page)
>  {
> @@ -2658,7 +2672,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;
>