linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo
@ 2019-06-21 17:30 Waiman Long
  2019-06-22  2:56 ` Shakeel Butt
  2019-06-26 19:58 ` Roman Gushchin
  0 siblings, 2 replies; 4+ messages in thread
From: Waiman Long @ 2019-06-21 17:30 UTC (permalink / raw)
  To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton
  Cc: linux-mm, linux-kernel, Michal Hocko, Roman Gushchin,
	Johannes Weiner, Shakeel Butt, Vladimir Davydov, Waiman Long

With Roman's kmem cache reparent patch, multiple kmem caches of the same
type can be seen attached to the same memcg id. All of them, except
maybe one, are reparent'ed kmem caches. It can be useful to tag those
reparented caches by adding a new slab flag "SLAB_DEACTIVATED" to those
kmem caches that will be reparent'ed if it cannot be destroyed completely.

For the reparent'ed memcg kmem caches, the tag ":deact" will now be
shown in <debugfs>/memcg_slabinfo.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/slab.h |  4 ++++
 mm/slab.c            |  1 +
 mm/slab_common.c     | 14 ++++++++------
 mm/slub.c            |  1 +
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index fecf40b7be69..19ab1380f875 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -116,6 +116,10 @@
 /* Objects are reclaimable */
 #define SLAB_RECLAIM_ACCOUNT	((slab_flags_t __force)0x00020000U)
 #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
+
+/* Slab deactivation flag */
+#define SLAB_DEACTIVATED	((slab_flags_t __force)0x10000000U)
+
 /*
  * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
  *
diff --git a/mm/slab.c b/mm/slab.c
index a2e93adf1df0..e8c7743fc283 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -2245,6 +2245,7 @@ int __kmem_cache_shrink(struct kmem_cache *cachep)
 #ifdef CONFIG_MEMCG
 void __kmemcg_cache_deactivate(struct kmem_cache *cachep)
 {
+	cachep->flags |= SLAB_DEACTIVATED;
 	__kmem_cache_shrink(cachep);
 }
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 146d8eaa639c..85cf0c374303 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1533,7 +1533,7 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
 	struct slabinfo sinfo;
 
 	mutex_lock(&slab_mutex);
-	seq_puts(m, "# <name> <css_id[:dead]> <active_objs> <num_objs>");
+	seq_puts(m, "# <name> <css_id[:dead|deact]> <active_objs> <num_objs>");
 	seq_puts(m, " <active_slabs> <num_slabs>\n");
 	list_for_each_entry(s, &slab_root_caches, root_caches_node) {
 		/*
@@ -1544,22 +1544,24 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
 
 		memset(&sinfo, 0, sizeof(sinfo));
 		get_slabinfo(s, &sinfo);
-		seq_printf(m, "%-17s root      %6lu %6lu %6lu %6lu\n",
+		seq_printf(m, "%-17s root       %6lu %6lu %6lu %6lu\n",
 			   cache_name(s), sinfo.active_objs, sinfo.num_objs,
 			   sinfo.active_slabs, sinfo.num_slabs);
 
 		for_each_memcg_cache(c, s) {
 			struct cgroup_subsys_state *css;
-			char *dead = "";
+			char *status = "";
 
 			css = &c->memcg_params.memcg->css;
 			if (!(css->flags & CSS_ONLINE))
-				dead = ":dead";
+				status = ":dead";
+			else if (c->flags & SLAB_DEACTIVATED)
+				status = ":deact";
 
 			memset(&sinfo, 0, sizeof(sinfo));
 			get_slabinfo(c, &sinfo);
-			seq_printf(m, "%-17s %4d%5s %6lu %6lu %6lu %6lu\n",
-				   cache_name(c), css->id, dead,
+			seq_printf(m, "%-17s %4d%-6s %6lu %6lu %6lu %6lu\n",
+				   cache_name(c), css->id, status,
 				   sinfo.active_objs, sinfo.num_objs,
 				   sinfo.active_slabs, sinfo.num_slabs);
 		}
diff --git a/mm/slub.c b/mm/slub.c
index a384228ff6d3..c965b4413658 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4057,6 +4057,7 @@ void __kmemcg_cache_deactivate(struct kmem_cache *s)
 	 */
 	slub_set_cpu_partial(s, 0);
 	s->min_partial = 0;
