Re: [PATCH v6 12/19] mm: memcg/slab: use a single set of kmem_caches for all accounted allocations

2020-06-22 Thread Shakeel Butt
On Mon, Jun 8, 2020 at 4:07 PM Roman Gushchin  wrote:
>
> This is fairly big but mostly red patch, which makes all accounted
> slab allocations use a single set of kmem_caches instead of
> creating a separate set for each memory cgroup.
>
> Because the number of non-root kmem_caches is now capped by the number
> of root kmem_caches, there is no need to shrink or destroy them
> prematurely. They can be perfectly destroyed together with their
> root counterparts. This allows to dramatically simplify the
> management of non-root kmem_caches and delete a ton of code.
>
> This patch performs the following changes:
> 1) introduces memcg_params.memcg_cache pointer to represent the
>kmem_cache which will be used for all non-root allocations
> 2) reuses the existing memcg kmem_cache creation mechanism
>to create memcg kmem_cache on the first allocation attempt
> 3) memcg kmem_caches are named -memcg,
>e.g. dentry-memcg
> 4) simplifies memcg_kmem_get_cache() to just return memcg kmem_cache
>or schedule it's creation and return the root cache
> 5) removes almost all non-root kmem_cache management code
>(separate refcounter, reparenting, shrinking, etc)
> 6) makes slab debugfs to display root_mem_cgroup css id and never
>show :dead and :deact flags in the memcg_slabinfo attribute.
>
> Following patches in the series will simplify the kmem_cache creation.
>
> Signed-off-by: Roman Gushchin 
> Reviewed-by: Vlastimil Babka 

This is a very satisfying patch.

Reviewed-by: Shakeel Butt 


[PATCH v6 12/19] mm: memcg/slab: use a single set of kmem_caches for all accounted allocations

2020-06-08 Thread Roman Gushchin
This is fairly big but mostly red patch, which makes all accounted
slab allocations use a single set of kmem_caches instead of
creating a separate set for each memory cgroup.

Because the number of non-root kmem_caches is now capped by the number
of root kmem_caches, there is no need to shrink or destroy them
prematurely. They can be perfectly destroyed together with their
root counterparts. This allows to dramatically simplify the
management of non-root kmem_caches and delete a ton of code.

This patch performs the following changes:
1) introduces memcg_params.memcg_cache pointer to represent the
   kmem_cache which will be used for all non-root allocations
2) reuses the existing memcg kmem_cache creation mechanism
   to create memcg kmem_cache on the first allocation attempt
3) memcg kmem_caches are named -memcg,
   e.g. dentry-memcg
4) simplifies memcg_kmem_get_cache() to just return memcg kmem_cache
   or schedule it's creation and return the root cache
5) removes almost all non-root kmem_cache management code
   (separate refcounter, reparenting, shrinking, etc)
6) makes slab debugfs to display root_mem_cgroup css id and never
   show :dead and :deact flags in the memcg_slabinfo attribute.

Following patches in the series will simplify the kmem_cache creation.

Signed-off-by: Roman Gushchin 
Reviewed-by: Vlastimil Babka 
---
 include/linux/memcontrol.h |   5 +-
 include/linux/slab.h   |   5 +-
 mm/memcontrol.c| 163 +++---
 mm/slab.c  |  16 +-
 mm/slab.h  | 145 -
 mm/slab_common.c   | 426 -
 mm/slub.c  |  38 +---
 7 files changed, 128 insertions(+), 670 deletions(-)

diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index ba7065c0922a..e2c4d54aa1f6 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -317,7 +317,6 @@ struct mem_cgroup {
 /* Index in the kmem_cache->memcg_params.memcg_caches array */
int kmemcg_id;
enum memcg_kmem_state kmem_state;
-   struct list_head kmem_caches;
struct obj_cgroup __rcu *objcg;
struct list_head objcg_list;
 #endif
@@ -1404,9 +1403,7 @@ static inline void memcg_set_shrinker_bit(struct 
mem_cgroup *memcg,
 }
 #endif
 
-struct kmem_cache *memcg_kmem_get_cache(struct kmem_cache *cachep,
-   struct obj_cgroup **objcgp);
-void memcg_kmem_put_cache(struct kmem_cache *cachep);
+struct kmem_cache *memcg_kmem_get_cache(struct kmem_cache *cachep);
 
 #ifdef CONFIG_MEMCG_KMEM
 int __memcg_kmem_charge(struct mem_cgroup *memcg, gfp_t gfp,
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 6d454886bcaf..310768bfa8d2 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -155,8 +155,7 @@ struct kmem_cache *kmem_cache_create_usercopy(const char 
*name,
 void kmem_cache_destroy(struct kmem_cache *);
 int kmem_cache_shrink(struct kmem_cache *);
 
-void memcg_create_kmem_cache(struct mem_cgroup *, struct kmem_cache *);
-void memcg_deactivate_kmem_caches(struct mem_cgroup *, struct mem_cgroup *);
+void memcg_create_kmem_cache(struct kmem_cache *cachep);
 
 /*
  * Please use this macro to create slab caches. Simply specify the
@@ -578,8 +577,6 @@ static __always_inline void *kmalloc_node(size_t size, 
gfp_t flags, int node)
return __kmalloc_node(size, flags, node);
 }
 
-int memcg_update_all_caches(int num_memcgs);
-
 /**
  * kmalloc_array - allocate memory for an array.
  * @n: number of elements.
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 51e85d05095c..995204f65217 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -350,7 +350,7 @@ static void memcg_reparent_objcgs(struct mem_cgroup *memcg,
 }
 
 /*
- * This will be the memcg's index in each cache's ->memcg_params.memcg_caches.
+ * This will be used as a shrinker list's index.
  * The main reason for not using cgroup id for this:
  *  this works better in sparse environments, where we have a lot of memcgs,
  *  but only a few kmem-limited. Or also, if we have, for instance, 200
@@ -569,20 +569,16 @@ ino_t page_cgroup_ino(struct page *page)
unsigned long ino = 0;
 
rcu_read_lock();
-   if (PageSlab(page) && !PageTail(page)) {
-   memcg = memcg_from_slab_page(page);
-   } else {
-   memcg = page->mem_cgroup;
+   memcg = page->mem_cgroup;
 
-   /*
-* The lowest bit set means that memcg isn't a valid
-* memcg pointer, but a obj_cgroups pointer.
-* In this case the page is shared and doesn't belong
-* to any specific memory cgroup.
-*/
-   if ((unsigned long) memcg & 0x1UL)
-   memcg = NULL;
-   }
+   /*
+* The lowest bit set means that memcg isn't a valid
+* memcg pointer, but a obj_cgroups pointer.
+* In this case the page is shared and doesn't