Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
On 10/23/2012 09:50 PM, JoonSoo Kim wrote:
>> -struct kmem_cache *__kmem_cache_alias(const char *name, size_t size,
>> > - size_t align, unsigned long flags, void (*ctor)(void *))
>> > +struct kmem_cache *
>> > +__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t
>> > size,
>> > + size_t align, unsigned long flags, void (*ctor)(void *))
>> > {
>> > struct kmem_cache *s;
>> >
>> > - s = find_mergeable(size, align, flags, name, ctor);
>> > + s = find_mergeable(memcg, size, align, flags, name, ctor);
>> > if (s) {
>> > s->refcount++;
>> > /*
> If your intention is that find_mergeable() works for memcg-slab-caches
> properly,
> it cannot works properly with this code.
> When memcg is not NULL, slab cache is only added to memcg's slab cache list.
> find_mergeable() only interate on original-slab-cache list.
> So memcg slab cache never be mergeable.
Actually, recent results made me reconsider this.
I split this in multiple lists so we could transverse the lists faster
for /proc/slabinfo.
Turns out, there are many places that will rely on the ability to scan
through *all* caches in the system (root or not). This is one (easily
fixable) example, but there are others, like the hotplug handlers.
That said, I don't think that /proc/slabinfo is *that* performance
sensitive, so it is better to just skip the non-root caches, and just
keep all caches in the global list.
Maybe we would still benefit from a memcg-side list, for example, when
we're destructing memcg, so I'll consider keeping that (with a list
field in memcg_params). But even for that one, is still doable to
transverse the whole list...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
On 10/23/2012 09:50 PM, JoonSoo Kim wrote:
-struct kmem_cache *__kmem_cache_alias(const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *))
+struct kmem_cache *
+__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t
size,
+ size_t align, unsigned long flags, void (*ctor)(void *))
{
struct kmem_cache *s;
- s = find_mergeable(size, align, flags, name, ctor);
+ s = find_mergeable(memcg, size, align, flags, name, ctor);
if (s) {
s-refcount++;
/*
If your intention is that find_mergeable() works for memcg-slab-caches
properly,
it cannot works properly with this code.
When memcg is not NULL, slab cache is only added to memcg's slab cache list.
find_mergeable() only interate on original-slab-cache list.
So memcg slab cache never be mergeable.
Actually, recent results made me reconsider this.
I split this in multiple lists so we could transverse the lists faster
for /proc/slabinfo.
Turns out, there are many places that will rely on the ability to scan
through *all* caches in the system (root or not). This is one (easily
fixable) example, but there are others, like the hotplug handlers.
That said, I don't think that /proc/slabinfo is *that* performance
sensitive, so it is better to just skip the non-root caches, and just
keep all caches in the global list.
Maybe we would still benefit from a memcg-side list, for example, when
we're destructing memcg, so I'll consider keeping that (with a list
field in memcg_params). But even for that one, is still doable to
transverse the whole list...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
On 10/23/2012 09:50 PM, JoonSoo Kim wrote:
> 2012/10/19 Glauber Costa :
>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 6a1e096..59f6d54 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -339,6 +339,12 @@ struct mem_cgroup {
>> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
>> struct tcp_memcontrol tcp_mem;
>> #endif
>> +#if defined(CONFIG_MEMCG_KMEM)
>> + /* analogous to slab_common's slab_caches list. per-memcg */
>> + struct list_head memcg_slab_caches;
>> + /* Not a spinlock, we can take a lot of time walking the list */
>> + struct mutex slab_caches_mutex;
>> +#endif
>> };
>
> It is better to change name of "slab_caches_mutex to something
> representing slab cache mutex of memcg.
>
Why do you think so? I particularly think keeping the same name is
better, because it shows us that this is a memcg version of the global
mutex, and wraps all caches. One is unlikely to confuse between them,
because one of them is a global name, and the other is always inside a
struct.
>> +int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s)
>> +{
>> + size_t size = sizeof(struct memcg_cache_params);
>> +
>> + if (!memcg_kmem_enabled())
>> + return 0;
>> +
>> + s->memcg_params = kzalloc(size, GFP_KERNEL);
>> + if (!s->memcg_params)
>> + return -ENOMEM;
>> +
>> + if (memcg)
>> + s->memcg_params->memcg = memcg;
>> + return 0;
>> +}
>
> Now, I don't read full-patchset and I have not enough knowledge about cgroup.
> So I have a question.
> When memcg_kmem_enable, creation kmem_cache of normal user(not
> included in cgroup) also allocate memcg_params.
> Is it right behavior?
>
Yes. I would even add that *specially* the normal kmem cache gets this.
This is where it will hold the vector of memcg caches.
Also note this only happens when a memcg becomes kmem-limited, so people
not using memcg at all won't pay this price.
This should be documented over struct memcg_params.
>> +static inline bool cache_match_memcg(struct kmem_cache *cachep,
>> +struct mem_cgroup *memcg)
>> +{
>> + return (is_root_cache(cachep) && !memcg) ||
>> + (cachep->memcg_params->memcg == memcg);
>> +}
>
> When cachep->memcg_params == NULL and memcg is not NULL, NULL pointer
> deref may be occurred.
> Is there no situation like above?
>
is_root_cache already test for this, so if this is the case, we'll exit
the conditional on that test.
Every cache without a valid memcg_params pointer is a root cache.
If memcg != NULL, and memcg_params == NULL, this is a serious bug.
I just didn't BUG(), because I really didn't see the point. It is just
added instructions, and we're unlikely to make that far in that case.
>> dump_stack();
>> @@ -66,7 +68,8 @@ static int kmem_cache_sanity_check(const char *name,
>> size_t size)
>> return 0;
>> }
>> #else
>> -static inline int kmem_cache_sanity_check(const char *name, size_t size)
>> +static inline int kmem_cache_sanity_check(struct mem_cgroup *memcg,
>> + const char *name, size_t size)
>> {
>> return 0;
>> }
>> @@ -97,8 +100,9 @@ static inline int kmem_cache_sanity_check(const char
>> *name, size_t size)
>> * as davem.
>> */
>>
>> -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,
>> + size_t align, unsigned long flags, void (*ctor)(void
>> *))
>> {
>> struct kmem_cache *s = NULL;
>> int err = 0;
>> @@ -106,7 +110,7 @@ struct kmem_cache *kmem_cache_create(const char *name,
>> size_t size, size_t align
>> get_online_cpus();
>> mutex_lock(_mutex);
>>
>> - if (!kmem_cache_sanity_check(name, size) == 0)
>> + if (!kmem_cache_sanity_check(memcg, name, size) == 0)
>> goto out_locked;
>
> This compare is somewhat ugly.
> How about "if(kmem_cache_sanity_check(memcg, name, size))"?
>
Well, the check was already there =)
At least it becomes clear that the only thing I am changing is that i
know pass a memcg pointer.
As for the proposed change, it gives the impression that if you succeed
in the check, you will exit the function.
At least this way, it becomes clear that you exit when you fail a
sanity_check. So taking the semantics into consideration, I don't really
dislike it.
>> {
>> @@ -3916,17 +3917,20 @@ static struct kmem_cache *find_mergeable(size_t size,
>> if (s->size - size >= sizeof(void *))
>> continue;
>>
>> + if (!cache_match_memcg(s, memcg))
>> + continue;
>> return s;
>> }
>> return NULL;
>> }
>>
>>
Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
On 10/23/2012 09:50 PM, JoonSoo Kim wrote:
2012/10/19 Glauber Costa glom...@parallels.com:
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6a1e096..59f6d54 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -339,6 +339,12 @@ struct mem_cgroup {
#if defined(CONFIG_MEMCG_KMEM) defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
+#if defined(CONFIG_MEMCG_KMEM)
+ /* analogous to slab_common's slab_caches list. per-memcg */
+ struct list_head memcg_slab_caches;
+ /* Not a spinlock, we can take a lot of time walking the list */
+ struct mutex slab_caches_mutex;
+#endif
};
It is better to change name of slab_caches_mutex to something
representing slab cache mutex of memcg.
Why do you think so? I particularly think keeping the same name is
better, because it shows us that this is a memcg version of the global
mutex, and wraps all caches. One is unlikely to confuse between them,
because one of them is a global name, and the other is always inside a
struct.
+int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s)
+{
+ size_t size = sizeof(struct memcg_cache_params);
+
+ if (!memcg_kmem_enabled())
+ return 0;
+
+ s-memcg_params = kzalloc(size, GFP_KERNEL);
+ if (!s-memcg_params)
+ return -ENOMEM;
+
+ if (memcg)
+ s-memcg_params-memcg = memcg;
+ return 0;
+}
Now, I don't read full-patchset and I have not enough knowledge about cgroup.
So I have a question.
When memcg_kmem_enable, creation kmem_cache of normal user(not
included in cgroup) also allocate memcg_params.
Is it right behavior?
Yes. I would even add that *specially* the normal kmem cache gets this.
This is where it will hold the vector of memcg caches.
Also note this only happens when a memcg becomes kmem-limited, so people
not using memcg at all won't pay this price.
This should be documented over struct memcg_params.
+static inline bool cache_match_memcg(struct kmem_cache *cachep,
+struct mem_cgroup *memcg)
+{
+ return (is_root_cache(cachep) !memcg) ||
+ (cachep-memcg_params-memcg == memcg);
+}
When cachep-memcg_params == NULL and memcg is not NULL, NULL pointer
deref may be occurred.
Is there no situation like above?
is_root_cache already test for this, so if this is the case, we'll exit
the conditional on that test.
Every cache without a valid memcg_params pointer is a root cache.
If memcg != NULL, and memcg_params == NULL, this is a serious bug.
I just didn't BUG(), because I really didn't see the point. It is just
added instructions, and we're unlikely to make that far in that case.
dump_stack();
@@ -66,7 +68,8 @@ static int kmem_cache_sanity_check(const char *name,
size_t size)
return 0;
}
#else
-static inline int kmem_cache_sanity_check(const char *name, size_t size)
+static inline int kmem_cache_sanity_check(struct mem_cgroup *memcg,
+ const char *name, size_t size)
{
return 0;
}
@@ -97,8 +100,9 @@ static inline int kmem_cache_sanity_check(const char
*name, size_t size)
* as davem.
*/
-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,
+ size_t align, unsigned long flags, void (*ctor)(void
*))
{
struct kmem_cache *s = NULL;
int err = 0;
@@ -106,7 +110,7 @@ struct kmem_cache *kmem_cache_create(const char *name,
size_t size, size_t align
get_online_cpus();
mutex_lock(slab_mutex);
- if (!kmem_cache_sanity_check(name, size) == 0)
+ if (!kmem_cache_sanity_check(memcg, name, size) == 0)
goto out_locked;
This compare is somewhat ugly.
How about if(kmem_cache_sanity_check(memcg, name, size))?
Well, the check was already there =)
At least it becomes clear that the only thing I am changing is that i
know pass a memcg pointer.
As for the proposed change, it gives the impression that if you succeed
in the check, you will exit the function.
At least this way, it becomes clear that you exit when you fail a
sanity_check. So taking the semantics into consideration, I don't really
dislike it.
{
@@ -3916,17 +3917,20 @@ static struct kmem_cache *find_mergeable(size_t size,
if (s-size - size = sizeof(void *))
continue;
+ if (!cache_match_memcg(s, memcg))
+ continue;
return s;
}
return NULL;
}
-struct kmem_cache *__kmem_cache_alias(const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *))
+struct kmem_cache *
Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
2012/10/19 Glauber Costa :
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6a1e096..59f6d54 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -339,6 +339,12 @@ struct mem_cgroup {
> #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
> struct tcp_memcontrol tcp_mem;
> #endif
> +#if defined(CONFIG_MEMCG_KMEM)
> + /* analogous to slab_common's slab_caches list. per-memcg */
> + struct list_head memcg_slab_caches;
> + /* Not a spinlock, we can take a lot of time walking the list */
> + struct mutex slab_caches_mutex;
> +#endif
> };
It is better to change name of "slab_caches_mutex to something
representing slab cache mutex of memcg.
> +int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s)
> +{
> + size_t size = sizeof(struct memcg_cache_params);
> +
> + if (!memcg_kmem_enabled())
> + return 0;
> +
> + s->memcg_params = kzalloc(size, GFP_KERNEL);
> + if (!s->memcg_params)
> + return -ENOMEM;
> +
> + if (memcg)
> + s->memcg_params->memcg = memcg;
> + return 0;
> +}
Now, I don't read full-patchset and I have not enough knowledge about cgroup.
So I have a question.
When memcg_kmem_enable, creation kmem_cache of normal user(not
included in cgroup) also allocate memcg_params.
Is it right behavior?
> +void memcg_release_cache(struct kmem_cache *s)
> +{
> + kfree(s->memcg_params);
> +}
> +
> /*
> * We need to verify if the allocation against current->mm->owner's memcg is
> * possible for the given order. But the page is not allocated yet, so we'll
> diff --git a/mm/slab.h b/mm/slab.h
> index 5ee1851..c35ecce 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -35,12 +35,15 @@ extern struct kmem_cache *kmem_cache;
> /* Functions provided by the slab allocators */
> extern int __kmem_cache_create(struct kmem_cache *, unsigned long flags);
>
> +struct mem_cgroup;
> #ifdef CONFIG_SLUB
> -struct kmem_cache *__kmem_cache_alias(const char *name, size_t size,
> - size_t align, unsigned long flags, void (*ctor)(void *));
> +struct kmem_cache *
> +__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
> + size_t align, unsigned long flags, void (*ctor)(void *));
> #else
> -static inline struct kmem_cache *__kmem_cache_alias(const char *name, size_t
> size,
> - size_t align, unsigned long flags, void (*ctor)(void *))
> +static inline struct kmem_cache *
> +__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
> + size_t align, unsigned long flags, void (*ctor)(void *))
> { return NULL; }
> #endif
>
> @@ -98,11 +101,23 @@ static inline bool is_root_cache(struct kmem_cache *s)
> {
> return !s->memcg_params || s->memcg_params->is_root_cache;
> }
> +
> +static inline bool cache_match_memcg(struct kmem_cache *cachep,
> +struct mem_cgroup *memcg)
> +{
> + return (is_root_cache(cachep) && !memcg) ||
> + (cachep->memcg_params->memcg == memcg);
> +}
When cachep->memcg_params == NULL and memcg is not NULL, NULL pointer
deref may be occurred.
Is there no situation like above?
> #else
> static inline bool is_root_cache(struct kmem_cache *s)
> {
> return true;
> }
>
> +static inline bool cache_match_memcg(struct kmem_cache *cachep,
> +struct mem_cgroup *memcg)
> +{
> + return true;
> +}
> #endif
> #endif
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index bf4b4f1..f97f7b8 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include "slab.h"
>
> @@ -27,7 +28,8 @@ DEFINE_MUTEX(slab_mutex);
> struct kmem_cache *kmem_cache;
>
> #ifdef CONFIG_DEBUG_VM
> -static int kmem_cache_sanity_check(const char *name, size_t size)
> +static int kmem_cache_sanity_check(struct mem_cgroup *memcg, const char
> *name,
> + size_t size)
> {
> struct kmem_cache *s = NULL;
>
> @@ -53,7 +55,7 @@ static int kmem_cache_sanity_check(const char *name, size_t
> size)
> continue;
> }
>
> - if (!strcmp(s->name, name)) {
> + if (cache_match_memcg(s, memcg) && !strcmp(s->name, name)) {
> pr_err("%s (%s): Cache name already exists.\n",
>__func__, name);
> dump_stack();
> @@ -66,7 +68,8 @@ static int kmem_cache_sanity_check(const char *name, size_t
> size)
> return 0;
> }
> #else
> -static inline int kmem_cache_sanity_check(const char *name, size_t size)
> +static inline int kmem_cache_sanity_check(struct mem_cgroup *memcg,
> + const char *name, size_t size)
> {
> return 0;
> }
> @@ -97,8 +100,9 @@ static inline int kmem_cache_sanity_check(const char
> *name,
Re: [PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
2012/10/19 Glauber Costa glom...@parallels.com:
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6a1e096..59f6d54 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -339,6 +339,12 @@ struct mem_cgroup {
#if defined(CONFIG_MEMCG_KMEM) defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
+#if defined(CONFIG_MEMCG_KMEM)
+ /* analogous to slab_common's slab_caches list. per-memcg */
+ struct list_head memcg_slab_caches;
+ /* Not a spinlock, we can take a lot of time walking the list */
+ struct mutex slab_caches_mutex;
+#endif
};
It is better to change name of slab_caches_mutex to something
representing slab cache mutex of memcg.
+int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s)
+{
+ size_t size = sizeof(struct memcg_cache_params);
+
+ if (!memcg_kmem_enabled())
+ return 0;
+
+ s-memcg_params = kzalloc(size, GFP_KERNEL);
+ if (!s-memcg_params)
+ return -ENOMEM;
+
+ if (memcg)
+ s-memcg_params-memcg = memcg;
+ return 0;
+}
Now, I don't read full-patchset and I have not enough knowledge about cgroup.
So I have a question.
When memcg_kmem_enable, creation kmem_cache of normal user(not
included in cgroup) also allocate memcg_params.
Is it right behavior?
+void memcg_release_cache(struct kmem_cache *s)
+{
+ kfree(s-memcg_params);
+}
+
/*
* We need to verify if the allocation against current-mm-owner's memcg is
* possible for the given order. But the page is not allocated yet, so we'll
diff --git a/mm/slab.h b/mm/slab.h
index 5ee1851..c35ecce 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -35,12 +35,15 @@ extern struct kmem_cache *kmem_cache;
/* Functions provided by the slab allocators */
extern int __kmem_cache_create(struct kmem_cache *, unsigned long flags);
+struct mem_cgroup;
#ifdef CONFIG_SLUB
-struct kmem_cache *__kmem_cache_alias(const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *));
+struct kmem_cache *
+__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
+ size_t align, unsigned long flags, void (*ctor)(void *));
#else
-static inline struct kmem_cache *__kmem_cache_alias(const char *name, size_t
size,
- size_t align, unsigned long flags, void (*ctor)(void *))
+static inline struct kmem_cache *
+__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
+ size_t align, unsigned long flags, void (*ctor)(void *))
{ return NULL; }
#endif
@@ -98,11 +101,23 @@ static inline bool is_root_cache(struct kmem_cache *s)
{
return !s-memcg_params || s-memcg_params-is_root_cache;
}
+
+static inline bool cache_match_memcg(struct kmem_cache *cachep,
+struct mem_cgroup *memcg)
+{
+ return (is_root_cache(cachep) !memcg) ||
+ (cachep-memcg_params-memcg == memcg);
+}
When cachep-memcg_params == NULL and memcg is not NULL, NULL pointer
deref may be occurred.
Is there no situation like above?
#else
static inline bool is_root_cache(struct kmem_cache *s)
{
return true;
}
+static inline bool cache_match_memcg(struct kmem_cache *cachep,
+struct mem_cgroup *memcg)
+{
+ return true;
+}
#endif
#endif
diff --git a/mm/slab_common.c b/mm/slab_common.c
index bf4b4f1..f97f7b8 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -18,6 +18,7 @@
#include asm/cacheflush.h
#include asm/tlbflush.h
#include asm/page.h
+#include linux/memcontrol.h
#include slab.h
@@ -27,7 +28,8 @@ DEFINE_MUTEX(slab_mutex);
struct kmem_cache *kmem_cache;
#ifdef CONFIG_DEBUG_VM
-static int kmem_cache_sanity_check(const char *name, size_t size)
+static int kmem_cache_sanity_check(struct mem_cgroup *memcg, const char
*name,
+ size_t size)
{
struct kmem_cache *s = NULL;
@@ -53,7 +55,7 @@ static int kmem_cache_sanity_check(const char *name, size_t
size)
continue;
}
- if (!strcmp(s-name, name)) {
+ if (cache_match_memcg(s, memcg) !strcmp(s-name, name)) {
pr_err(%s (%s): Cache name already exists.\n,
__func__, name);
dump_stack();
@@ -66,7 +68,8 @@ static int kmem_cache_sanity_check(const char *name, size_t
size)
return 0;
}
#else
-static inline int kmem_cache_sanity_check(const char *name, size_t size)
+static inline int kmem_cache_sanity_check(struct mem_cgroup *memcg,
+ const char *name, size_t size)
{
return 0;
}
@@ -97,8 +100,9 @@ static inline int kmem_cache_sanity_check(const char
*name, size_t size)
* as davem.
*/
-struct kmem_cache *kmem_cache_create(const
[PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
Allow a memcg parameter to be passed during cache creation. When the slub
allocator is being used, it 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.
A helper is provided, 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.
[ v2: moved to idr/ida instead of redoing the indexes ]
[ v3: moved call to ida_init away from cgroup creation to fix a bug ]
[ v4: no longer using the index mechanism ]
Signed-off-by: Glauber Costa
CC: Christoph Lameter
CC: Pekka Enberg
CC: Michal Hocko
CC: Kamezawa Hiroyuki
CC: Johannes Weiner
CC: Suleiman Souhlal
CC: Tejun Heo
---
include/linux/memcontrol.h | 22 +
include/linux/slab.h | 12 +++-
mm/memcontrol.c| 49 ++
mm/slab.h | 23 ++
mm/slab_common.c | 40 +++--
mm/slub.c | 16 +++
6 files changed, 143 insertions(+), 19 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index fd4ff56..4f89a45 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -28,6 +28,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 {
@@ -414,6 +415,11 @@ void __memcg_kmem_commit_charge(struct page *page,
struct mem_cgroup *memcg, int order);
void __memcg_kmem_uncharge_pages(struct page *page, int order);
+int memcg_css_id(struct mem_cgroup *memcg);
+int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s);
+void memcg_release_cache(struct kmem_cache *cachep);
+void memcg_cache_list_add(struct mem_cgroup *memcg, struct kmem_cache *cachep);
+
/**
* memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
* @gfp: the gfp allocation flags.
@@ -505,6 +511,22 @@ static inline void
memcg_kmem_commit_charge(struct page *page, struct mem_cgroup *memcg, int
order)
{
}
+
+static inline int memcg_register_cache(struct mem_cgroup *memcg,
+ struct kmem_cache *s)
+{
+ return 0;
+}
+
+static inline void memcg_release_cache(struct kmem_cache *cachep)
+{
+}
+
+static inline void memcg_cache_list_add(struct mem_cgroup *memcg,
+ struct kmem_cache *s)
+{
+ BUG();
+}
#endif /* CONFIG_MEMCG_KMEM */
#endif /* _LINUX_MEMCONTROL_H */
diff --git a/include/linux/slab.h b/include/linux/slab.h
index e4ea48a..b22a158 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -116,6 +116,7 @@ struct kmem_cache {
};
#endif
+struct mem_cgroup;
/*
* struct kmem_cache related prototypes
*/
@@ -125,6 +126,9 @@ int slab_is_available(void);
struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
unsigned long,
void (*)(void *));
+struct kmem_cache *
+kmem_cache_create_memcg(struct mem_cgroup *, const char *, size_t, size_t,
+ unsigned long, void (*)(void *));
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
void kmem_cache_free(struct kmem_cache *, void *);
@@ -193,15 +197,21 @@ unsigned int kmem_cache_size(struct kmem_cache *);
* Child caches will hold extra metadata needed for its operation. Fields are:
*
* @memcg: pointer to the memcg this cache belongs to
+ * @root_cache: pointer to the global, root cache, this cache was derived from
*/
struct memcg_cache_params {
bool is_root_cache;
union {
struct kmem_cache *memcg_caches[0];
- struct mem_cgroup *memcg;
+ struct {
+ struct mem_cgroup *memcg;
+ struct kmem_cache *root_cache;
+ };
};
};
+int memcg_update_all_caches(int num_memcgs);
+
/*
* Common kmalloc functions provided by all allocators
*/
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6a1e096..59f6d54 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -339,6 +339,12 @@ struct mem_cgroup {
#if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
+#if defined(CONFIG_MEMCG_KMEM)
+ /* analogous to slab_common's slab_caches list. per-memcg */
+ struct list_head memcg_slab_caches;
+ /* Not a spinlock, we can take a lot of time walking the list */
+ struct mutex slab_caches_mutex;
+#endif
};
/* internal only representation about the status of kmem accounting. */
@@ -2755,6 +2761,49 @@ static void memcg_uncharge_kmem(struct mem_cgroup
*memcg, u64 size)
[PATCH v5 06/18] consider a memcg parameter in kmem_create_cache
Allow a memcg parameter to be passed during cache creation. When the slub
allocator is being used, it 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.
A helper is provided, 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.
[ v2: moved to idr/ida instead of redoing the indexes ]
[ v3: moved call to ida_init away from cgroup creation to fix a bug ]
[ v4: no longer using the index mechanism ]
Signed-off-by: Glauber Costa glom...@parallels.com
CC: Christoph Lameter c...@linux.com
CC: Pekka Enberg penb...@cs.helsinki.fi
CC: Michal Hocko mho...@suse.cz
CC: Kamezawa Hiroyuki kamezawa.hir...@jp.fujitsu.com
CC: Johannes Weiner han...@cmpxchg.org
CC: Suleiman Souhlal sulei...@google.com
CC: Tejun Heo t...@kernel.org
---
include/linux/memcontrol.h | 22 +
include/linux/slab.h | 12 +++-
mm/memcontrol.c| 49 ++
mm/slab.h | 23 ++
mm/slab_common.c | 40 +++--
mm/slub.c | 16 +++
6 files changed, 143 insertions(+), 19 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index fd4ff56..4f89a45 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -28,6 +28,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 {
@@ -414,6 +415,11 @@ void __memcg_kmem_commit_charge(struct page *page,
struct mem_cgroup *memcg, int order);
void __memcg_kmem_uncharge_pages(struct page *page, int order);
+int memcg_css_id(struct mem_cgroup *memcg);
+int memcg_register_cache(struct mem_cgroup *memcg, struct kmem_cache *s);
+void memcg_release_cache(struct kmem_cache *cachep);
+void memcg_cache_list_add(struct mem_cgroup *memcg, struct kmem_cache *cachep);
+
/**
* memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
* @gfp: the gfp allocation flags.
@@ -505,6 +511,22 @@ static inline void
memcg_kmem_commit_charge(struct page *page, struct mem_cgroup *memcg, int
order)
{
}
+
+static inline int memcg_register_cache(struct mem_cgroup *memcg,
+ struct kmem_cache *s)
+{
+ return 0;
+}
+
+static inline void memcg_release_cache(struct kmem_cache *cachep)
+{
+}
+
+static inline void memcg_cache_list_add(struct mem_cgroup *memcg,
+ struct kmem_cache *s)
+{
+ BUG();
+}
#endif /* CONFIG_MEMCG_KMEM */
#endif /* _LINUX_MEMCONTROL_H */
diff --git a/include/linux/slab.h b/include/linux/slab.h
index e4ea48a..b22a158 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -116,6 +116,7 @@ struct kmem_cache {
};
#endif
+struct mem_cgroup;
/*
* struct kmem_cache related prototypes
*/
@@ -125,6 +126,9 @@ int slab_is_available(void);
struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
unsigned long,
void (*)(void *));
+struct kmem_cache *
+kmem_cache_create_memcg(struct mem_cgroup *, const char *, size_t, size_t,
+ unsigned long, void (*)(void *));
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
void kmem_cache_free(struct kmem_cache *, void *);
@@ -193,15 +197,21 @@ unsigned int kmem_cache_size(struct kmem_cache *);
* Child caches will hold extra metadata needed for its operation. Fields are:
*
* @memcg: pointer to the memcg this cache belongs to
+ * @root_cache: pointer to the global, root cache, this cache was derived from
*/
struct memcg_cache_params {
bool is_root_cache;
union {
struct kmem_cache *memcg_caches[0];
- struct mem_cgroup *memcg;
+ struct {
+ struct mem_cgroup *memcg;
+ struct kmem_cache *root_cache;
+ };
};
};
+int memcg_update_all_caches(int num_memcgs);
+
/*
* Common kmalloc functions provided by all allocators
*/
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6a1e096..59f6d54 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -339,6 +339,12 @@ struct mem_cgroup {
#if defined(CONFIG_MEMCG_KMEM) defined(CONFIG_INET)
struct tcp_memcontrol tcp_mem;
#endif
+#if defined(CONFIG_MEMCG_KMEM)
+ /* analogous to slab_common's slab_caches list. per-memcg */
+ struct list_head memcg_slab_caches;
+ /* Not a spinlock, we can take a lot of time walking the list */
+ struct mutex slab_caches_mutex;
+#endif
};
/* internal only