+	s->flags |= SLAB_DEACTIVATED;
 }
 #endif	/* CONFIG_MEMCG */
 
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo
  2019-06-21 17:30 [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo Waiman Long
@ 2019-06-22  2:56 ` Shakeel Butt
  2019-06-26 19:58 ` Roman Gushchin
  1 sibling, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2019-06-22  2:56 UTC (permalink / raw)
  To: Waiman Long
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, Linux MM, LKML, Michal Hocko, Roman Gushchin,
	Johannes Weiner, Vladimir Davydov

On Fri, Jun 21, 2019 at 10:30 AM Waiman Long <longman@redhat.com> wrote:
>
> With Roman's kmem cache reparent patch, multiple kmem caches of the same
> type can be seen attached to the same memcg id. All of them, except
> maybe one, are reparent'ed kmem caches. It can be useful to tag those
> reparented caches by adding a new slab flag "SLAB_DEACTIVATED" to those
> kmem caches that will be reparent'ed if it cannot be destroyed completely.
>
> For the reparent'ed memcg kmem caches, the tag ":deact" will now be
> shown in <debugfs>/memcg_slabinfo.
>
> Signed-off-by: Waiman Long <longman@redhat.com>

Reviewed-by: Shakeel Butt <shakeelb@google.com>

> ---
>  include/linux/slab.h |  4 ++++
>  mm/slab.c            |  1 +
>  mm/slab_common.c     | 14 ++++++++------
>  mm/slub.c            |  1 +
>  4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index fecf40b7be69..19ab1380f875 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -116,6 +116,10 @@
>  /* Objects are reclaimable */
>  #define SLAB_RECLAIM_ACCOUNT   ((slab_flags_t __force)0x00020000U)
>  #define SLAB_TEMPORARY         SLAB_RECLAIM_ACCOUNT    /* Objects are short-lived */
> +
> +/* Slab deactivation flag */
> +#define SLAB_DEACTIVATED       ((slab_flags_t __force)0x10000000U)
> +
>  /*
>   * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
>   *
> diff --git a/mm/slab.c b/mm/slab.c
> index a2e93adf1df0..e8c7743fc283 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2245,6 +2245,7 @@ int __kmem_cache_shrink(struct kmem_cache *cachep)
>  #ifdef CONFIG_MEMCG
>  void __kmemcg_cache_deactivate(struct kmem_cache *cachep)
>  {
> +       cachep->flags |= SLAB_DEACTIVATED;
>         __kmem_cache_shrink(cachep);
>  }
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 146d8eaa639c..85cf0c374303 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1533,7 +1533,7 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
>         struct slabinfo sinfo;
>
>         mutex_lock(&slab_mutex);
> -       seq_puts(m, "# <name> <css_id[:dead]> <active_objs> <num_objs>");
> +       seq_puts(m, "# <name> <css_id[:dead|deact]> <active_objs> <num_objs>");
>         seq_puts(m, " <active_slabs> <num_slabs>\n");
>         list_for_each_entry(s, &slab_root_caches, root_caches_node) {
>                 /*
> @@ -1544,22 +1544,24 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
>
>                 memset(&sinfo, 0, sizeof(sinfo));
>                 get_slabinfo(s, &sinfo);
> -               seq_printf(m, "%-17s root      %6lu %6lu %6lu %6lu\n",
> +               seq_printf(m, "%-17s root       %6lu %6lu %6lu %6lu\n",
>                            cache_name(s), sinfo.active_objs, sinfo.num_objs,
>                            sinfo.active_slabs, sinfo.num_slabs);
>
>                 for_each_memcg_cache(c, s) {
>                         struct cgroup_subsys_state *css;
> -                       char *dead = "";
> +                       char *status = "";
>
>                         css = &c->memcg_params.memcg->css;
>                         if (!(css->flags & CSS_ONLINE))
> -                               dead = ":dead";
> +                               status = ":dead";
> +                       else if (c->flags & SLAB_DEACTIVATED)
> +                               status = ":deact";
>
>                         memset(&sinfo, 0, sizeof(sinfo));
>                         get_slabinfo(c, &sinfo);
> -                       seq_printf(m, "%-17s %4d%5s %6lu %6lu %6lu %6lu\n",
> -                                  cache_name(c), css->id, dead,
> +                       seq_printf(m, "%-17s %4d%-6s %6lu %6lu %6lu %6lu\n",
> +                                  cache_name(c), css->id, status,
>                                    sinfo.active_objs, sinfo.num_objs,
>                                    sinfo.active_slabs, sinfo.num_slabs);
>                 }
> diff --git a/mm/slub.c b/mm/slub.c
> index a384228ff6d3..c965b4413658 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4057,6 +4057,7 @@ void __kmemcg_cache_deactivate(struct kmem_cache *s)
>          */
>         slub_set_cpu_partial(s, 0);
>         s->min_partial = 0;
> +       s->flags |= SLAB_DEACTIVATED;
>  }
>  #endif /* CONFIG_MEMCG */
>
> --
> 2.18.1
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo
  2019-06-21 17:30 [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo Waiman Long
  2019-06-22  2:56 ` Shakeel Butt
@ 2019-06-26 19:58 ` Roman Gushchin
  2019-06-27 18:45   ` Waiman Long
  1 sibling, 1 reply; 4+ messages in thread
From: Roman Gushchin @ 2019-06-26 19:58 UTC (permalink / raw)
  To: Waiman Long
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
	Johannes Weiner, Shakeel Butt, Vladimir Davydov

On Fri, Jun 21, 2019 at 01:30:05PM -0400, Waiman Long wrote:
> With Roman's kmem cache reparent patch, multiple kmem caches of the same
> type can be seen attached to the same memcg id. All of them, except
> maybe one, are reparent'ed kmem caches. It can be useful to tag those
> reparented caches by adding a new slab flag "SLAB_DEACTIVATED" to those
> kmem caches that will be reparent'ed if it cannot be destroyed completely.
> 
> For the reparent'ed memcg kmem caches, the tag ":deact" will now be
> shown in <debugfs>/memcg_slabinfo.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>

Hi Waiman!

Sorry for the late reply. The patch overall looks good to me,
except one nit. Please feel free to use my ack:
Acked-by: Roman Gushchin <guro@fb.com>

> ---
>  include/linux/slab.h |  4 ++++
>  mm/slab.c            |  1 +
>  mm/slab_common.c     | 14 ++++++++------
>  mm/slub.c            |  1 +
>  4 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index fecf40b7be69..19ab1380f875 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -116,6 +116,10 @@
>  /* Objects are reclaimable */
>  #define SLAB_RECLAIM_ACCOUNT	((slab_flags_t __force)0x00020000U)
>  #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
> +
> +/* Slab deactivation flag */
> +#define SLAB_DEACTIVATED	((slab_flags_t __force)0x10000000U)
> +
>  /*
>   * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
>   *
> diff --git a/mm/slab.c b/mm/slab.c
> index a2e93adf1df0..e8c7743fc283 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -2245,6 +2245,7 @@ int __kmem_cache_shrink(struct kmem_cache *cachep)
>  #ifdef CONFIG_MEMCG
>  void __kmemcg_cache_deactivate(struct kmem_cache *cachep)
>  {
> +	cachep->flags |= SLAB_DEACTIVATED;

A nit: it can be done from kmemcg_cache_deactivate() instead,
and then you don't have to do it in slab and slub separately.

Since it's not slab- or slub-specific code, it'd be better, IMO,
to put it into slab_common.c.

Thanks!

>  	__kmem_cache_shrink(cachep);
>  }
>  
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 146d8eaa639c..85cf0c374303 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1533,7 +1533,7 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
>  	struct slabinfo sinfo;
>  
>  	mutex_lock(&slab_mutex);
> -	seq_puts(m, "# <name> <css_id[:dead]> <active_objs> <num_objs>");
> +	seq_puts(m, "# <name> <css_id[:dead|deact]> <active_objs> <num_objs>");
>  	seq_puts(m, " <active_slabs> <num_slabs>\n");
>  	list_for_each_entry(s, &slab_root_caches, root_caches_node) {
>  		/*
> @@ -1544,22 +1544,24 @@ static int memcg_slabinfo_show(struct seq_file *m, void *unused)
>  
>  		memset(&sinfo, 0, sizeof(sinfo));
>  		get_slabinfo(s, &sinfo);
> -		seq_printf(m, "%-17s root      %6lu %6lu %6lu %6lu\n",
> +		seq_printf(m, "%-17s root       %6lu %6lu %6lu %6lu\n",
>  			   cache_name(s), sinfo.active_objs, sinfo.num_objs,
>  			   sinfo.active_slabs, sinfo.num_slabs);
>  
>  		for_each_memcg_cache(c, s) {
>  			struct cgroup_subsys_state *css;
> -			char *dead = "";
> +			char *status = "";
>  
>  			css = &c->memcg_params.memcg->css;
>  			if (!(css->flags & CSS_ONLINE))
> -				dead = ":dead";
> +				status = ":dead";
> +			else if (c->flags & SLAB_DEACTIVATED)
> +				status = ":deact";
>  
>  			memset(&sinfo, 0, sizeof(sinfo));
>  			get_slabinfo(c, &sinfo);
> -			seq_printf(m, "%-17s %4d%5s %6lu %6lu %6lu %6lu\n",
> -				   cache_name(c), css->id, dead,
> +			seq_printf(m, "%-17s %4d%-6s %6lu %6lu %6lu %6lu\n",
> +				   cache_name(c), css->id, status,
>  				   sinfo.active_objs, sinfo.num_objs,
>  				   sinfo.active_slabs, sinfo.num_slabs);
>  		}
> diff --git a/mm/slub.c b/mm/slub.c
> index a384228ff6d3..c965b4413658 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4057,6 +4057,7 @@ void __kmemcg_cache_deactivate(struct kmem_cache *s)
>  	 */
>  	slub_set_cpu_partial(s, 0);
>  	s->min_partial = 0;
> +	s->flags |= SLAB_DEACTIVATED;
>  }
>  #endif	/* CONFIG_MEMCG */
>  
> -- 
> 2.18.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo
  2019-06-26 19:58 ` Roman Gushchin
@ 2019-06-27 18:45   ` Waiman Long
  0 siblings, 0 replies; 4+ messages in thread
From: Waiman Long @ 2019-06-27 18:45 UTC (permalink / raw)
  To: Roman Gushchin
  Cc: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
	Andrew Morton, linux-mm, linux-kernel, Michal Hocko,
	Johannes Weiner, Shakeel Butt, Vladimir Davydov

On 6/26/19 3:58 PM, Roman Gushchin wrote:
> On Fri, Jun 21, 2019 at 01:30:05PM -0400, Waiman Long wrote:
>> With Roman's kmem cache reparent patch, multiple kmem caches of the same
>> type can be seen attached to the same memcg id. All of them, except
>> maybe one, are reparent'ed kmem caches. It can be useful to tag those
>> reparented caches by adding a new slab flag "SLAB_DEACTIVATED" to those
>> kmem caches that will be reparent'ed if it cannot be destroyed completely.
>>
>> For the reparent'ed memcg kmem caches, the tag ":deact" will now be
>> shown in <debugfs>/memcg_slabinfo.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> Hi Waiman!
>
> Sorry for the late reply. The patch overall looks good to me,
> except one nit. Please feel free to use my ack:
> Acked-by: Roman Gushchin <guro@fb.com>
>
>> ---
>>  include/linux/slab.h |  4 ++++
>>  mm/slab.c            |  1 +
>>  mm/slab_common.c     | 14 ++++++++------
>>  mm/slub.c            |  1 +
>>  4 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>> index fecf40b7be69..19ab1380f875 100644
>> --- a/include/linux/slab.h
>> +++ b/include/linux/slab.h
>> @@ -116,6 +116,10 @@
>>  /* Objects are reclaimable */
>>  #define SLAB_RECLAIM_ACCOUNT	((slab_flags_t __force)0x00020000U)
>>  #define SLAB_TEMPORARY		SLAB_RECLAIM_ACCOUNT	/* Objects are short-lived */
>> +
>> +/* Slab deactivation flag */
>> +#define SLAB_DEACTIVATED	((slab_flags_t __force)0x10000000U)
>> +
>>  /*
>>   * ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
>>   *
>> diff --git a/mm/slab.c b/mm/slab.c
>> index a2e93adf1df0..e8c7743fc283 100644
>> --- a/mm/slab.c
>> +++ b/mm/slab.c
>> @@ -2245,6 +2245,7 @@ int __kmem_cache_shrink(struct kmem_cache *cachep)
>>  #ifdef CONFIG_MEMCG
>>  void __kmemcg_cache_deactivate(struct kmem_cache *cachep)
>>  {
>> +	cachep->flags |= SLAB_DEACTIVATED;
> A nit: it can be done from kmemcg_cache_deactivate() instead,
> and then you don't have to do it in slab and slub separately.
>
> Since it's not slab- or slub-specific code, it'd be better, IMO,
> to put it into slab_common.c.

Thanks for the suggestion.

You are right. It will be cleaner to set the flag in
kmemcg_cache_deactivate(). I have just sent out a v2 patch to do that.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-06-27 18:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 17:30 [PATCH-next] mm, memcg: Add ":deact" tag for reparented kmem caches in memcg_slabinfo Waiman Long
2019-06-22  2:56 ` Shakeel Butt
2019-06-26 19:58 ` Roman Gushchin
2019-06-27 18:45   ` Waiman Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).