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 <[email protected]>
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 <[email protected]> > --- > 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 >
