OpenVZ Forum - RDF feed
type: channel
items: [PATCH 00/23] slab+slub accounting for memcg
items: [PATCH 01/23] slub: don't create a copy of the name string in kmem_cache_create
items: [PATCH 02/23] slub: always get the cache from its page in kfree
items: [PATCH 03/23] slab: rename gfpflags to allocflags
items: [PATCH 04/23] memcg: Make it possible to use the stock for more than one page.
items: [PATCH 05/23] memcg: Reclaim when more than one page needed.
items: [PATCH 06/23] slab: use obj_size field of struct kmem_cache when not debugging
items: [PATCH 07/23] change defines to an enum
items: [PATCH 08/23] don't force return value checking in res_counter_charge_nofail
items: [PATCH 09/23] kmem slab accounting basic infrastructure
items: [PATCH 10/23] slab/slub: struct memcg_params
items: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
items: [PATCH 12/23] slab: pass memcg parameter to kmem_cache_create
items: [PATCH 13/23] slub: create duplicate cache
items: [PATCH 14/23] slub: provide kmalloc_no_account
items: [PATCH 15/23] slab: create duplicate cache
items: [PATCH 16/23] slab: provide kmalloc_no_account
items: [PATCH 17/23] kmem controller charge/uncharge infrastructure
items: [PATCH 18/23] slub: charge allocation to a memcg
items: [PATCH 19/23] slab: per-memcg accounting of slab caches
items: [PATCH 20/23] memcg: disable kmem code when not in use.
items: [PATCH 21/23] memcg: Track all the memcg children of a kmem_cache.
items: [PATCH 22/23] memcg: Per-memcg memory.kmem.slabinfo file.
items: [PATCH 23/23] slub: create slabinfo file for memcg
items: Re: [PATCH 00/23] slab+slub accounting for memcg
items: Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
items: Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
items: Re: [PATCH 13/23] slub: create duplicate cache
items: Re: [PATCH 04/23] memcg: Make it possible to use the stock for more than one page.
items: Re: [PATCH 05/23] memcg: Reclaim when more than one page needed.
items: Re: [PATCH 07/23] change defines to an enum
items: Re: [PATCH 08/23] don't force return value checking in res_counter_charge_nofail
items: Re: [PATCH 09/23] kmem slab accounting basic infrastructure
items: Re: [PATCH 16/23] slab: provide kmalloc_no_account
items: [PATCH 0/3] A few fixes for '[PATCH 00/23] slab+slub accounting for memcg' series
items: [PATCH 1/3] slab: Proper off-slabs handling when duplicating caches
items: [PATCH 2/3] slab: Fix imbalanced rcu locking
items: [PATCH 3/3] slab: Get rid of mem_cgroup_put_kmem_cache()
items: Re: [PATCH 10/23] slab/slub: struct memcg_params
items: Re: [PATCH 12/23] slab: pass memcg parameter to kmem_cache_create
items: Re: [PATCH 19/23] slab: per-memcg accounting of slab caches

[PATCH 00/23] slab+slub accounting for memcg
type: item
description: Hi, This is my current attempt at getting the kmem controller into a mergeable state. IMHO, all the important bits are there, and it should't change *that* much from now on. I am, however, expecting at least a couple more interactions before we sort all the edges out. This series works for both the slub and the slab. One of my main goals was to make sure that the interfaces we are creating actually makes sense for both allocators. I did some adaptations to the slab-specific patches, but the bulk of it comes from Suleiman's patches. I did the best to use his patches as-is where possible so to keep authorship information. When not possible, I tried to be fair and quote it in the commit message. In this series, all existing caches are created per-memcg after its first hit. The main reason is, during discussions in the memory summit we came into agreement that the fragmentation problems that could arise from creating all of them are mitigated by the typically small quantity of caches in the system (order of a few megabytes total for sparsely used caches). The lazy creation from Suleiman is kept, although a bit modified. For instance, I now use a locked scheme instead of cmpxcgh to make sure cache creation won't fail due to duplicates, which simplifies things by quite a bit. The slub is a bit more complex than what I came up with in my slub-only series. The reason is we did not need to use the cache-selection logic in the allocator itself - it was done by the cache users. But since now we are lazy creating all caches, this is simply no longer doable. I am leaving destruction of caches out of the series, although most of the infrastructure for that is here, since we did it in earlier series. This is basically because right now Kame is reworking it for user memcg, and I like the new proposed behavior a lot more. We all seemed to have agreed that reclaim is an interesting problem by itself, and is not included in this already too complicated series. Please note that this is…
creator: Glauber Costa
date: 2012-04-20T21:57:08-00:00

[PATCH 01/23] slub: don't create a copy of the name string in kmem_cache_create
type: item
description: When creating a cache, slub keeps a copy of the cache name through strdup. The slab however, doesn't do that. This means that everyone registering caches have to keep a copy themselves anyway, since code needs to work on all allocators. Having slab create a copy of it as well may very well be the right thing to do: but at this point, the callers are already there My motivation for it comes from the kmem slab cache controller for memcg. Because we create duplicate caches, having a more consistent behavior here really helps. I am sending the patch, however, more to probe on your opinion about it. If you guys agree, but don't want to merge it - since it is not fixing anything, nor improving any situation etc, I am more than happy to carry it in my series until it gets merged (fingers crossed). Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> --- mm/slub.c | 14 ++------------ 1 files changed, 2 insertions(+), 12 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index ffe13fd..af8cee9 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3925,7 +3925,6 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, size_t align, unsigned long flags, void (*ctor)(void *)) { struct kmem_cache *s; - char *n; if (WARN_ON(!name)) return NULL; @@ -3949,26 +3948,20 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, return s; } - n = kstrdup(name, GFP_KERNEL); - if (!n) - goto err; - s = kmalloc(kmem_size, GFP_KERNEL); if (s) { - if (kmem_cache_open(s, n, + if (kmem_cache_open(s, name, size, align, flags, ctor)) { list_add(&s->list, &slab_caches); up_write(&slub_lock); if (sysfs_slab_add(s)) { down_write(&slub_lock); list_del(&s->list); - kfree(n); kfree(s); goto err; } return s; } - kfree(n); kfree(s); } err: @@ -5212,7 +5205,6 @@ static void kmem_cache_release(struct kobject *kobj) { struct kmem_cache *s = to_slab(kobj); - kfree(s->name); kfree(s); } @@ -5318,11 +5310,9 @@ static i…
creator: Glauber Costa
date: 2012-04-20T21:57:09-00:00

[PATCH 02/23] slub: always get the cache from its page in kfree
type: item
description: struct page already have this information. If we start chaining caches, this information will always be more trustworthy than whatever is passed into the function Signed-off-by: Glauber Costa <glommer@parallels.com> --- mm/slub.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index af8cee9..2652e7c 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2600,7 +2600,7 @@ void kmem_cache_free(struct kmem_cache *s, void *x) page = virt_to_head_page(x); - slab_free(s, page, x, _RET_IP_); + slab_free(page->slab, page, x, _RET_IP_); trace_kmem_cache_free(_RET_IP_, x); } -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-20T21:57:10-00:00

[PATCH 03/23] slab: rename gfpflags to allocflags
type: item
description: A consistent name with slub saves us an acessor function. In both caches, this field represents the same thing. We would like to use it from the mem_cgroup code. Signed-off-by: Glauber Costa <glommer@parallels.com> --- include/linux/slab_def.h | 2 +- mm/slab.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index fbd1117..d41effe 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -39,7 +39,7 @@ struct kmem_cache { unsigned int gfporder; /* force GFP flags, e.g. GFP_DMA */ - gfp_t gfpflags; + gfp_t allocflags; size_t colour; /* cache colouring range */ unsigned int colour_off; /* colour offset */ diff --git a/mm/slab.c b/mm/slab.c index e901a36..c6e5ab8 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -1798,7 +1798,7 @@ static void *kmem_getpages(struct kmem_cache *cachep, gfp_t flags, int nodeid) flags |= __GFP_COMP; #endif - flags |= cachep->gfpflags; + flags |= cachep->allocflags; if (cachep->flags & SLAB_RECLAIM_ACCOUNT) flags |= __GFP_RECLAIMABLE; @@ -2508,9 +2508,9 @@ kmem_cache_create (const char *name, size_t size, size_t align, cachep->colour = left_over / cachep->colour_off; cachep->slab_size = slab_size; cachep->flags = flags; - cachep->gfpflags = 0; + cachep->allocflags = 0; if (CONFIG_ZONE_DMA_FLAG && (flags & SLAB_CACHE_DMA)) - cachep->gfpflags |= GFP_DMA; + cachep->allocflags |= GFP_DMA; cachep->buffer_size = size; cachep->reciprocal_buffer_size = reciprocal_value(size); @@ -2857,9 +2857,9 @@ static void kmem_flagcheck(struct kmem_cache *cachep, gfp_t flags) { if (CONFIG_ZONE_DMA_FLAG) { if (flags & GFP_DMA) - BUG_ON(!(cachep->gfpflags & GFP_DMA)); + BUG_ON(!(cachep->allocflags & GFP_DMA)); else - BUG_ON(cachep->gfpflags & GFP_DMA); + BUG_ON(cachep->allocflags & GFP_DMA); } } -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-20T21:57:11-00:00

[PATCH 04/23] memcg: Make it possible to use the stock for more than one page.
type: item
description: From: Suleiman Souhlal <ssouhlal@FreeBSD.org> Signed-off-by: Suleiman Souhlal <suleiman@google.com> --- mm/memcontrol.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 932a734..4b94b2d 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1998,19 +1998,19 @@ static DEFINE_PER_CPU(struct memcg_stock_pcp, memcg_stock); static DEFINE_MUTEX(percpu_charge_mutex); /* - * Try to consume stocked charge on this cpu. If success, one page is consumed - * from local stock and true is returned. If the stock is 0 or charges from a - * cgroup which is not current target, returns false. This stock will be - * refilled. + * Try to consume stocked charge on this cpu. If success, nr_pages pages are + * consumed from local stock and true is returned. If the stock is 0 or + * charges from a cgroup which is not current target, returns false. + * This stock will be refilled. */ -static bool consume_stock(struct mem_cgroup *memcg) +static bool consume_stock(struct mem_cgroup *memcg, int nr_pages) { struct memcg_stock_pcp *stock; bool ret = true; stock = &get_cpu_var(memcg_stock); - if (memcg == stock->cached && stock->nr_pages) - stock->nr_pages--; + if (memcg == stock->cached && stock->nr_pages >= nr_pages) + stock->nr_pages -= nr_pages; else /* need to call res_counter_charge */ ret = false; put_cpu_var(memcg_stock); @@ -2309,7 +2309,7 @@ again: VM_BUG_ON(css_is_removed(&memcg->css)); if (mem_cgroup_is_root(memcg)) goto done; - if (nr_pages == 1 && consume_stock(memcg)) + if (consume_stock(memcg, nr_pages)) goto done; css_get(&memcg->css); } else { @@ -2334,7 +2334,7 @@ again: rcu_read_unlock(); goto done; } - if (nr_pages == 1 && consume_stock(memcg)) { + if (consume_stock(memcg, nr_pages)) { /* * It seems dagerous to access memcg without css_get(). * But considering how consume_stok works, it's not -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-20T21:57:12-00:00

[PATCH 05/23] memcg: Reclaim when more than one page needed.
type: item
description: From: Suleiman Souhlal <ssouhlal@FreeBSD.org> mem_cgroup_do_charge() was written before slab accounting, and expects three cases: being called for 1 page, being called for a stock of 32 pages, or being called for a hugepage. If we call for 2 pages (and several slabs used in process creation are such, at least with the debug options I had), it assumed it's being called for stock and just retried without reclaiming. Fix that by passing down a minsize argument in addition to the csize. And what to do about that (csize == PAGE_SIZE && ret) retry? If it's needed at all (and presumably is since it's there, perhaps to handle races), then it should be extended to more than PAGE_SIZE, yet how far? And should there be a retry count limit, of what? For now retry up to COSTLY_ORDER (as page_alloc.c does), stay safe with a cond_resched(), and make sure not to do it if __GFP_NORETRY. Signed-off-by: Suleiman Souhlal <suleiman@google.com> --- mm/memcontrol.c | 18 +++++++++++------- 1 files changed, 11 insertions(+), 7 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 4b94b2d..cbffc4c 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2187,7 +2187,8 @@ enum { }; static int mem_cgroup_do_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, - unsigned int nr_pages, bool oom_check) + unsigned int nr_pages, unsigned int min_pages, + bool oom_check) { unsigned long csize = nr_pages * PAGE_SIZE; struct mem_cgroup *mem_over_limit; @@ -2210,18 +2211,18 @@ static int mem_cgroup_do_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, } else mem_over_limit = mem_cgroup_from_res_counter(fail_res, res); /* - * nr_pages can be either a huge page (HPAGE_PMD_NR), a batch - * of regular pages (CHARGE_BATCH), or a single regular page (1). - * * Never reclaim on behalf of optional batching, retry with a * single page instead. */ - if (nr_pages == CHARGE_BATCH) + if (nr_pages > min_pages) return CHARGE_RETRY; if (!(gfp_mask & __GFP_WAIT)) return CHARGE_WOULDBLOCK; + if (gfp_mask & __GFP_NOR…
creator: Glauber Costa
date: 2012-04-20T21:57:13-00:00

[PATCH 06/23] slab: use obj_size field of struct kmem_cache when not debugging
type: item
description: The kmem controller needs to keep track of the object size of a cache so it can later on create a per-memcg duplicate. Logic to keep track of that already exists, but it is only enable while debugging. This patch makes it also available when the kmem controller code is compiled in. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> --- include/linux/slab_def.h | 4 +++- mm/slab.c | 37 ++++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index d41effe..cba3139 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -78,8 +78,10 @@ struct kmem_cache { * variables contain the offset to the user object and its size. */ int obj_offset; - int obj_size; #endif /* CONFIG_DEBUG_SLAB */ +#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM) + int obj_size; +#endif /* 6) per-cpu/per-node data, touched during every alloc/free */ /* diff --git a/mm/slab.c b/mm/slab.c index c6e5ab8..a0d51dd 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -413,8 +413,28 @@ static void kmem_list3_init(struct kmem_list3 *parent) #define STATS_INC_FREEMISS(x) do { } while (0) #endif -#if DEBUG +#if defined(CONFIG_DEBUG_SLAB) || defined(CONFIG_CGROUP_MEM_RES_CTLR_KMEM) +static int obj_size(struct kmem_cache *cachep) +{ + return cachep->obj_size; +} +static void set_obj_size(struct kmem_cache *cachep, int size) +{ + cachep->obj_size = size; +} + +#else +static int obj_size(struct kmem_cache *cachep) +{ + return cachep->buffer_size; +} + +static void set_obj_size(struct kmem_cache *cachep, int size) +{ +} +#endif +#if DEBUG /* * memory layout of objects: * 0 : objp @@ -433,11 +453,6 @@ static int obj_offset(struct kmem_cache *cachep) return cachep->obj_offset; } -static int obj_size(struct kmem_cache *cachep) -{ - return cachep->obj_size; -} - static unsigned long long *dbg_redzone1(struct kmem_…
creator: Glauber Costa
date: 2012-04-20T21:57:14-00:00

[PATCH 07/23] change defines to an enum
type: item
description: This is just a cleanup patch for clarity of expression. In earlier submissions, people asked it to be in a separate patch, so here it is. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index cbffc4c..2810228 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -374,9 +374,12 @@ enum charge_type { }; /* for encoding cft->private value on file */ -#define _MEM (0) -#define _MEMSWAP (1) -#define _OOM_TYPE (2) +enum res_type { + _MEM, + _MEMSWAP, + _OOM_TYPE, +}; + #define MEMFILE_PRIVATE(x, val) (((x) << 16) | (val)) #define MEMFILE_TYPE(val) (((val) >> 16) & 0xffff) #define MEMFILE_ATTR(val) ((val) & 0xffff) -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-20T21:57:15-00:00

[PATCH 08/23] don't force return value checking in res_counter_charge_nofail
type: item
description: Since we will succeed with the allocation no matter what, there isn't the need to use __must_check with it. It can very well be optional. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Michal Hocko <mhocko@suse.cz> --- include/linux/res_counter.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/include/linux/res_counter.h b/include/linux/res_counter.h index da81af0..f7621cf 100644 --- a/include/linux/res_counter.h +++ b/include/linux/res_counter.h @@ -119,7 +119,7 @@ int __must_check res_counter_charge_locked(struct res_counter *counter, unsigned long val); int __must_check res_counter_charge(struct res_counter *counter, unsigned long val, struct res_counter **limit_fail_at); -int __must_check res_counter_charge_nofail(struct res_counter *counter, +int res_counter_charge_nofail(struct res_counter *counter, unsigned long val, struct res_counter **limit_fail_at); /* -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-20T21:57:16-00:00

[PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: This patch adds the basic infrastructure for the accounting of the slab caches. To control that, the following files are created: * memory.kmem.usage_in_bytes * memory.kmem.limit_in_bytes * memory.kmem.failcnt * memory.kmem.max_usage_in_bytes They have the same meaning of their user memory counterparts. They reflect the state of the "kmem" res_counter. The code is not enabled until a limit is set. This can be tested by the flag "kmem_accounted". This means that after the patch is applied, no behavioral changes exists for whoever is still using memcg to control their memory usage. We always account to both user and kernel resource_counters. This effectively means that an independent kernel limit is in place when the limit is set to a lower value than the user memory. A equal or higher value means that the user limit will always hit first, meaning that kmem is effectively unlimited. People who want to track kernel memory but not limit it, can set this limit to a very high number (like RESOURCE_MAX - 1page - that no one will ever hit, or equal to the user memory) Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 79 insertions(+), 1 deletions(-) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 2810228..36f1e6b 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -252,6 +252,10 @@ struct mem_cgroup { }; /* + * the counter to account for kernel memory usage. + */ + struct res_counter kmem; + /* * Per cgroup active and inactive list, similar to the * per zone LRU lists. */ @@ -266,6 +270,7 @@ struct mem_cgroup { * Should the accounting and control be hierarchical, per subtree? */ bool use_hierarchy; + bool kmem_accounted; bool oom_lock; atomic_t under_oom; @@ -378,6 +383,7 @@ enum res_type { _MEM, _MEMSWAP, _OOM_TYPE, + _KMEM, }; #define MEM…
creator: Glauber Costa
date: 2012-04-20T21:57:17-00:00

[PATCH 10/23] slab/slub: struct memcg_params
type: item
description: For the kmem slab controller, we need to record some extra information in the kmem_cache structure. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/slab.h | 15 +++++++++++++++ include/linux/slab_def.h | 4 ++++ include/linux/slub_def.h | 3 +++ 3 files changed, 22 insertions(+), 0 deletions(-) diff --git a/include/linux/slab.h b/include/linux/slab.h index a595dce..a5127e1 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -153,6 +153,21 @@ unsigned int kmem_cache_size(struct kmem_cache *); #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) #endif +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM +struct mem_cgroup_cache_params { + struct mem_cgroup *memcg; + int id; + +#ifdef CONFIG_SLAB + /* Original cache parameters, used when creating a memcg cache */ + size_t orig_align; + atomic_t refcnt; + +#endif + struct list_head destroyed_list; /* Used when deleting cpuset cache */ +}; +#endif + /* * Common kmalloc functions provided by all allocators */ diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index cba3139..06e4a3e 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -83,6 +83,10 @@ struct kmem_cache { int obj_size; #endif +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM + struct mem_cgroup_cache_params memcg_params; +#endif + /* 6) per-cpu/per-node data, touched during every alloc/free */ /* * We put array[] at the end of kmem_cache, because we want to size diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h index c2f8c8b..5f5e942 100644 --- a/include/linux/slub_def.h +++ b/include/linux/slub_def.h @@ -102,6 +102,9 @@ struct kmem_cache { #ifdef CONFIG_SYSFS struct kobject kobj; /* For sysfs */ #endif +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM + str…
creator: Glauber Costa
date: 2012-04-20T21:57:18-00:00

[PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: Allow a memcg parameter to be passed during cache creation. The slub allocator will only merge caches that belong to the same memcg. Default function is created as a wrapper, passing NULL to the memcg version. We only merge caches that belong to the same memcg. >From the memcontrol.c side, 3 helper functions are created: 1) memcg_css_id: because slub needs a unique cache name for sysfs. Since this is visible, but not the canonical location for slab data, the cache name is not used, the css_id should suffice. 2) mem_cgroup_register_cache: is responsible for assigning a unique index to each cache, and other general purpose setup. The index is only assigned for the root caches. All others are assigned index == -1. 3) mem_cgroup_release_cache: can be called from the root cache destruction, and will release the index for other caches. This index mechanism was developed by Suleiman Souhlal. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/memcontrol.h | 14 ++++++++++++++ include/linux/slab.h | 6 ++++++ mm/memcontrol.c | 29 +++++++++++++++++++++++++++++ mm/slub.c | 31 +++++++++++++++++++++++++++---- 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index f94efd2..99e14b9 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -26,6 +26,7 @@ struct mem_cgroup; struct page_cgroup; struct page; struct mm_struct; +struct kmem_cache; /* Stats that can be updated by kernel. */ enum mem_cgroup_page_stat_item { @@ -440,7 +441,20 @@ struct sock; #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM void sock_update_memcg(struct sock *sk); void sock_release_memcg(struct sock *sk); +int memcg_css_id(struct mem_cgroup *memcg); +void mem_cgroup…
creator: Glauber Costa
date: 2012-04-20T21:57:19-00:00

[PATCH 12/23] slab: pass memcg parameter to kmem_cache_create
type: item
description: Allow a memcg parameter to be passed during cache creation. Default function is created as a wrapper, passing NULL to the memcg version. We only merge caches that belong to the same memcg. This code was mostly written by Suleiman Souhlal and only adapted to my patchset, plus a couple of simplifications Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- mm/slab.c | 38 +++++++++++++++++++++++++++++--------- 1 files changed, 29 insertions(+), 9 deletions(-) diff --git a/mm/slab.c b/mm/slab.c index a0d51dd..362bb6e 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -2287,14 +2287,15 @@ static int __init_refok setup_cpu_cache(struct kmem_cache *cachep, gfp_t gfp) * cacheline. This can be beneficial if you're counting cycles as closely * as davem. */ -struct kmem_cache * -kmem_cache_create (const char *name, size_t size, size_t align, - unsigned long flags, void (*ctor)(void *)) +static struct kmem_cache * +__kmem_cache_create(struct mem_cgroup *memcg, const char *name, size_t size, + size_t align, unsigned long flags, void (*ctor)(void *)) { - size_t left_over, slab_size, ralign; + size_t left_over, orig_align, ralign, slab_size; struct kmem_cache *cachep = NULL, *pc; gfp_t gfp; + orig_align = align; /* * Sanity checks... these are all serious usage bugs. */ @@ -2311,7 +2312,6 @@ kmem_cache_create (const char *name, size_t size, size_t align, */ if (slab_is_available()) { get_online_cpus(); - mutex_lock(&cache_chain_mutex); } list_for_each_entry(pc, &cache_chain, next) { @@ -2331,9 +2331,9 @@ kmem_cache_create (const char *name, size_t size, size_t align, continue; } - if (!strcmp(pc->name, name)) { + if (!strcmp(pc->name, name) && !memcg) { printk(KERN_ERR - "kmem_cache_create: duplicate cache %s\n", name); + "kmem_cache_crea…
creator: Glauber Costa
date: 2012-04-22T23:53:29-00:00

[PATCH 13/23] slub: create duplicate cache
type: item
description: This patch provides kmem_cache_dup(), that duplicates a cache for a memcg, preserving its creation properties. Object size, alignment and flags are all respected. When a duplicate cache is created, the parent cache cannot be destructed during the child lifetime. To assure this, its reference count is increased if the cache creation succeeds. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/memcontrol.h | 3 +++ include/linux/slab.h | 3 +++ mm/memcontrol.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ mm/slub.c | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 87 insertions(+), 0 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 99e14b9..493ecdd 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -445,6 +445,9 @@ int memcg_css_id(struct mem_cgroup *memcg); void mem_cgroup_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s); void mem_cgroup_release_cache(struct kmem_cache *cachep); +extern char *mem_cgroup_cache_name(struct mem_cgroup *memcg, + struct kmem_cache *cachep); + #else static inline void mem_cgroup_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s) diff --git a/include/linux/slab.h b/include/linux/slab.h index c7a7e05..909b508 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -323,6 +323,9 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long); #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM #define MAX_KMEM_CACHE_TYPES 400 +extern struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg, + struct kmem_cache *cachep); +void kmem_cache_drop_ref(struct kmem_cache *cachep); #else #define MAX_KMEM_CACHE_TYPES 0 #endif /* CONFIG_CGROUP_MEM_RES_CTLR_KMEM */ diff --git a/mm/memc…
creator: Glauber Costa
date: 2012-04-22T23:53:30-00:00

[PATCH 14/23] slub: provide kmalloc_no_account
type: item
description: Some allocations need to be accounted to the root memcg regardless of their context. One trivial example, is the allocations we do during the memcg slab cache creation themselves. Strictly speaking, they could go to the parent, but it is way easier to bill them to the root cgroup. Only generic kmalloc allocations are allowed to be bypassed. The function is not exported, because drivers code should always be accounted. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/slub_def.h | 1 + mm/slub.c | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-) diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h index 5f5e942..9a8000a 100644 --- a/include/linux/slub_def.h +++ b/include/linux/slub_def.h @@ -221,6 +221,7 @@ static __always_inline struct kmem_cache *kmalloc_slab(size_t size) } void *kmem_cache_alloc(struct kmem_cache *, gfp_t); +void *kmalloc_no_account(size_t size, gfp_t); void *__kmalloc(size_t size, gfp_t flags); static __always_inline void * diff --git a/mm/slub.c b/mm/slub.c index 2285a96..d754b06 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -3359,6 +3359,27 @@ void *__kmalloc(size_t size, gfp_t flags) } EXPORT_SYMBOL(__kmalloc); +void *kmalloc_no_account(size_t size, gfp_t flags) +{ + struct kmem_cache *s; + void *ret; + + if (unlikely(size > SLUB_MAX_SIZE)) + return kmalloc_large(size, flags); + + s = get_slab(size, flags); + + if (unlikely(ZERO_OR_NULL_PTR(s))) + return s; + + ret = slab_alloc(s, flags, NUMA_NO_NODE, _RET_IP_); + + trace_kmalloc(_RET_IP_, ret, size, s->size, flags); + + return ret; +} +EXPORT_SYMBOL(kmalloc_no_account); + #ifdef CONFIG_NUMA static void *kmalloc_large_node(size_t size, gfp_t flags, int node) { -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-22T23:53:31-00:00

[PATCH 15/23] slab: create duplicate cache
type: item
description: This patch provides kmem_cache_dup(), that duplicates a cache for a memcg, preserving its creation properties. Object size, alignment and flags are all respected. An exception is the SLAB_PANIC flag, since cache creation inside a memcg should not be fatal. This code is mostly written by Suleiman Souhlal, with some adaptations and simplifications by me. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- mm/slab.c | 36 ++++++++++++++++++++++++++++++++++++ 1 files changed, 36 insertions(+), 0 deletions(-) diff --git a/mm/slab.c b/mm/slab.c index 362bb6e..c4ef684 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -301,6 +301,8 @@ static void free_block(struct kmem_cache *cachep, void **objpp, int len, int node); static int enable_cpucache(struct kmem_cache *cachep, gfp_t gfp); static void cache_reap(struct work_struct *unused); +static int do_tune_cpucache(struct kmem_cache *cachep, int limit, + int batchcount, int shared, gfp_t gfp); /* * This function must be completely optimized away if a constant is passed to @@ -2593,6 +2595,40 @@ kmem_cache_create(const char *name, size_t size, size_t align, } EXPORT_SYMBOL(kmem_cache_create); +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM +struct kmem_cache * +kmem_cache_dup(struct mem_cgroup *memcg, struct kmem_cache *cachep) +{ + struct kmem_cache *new; + int flags; + char *name; + + name = mem_cgroup_cache_name(memcg, cachep); + if (!name) + return NULL; + + flags = cachep->flags & ~SLAB_PANIC; + mutex_lock(&cache_chain_mutex); + new = __kmem_cache_create(memcg, name, obj_size(cachep), + cachep->memcg_params.orig_align, flags, cachep->ctor); + + if (new == NULL) { + mutex_unlock(&cache_chain_mutex); + kfree(name); + return NULL; + } + + if ((cachep->limit != new->limit) || + (cachep->batch…
creator: Glauber Costa
date: 2012-04-22T23:53:32-00:00

[PATCH 16/23] slab: provide kmalloc_no_account
type: item
description: Some allocations need to be accounted to the root memcg regardless of their context. One trivial example, is the allocations we do during the memcg slab cache creation themselves. Strictly speaking, they could go to the parent, but it is way easier to bill them to the root cgroup. Only generic kmalloc allocations are allowed to be bypassed. The function is not exported, because drivers code should always be accounted. This code is mosly written by Suleiman Souhlal. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/slab_def.h | 1 + mm/slab.c | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 0 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index 06e4a3e..54d25d7 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -114,6 +114,7 @@ extern struct cache_sizes malloc_sizes[]; void *kmem_cache_alloc(struct kmem_cache *, gfp_t); void *__kmalloc(size_t size, gfp_t flags); +void *kmalloc_no_account(size_t size, gfp_t flags); #ifdef CONFIG_TRACING extern void *kmem_cache_alloc_trace(size_t size, diff --git a/mm/slab.c b/mm/slab.c index c4ef684..13948c3 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -3960,6 +3960,29 @@ void *__kmalloc(size_t size, gfp_t flags) } EXPORT_SYMBOL(__kmalloc); +static __always_inline void *__do_kmalloc_no_account(size_t size, gfp_t flags, + void *caller) +{ + struct kmem_cache *cachep; + void *ret; + + cachep = __find_general_cachep(size, flags); + if (unlikely(ZERO_OR_NULL_PTR(cachep))) + return cachep; + + ret = __cache_alloc(cachep, flags, caller); + trace_kmalloc((unsigned long)caller, ret, size, + cachep->buffer_size, flags); + + return ret; +} + +void *kmalloc_no_account(size_t size, gfp_t flags) +{ + return __do_kmall…
creator: Glauber Costa
date: 2012-04-22T23:53:33-00:00

[PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: With all the dependencies already in place, this patch introduces the charge/uncharge functions for the slab cache accounting in memcg. Before we can charge a cache, we need to select the right cache. This is done by using the function __mem_cgroup_get_kmem_cache(). If we should use the root kmem cache, this function tries to detect that and return as early as possible. The charge and uncharge functions comes in two flavours: * __mem_cgroup_(un)charge_slab(), that assumes the allocation is a slab page, and * __mem_cgroup_(un)charge_kmem(), that does not. This later exists because the slub allocator draws the larger kmalloc allocations from the page allocator. In memcontrol.h those functions are wrapped in inline acessors. The idea is to later on, patch those with jump labels, so we don't incur any overhead when no mem cgroups are being used. Because the slub allocator tends to inline the allocations whenever it can, those functions need to be exported so modules can make use of it properly. I apologize in advance to the reviewers. This patch is quite big, but I was not able to split it any further due to all the dependencies between the code. This code is inspired by the code written by Suleiman Souhlal, but heavily changed. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/memcontrol.h | 68 ++++++++ init/Kconfig | 2 +- mm/memcontrol.c | 373 +++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 441 insertions(+), 2 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index 493ecdd..c1c1302 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -448,6 +448,21 @@ void mem_cgroup_release_cache(struct kmem_cache *cachep); extern char *mem_cgroup…
creator: Glauber Costa
date: 2012-04-22T23:53:34-00:00

[PATCH 18/23] slub: charge allocation to a memcg
type: item
description: This patch charges allocation of a slab object to a particular memcg. The cache is selected with mem_cgroup_get_kmem_cache(), which is the biggest overhead we pay here, because it happens at all allocations. However, other than forcing a function call, this function is not very expensive, and try to return as soon as we realize we are not a memcg cache. The charge/uncharge functions are heavier, but are only called for new page allocations. The kmalloc_no_account variant is patched so the base function is used and we don't even try to do cache selection. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/slub_def.h | 32 ++++++++++-- mm/slub.c | 124 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 138 insertions(+), 18 deletions(-) diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h index 9a8000a..e75efcb 100644 --- a/include/linux/slub_def.h +++ b/include/linux/slub_def.h @@ -13,6 +13,7 @@ #include <linux/kobject.h> #include <linux/kmemleak.h> +#include <linux/memcontrol.h> enum stat_item { ALLOC_FASTPATH, /* Allocation from cpu slab */ @@ -210,14 +211,21 @@ static __always_inline int kmalloc_index(size_t size) * This ought to end up with a global pointer to the right cache * in kmalloc_caches. */ -static __always_inline struct kmem_cache *kmalloc_slab(size_t size) +static __always_inline struct kmem_cache *kmalloc_slab(gfp_t flags, size_t size) { + struct kmem_cache *s; int index = kmalloc_index(size); if (index == 0) return NULL; - return kmalloc_caches[index]; + s = kmalloc_caches[index]; + + rcu_read_lock(); + s = mem_cgroup_get_kmem_cache(s, flags); + rcu_read_unlock(); + + return s; } void *kmem_cache_alloc(struct kmem_cache *, gfp_t); @@ -225,13 +233,27 @@ void *km…
creator: Glauber Costa
date: 2012-04-22T23:53:35-00:00

[PATCH 19/23] slab: per-memcg accounting of slab caches
type: item
description: This patch charges allocation of a slab object to a particular memcg. The cache is selected with mem_cgroup_get_kmem_cache(), which is the biggest overhead we pay here, because it happens at all allocations. However, other than forcing a function call, this function is not very expensive, and try to return as soon as we realize we are not a memcg cache. The charge/uncharge functions are heavier, but are only called for new page allocations. Code is heavily inspired by Suleiman's, with adaptations to the patchset and minor simplifications by me. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/slab_def.h | 66 ++++++++++++++++++++++++++++- mm/slab.c | 105 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 162 insertions(+), 9 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index 54d25d7..c4f7e45 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -51,7 +51,7 @@ struct kmem_cache { void (*ctor)(void *obj); /* 4) cache creation/removal */ - const char *name; + char *name; struct list_head next; /* 5) statistics */ @@ -219,4 +219,68 @@ found: #endif /* CONFIG_NUMA */ +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM + +void kmem_cache_drop_ref(struct kmem_cache *cachep); + +static inline void +kmem_cache_get_ref(struct kmem_cache *cachep) +{ + if (cachep->memcg_params.id == -1 && + unlikely(!atomic_add_unless(&cachep->memcg_params.refcnt, 1, 0))) + BUG(); +} + +static inline void +mem_cgroup_put_kmem_cache(struct kmem_cache *cachep) +{ + rcu_read_unlock(); +} + +static inline void +mem_cgroup_kmem_cache_prepare_sleep(struct kmem_cache *cachep) +{ + /* + * Make sure the cache doesn't get freed while we have interrupts + * enabled. + */ + kmem_cache_ge…
creator: Glauber Costa
date: 2012-04-22T23:53:36-00:00

[PATCH 20/23] memcg: disable kmem code when not in use.
type: item
description: We can use jump labels to patch the code in or out when not used. Because the assignment: memcg->kmem_accounted = true is done after the jump labels increment, we guarantee that the root memcg will always be selected until all call sites are patched (see mem_cgroup_kmem_enabled). This guarantees that no mischarges are applied. Jump label decrement happens when the last reference count from the memcg dies. This will only happen when the caches are all dead. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- include/linux/memcontrol.h | 4 +++- mm/memcontrol.c | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h index c1c1302..25c4324 100644 --- a/include/linux/memcontrol.h +++ b/include/linux/memcontrol.h @@ -21,6 +21,7 @@ #define _LINUX_MEMCONTROL_H #include <linux/cgroup.h> #include <linux/vm_event_item.h> +#include <linux/jump_label.h> struct mem_cgroup; struct page_cgroup; @@ -460,7 +461,8 @@ void __mem_cgroup_uncharge_kmem(size_t size); struct kmem_cache * __mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, gfp_t gfp); -#define mem_cgroup_kmem_on 1 +extern struct static_key mem_cgroup_kmem_enabled_key; +#define mem_cgroup_kmem_on static_key_false(&mem_cgroup_kmem_enabled_key) void mem_cgroup_destroy_cache(struct kmem_cache *cachep); #else diff --git a/mm/memcontrol.c b/mm/memcontrol.c index ae61e99..547b632 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -422,6 +422,10 @@ static void mem_cgroup_put(struct mem_cgroup *memcg); #include <net/sock.h> #include <net/ip.h> +struct static_key mem_cgroup_kmem_enabled_key; +/* so modules can inline the checks */ +EXPORT_SYMBOL(mem_cgroup_kmem_enabled_key); + static bool…
creator: Glauber Costa
date: 2012-04-22T23:53:37-00:00

[PATCH 21/23] memcg: Track all the memcg children of a kmem_cache.
type: item
description: From: Suleiman Souhlal <ssouhlal@FreeBSD.org> This enables us to remove all the children of a kmem_cache being destroyed, if for example the kernel module it's being used in gets unloaded. Otherwise, the children will still point to the destroyed parent. We also use this to propagate /proc/slabinfo settings to all the children of a cache, when, for example, changing its batchsize. Signed-off-by: Suleiman Souhlal <suleiman@google.com> --- include/linux/slab.h | 1 + mm/slab.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/include/linux/slab.h b/include/linux/slab.h index 909b508..0dc49fa 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -163,6 +163,7 @@ struct mem_cgroup_cache_params { size_t orig_align; atomic_t refcnt; + struct list_head sibling_list; #endif struct list_head destroyed_list; /* Used when deleting cpuset cache */ }; diff --git a/mm/slab.c b/mm/slab.c index ac0916b..86f2275 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -2561,6 +2561,7 @@ __kmem_cache_create(struct mem_cgroup *memcg, const char *name, size_t size, #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM mem_cgroup_register_cache(memcg, cachep); atomic_set(&cachep->memcg_params.refcnt, 1); + INIT_LIST_HEAD(&cachep->memcg_params.sibling_list); #endif if (setup_cpu_cache(cachep, gfp)) { @@ -2628,6 +2629,8 @@ kmem_cache_dup(struct mem_cgroup *memcg, struct kmem_cache *cachep) return NULL; } + list_add(&new->memcg_params.sibling_list, + &cachep->memcg_params.sibling_list); if ((cachep->limit != new->limit) || (cachep->batchcount != new->batchcount) || (cachep->shared != new->shared)) @@ -2815,6 +2818,29 @@ void kmem_cache_destroy(struct kmem_cache *cachep) { BUG_ON(!cachep || in_interrupt()); +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM + /* Destroy all the children caches if we aren't a memcg cache */ + if (cachep->memcg_params.id != -1) { + struct kmem_cache *c; + struct mem_cgroup_cache_params *p, *tmp; + + mutex_lock(&cache_chain_…
creator: Glauber Costa
date: 2012-04-22T23:53:38-00:00

[PATCH 22/23] memcg: Per-memcg memory.kmem.slabinfo file.
type: item
description: From: Suleiman Souhlal <ssouhlal@FreeBSD.org> This file shows all the kmem_caches used by a memcg. Signed-off-by: Suleiman Souhlal <suleiman@google.com> --- include/linux/slab.h | 1 + mm/memcontrol.c | 17 ++++++++++ mm/slab.c | 88 +++++++++++++++++++++++++++++++++++++------------- mm/slub.c | 5 +++ 4 files changed, 88 insertions(+), 23 deletions(-) diff --git a/include/linux/slab.h b/include/linux/slab.h index 0dc49fa..6932205 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -327,6 +327,7 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long); extern struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg, struct kmem_cache *cachep); void kmem_cache_drop_ref(struct kmem_cache *cachep); +int mem_cgroup_slabinfo(struct mem_cgroup *mem, struct seq_file *m); #else #define MAX_KMEM_CACHE_TYPES 0 #endif /* CONFIG_CGROUP_MEM_RES_CTLR_KMEM */ diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 547b632..46ebd11 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -5092,6 +5092,19 @@ static int mem_control_numa_stat_open(struct inode *unused, struct file *file) #endif /* CONFIG_NUMA */ #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM +static int mem_cgroup_slabinfo_show(struct cgroup *cgroup, struct cftype *ctf, + struct seq_file *m) +{ + struct mem_cgroup *mem; + + mem = mem_cgroup_from_cont(cgroup); + + if (mem == root_mem_cgroup) + mem = NULL; + + return mem_cgroup_slabinfo(mem, m); +} + static struct cftype kmem_cgroup_files[] = { { .name = "kmem.limit_in_bytes", @@ -5116,6 +5129,10 @@ static struct cftype kmem_cgroup_files[] = { .trigger = mem_cgroup_reset, .read = mem_cgroup_read, }, + { + .name = "kmem.slabinfo", + .read_seq_string = mem_cgroup_slabinfo_show, + }, {}, }; diff --git a/mm/slab.c b/mm/slab.c index 86f2275..3e13fef 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -4518,21 +4519,26 @@ static void s_stop(struct seq_file *m, void *p) mutex_unlock(&cache_chain_mutex); } -static int s_show(struct seq_file *m, void *p) -{ - struct kmem_cache *ca…
creator: Glauber Costa
date: 2012-04-22T23:53:39-00:00

[PATCH 23/23] slub: create slabinfo file for memcg
type: item
description: This patch implements mem_cgroup_slabinfo() for the slub. With that, we can also probe the used caches for it. Signed-off-by: Glauber Costa <glommer@parallels.com> CC: Christoph Lameter <cl@linux.com> CC: Pekka Enberg <penberg@cs.helsinki.fi> CC: Michal Hocko <mhocko@suse.cz> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> CC: Johannes Weiner <hannes@cmpxchg.org> CC: Suleiman Souhlal <suleiman@google.com> --- mm/slub.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 1031d4d..495a4f1 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -4150,6 +4150,33 @@ void kmem_cache_drop_ref(struct kmem_cache *s) int mem_cgroup_slabinfo(struct mem_cgroup *memcg, struct seq_file *m) { + struct kmem_cache *s; + int node; + unsigned long nr_objs = 0; + unsigned long nr_free = 0; + + seq_printf(m, "# name <active_objs> <num_objs> <objsize>\n"); + + down_read(&slub_lock); + list_for_each_entry(s, &slab_caches, list) { + if (s->memcg_params.memcg != memcg) + continue; + + for_each_online_node(node) { + struct kmem_cache_node *n = get_node(s, node); + + if (!n) + continue; + + nr_objs += atomic_long_read(&n->total_objects); + nr_free += count_partial(n, count_free); + } + + seq_printf(m, "%-17s %6lu %6lu %6u\n", s->name, + nr_objs - nr_free, nr_objs, s->size); + } + up_read(&slub_lock); + return 0; } #endif -- 1.7.7.6
creator: Glauber Costa
date: 2012-04-22T23:53:40-00:00

Re: [PATCH 00/23] slab+slub accounting for memcg
type: item
description: On 04/20/2012 06:57 PM, Glauber Costa wrote: > Hi, > > This is my current attempt at getting the kmem controller > into a mergeable state. IMHO, all the important bits are there, and it should't > change *that* much from now on. I am, however, expecting at least a couple more > interactions before we sort all the edges out. > > This series works for both the slub and the slab. One of my main goals was to > make sure that the interfaces we are creating actually makes sense for both > allocators. > > I did some adaptations to the slab-specific patches, but the bulk of it > comes from Suleiman's patches. I did the best to use his patches > as-is where possible so to keep authorship information. When not possible, > I tried to be fair and quote it in the commit message. > > In this series, all existing caches are created per-memcg after its first hit. > The main reason is, during discussions in the memory summit we came into > agreement that the fragmentation problems that could arise from creating all > of them are mitigated by the typically small quantity of caches in the system > (order of a few megabytes total for sparsely used caches). > The lazy creation from Suleiman is kept, although a bit modified. For instance, > I now use a locked scheme instead of cmpxcgh to make sure cache creation won't > fail due to duplicates, which simplifies things by quite a bit. > > The slub is a bit more complex than what I came up with in my slub-only > series. The reason is we did not need to use the cache-selection logic > in the allocator itself - it was done by the cache users. But since now > we are lazy creating all caches, this is simply no longer doable. > > I am leaving destruction of caches out of the series, although most > of the infrastructure for that is here, since we did it in earlier > series. This is basically because right now Kame is reworking it for > user memcg, and I like the new proposed behavior a lot more. We all seemed > to have agreed that reclaim is an …
creator: Glauber Costa
date: 2012-04-22T23:59:15-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Sun, 22 Apr 2012, Glauber Costa wrote: > +/* > + * Return the kmem_cache we're supposed to use for a slab allocation. > + * If we are in interrupt context or otherwise have an allocation that > + * can't fail, we return the original cache. > + * Otherwise, we will try to use the current memcg's version of the cache. > + * > + * If the cache does not exist yet, if we are the first user of it, > + * we either create it immediately, if possible, or create it asynchronously > + * in a workqueue. > + * In the latter case, we will let the current allocation go through with > + * the original cache. > + * > + * This function returns with rcu_read_lock() held. > + */ > +struct kmem_cache *__mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, > + gfp_t gfp) > +{ > + struct mem_cgroup *memcg; > + int idx; > + > + gfp |= cachep->allocflags; > + > + if ((current->mm == NULL)) > + return cachep; > + > + if (cachep->memcg_params.memcg) > + return cachep; > + > + idx = cachep->memcg_params.id; > + VM_BUG_ON(idx == -1); > + > + memcg = mem_cgroup_from_task(current); > + if (!mem_cgroup_kmem_enabled(memcg)) > + return cachep; > + > + if (rcu_access_pointer(memcg->slabs[idx]) == NULL) { > + memcg_create_cache_enqueue(memcg, cachep); > + return cachep; > + } > + > + return rcu_dereference(memcg->slabs[idx]); > +} > +EXPORT_SYMBOL(__mem_cgroup_get_kmem_cache); > + > +void mem_cgroup_remove_child_kmem_cache(struct kmem_cache *cachep, int id) > +{ > + rcu_assign_pointer(cachep->memcg_params.memcg->slabs[id], NULL); > +} > + > +bool __mem_cgroup_charge_kmem(gfp_t gfp, size_t size) > +{ > + struct mem_cgroup *memcg; > + bool ret = true; > + > + rcu_read_lock(); > + memcg = mem_cgroup_from_task(current); This seems horribly inconsistent with memcg charging of user memory since it charges to p->mm->owner and you're charging to p. So a thread attached to a memcg can charge user memory to one memcg while charging slab to another memcg? > + > + if (!mem_cgroup_kmem_enabled(memcg)) > + goto …
creator: David Rientjes
date: 2012-04-23T22:25:59-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: On Fri, Apr 20, 2012 at 06:57:19PM -0300, Glauber Costa wrote: > diff --git a/mm/slub.c b/mm/slub.c > index 2652e7c..86e40cc 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -32,6 +32,7 @@ > #include <linux/prefetch.h> > > #include <trace/events/kmem.h> > +#include <linux/memcontrol.h> > > /* > * Lock order: > @@ -3880,7 +3881,7 @@ static int slab_unmergeable(struct kmem_cache *s) > return 0; > } > > -static struct kmem_cache *find_mergeable(size_t size, > +static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size, > size_t align, unsigned long flags, const char *name, > void (*ctor)(void *)) > { > @@ -3916,21 +3917,29 @@ static struct kmem_cache *find_mergeable(size_t size, > if (s->size - size >= sizeof(void *)) > continue; > > + if (memcg && s->memcg_params.memcg != memcg) > + continue; > + This probably won't build without CONFIG_CGROUP_MEM_RES_CTLR_KMEM ? > return s; > } > return NULL; > } > > -struct kmem_cache *kmem_cache_create(const char *name, size_t size, > - size_t align, unsigned long flags, void (*ctor)(void *)) > +struct kmem_cache * > +kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size, Does that build without CONFIG_CGROUP_MEM_RES_CTLR ? > + size_t align, unsigned long flags, void (*ctor)(void *)) > { > struct kmem_cache *s; > > if (WARN_ON(!name)) > return NULL; > > +#ifndef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > + WARN_ON(memcg != NULL); > +#endif > + > down_write(&slub_lock); > - s = find_mergeable(size, align, flags, name, ctor); > + s = find_mergeable(memcg, size, align, flags, name, ctor); > if (s) { > s->refcount++; > /* > @@ -3954,12 +3963,15 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, > size, align, flags, ctor)) { > list_add(&s->list, &slab_caches); > up_write(&slub_lock); > + mem_cgroup_register_cache(memcg, s); How do you handle when the memcg cgroup gets destroyed? Also that means only one memcg cgroup can be accounted for a given slab cache? What if that memcg cgrou…
creator: Frederic Weisbecker
date: 2012-04-24T14:03:31-00:00

Re: [PATCH 13/23] slub: create duplicate cache
type: item
description: On Sun, Apr 22, 2012 at 08:53:30PM -0300, Glauber Costa wrote: > This patch provides kmem_cache_dup(), that duplicates > a cache for a memcg, preserving its creation properties. > Object size, alignment and flags are all respected. > > When a duplicate cache is created, the parent cache cannot > be destructed during the child lifetime. To assure this, > its reference count is increased if the cache creation > succeeds. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/memcontrol.h | 3 +++ > include/linux/slab.h | 3 +++ > mm/memcontrol.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > mm/slub.c | 37 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 87 insertions(+), 0 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 99e14b9..493ecdd 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -445,6 +445,9 @@ int memcg_css_id(struct mem_cgroup *memcg); > void mem_cgroup_register_cache(struct mem_cgroup *memcg, > struct kmem_cache *s); > void mem_cgroup_release_cache(struct kmem_cache *cachep); > +extern char *mem_cgroup_cache_name(struct mem_cgroup *memcg, > + struct kmem_cache *cachep); > + > #else > static inline void mem_cgroup_register_cache(struct mem_cgroup *memcg, > struct kmem_cache *s) > diff --git a/include/linux/slab.h b/include/linux/slab.h > index c7a7e05..909b508 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -323,6 +323,9 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long); > > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > #define MAX_KMEM_CACHE_TYPES 400 > +extern struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg, > + struct kmem_cache *cachep);…
creator: Frederic Weisbecker
date: 2012-04-24T14:18:48-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Mon, Apr 23, 2012 at 03:25:59PM -0700, David Rientjes wrote: > On Sun, 22 Apr 2012, Glauber Costa wrote: > > > +/* > > + * Return the kmem_cache we're supposed to use for a slab allocation. > > + * If we are in interrupt context or otherwise have an allocation that > > + * can't fail, we return the original cache. > > + * Otherwise, we will try to use the current memcg's version of the cache. > > + * > > + * If the cache does not exist yet, if we are the first user of it, > > + * we either create it immediately, if possible, or create it asynchronously > > + * in a workqueue. > > + * In the latter case, we will let the current allocation go through with > > + * the original cache. > > + * > > + * This function returns with rcu_read_lock() held. > > + */ > > +struct kmem_cache *__mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, > > + gfp_t gfp) > > +{ > > + struct mem_cgroup *memcg; > > + int idx; > > + > > + gfp |= cachep->allocflags; > > + > > + if ((current->mm == NULL)) > > + return cachep; > > + > > + if (cachep->memcg_params.memcg) > > + return cachep; > > + > > + idx = cachep->memcg_params.id; > > + VM_BUG_ON(idx == -1); > > + > > + memcg = mem_cgroup_from_task(current); > > + if (!mem_cgroup_kmem_enabled(memcg)) > > + return cachep; > > + > > + if (rcu_access_pointer(memcg->slabs[idx]) == NULL) { > > + memcg_create_cache_enqueue(memcg, cachep); > > + return cachep; > > + } > > + > > + return rcu_dereference(memcg->slabs[idx]); > > +} > > +EXPORT_SYMBOL(__mem_cgroup_get_kmem_cache); > > + > > +void mem_cgroup_remove_child_kmem_cache(struct kmem_cache *cachep, int id) > > +{ > > + rcu_assign_pointer(cachep->memcg_params.memcg->slabs[id], NULL); > > +} > > + > > +bool __mem_cgroup_charge_kmem(gfp_t gfp, size_t size) > > +{ > > + struct mem_cgroup *memcg; > > + bool ret = true; > > + > > + rcu_read_lock(); > > + memcg = mem_cgroup_from_task(current); > > This seems horribly inconsistent with memcg charging of user memory since > it charges to p->mm->owner …
creator: Frederic Weisbecker
date: 2012-04-24T14:22:34-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: On 04/24/2012 11:03 AM, Frederic Weisbecker wrote: > On Fri, Apr 20, 2012 at 06:57:19PM -0300, Glauber Costa wrote: >> diff --git a/mm/slub.c b/mm/slub.c >> index 2652e7c..86e40cc 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -32,6 +32,7 @@ >> #include<linux/prefetch.h> >> >> #include<trace/events/kmem.h> >> +#include<linux/memcontrol.h> >> >> /* >> * Lock order: >> @@ -3880,7 +3881,7 @@ static int slab_unmergeable(struct kmem_cache *s) >> return 0; >> } >> >> -static struct kmem_cache *find_mergeable(size_t size, >> +static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size, >> size_t align, unsigned long flags, const char *name, >> void (*ctor)(void *)) >> { >> @@ -3916,21 +3917,29 @@ static struct kmem_cache *find_mergeable(size_t size, >> if (s->size - size>= sizeof(void *)) >> continue; >> >> + if (memcg&& s->memcg_params.memcg != memcg) >> + continue; >> + > > This probably won't build without CONFIG_CGROUP_MEM_RES_CTLR_KMEM ? Probably not, thanks. > >> return s; >> } >> return NULL; >> } >> >> -struct kmem_cache *kmem_cache_create(const char *name, size_t size, >> - size_t align, unsigned long flags, void (*ctor)(void *)) >> +struct kmem_cache * >> +kmem_cache_create_memcg(struct mem_cgroup *memcg, const char *name, size_t size, > > Does that build without CONFIG_CGROUP_MEM_RES_CTLR ? Yes, because MEM_RES_CTLR_KMEM is dependent on RES_CTLR. > >> + size_t align, unsigned long flags, void (*ctor)(void *)) >> { >> struct kmem_cache *s; >> >> if (WARN_ON(!name)) >> return NULL; >> >> +#ifndef CONFIG_CGROUP_MEM_RES_CTLR_KMEM >> + WARN_ON(memcg != NULL); >> +#endif >> + >> down_write(&slub_lock); >> - s = find_mergeable(size, align, flags, name, ctor); >> + s = find_mergeable(memcg, size, align, flags, name, ctor); >> if (s) { >> s->refcount++; >> /* >> @@ -3954,12 +3963,15 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size, >> size, align, flags, ctor)) { >> list_add(&s->list,&slab_caches); >> up_write(&slub_lock); >> +…
creator: Glauber Costa
date: 2012-04-24T14:27:50-00:00

Re: [PATCH 13/23] slub: create duplicate cache
type: item
description: On 04/24/2012 11:18 AM, Frederic Weisbecker wrote: > On Sun, Apr 22, 2012 at 08:53:30PM -0300, Glauber Costa wrote: >> This patch provides kmem_cache_dup(), that duplicates >> a cache for a memcg, preserving its creation properties. >> Object size, alignment and flags are all respected. >> >> When a duplicate cache is created, the parent cache cannot >> be destructed during the child lifetime. To assure this, >> its reference count is increased if the cache creation >> succeeds. >> >> Signed-off-by: Glauber Costa<glommer@parallels.com> >> CC: Christoph Lameter<cl@linux.com> >> CC: Pekka Enberg<penberg@cs.helsinki.fi> >> CC: Michal Hocko<mhocko@suse.cz> >> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >> CC: Johannes Weiner<hannes@cmpxchg.org> >> CC: Suleiman Souhlal<suleiman@google.com> >> --- >> include/linux/memcontrol.h | 3 +++ >> include/linux/slab.h | 3 +++ >> mm/memcontrol.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> mm/slub.c | 37 +++++++++++++++++++++++++++++++++++++ >> 4 files changed, 87 insertions(+), 0 deletions(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index 99e14b9..493ecdd 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -445,6 +445,9 @@ int memcg_css_id(struct mem_cgroup *memcg); >> void mem_cgroup_register_cache(struct mem_cgroup *memcg, >> struct kmem_cache *s); >> void mem_cgroup_release_cache(struct kmem_cache *cachep); >> +extern char *mem_cgroup_cache_name(struct mem_cgroup *memcg, >> + struct kmem_cache *cachep); >> + >> #else >> static inline void mem_cgroup_register_cache(struct mem_cgroup *memcg, >> struct kmem_cache *s) >> diff --git a/include/linux/slab.h b/include/linux/slab.h >> index c7a7e05..909b508 100644 >> --- a/include/linux/slab.h >> +++ b/include/linux/slab.h >> @@ -323,6 +323,9 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long); >> >> #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM >> #define MAX_KMEM_CACHE_TYPES 400 >> +extern…
creator: Glauber Costa
date: 2012-04-24T14:37:59-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On 04/24/2012 11:22 AM, Frederic Weisbecker wrote: > On Mon, Apr 23, 2012 at 03:25:59PM -0700, David Rientjes wrote: >> On Sun, 22 Apr 2012, Glauber Costa wrote: >> >>> +/* >>> + * Return the kmem_cache we're supposed to use for a slab allocation. >>> + * If we are in interrupt context or otherwise have an allocation that >>> + * can't fail, we return the original cache. >>> + * Otherwise, we will try to use the current memcg's version of the cache. >>> + * >>> + * If the cache does not exist yet, if we are the first user of it, >>> + * we either create it immediately, if possible, or create it asynchronously >>> + * in a workqueue. >>> + * In the latter case, we will let the current allocation go through with >>> + * the original cache. >>> + * >>> + * This function returns with rcu_read_lock() held. >>> + */ >>> +struct kmem_cache *__mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, >>> + gfp_t gfp) >>> +{ >>> + struct mem_cgroup *memcg; >>> + int idx; >>> + >>> + gfp |= cachep->allocflags; >>> + >>> + if ((current->mm == NULL)) >>> + return cachep; >>> + >>> + if (cachep->memcg_params.memcg) >>> + return cachep; >>> + >>> + idx = cachep->memcg_params.id; >>> + VM_BUG_ON(idx == -1); >>> + >>> + memcg = mem_cgroup_from_task(current); >>> + if (!mem_cgroup_kmem_enabled(memcg)) >>> + return cachep; >>> + >>> + if (rcu_access_pointer(memcg->slabs[idx]) == NULL) { >>> + memcg_create_cache_enqueue(memcg, cachep); >>> + return cachep; >>> + } >>> + >>> + return rcu_dereference(memcg->slabs[idx]); >>> +} >>> +EXPORT_SYMBOL(__mem_cgroup_get_kmem_cache); >>> + >>> +void mem_cgroup_remove_child_kmem_cache(struct kmem_cache *cachep, int id) >>> +{ >>> + rcu_assign_pointer(cachep->memcg_params.memcg->slabs[id], NULL); >>> +} >>> + >>> +bool __mem_cgroup_charge_kmem(gfp_t gfp, size_t size) >>> +{ >>> + struct mem_cgroup *memcg; >>> + bool ret = true; >>> + >>> + rcu_read_lock(); >>> + memcg = mem_cgroup_from_task(current); >> >> This seems horribly inconsistent with memcg ch…
creator: Glauber Costa
date: 2012-04-24T14:40:34-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Tue, 24 Apr 2012, Frederic Weisbecker wrote: > > This seems horribly inconsistent with memcg charging of user memory since > > it charges to p->mm->owner and you're charging to p. So a thread attached > > to a memcg can charge user memory to one memcg while charging slab to > > another memcg? > > Charging to the thread rather than the process seem to me the right behaviour: > you can have two threads of a same process attached to different cgroups. > > Perhaps it is the user memory memcg that needs to be fixed? > No, because memory is represented by mm_struct, not task_struct, so you must charge to p->mm->owner to allow for moving threads amongst memcgs later for memory.move_charge_at_immigrate. You shouldn't be able to charge two different memcgs for memory represented by a single mm. > > > + > > > + if (!mem_cgroup_kmem_enabled(memcg)) > > > + goto out; > > > + > > > + mem_cgroup_get(memcg); > > > + ret = memcg_charge_kmem(memcg, gfp, size) == 0; > > > + if (ret) > > > + mem_cgroup_put(memcg); > > > +out: > > > + rcu_read_unlock(); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(__mem_cgroup_charge_kmem); > > > + > > > +void __mem_cgroup_uncharge_kmem(size_t size) > > > +{ > > > + struct mem_cgroup *memcg; > > > + > > > + rcu_read_lock(); > > > + memcg = mem_cgroup_from_task(current); > > > + > > > + if (!mem_cgroup_kmem_enabled(memcg)) > > > + goto out; > > > + > > > + mem_cgroup_put(memcg); > > > + memcg_uncharge_kmem(memcg, size); > > > +out: > > > + rcu_read_unlock(); > > > +} > > > +EXPORT_SYMBOL(__mem_cgroup_uncharge_kmem);
creator: David Rientjes
date: 2012-04-24T20:21:43-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Tue, 24 Apr 2012, Glauber Costa wrote: > I think memcg is not necessarily wrong. That is because threads in a process > share an address space, and you will eventually need to map a page to deliver > it to userspace. The mm struct points you to the owner of that. > > But that is not necessarily true for things that live in the kernel address > space. > > Do you view this differently ? > Yes, for user memory, I see charging to p->mm->owner as allowing that process to eventually move and be charged to a different memcg and there's no way to do proper accounting if the charge is split amongst different memcgs because of thread membership to a set of memcgs. This is consistent with charges for shared memory being moved when a thread mapping it moves to a new memcg, as well.
creator: David Rientjes
date: 2012-04-24T20:25:57-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On 04/24/2012 05:25 PM, David Rientjes wrote: > On Tue, 24 Apr 2012, Glauber Costa wrote: > >> I think memcg is not necessarily wrong. That is because threads in a process >> share an address space, and you will eventually need to map a page to deliver >> it to userspace. The mm struct points you to the owner of that. >> >> But that is not necessarily true for things that live in the kernel address >> space. >> >> Do you view this differently ? >> > > Yes, for user memory, I see charging to p->mm->owner as allowing that > process to eventually move and be charged to a different memcg and there's > no way to do proper accounting if the charge is split amongst different > memcgs because of thread membership to a set of memcgs. This is > consistent with charges for shared memory being moved when a thread > mapping it moves to a new memcg, as well. But that's the problem. When we are dealing with kernel memory, we are allocating a whole slab page. It is essentially impossible to track, given a page, which task allocated which object.
creator: Glauber Costa
date: 2012-04-24T21:36:02-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Tue, 24 Apr 2012, Glauber Costa wrote: > > Yes, for user memory, I see charging to p->mm->owner as allowing that > > process to eventually move and be charged to a different memcg and there's > > no way to do proper accounting if the charge is split amongst different > > memcgs because of thread membership to a set of memcgs. This is > > consistent with charges for shared memory being moved when a thread > > mapping it moves to a new memcg, as well. > > But that's the problem. > > When we are dealing with kernel memory, we are allocating a whole slab page. > It is essentially impossible to track, given a page, which task allocated > which object. > Right, so you have to make the distinction that slab charges cannot be migrated by memory.move_charge_at_immigrate (and it's not even specified to do anything beyond user pages in Documentation/cgroups/memory.txt), but it would be consistent to charge the same memcg for a process's slab allocations as the process's user allocations. My response was why we shouldn't be charging user pages to mem_cgroup_from_task(current) rather than mem_cgroup_from_task(current->mm->owner) which is what is currently implemented. If that can't be changed so that we can still migrate user memory amongst memcgs for memory.move_charge_at_immigrate, then it seems consistent to have all allocations done by a task to be charged to the same memcg. Hence, I suggested current->mm->owner for slab charging as well.
creator: David Rientjes
date: 2012-04-24T22:54:20-00:00

Re: [PATCH 04/23] memcg: Make it possible to use the stock for more than one page.
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > From: Suleiman Souhlal <ssouhlal@FreeBSD.org> > > Signed-off-by: Suleiman Souhlal <suleiman@google.com> ok, should work enough. Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T00:59:48-00:00

Re: [PATCH 05/23] memcg: Reclaim when more than one page needed.
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > From: Suleiman Souhlal <ssouhlal@FreeBSD.org> > > mem_cgroup_do_charge() was written before slab accounting, and expects > three cases: being called for 1 page, being called for a stock of 32 pages, > or being called for a hugepage. If we call for 2 pages (and several slabs > used in process creation are such, at least with the debug options I had), > it assumed it's being called for stock and just retried without reclaiming. > > Fix that by passing down a minsize argument in addition to the csize. > > And what to do about that (csize == PAGE_SIZE && ret) retry? If it's > needed at all (and presumably is since it's there, perhaps to handle > races), then it should be extended to more than PAGE_SIZE, yet how far? IIRC, it was for preventing rapid OOM kill and reducing latency. > And should there be a retry count limit, of what? For now retry up to > COSTLY_ORDER (as page_alloc.c does), stay safe with a cond_resched(), > and make sure not to do it if __GFP_NORETRY. > > Signed-off-by: Suleiman Souhlal <suleiman@google.com> Hmm, maybe ok. Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > --- > mm/memcontrol.c | 18 +++++++++++------- > 1 files changed, 11 insertions(+), 7 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 4b94b2d..cbffc4c 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2187,7 +2187,8 @@ enum { > }; > > static int mem_cgroup_do_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, > - unsigned int nr_pages, bool oom_check) > + unsigned int nr_pages, unsigned int min_pages, > + bool oom_check) > { > unsigned long csize = nr_pages * PAGE_SIZE; > struct mem_cgroup *mem_over_limit; > @@ -2210,18 +2211,18 @@ static int mem_cgroup_do_charge(struct mem_cgroup *memcg, gfp_t gfp_mask, > } else > mem_over_limit = mem_cgroup_from_res_counter(fail_res, res); > /* > - * nr_pages can be either a huge page (HPAGE_PMD_NR), a batch > - * of regular pages (CHARGE_BATCH), or a single regu…
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:16:42-00:00

Re: [PATCH 07/23] change defines to an enum
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > This is just a cleanup patch for clarity of expression. > In earlier submissions, people asked it to be in a separate > patch, so here it is. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:18:42-00:00

Re: [PATCH 08/23] don't force return value checking in res_counter_charge_nofail
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > Since we will succeed with the allocation no matter what, there > isn't the need to use __must_check with it. It can very well > be optional. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Michal Hocko <mhocko@suse.cz> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:28:31-00:00

Re: [PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > This patch adds the basic infrastructure for the accounting of the slab > caches. To control that, the following files are created: > > * memory.kmem.usage_in_bytes > * memory.kmem.limit_in_bytes > * memory.kmem.failcnt > * memory.kmem.max_usage_in_bytes > > They have the same meaning of their user memory counterparts. They reflect > the state of the "kmem" res_counter. > > The code is not enabled until a limit is set. This can be tested by the flag > "kmem_accounted". This means that after the patch is applied, no behavioral > changes exists for whoever is still using memcg to control their memory usage. > Hmm, res_counter never goes naeative ? > We always account to both user and kernel resource_counters. This effectively > means that an independent kernel limit is in place when the limit is set > to a lower value than the user memory. A equal or higher value means that the > user limit will always hit first, meaning that kmem is effectively unlimited. > > People who want to track kernel memory but not limit it, can set this limit > to a very high number (like RESOURCE_MAX - 1page - that no one will ever hit, > or equal to the user memory) > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> The code itself seems fine. Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:32:32-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: (2012/04/21 6:57), Glauber Costa wrote: > Allow a memcg parameter to be passed during cache creation. > The slub allocator will only merge caches that belong to > the same memcg. > > Default function is created as a wrapper, passing NULL > to the memcg version. We only merge caches that belong > to the same memcg. > >>From the memcontrol.c side, 3 helper functions are created: > > 1) memcg_css_id: because slub needs a unique cache name > for sysfs. Since this is visible, but not the canonical > location for slab data, the cache name is not used, the > css_id should suffice. > > 2) mem_cgroup_register_cache: is responsible for assigning > a unique index to each cache, and other general purpose > setup. The index is only assigned for the root caches. All > others are assigned index == -1. > > 3) mem_cgroup_release_cache: can be called from the root cache > destruction, and will release the index for other caches. > > This index mechanism was developed by Suleiman Souhlal. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/memcontrol.h | 14 ++++++++++++++ > include/linux/slab.h | 6 ++++++ > mm/memcontrol.c | 29 +++++++++++++++++++++++++++++ > mm/slub.c | 31 +++++++++++++++++++++++++++---- > 4 files changed, 76 insertions(+), 4 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index f94efd2..99e14b9 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -26,6 +26,7 @@ struct mem_cgroup; > struct page_cgroup; > struct page; > struct mm_struct; > +struct kmem_cache; > > /* Stats that can be updated by kernel. */ > enum mem_cgroup_page_stat_item { > @@ -440,7 +441,20 @@ struct sock; > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM…
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:38:59-00:00

Re: [PATCH 16/23] slab: provide kmalloc_no_account
type: item
description: (2012/04/23 8:53), Glauber Costa wrote: > Some allocations need to be accounted to the root memcg regardless > of their context. One trivial example, is the allocations we do > during the memcg slab cache creation themselves. Strictly speaking, > they could go to the parent, but it is way easier to bill them to > the root cgroup. > > Only generic kmalloc allocations are allowed to be bypassed. > > The function is not exported, because drivers code should always > be accounted. > > This code is mosly written by Suleiman Souhlal. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> Seems reasonable. Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Hmm...but can't we find the 'context' in automatic way ? -Kame > --- > include/linux/slab_def.h | 1 + > mm/slab.c | 23 +++++++++++++++++++++++ > 2 files changed, 24 insertions(+), 0 deletions(-) > > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h > index 06e4a3e..54d25d7 100644 > --- a/include/linux/slab_def.h > +++ b/include/linux/slab_def.h > @@ -114,6 +114,7 @@ extern struct cache_sizes malloc_sizes[]; > > void *kmem_cache_alloc(struct kmem_cache *, gfp_t); > void *__kmalloc(size_t size, gfp_t flags); > +void *kmalloc_no_account(size_t size, gfp_t flags); > > #ifdef CONFIG_TRACING > extern void *kmem_cache_alloc_trace(size_t size, > diff --git a/mm/slab.c b/mm/slab.c > index c4ef684..13948c3 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -3960,6 +3960,29 @@ void *__kmalloc(size_t size, gfp_t flags) > } > EXPORT_SYMBOL(__kmalloc); > > +static __always_inline void *__do_kmalloc_no_account(size_t size, gfp_t flags, > + void *caller) > +{ > + struct kmem_cache *cachep; > + void *ret; > + > + cachep = __find_general_cachep(size, fl…
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:44:35-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: (2012/04/24 23:22), Frederic Weisbecker wrote: > On Mon, Apr 23, 2012 at 03:25:59PM -0700, David Rientjes wrote: >> On Sun, 22 Apr 2012, Glauber Costa wrote: >> >>> +/* >>> + * Return the kmem_cache we're supposed to use for a slab allocation. >>> + * If we are in interrupt context or otherwise have an allocation that >>> + * can't fail, we return the original cache. >>> + * Otherwise, we will try to use the current memcg's version of the cache. >>> + * >>> + * If the cache does not exist yet, if we are the first user of it, >>> + * we either create it immediately, if possible, or create it asynchronously >>> + * in a workqueue. >>> + * In the latter case, we will let the current allocation go through with >>> + * the original cache. >>> + * >>> + * This function returns with rcu_read_lock() held. >>> + */ >>> +struct kmem_cache *__mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, >>> + gfp_t gfp) >>> +{ >>> + struct mem_cgroup *memcg; >>> + int idx; >>> + >>> + gfp |= cachep->allocflags; >>> + >>> + if ((current->mm == NULL)) >>> + return cachep; >>> + >>> + if (cachep->memcg_params.memcg) >>> + return cachep; >>> + >>> + idx = cachep->memcg_params.id; >>> + VM_BUG_ON(idx == -1); >>> + >>> + memcg = mem_cgroup_from_task(current); >>> + if (!mem_cgroup_kmem_enabled(memcg)) >>> + return cachep; >>> + >>> + if (rcu_access_pointer(memcg->slabs[idx]) == NULL) { >>> + memcg_create_cache_enqueue(memcg, cachep); >>> + return cachep; >>> + } >>> + >>> + return rcu_dereference(memcg->slabs[idx]); >>> +} >>> +EXPORT_SYMBOL(__mem_cgroup_get_kmem_cache); >>> + >>> +void mem_cgroup_remove_child_kmem_cache(struct kmem_cache *cachep, int id) >>> +{ >>> + rcu_assign_pointer(cachep->memcg_params.memcg->slabs[id], NULL); >>> +} >>> + >>> +bool __mem_cgroup_charge_kmem(gfp_t gfp, size_t size) >>> +{ >>> + struct mem_cgroup *memcg; >>> + bool ret = true; >>> + >>> + rcu_read_lock(); >>> + memcg = mem_cgroup_from_task(current); >> >> This seems horribly inconsistent with memcg chargi…
creator: KAMEZAWA Hiroyuki
date: 2012-04-25T01:56:16-00:00

Re: [PATCH 16/23] slab: provide kmalloc_no_account
type: item
description: On 04/24/2012 10:44 PM, KAMEZAWA Hiroyuki wrote: > (2012/04/23 8:53), Glauber Costa wrote: > >> Some allocations need to be accounted to the root memcg regardless >> of their context. One trivial example, is the allocations we do >> during the memcg slab cache creation themselves. Strictly speaking, >> they could go to the parent, but it is way easier to bill them to >> the root cgroup. >> >> Only generic kmalloc allocations are allowed to be bypassed. >> >> The function is not exported, because drivers code should always >> be accounted. >> >> This code is mosly written by Suleiman Souhlal. >> >> Signed-off-by: Glauber Costa<glommer@parallels.com> >> CC: Christoph Lameter<cl@linux.com> >> CC: Pekka Enberg<penberg@cs.helsinki.fi> >> CC: Michal Hocko<mhocko@suse.cz> >> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >> CC: Johannes Weiner<hannes@cmpxchg.org> >> CC: Suleiman Souhlal<suleiman@google.com> > > > Seems reasonable. > Reviewed-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> > > Hmm...but can't we find the 'context' in automatic way ? > Not that I can think of. Well, actually, not without adding some tests to the allocation path I'd rather not (like testing for the return address and then doing a table lookup, etc) An option would be to store it in the task_struct. So we would allocate as following: memcg_skip_account_start(p); do_a_bunch_of_allocations(); memcg_skip_account_stop(p); The problem with that, is that it is quite easy to abuse. but if we don't export that to modules, it would be acceptable. Question is, given the fact that the number of kmalloc_no_account() is expected to be really small, is it worth it?
creator: Glauber Costa
date: 2012-04-25T14:29:22-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: On 04/24/2012 10:38 PM, KAMEZAWA Hiroyuki wrote: > (2012/04/21 6:57), Glauber Costa wrote: > >> Allow a memcg parameter to be passed during cache creation. >> The slub allocator will only merge caches that belong to >> the same memcg. >> >> Default function is created as a wrapper, passing NULL >> to the memcg version. We only merge caches that belong >> to the same memcg. >> >> > From the memcontrol.c side, 3 helper functions are created: >> >> 1) memcg_css_id: because slub needs a unique cache name >> for sysfs. Since this is visible, but not the canonical >> location for slab data, the cache name is not used, the >> css_id should suffice. >> >> 2) mem_cgroup_register_cache: is responsible for assigning >> a unique index to each cache, and other general purpose >> setup. The index is only assigned for the root caches. All >> others are assigned index == -1. >> >> 3) mem_cgroup_release_cache: can be called from the root cache >> destruction, and will release the index for other caches. >> >> This index mechanism was developed by Suleiman Souhlal. >> >> Signed-off-by: Glauber Costa<glommer@parallels.com> >> CC: Christoph Lameter<cl@linux.com> >> CC: Pekka Enberg<penberg@cs.helsinki.fi> >> CC: Michal Hocko<mhocko@suse.cz> >> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >> CC: Johannes Weiner<hannes@cmpxchg.org> >> CC: Suleiman Souhlal<suleiman@google.com> >> --- >> include/linux/memcontrol.h | 14 ++++++++++++++ >> include/linux/slab.h | 6 ++++++ >> mm/memcontrol.c | 29 +++++++++++++++++++++++++++++ >> mm/slub.c | 31 +++++++++++++++++++++++++++---- >> 4 files changed, 76 insertions(+), 4 deletions(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index f94efd2..99e14b9 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -26,6 +26,7 @@ struct mem_cgroup; >> struct page_cgroup; >> struct page; >> struct mm_struct; >> +struct kmem_cache; >> >> /* Stats that can be updated by kernel. */ >> enum mem_c…
creator: Glauber Costa
date: 2012-04-25T14:37:22-00:00

Re: [PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: On 04/24/2012 10:32 PM, KAMEZAWA Hiroyuki wrote: > (2012/04/21 6:57), Glauber Costa wrote: > >> This patch adds the basic infrastructure for the accounting of the slab >> caches. To control that, the following files are created: >> >> * memory.kmem.usage_in_bytes >> * memory.kmem.limit_in_bytes >> * memory.kmem.failcnt >> * memory.kmem.max_usage_in_bytes >> >> They have the same meaning of their user memory counterparts. They reflect >> the state of the "kmem" res_counter. >> >> The code is not enabled until a limit is set. This can be tested by the flag >> "kmem_accounted". This means that after the patch is applied, no behavioral >> changes exists for whoever is still using memcg to control their memory usage. >> > > Hmm, res_counter never goes naeative ? Why would it? This one has more or less the same logic as the sock buffers. If we are not accounted, the caches don't get created. If the caches don't get created, we don't release them. (this is modulo bugs, of course)
creator: Glauber Costa
date: 2012-04-25T14:38:57-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On 04/24/2012 07:54 PM, David Rientjes wrote: > On Tue, 24 Apr 2012, Glauber Costa wrote: > >>> Yes, for user memory, I see charging to p->mm->owner as allowing that >>> process to eventually move and be charged to a different memcg and there's >>> no way to do proper accounting if the charge is split amongst different >>> memcgs because of thread membership to a set of memcgs. This is >>> consistent with charges for shared memory being moved when a thread >>> mapping it moves to a new memcg, as well. >> >> But that's the problem. >> >> When we are dealing with kernel memory, we are allocating a whole slab page. >> It is essentially impossible to track, given a page, which task allocated >> which object. >> > > Right, so you have to make the distinction that slab charges cannot be > migrated by memory.move_charge_at_immigrate (and it's not even specified > to do anything beyond user pages in Documentation/cgroups/memory.txt), Never intended to. > but > it would be consistent to charge the same memcg for a process's slab > allocations as the process's user allocations. > > My response was why we shouldn't be charging user pages to > mem_cgroup_from_task(current) rather than > mem_cgroup_from_task(current->mm->owner) which is what is currently > implemented. Ah, all right. Well, for user memory I agree with you. My point was exactly that user memory can always be pinpointed to a specific address space, while kernel memory can't. > > If that can't be changed so that we can still migrate user memory amongst > memcgs for memory.move_charge_at_immigrate, then it seems consistent to > have all allocations done by a task to be charged to the same memcg. > Hence, I suggested current->mm->owner for slab charging as well. All right. This can be done. Although I don't see this as a must for slab as already explained, I certainly don't oppose doing so as well.
creator: Glauber Costa
date: 2012-04-25T14:43:28-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: > > About kmem, if we count task_struct, page tables, etc...which can be freed by > OOM-Killer i.e. it's allocated for 'process', should be aware of OOM problem. > Using mm->owner makes sense to me until someone finds a great idea to handle > OOM situation rather than task killing. > noted, will update.
creator: Glauber Costa
date: 2012-04-25T14:44:22-00:00

Re: [PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: (2012/04/25 23:38), Glauber Costa wrote: > On 04/24/2012 10:32 PM, KAMEZAWA Hiroyuki wrote: >> (2012/04/21 6:57), Glauber Costa wrote: >> >>> This patch adds the basic infrastructure for the accounting of the slab >>> caches. To control that, the following files are created: >>> >>> * memory.kmem.usage_in_bytes >>> * memory.kmem.limit_in_bytes >>> * memory.kmem.failcnt >>> * memory.kmem.max_usage_in_bytes >>> >>> They have the same meaning of their user memory counterparts. They reflect >>> the state of the "kmem" res_counter. >>> >>> The code is not enabled until a limit is set. This can be tested by the flag >>> "kmem_accounted". This means that after the patch is applied, no behavioral >>> changes exists for whoever is still using memcg to control their memory usage. >>> >> >> Hmm, res_counter never goes naeative ? > > Why would it? > > This one has more or less the same logic as the sock buffers. > > If we are not accounted, the caches don't get created. If the caches > don't get created, we don't release them. (this is modulo bugs, of course) Okay. Please note how the logic works in description or Doc. It's a bit complicated part. Thanks, -Kame
creator: KAMEZAWA Hiroyuki
date: 2012-04-26T00:08:39-00:00

Re: [PATCH 16/23] slab: provide kmalloc_no_account
type: item
description: (2012/04/25 23:29), Glauber Costa wrote: > On 04/24/2012 10:44 PM, KAMEZAWA Hiroyuki wrote: >> (2012/04/23 8:53), Glauber Costa wrote: >> >>> Some allocations need to be accounted to the root memcg regardless >>> of their context. One trivial example, is the allocations we do >>> during the memcg slab cache creation themselves. Strictly speaking, >>> they could go to the parent, but it is way easier to bill them to >>> the root cgroup. >>> >>> Only generic kmalloc allocations are allowed to be bypassed. >>> >>> The function is not exported, because drivers code should always >>> be accounted. >>> >>> This code is mosly written by Suleiman Souhlal. >>> >>> Signed-off-by: Glauber Costa<glommer@parallels.com> >>> CC: Christoph Lameter<cl@linux.com> >>> CC: Pekka Enberg<penberg@cs.helsinki.fi> >>> CC: Michal Hocko<mhocko@suse.cz> >>> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >>> CC: Johannes Weiner<hannes@cmpxchg.org> >>> CC: Suleiman Souhlal<suleiman@google.com> >> >> >> Seems reasonable. >> Reviewed-by: KAMEZAWA Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >> >> Hmm...but can't we find the 'context' in automatic way ? >> > > Not that I can think of. Well, actually, not without adding some tests > to the allocation path I'd rather not (like testing for the return > address and then doing a table lookup, etc) > > An option would be to store it in the task_struct. So we would allocate > as following: > > memcg_skip_account_start(p); > do_a_bunch_of_allocations(); > memcg_skip_account_stop(p); > > The problem with that, is that it is quite easy to abuse. > but if we don't export that to modules, it would be acceptable. > > Question is, given the fact that the number of kmalloc_no_account() is > expected to be really small, is it worth it? > ok, but.... There was an idea __GFP_NOACCOUNT, which is better ? Are you afraid that__GFP_NOACCOUNT can be spread too much rather than kmalloc_no_account() ? Thanks, -Kame
creator: KAMEZAWA Hiroyuki
date: 2012-04-26T00:13:51-00:00

Re: [PATCH 13/23] slub: create duplicate cache
type: item
description: On Tue, Apr 24, 2012 at 11:37:59AM -0300, Glauber Costa wrote: > On 04/24/2012 11:18 AM, Frederic Weisbecker wrote: > >On Sun, Apr 22, 2012 at 08:53:30PM -0300, Glauber Costa wrote: > >>This patch provides kmem_cache_dup(), that duplicates > >>a cache for a memcg, preserving its creation properties. > >>Object size, alignment and flags are all respected. > >> > >>When a duplicate cache is created, the parent cache cannot > >>be destructed during the child lifetime. To assure this, > >>its reference count is increased if the cache creation > >>succeeds. > >> > >>Signed-off-by: Glauber Costa<glommer@parallels.com> > >>CC: Christoph Lameter<cl@linux.com> > >>CC: Pekka Enberg<penberg@cs.helsinki.fi> > >>CC: Michal Hocko<mhocko@suse.cz> > >>CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> > >>CC: Johannes Weiner<hannes@cmpxchg.org> > >>CC: Suleiman Souhlal<suleiman@google.com> > >>--- > >> include/linux/memcontrol.h | 3 +++ > >> include/linux/slab.h | 3 +++ > >> mm/memcontrol.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > >> mm/slub.c | 37 +++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 87 insertions(+), 0 deletions(-) > >> > >>diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > >>index 99e14b9..493ecdd 100644 > >>--- a/include/linux/memcontrol.h > >>+++ b/include/linux/memcontrol.h > >>@@ -445,6 +445,9 @@ int memcg_css_id(struct mem_cgroup *memcg); > >> void mem_cgroup_register_cache(struct mem_cgroup *memcg, > >> struct kmem_cache *s); > >> void mem_cgroup_release_cache(struct kmem_cache *cachep); > >>+extern char *mem_cgroup_cache_name(struct mem_cgroup *memcg, > >>+ struct kmem_cache *cachep); > >>+ > >> #else > >> static inline void mem_cgroup_register_cache(struct mem_cgroup *memcg, > >> struct kmem_cache *s) > >>diff --git a/include/linux/slab.h b/include/linux/slab.h > >>index c7a7e05..909b508 100644 > >>--- a/include/linux/slab.h > >>+++ b/include/linux/slab.h > >>@@ -323,6 +323,9 @@ extern void *__kmalloc_track_caller(…
creator: Frederic Weisbecker
date: 2012-04-26T13:10:55-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Tue, Apr 24, 2012 at 01:21:43PM -0700, David Rientjes wrote: > On Tue, 24 Apr 2012, Frederic Weisbecker wrote: > > > > This seems horribly inconsistent with memcg charging of user memory since > > > it charges to p->mm->owner and you're charging to p. So a thread attached > > > to a memcg can charge user memory to one memcg while charging slab to > > > another memcg? > > > > Charging to the thread rather than the process seem to me the right behaviour: > > you can have two threads of a same process attached to different cgroups. > > > > Perhaps it is the user memory memcg that needs to be fixed? > > > > No, because memory is represented by mm_struct, not task_struct, so you > must charge to p->mm->owner to allow for moving threads amongst memcgs > later for memory.move_charge_at_immigrate. You shouldn't be able to > charge two different memcgs for memory represented by a single mm. The idea I had was more that only the memcg of the thread that does the allocation is charged. But the problem is that this allocation can be later deallocated from another thread. So probably charging the owner is indeed the only sane way to go with user memory.
creator: Frederic Weisbecker
date: 2012-04-27T11:38:45-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Wed, Apr 25, 2012 at 10:56:16AM +0900, KAMEZAWA Hiroyuki wrote: > (2012/04/24 23:22), Frederic Weisbecker wrote: > > > On Mon, Apr 23, 2012 at 03:25:59PM -0700, David Rientjes wrote: > >> On Sun, 22 Apr 2012, Glauber Costa wrote: > >> > >>> +/* > >>> + * Return the kmem_cache we're supposed to use for a slab allocation. > >>> + * If we are in interrupt context or otherwise have an allocation that > >>> + * can't fail, we return the original cache. > >>> + * Otherwise, we will try to use the current memcg's version of the cache. > >>> + * > >>> + * If the cache does not exist yet, if we are the first user of it, > >>> + * we either create it immediately, if possible, or create it asynchronously > >>> + * in a workqueue. > >>> + * In the latter case, we will let the current allocation go through with > >>> + * the original cache. > >>> + * > >>> + * This function returns with rcu_read_lock() held. > >>> + */ > >>> +struct kmem_cache *__mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, > >>> + gfp_t gfp) > >>> +{ > >>> + struct mem_cgroup *memcg; > >>> + int idx; > >>> + > >>> + gfp |= cachep->allocflags; > >>> + > >>> + if ((current->mm == NULL)) > >>> + return cachep; > >>> + > >>> + if (cachep->memcg_params.memcg) > >>> + return cachep; > >>> + > >>> + idx = cachep->memcg_params.id; > >>> + VM_BUG_ON(idx == -1); > >>> + > >>> + memcg = mem_cgroup_from_task(current); > >>> + if (!mem_cgroup_kmem_enabled(memcg)) > >>> + return cachep; > >>> + > >>> + if (rcu_access_pointer(memcg->slabs[idx]) == NULL) { > >>> + memcg_create_cache_enqueue(memcg, cachep); > >>> + return cachep; > >>> + } > >>> + > >>> + return rcu_dereference(memcg->slabs[idx]); > >>> +} > >>> +EXPORT_SYMBOL(__mem_cgroup_get_kmem_cache); > >>> + > >>> +void mem_cgroup_remove_child_kmem_cache(struct kmem_cache *cachep, int id) > >>> +{ > >>> + rcu_assign_pointer(cachep->memcg_params.memcg->slabs[id], NULL); > >>> +} > >>> + > >>> +bool __mem_cgroup_charge_kmem(gfp_t gfp, size_t size) > >>> +{ > >>> …
creator: Frederic Weisbecker
date: 2012-04-27T12:22:22-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Fri, 27 Apr 2012, Frederic Weisbecker wrote: > > No, because memory is represented by mm_struct, not task_struct, so you > > must charge to p->mm->owner to allow for moving threads amongst memcgs > > later for memory.move_charge_at_immigrate. You shouldn't be able to > > charge two different memcgs for memory represented by a single mm. > > The idea I had was more that only the memcg of the thread that does the allocation > is charged. But the problem is that this allocation can be later deallocated > from another thread. So probably charging the owner is indeed the only sane > way to go with user memory. > It's all really the same concept: if we want to move memory of a process, willingly free memory in the process itself, or free memory of a process by way of the oom killer, we need a way to do that for the entire process so the accounting makes sense afterwards. And since we have that requirement for user memory, it makes sense that its consistent with slab as well. I don't think a thread of a process should be able to charge slab to one memcg while its user memory is charged to another memcg.
creator: David Rientjes
date: 2012-04-27T18:13:31-00:00

[PATCH 0/3] A few fixes for '[PATCH 00/23] slab+slub accounting for memcg' series
type: item
description: Hello Glauber, On Fri, Apr 20, 2012 at 06:57:08PM -0300, Glauber Costa wrote: > This is my current attempt at getting the kmem controller > into a mergeable state. IMHO, all the important bits are there, and it should't > change *that* much from now on. I am, however, expecting at least a couple more > interactions before we sort all the edges out. > > This series works for both the slub and the slab. One of my main goals was to > make sure that the interfaces we are creating actually makes sense for both > allocators. > > I did some adaptations to the slab-specific patches, but the bulk of it > comes from Suleiman's patches. I did the best to use his patches > as-is where possible so to keep authorship information. When not possible, > I tried to be fair and quote it in the commit message. > > In this series, all existing caches are created per-memcg after its first hit. > The main reason is, during discussions in the memory summit we came into > agreement that the fragmentation problems that could arise from creating all > of them are mitigated by the typically small quantity of caches in the system > (order of a few megabytes total for sparsely used caches). > The lazy creation from Suleiman is kept, although a bit modified. For instance, > I now use a locked scheme instead of cmpxcgh to make sure cache creation won't > fail due to duplicates, which simplifies things by quite a bit. > > The slub is a bit more complex than what I came up with in my slub-only > series. The reason is we did not need to use the cache-selection logic > in the allocator itself - it was done by the cache users. But since now > we are lazy creating all caches, this is simply no longer doable. > > I am leaving destruction of caches out of the series, although most > of the infrastructure for that is here, since we did it in earlier > series. This is basically because right now Kame is reworking it for > user memcg, and I like the new proposed behavior a lot more. We all seemed > to have a…
creator: Anton Vorontsov
date: 2012-04-30T09:59:19-00:00

[PATCH 1/3] slab: Proper off-slabs handling when duplicating caches
type: item
description: OFF_SLAB is not CREATE_MASK bit, so we should clear it before calling __kmem_cache_create(), otherwise kernel gets very upset, see below. As a side effect, now we let slab to reevaluate off-slab decision, but the decision will be the same, because whether we do off-slabs only depend on the size and create_mask bits. ------------[ cut here ]------------ kernel BUG at mm/slab.c:2376! invalid opcode: 0000 [#1] SMP CPU 0 Pid: 14, comm: kworker/0:1 Not tainted 3.4.0-rc4+ #32 Bochs Bochs RIP: 0010:[<ffffffff810c1839>] [<ffffffff810c1839>] __kmem_cache_create+0x609/0x650 RSP: 0018:ffff8800072c9c90 EFLAGS: 00010286 RAX: 0000000000000800 RBX: ffffffff81f26bf8 RCX: 000000000000000b RDX: 000000000000000c RSI: 000000000000000b RDI: ffff8800065c66f8 RBP: ffff8800072c9d40 R08: ffffffff80002800 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000001 R12: ffff8800072c8000 R13: ffff8800072c9fd8 R14: ffffffffffffffff R15: ffff8800072c9d0c FS: 00007f45eb0f2700(0000) GS:ffff880007c00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b CR2: ffffffffff600400 CR3: 000000000650e000 CR4: 00000000000006b0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 Process kworker/0:1 (pid: 14, threadinfo ffff8800072c8000, task ffff88000725d100) Stack: ffff8800072c9cb0 0000000000000000 ffffc9000000c000 ffffffff81621e80 ffff8800072c9cc0 ffffffff81621e80 ffff8800072c9d40 ffffffff81355cbf ffffffff810c1944 0000000000000000 ffffffff81621ec0 ffffffff80002800 Call Trace: [<ffffffff81355cbf>] ? mutex_lock_nested+0x26f/0x340 [<ffffffff810c1944>] ? kmem_cache_dup+0x44/0x110 [<ffffffff810c2aa0>] ? memcg_create_kmem_cache+0xd0/0xd0 [<ffffffff810c196b>] kmem_cache_dup+0x6b/0x110 [<ffffffff810c2a73>] memcg_create_kmem_cache+0xa3/0xd0 [<ffffffff810c2b1a>] memcg_create_cache_work_func+0x7a/0xe0 [<ffffffff810405d4>] process_one_work+0x174/0x450 [<ffffffff81040576>] ? process_one_work+0x116/0x450 [<ff…
creator: Anton Vorontsov
date: 2012-04-30T10:01:21-00:00

[PATCH 2/3] slab: Fix imbalanced rcu locking
type: item
description: Not sure why the code tries to unlock the rcu. The only case where slab grabs the lock is around mem_cgroup_get_kmem_cache() call, which won't result into calling cache_grow() (that tries to unlock the rcu). ===================================== [ BUG: bad unlock balance detected! ] 3.4.0-rc4+ #33 Not tainted ------------------------------------- swapper/0/0 is trying to release lock (rcu_read_lock) at: [<ffffffff8134f0b4>] cache_grow.constprop.63+0xe8/0x371 but there are no more locks to release! other info that might help us debug this: no locks held by swapper/0/0. stack backtrace: Pid: 0, comm: swapper/0 Not tainted 3.4.0-rc4+ #33 Call Trace: [<ffffffff8134f0b4>] ? cache_grow.constprop.63+0xe8/0x371 [<ffffffff8134cf09>] print_unlock_inbalance_bug.part.26+0xd1/0xd9 [<ffffffff8134f0b4>] ? cache_grow.constprop.63+0xe8/0x371 [<ffffffff8106865e>] print_unlock_inbalance_bug+0x4e/0x50 [<ffffffff8134f0b4>] ? cache_grow.constprop.63+0xe8/0x371 [<ffffffff8106cb26>] __lock_release+0xd6/0xe0 [<ffffffff8106cb8c>] lock_release+0x5c/0x80 [<ffffffff8134f0cc>] cache_grow.constprop.63+0x100/0x371 [<ffffffff8134f5c6>] cache_alloc_refill+0x289/0x2dc [<ffffffff810bf682>] ? kmem_cache_alloc+0x92/0x260 [<ffffffff81676a0f>] ? pidmap_init+0x79/0xb2 [<ffffffff810bf842>] kmem_cache_alloc+0x252/0x260 [<ffffffff810bf5f0>] ? kmem_freepages+0x180/0x180 [<ffffffff81676a0f>] pidmap_init+0x79/0xb2 [<ffffffff81667aa3>] start_kernel+0x297/0x2f8 [<ffffffff8166769e>] ? repair_env_string+0x5a/0x5a [<ffffffff816672fd>] x86_64_start_reservations+0x101/0x105 [<ffffffff816673f1>] x86_64_start_kernel+0xf0/0xf7 Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org> --- include/linux/slab_def.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index c4f7e45..2d371ae 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -245,13 +245,11 @@ mem_cgroup_kmem_cache_prepare_sleep(struct kmem_cache *cachep) * enabled. */ kmem_cache_get_ref(…
creator: Anton Vorontsov
date: 2012-04-30T10:01:39-00:00

[PATCH 3/3] slab: Get rid of mem_cgroup_put_kmem_cache()
type: item
description: The function is no longer used, so can be safely removed. Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org> --- include/linux/slab_def.h | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h index 2d371ae..72ea626 100644 --- a/include/linux/slab_def.h +++ b/include/linux/slab_def.h @@ -232,12 +232,6 @@ kmem_cache_get_ref(struct kmem_cache *cachep) } static inline void -mem_cgroup_put_kmem_cache(struct kmem_cache *cachep) -{ - rcu_read_unlock(); -} - -static inline void mem_cgroup_kmem_cache_prepare_sleep(struct kmem_cache *cachep) { /* @@ -266,11 +260,6 @@ kmem_cache_drop_ref(struct kmem_cache *cachep) } static inline void -mem_cgroup_put_kmem_cache(struct kmem_cache *cachep) -{ -} - -static inline void mem_cgroup_kmem_cache_prepare_sleep(struct kmem_cache *cachep) { } -- 1.7.9.2
creator: Anton Vorontsov
date: 2012-04-30T10:02:02-00:00

Re: [PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: On Fri, Apr 20, 2012 at 2:57 PM, Glauber Costa <glommer@parallels.com> wrote: > This patch adds the basic infrastructure for the accounting of the slab > caches. To control that, the following files are created: > > * memory.kmem.usage_in_bytes > * memory.kmem.limit_in_bytes > * memory.kmem.failcnt > * memory.kmem.max_usage_in_bytes > > They have the same meaning of their user memory counterparts. They reflect > the state of the "kmem" res_counter. > > The code is not enabled until a limit is set. This can be tested by the flag > "kmem_accounted". This means that after the patch is applied, no behavioral > changes exists for whoever is still using memcg to control their memory usage. > > We always account to both user and kernel resource_counters. This effectively > means that an independent kernel limit is in place when the limit is set > to a lower value than the user memory. A equal or higher value means that the > user limit will always hit first, meaning that kmem is effectively unlimited. > > People who want to track kernel memory but not limit it, can set this limit > to a very high number (like RESOURCE_MAX - 1page - that no one will ever hit, > or equal to the user memory) > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > --- > mm/memcontrol.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 79 insertions(+), 1 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 2810228..36f1e6b 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -252,6 +252,10 @@ struct mem_cgroup { > }; > > /* > + * the counter to account for kernel memory usage. > + */ > + struct res_counter kmem; > + /* > * Per cgroup active and inactive list, similar to the > * per zone LRU lists. > */ > @@ -266,6 +270,7 @@ struct mem_cgroup { > * Should the accounting and control be hierarchical, per …
creator: Suleiman Souhlal
date: 2012-04-30T19:33:53-00:00

Re: [PATCH 10/23] slab/slub: struct memcg_params
type: item
description: On Fri, Apr 20, 2012 at 2:57 PM, Glauber Costa <glommer@parallels.com> wrote: > For the kmem slab controller, we need to record some extra > information in the kmem_cache structure. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/slab.h | 15 +++++++++++++++ > include/linux/slab_def.h | 4 ++++ > include/linux/slub_def.h | 3 +++ > 3 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index a595dce..a5127e1 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -153,6 +153,21 @@ unsigned int kmem_cache_size(struct kmem_cache *); > #define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) > #endif > > +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > +struct mem_cgroup_cache_params { > + struct mem_cgroup *memcg; > + int id; > + > +#ifdef CONFIG_SLAB > + /* Original cache parameters, used when creating a memcg cache */ > + size_t orig_align; > + atomic_t refcnt; > + > +#endif > + struct list_head destroyed_list; /* Used when deleting cpuset cache */ s,cpuset,memcg, Sorry about that. -- Suleiman
creator: Suleiman Souhlal
date: 2012-04-30T19:42:28-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: On Fri, Apr 20, 2012 at 2:57 PM, Glauber Costa <glommer@parallels.com> wrote: > Allow a memcg parameter to be passed during cache creation. > The slub allocator will only merge caches that belong to > the same memcg. > > Default function is created as a wrapper, passing NULL > to the memcg version. We only merge caches that belong > to the same memcg. > > From the memcontrol.c side, 3 helper functions are created: > > 1) memcg_css_id: because slub needs a unique cache name > for sysfs. Since this is visible, but not the canonical > location for slab data, the cache name is not used, the > css_id should suffice. > > 2) mem_cgroup_register_cache: is responsible for assigning > a unique index to each cache, and other general purpose > setup. The index is only assigned for the root caches. All > others are assigned index == -1. > > 3) mem_cgroup_release_cache: can be called from the root cache > destruction, and will release the index for other caches. > > This index mechanism was developed by Suleiman Souhlal. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/memcontrol.h | 14 ++++++++++++++ > include/linux/slab.h | 6 ++++++ > mm/memcontrol.c | 29 +++++++++++++++++++++++++++++ > mm/slub.c | 31 +++++++++++++++++++++++++++---- > 4 files changed, 76 insertions(+), 4 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index f94efd2..99e14b9 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -26,6 +26,7 @@ struct mem_cgroup; > struct page_cgroup; > struct page; > struct mm_struct; > +struct kmem_cache; > > /* Stats that can be updated by kernel. */ > enum mem_cgroup_page_stat_item { > @@ -440,7 +441,20 @@ struct sock; > …
creator: Suleiman Souhlal
date: 2012-04-30T19:51:18-00:00

Re: [PATCH 12/23] slab: pass memcg parameter to kmem_cache_create
type: item
description: On Sun, Apr 22, 2012 at 4:53 PM, Glauber Costa <glommer@parallels.com> wrote: > Allow a memcg parameter to be passed during cache creation. > > Default function is created as a wrapper, passing NULL > to the memcg version. We only merge caches that belong > to the same memcg. > > This code was mostly written by Suleiman Souhlal and > only adapted to my patchset, plus a couple of simplifications > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > mm/slab.c | 38 +++++++++++++++++++++++++++++--------- > 1 files changed, 29 insertions(+), 9 deletions(-) > > diff --git a/mm/slab.c b/mm/slab.c > index a0d51dd..362bb6e 100644 > --- a/mm/slab.c > +++ b/mm/slab.c > @@ -2287,14 +2287,15 @@ static int __init_refok setup_cpu_cache(struct kmem_cache *cachep, gfp_t gfp) > * cacheline. This can be beneficial if you're counting cycles as closely > * as davem. > */ > -struct kmem_cache * > -kmem_cache_create (const char *name, size_t size, size_t align, > - unsigned long flags, void (*ctor)(void *)) > +static struct kmem_cache * > +__kmem_cache_create(struct mem_cgroup *memcg, const char *name, size_t size, > + size_t align, unsigned long flags, void (*ctor)(void *)) > { > - size_t left_over, slab_size, ralign; > + size_t left_over, orig_align, ralign, slab_size; > struct kmem_cache *cachep = NULL, *pc; > gfp_t gfp; > > + orig_align = align; > /* > * Sanity checks... these are all serious usage bugs. > */ > @@ -2311,7 +2312,6 @@ kmem_cache_create (const char *name, size_t size, size_t align, > */ > if (slab_is_available()) { > get_online_cpus(); > - mutex_lock(&cache_chain_mutex); > } > > list_for_each_entry(pc, &cache_chain, next) { > @@ -2331,9 +2331,9 @@ kmem_cache_create (const char *name, size_t size, size…
creator: Suleiman Souhlal
date: 2012-04-30T19:54:36-00:00

Re: [PATCH 13/23] slub: create duplicate cache
type: item
description: On Sun, Apr 22, 2012 at 4:53 PM, Glauber Costa <glommer@parallels.com> wrote: > This patch provides kmem_cache_dup(), that duplicates > a cache for a memcg, preserving its creation properties. > Object size, alignment and flags are all respected. > > When a duplicate cache is created, the parent cache cannot > be destructed during the child lifetime. To assure this, > its reference count is increased if the cache creation > succeeds. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/memcontrol.h | 3 +++ > include/linux/slab.h | 3 +++ > mm/memcontrol.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ > mm/slub.c | 37 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 87 insertions(+), 0 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index 99e14b9..493ecdd 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -445,6 +445,9 @@ int memcg_css_id(struct mem_cgroup *memcg); > void mem_cgroup_register_cache(struct mem_cgroup *memcg, > struct kmem_cache *s); > void mem_cgroup_release_cache(struct kmem_cache *cachep); > +extern char *mem_cgroup_cache_name(struct mem_cgroup *memcg, > + struct kmem_cache *cachep); > + > #else > static inline void mem_cgroup_register_cache(struct mem_cgroup *memcg, > struct kmem_cache *s) > diff --git a/include/linux/slab.h b/include/linux/slab.h > index c7a7e05..909b508 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -323,6 +323,9 @@ extern void *__kmalloc_track_caller(size_t, gfp_t, unsigned long); > > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > #define MAX_KMEM_CACHE_TYPES 400 > +extern struct kmem_cache *kmem_cache_dup(struct mem_cgroup *memcg, > + struct kmem_…
creator: Suleiman Souhlal
date: 2012-04-30T20:15:23-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On Sun, Apr 22, 2012 at 4:53 PM, Glauber Costa <glommer@parallels.com> wrote: > With all the dependencies already in place, this patch introduces > the charge/uncharge functions for the slab cache accounting in memcg. > > Before we can charge a cache, we need to select the right cache. > This is done by using the function __mem_cgroup_get_kmem_cache(). > > If we should use the root kmem cache, this function tries to detect > that and return as early as possible. > > The charge and uncharge functions comes in two flavours: > * __mem_cgroup_(un)charge_slab(), that assumes the allocation is > a slab page, and > * __mem_cgroup_(un)charge_kmem(), that does not. This later exists > because the slub allocator draws the larger kmalloc allocations > from the page allocator. > > In memcontrol.h those functions are wrapped in inline acessors. > The idea is to later on, patch those with jump labels, so we don't > incur any overhead when no mem cgroups are being used. > > Because the slub allocator tends to inline the allocations whenever > it can, those functions need to be exported so modules can make use > of it properly. > > I apologize in advance to the reviewers. This patch is quite big, but > I was not able to split it any further due to all the dependencies > between the code. > > This code is inspired by the code written by Suleiman Souhlal, > but heavily changed. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/memcontrol.h | 68 ++++++++ > init/Kconfig | 2 +- > mm/memcontrol.c | 373 +++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 441 insertions(+), 2 deletions(-) > > + > +static struct kmem_cache *memcg_create_kmem_cache(struct mem_cgroup *memcg, > + struct kmem_ca…
creator: Suleiman Souhlal
date: 2012-04-30T20:56:46-00:00

Re: [PATCH 19/23] slab: per-memcg accounting of slab caches
type: item
description: On Sun, Apr 22, 2012 at 4:53 PM, Glauber Costa <glommer@parallels.com> wrote: > This patch charges allocation of a slab object to a particular > memcg. > > The cache is selected with mem_cgroup_get_kmem_cache(), > which is the biggest overhead we pay here, because > it happens at all allocations. However, other than forcing > a function call, this function is not very expensive, and > try to return as soon as we realize we are not a memcg cache. > > The charge/uncharge functions are heavier, but are only called > for new page allocations. > > Code is heavily inspired by Suleiman's, with adaptations to > the patchset and minor simplifications by me. > > Signed-off-by: Glauber Costa <glommer@parallels.com> > CC: Christoph Lameter <cl@linux.com> > CC: Pekka Enberg <penberg@cs.helsinki.fi> > CC: Michal Hocko <mhocko@suse.cz> > CC: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> > CC: Johannes Weiner <hannes@cmpxchg.org> > CC: Suleiman Souhlal <suleiman@google.com> > --- > include/linux/slab_def.h | 66 ++++++++++++++++++++++++++++- > mm/slab.c | 105 ++++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 162 insertions(+), 9 deletions(-) > > diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h > index 54d25d7..c4f7e45 100644 > --- a/include/linux/slab_def.h > +++ b/include/linux/slab_def.h > @@ -51,7 +51,7 @@ struct kmem_cache { > void (*ctor)(void *obj); > > /* 4) cache creation/removal */ > - const char *name; > + char *name; > struct list_head next; > > /* 5) statistics */ > @@ -219,4 +219,68 @@ found: > > #endif /* CONFIG_NUMA */ > > +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM > + > +void kmem_cache_drop_ref(struct kmem_cache *cachep); > + > +static inline void > +kmem_cache_get_ref(struct kmem_cache *cachep) > +{ > + if (cachep->memcg_params.id == -1 && > + unlikely(!atomic_add_unless(&cachep->memcg_params.refcnt, 1, 0))) > + BUG(); > +} > + > +static inline void > +mem_cgroup_put_kmem_cache(struct kmem_cache *cachep) > +{ > + rcu_read_unlock();…
creator: Suleiman Souhlal
date: 2012-04-30T21:25:47-00:00

Re: [PATCH 09/23] kmem slab accounting basic infrastructure
type: item
description: >> @@ -3951,8 +3966,26 @@ static int mem_cgroup_write(struct cgroup *cont, struct cftype *cft, >> break; >> if (type == _MEM) >> ret = mem_cgroup_resize_limit(memcg, val); >> - else >> + else if (type == _MEMSWAP) >> ret = mem_cgroup_resize_memsw_limit(memcg, val); >> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM >> + else if (type == _KMEM) { >> + ret = res_counter_set_limit(&memcg->kmem, val); >> + if (ret) >> + break; >> + /* >> + * Once enabled, can't be disabled. We could in theory >> + * disable it if we haven't yet created any caches, or >> + * if we can shrink them all to death. >> + * >> + * But it is not worth the trouble >> + */ >> + if (!memcg->kmem_accounted&& val != RESOURCE_MAX) >> + memcg->kmem_accounted = true; >> + } >> +#endif >> + else >> + return -EINVAL; >> break; >> case RES_SOFT_LIMIT: >> ret = res_counter_memparse_write_strategy(buffer,&val); > > Why is RESOURCE_MAX special? Because I am using the convention that setting it to any value different than that will enable accounting.
creator: Glauber Costa
date: 2012-05-02T15:15:43-00:00

Re: [PATCH 11/23] slub: consider a memcg parameter in kmem_create_cache
type: item
description: On 04/30/2012 04:51 PM, Suleiman Souhlal wrote: > On Fri, Apr 20, 2012 at 2:57 PM, Glauber Costa<glommer@parallels.com> wrote: >> Allow a memcg parameter to be passed during cache creation. >> The slub allocator will only merge caches that belong to >> the same memcg. >> >> Default function is created as a wrapper, passing NULL >> to the memcg version. We only merge caches that belong >> to the same memcg. >> >> From the memcontrol.c side, 3 helper functions are created: >> >> 1) memcg_css_id: because slub needs a unique cache name >> for sysfs. Since this is visible, but not the canonical >> location for slab data, the cache name is not used, the >> css_id should suffice. >> >> 2) mem_cgroup_register_cache: is responsible for assigning >> a unique index to each cache, and other general purpose >> setup. The index is only assigned for the root caches. All >> others are assigned index == -1. >> >> 3) mem_cgroup_release_cache: can be called from the root cache >> destruction, and will release the index for other caches. >> >> This index mechanism was developed by Suleiman Souhlal. >> >> Signed-off-by: Glauber Costa<glommer@parallels.com> >> CC: Christoph Lameter<cl@linux.com> >> CC: Pekka Enberg<penberg@cs.helsinki.fi> >> CC: Michal Hocko<mhocko@suse.cz> >> CC: Kamezawa Hiroyuki<kamezawa.hiroyu@jp.fujitsu.com> >> CC: Johannes Weiner<hannes@cmpxchg.org> >> CC: Suleiman Souhlal<suleiman@google.com> >> --- >> include/linux/memcontrol.h | 14 ++++++++++++++ >> include/linux/slab.h | 6 ++++++ >> mm/memcontrol.c | 29 +++++++++++++++++++++++++++++ >> mm/slub.c | 31 +++++++++++++++++++++++++++---- >> 4 files changed, 76 insertions(+), 4 deletions(-) >> >> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h >> index f94efd2..99e14b9 100644 >> --- a/include/linux/memcontrol.h >> +++ b/include/linux/memcontrol.h >> @@ -26,6 +26,7 @@ struct mem_cgroup; >> struct page_cgroup; >> struct page; >> struct mm_struct; >> +struct kmem_cache; >> >> /* Stats that can be upd…
creator: Glauber Costa
date: 2012-05-02T15:18:04-00:00

Re: [PATCH 17/23] kmem controller charge/uncharge infrastructure
type: item
description: On 04/30/2012 05:56 PM, Suleiman Souhlal wrote: >> + >> +static void kmem_cache_destroy_work_func(struct work_struct *w) >> +{ >> + struct kmem_cache *cachep; >> + char *name; >> + >> + spin_lock_irq(&cache_queue_lock); >> + while (!list_empty(&destroyed_caches)) { >> + cachep = container_of(list_first_entry(&destroyed_caches, >> + struct mem_cgroup_cache_params, destroyed_list), struct >> + kmem_cache, memcg_params); >> + name = (char *)cachep->name; >> + list_del(&cachep->memcg_params.destroyed_list); >> + spin_unlock_irq(&cache_queue_lock); >> + synchronize_rcu(); > > Is this synchronize_rcu() still needed, now that we don't use RCU to > protect memcgs from disappearing during allocation anymore? > > Also, should we drop the memcg reference we got in > memcg_create_kmem_cache() here? I have a reworked code I like better for this part. It reads as follows: static void kmem_cache_destroy_work_func(struct work_struct *w) { struct kmem_cache *cachep; const char *name; struct mem_cgroup_cache_params *params, *tmp; unsigned long flags; LIST_HEAD(delete_unlocked); synchronize_rcu(); spin_lock_irqsave(&cache_queue_lock, flags); list_for_each_entry_safe(params, tmp, &destroyed_caches, destroyed_list) { cachep = container_of(params, struct kmem_cache, memcg_params); list_move(&cachep->memcg_params.destroyed_list, &delete_unlocked); } spin_unlock_irqrestore(&cache_queue_lock, flags); list_for_each_entry_safe(params, tmp, &delete_unlocked, destroyed_list) { cachep = container_of(params, struct kmem_cache, memcg_params); list_del(&cachep->memcg_params.destroyed_list); name = cachep->name; mem_cgroup_put(cachep->memcg_params.memcg); kmem_cache_destroy(cachep); kfree(name); } } I think having a list in stack is better because we don't need to hold & drop the spinlock, and can achieve more parallelism if multiple cpus are scheduling the destroy worker. As you see, this version does a put() - so the answer to your question is yes. synchronize_rcu() also gets a new meaning, in the…
creator: Glauber Costa
date: 2012-05-02T15:34:44-00:00

Re: [PATCH 19/23] slab: per-memcg accounting of slab caches
type: item
description: >> @@ -3834,11 +3866,15 @@ static inline void __cache_free(struct kmem_cache *cachep, void *objp, >> */ >> void *kmem_cache_alloc(struct kmem_cache *cachep, gfp_t flags) >> { >> - void *ret = __cache_alloc(cachep, flags, __builtin_return_address(0)); >> + void *ret; >> + >> + rcu_read_lock(); >> + cachep = mem_cgroup_get_kmem_cache(cachep, flags); >> + rcu_read_unlock(); > > Don't we need to check in_interrupt(), current, __GFP_NOFAIL every > time we call mem_cgroup_cgroup_get_kmem_cache()? > > I would personally prefer if those checks were put inside > mem_cgroup_get_kmem_cache() instead of having to check for every > caller. > in_interrupt() yes, __GFP_NOFAIL I don't think so. __GFP_NOFAIL should lead to a res_counter_charge_nofail() in the end. The name similarity is no coincidence... From a code style PoV, it makes sense to bundle an in_interrupt() check here, but from a performance PoV, putting it in the callers can help us avoid the price of a function call. But well, looking at the code, I see it is not there as well... =( I plan to change memcontrol.h to look like this: static __always_inline struct kmem_cache * mem_cgroup_get_kmem_cache(struct kmem_cache *cachep, gfp_t gfp) { if (mem_cgroup_kmem_on && current->mm && !in_interrupt()) return __mem_cgroup_get_kmem_cache(cachep, gfp); return cachep; }
creator: Glauber Costa
date: 2012-05-02T15:40:37-00:00