* [PATCH -mm 1/6] memcg: get rid of memcg_create_cache_name
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 2/6] memcg: allocate memcg_caches array on first per memcg cache creation Vladimir Davydov
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
Instead of calling back to memcontrol.c from kmem_cache_create_memcg in
order to just create the name of a per memcg cache, let's allocate it in
place. We only need to pass the memcg name to kmem_cache_create_memcg
for that - everything else can be done in slab_common.c.
This patch also turns the buffer used to store memcg names to be
statically allocated while before it was kmalloc'ed on first use.
Saving those 256 bytes isn't worth complicating the code. If someone is
so tight on memory, they'd better disable CONFIG_MEMCG_KMEM altogether.
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/memcontrol.h | 2 --
include/linux/slab.h | 3 ++-
mm/memcontrol.c | 27 +++------------------------
mm/slab_common.c | 7 +++++--
4 files changed, 10 insertions(+), 29 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 1fa23244fe37..dfc2929a3877 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -492,8 +492,6 @@ void __memcg_kmem_uncharge_pages(struct page *page, int order);
int memcg_cache_id(struct mem_cgroup *memcg);
-char *memcg_create_cache_name(struct mem_cgroup *memcg,
- struct kmem_cache *root_cache);
int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
struct kmem_cache *root_cache);
void memcg_free_cache_params(struct kmem_cache *s);
diff --git a/include/linux/slab.h b/include/linux/slab.h
index ecbec9ccb80d..86e5b26fbdab 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -117,7 +117,8 @@ struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
void (*)(void *));
#ifdef CONFIG_MEMCG_KMEM
struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *,
- struct kmem_cache *);
+ struct kmem_cache *,
+ const char *);
#endif
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 19d620b3d69c..605d72044533 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3105,29 +3105,6 @@ int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
return 0;
}
-char *memcg_create_cache_name(struct mem_cgroup *memcg,
- struct kmem_cache *root_cache)
-{
- static char *buf = NULL;
-
- /*
- * We need a mutex here to protect the shared buffer. Since this is
- * expected to be called only on cache creation, we can employ the
- * slab_mutex for that purpose.
- */
- lockdep_assert_held(&slab_mutex);
-
- if (!buf) {
- buf = kmalloc(NAME_MAX + 1, GFP_KERNEL);
- if (!buf)
- return NULL;
- }
-
- cgroup_name(memcg->css.cgroup, buf, NAME_MAX + 1);
- return kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
- memcg_cache_id(memcg), buf);
-}
-
int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
struct kmem_cache *root_cache)
{
@@ -3168,6 +3145,7 @@ void memcg_free_cache_params(struct kmem_cache *s)
static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
struct kmem_cache *root_cache)
{
+ static char memcg_name[NAME_MAX + 1];
struct kmem_cache *cachep;
int id;
@@ -3183,7 +3161,8 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
if (cache_from_memcg_idx(root_cache, id))
return;
- cachep = kmem_cache_create_memcg(memcg, root_cache);
+ cgroup_name(memcg->css.cgroup, memcg_name, NAME_MAX + 1);
+ cachep = kmem_cache_create_memcg(memcg, root_cache, memcg_name);
/*
* If we could not create a memcg cache, do not complain, because
* that's not critical at all as we can always proceed with the root
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 4f16f374392a..6b6397d2e05e 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -264,13 +264,15 @@ EXPORT_SYMBOL(kmem_cache_create);
* kmem_cache_create_memcg - Create a cache for a memory cgroup.
* @memcg: The memory cgroup the new cache is for.
* @root_cache: The parent of the new cache.
+ * @memcg_name: The name of the memory cgroup.
*
* This function attempts to create a kmem cache that will serve allocation
* requests going from @memcg to @root_cache. The new cache inherits properties
* from its parent.
*/
struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg,
- struct kmem_cache *root_cache)
+ struct kmem_cache *root_cache,
+ const char *memcg_name)
{
struct kmem_cache *s = NULL;
char *cache_name;
@@ -280,7 +282,8 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg,
mutex_lock(&slab_mutex);
- cache_name = memcg_create_cache_name(memcg, root_cache);
+ cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
+ memcg_cache_id(memcg), memcg_name);
if (!cache_name)
goto out_unlock;
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm 2/6] memcg: allocate memcg_caches array on first per memcg cache creation
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 1/6] memcg: get rid of memcg_create_cache_name Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 3/6] memcg: cleanup memcg_caches arrays relocation path Vladimir Davydov
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
There is no need to allocate the memcg_caches array on kmem cache
creation, because we perfectly handle root caches without memcg_params
everywhere. Since not all caches will necessarily be used by memory
cgroups, this rather looks like a waste of memory. So let's allocate
the memcg_caches array on the first per memcg cache creation.
A couple of things to note about this patch:
- after it is applied memcg_{alloc, free}_cache_params do nothing for
root caches so that they can be inlined into memcg_kmem_{create,
destroy}_cache;
- since memcg_limited_groups_array_size is now accessed under
activate_kmem_mutex, we can move memcg_limited_groups_array_size
update (memcg_update_array_size) out of slab_mutex, which is nice,
because it isn't something specific to kmem caches - it will be used
in per memcg list_lru in the future;
- memcg_caches allocation scheme is now spread between slab_common.c
and memcontrol.c, but that'll be fixed by the next patches where I'll
move it completely to slab_common.c
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/slab.h | 1 +
mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++-------------------
mm/slab_common.c | 41 ++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 24 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 86e5b26fbdab..095ce8e47a36 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -119,6 +119,7 @@ struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *,
struct kmem_cache *,
const char *);
+int kmem_cache_init_memcg_array(struct kmem_cache *, int);
#endif
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 605d72044533..fa7cbce8f0cf 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3059,6 +3059,9 @@ int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
VM_BUG_ON(!is_root_cache(s));
+ if (!cur_params)
+ return 0;
+
if (num_groups > memcg_limited_groups_array_size) {
int i;
struct memcg_cache_params *new_params;
@@ -3099,8 +3102,7 @@ int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
* anyway.
*/
rcu_assign_pointer(s->memcg_params, new_params);
- if (cur_params)
- kfree_rcu(cur_params, rcu_head);
+ kfree_rcu(cur_params, rcu_head);
}
return 0;
}
@@ -3108,40 +3110,52 @@ int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
struct kmem_cache *root_cache)
{
- size_t size;
-
- if (!memcg_kmem_enabled())
+ if (!memcg)
return 0;
- if (!memcg) {
- size = offsetof(struct memcg_cache_params, memcg_caches);
- size += memcg_limited_groups_array_size * sizeof(void *);
- } else
- size = sizeof(struct memcg_cache_params);
-
- s->memcg_params = kzalloc(size, GFP_KERNEL);
+ s->memcg_params = kzalloc(sizeof(*s->memcg_params), GFP_KERNEL);
if (!s->memcg_params)
return -ENOMEM;
- if (memcg) {
- s->memcg_params->memcg = memcg;
- s->memcg_params->root_cache = root_cache;
- css_get(&memcg->css);
- } else
- s->memcg_params->is_root_cache = true;
+ s->memcg_params->memcg = memcg;
+ s->memcg_params->root_cache = root_cache;
+ css_get(&memcg->css);
return 0;
}
void memcg_free_cache_params(struct kmem_cache *s)
{
- if (!s->memcg_params)
+ if (is_root_cache(s))
return;
- if (!s->memcg_params->is_root_cache)
- css_put(&s->memcg_params->memcg->css);
+ css_put(&s->memcg_params->memcg->css);
kfree(s->memcg_params);
}
+/*
+ * Prepares the memcg_caches array of the given kmem cache for disposing
+ * memcgs' copies.
+ */
+static int memcg_prepare_kmem_cache(struct kmem_cache *cachep)
+{
+ int ret;
+
+ BUG_ON(!is_root_cache(cachep));
+
+ if (cachep->memcg_params)
+ return 0;
+
+ /* activate_kmem_mutex guarantees a stable value of
+ * memcg_limited_groups_array_size */
+ mutex_lock(&activate_kmem_mutex);
+ mutex_lock(&memcg_slab_mutex);
+ ret = kmem_cache_init_memcg_array(cachep,
+ memcg_limited_groups_array_size);
+ mutex_unlock(&memcg_slab_mutex);
+ mutex_unlock(&activate_kmem_mutex);
+ return ret;
+}
+
static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
struct kmem_cache *root_cache)
{
@@ -3287,10 +3301,13 @@ static void memcg_create_cache_work_func(struct work_struct *w)
struct mem_cgroup *memcg = cw->memcg;
struct kmem_cache *cachep = cw->cachep;
+ if (memcg_prepare_kmem_cache(cachep) != 0)
+ goto out;
+
mutex_lock(&memcg_slab_mutex);
memcg_kmem_create_cache(memcg, cachep);
mutex_unlock(&memcg_slab_mutex);
-
+out:
css_put(&memcg->css);
kfree(cw);
}
@@ -3371,8 +3388,7 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
struct mem_cgroup *memcg;
struct kmem_cache *memcg_cachep;
- VM_BUG_ON(!cachep->memcg_params);
- VM_BUG_ON(!cachep->memcg_params->is_root_cache);
+ VM_BUG_ON(!is_root_cache(cachep));
if (!current->mm || current->memcg_kmem_skip_account)
return cachep;
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 6b6397d2e05e..44bfc4b1ee09 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -77,6 +77,45 @@ static inline int kmem_cache_sanity_check(const char *name, size_t size)
#endif
#ifdef CONFIG_MEMCG_KMEM
+static int kmem_cache_alloc_memcg_array(struct kmem_cache *s, int nr_entries)
+{
+ size_t size;
+ struct memcg_cache_params *new;
+
+ size = offsetof(struct memcg_cache_params, memcg_caches);
+ size += nr_entries * sizeof(void *);
+
+ new = kzalloc(size, GFP_KERNEL);
+ if (!new)
+ return -ENOMEM;
+
+ new->is_root_cache = true;
+
+ /* matching rcu_dereference is in cache_from_memcg_idx */
+ rcu_assign_pointer(s->memcg_params, new);
+
+ return 0;
+}
+
+static void kmem_cache_free_memcg_array(struct kmem_cache *s)
+{
+ kfree(s->memcg_params);
+ s->memcg_params = NULL;
+}
+
+int kmem_cache_init_memcg_array(struct kmem_cache *s, int nr_entries)
+{
+ int ret = 0;
+
+ BUG_ON(!is_root_cache(s));
+
+ mutex_lock(&slab_mutex);
+ if (!s->memcg_params)
+ ret = kmem_cache_alloc_memcg_array(s, nr_entries);
+ mutex_unlock(&slab_mutex);
+ return ret;
+}
+
int memcg_update_all_caches(int num_memcgs)
{
struct kmem_cache *s;
@@ -317,6 +356,8 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
rc = __kmem_cache_destroy_memcg_children(s);
mutex_lock(&slab_mutex);
+ if (rc == 0)
+ kmem_cache_free_memcg_array(s);
return rc;
}
#else
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm 3/6] memcg: cleanup memcg_caches arrays relocation path
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 1/6] memcg: get rid of memcg_create_cache_name Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 2/6] memcg: allocate memcg_caches array on first per memcg cache creation Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 4/6] memcg: get rid of memcg_{alloc,free}_cache_params Vladimir Davydov
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
Current memcg_caches arrays relocation path looks rather awkward: on
setting kmem limit we call memcg_update_all_caches located on slab's
side, which walks over all root caches and calls back to memcontrol.c
through memcg_update_cache_size and memcg_update_array_size, which in
turn reallocate the memcg_caches array for a particular cache and update
the arrays' size respectively.
The first call from memcontrol.c to slab_common.c, which is
memcg_update_all_caches, is justified, because to iterate over root
caches we have to take the slab_mutex. However, I don't see any reason
why we can't reallocate a memcg_caches array on the slab's size or why
we should call memcg_update_cache_size under the slab_mutex. So let's
clean up this mess.
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/memcontrol.h | 3 --
include/linux/slab.h | 3 +-
mm/memcontrol.c | 125 +++++++++++++-------------------------------
mm/slab_common.c | 36 +++++++------
4 files changed, 59 insertions(+), 108 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index dfc2929a3877..6e59393e03f9 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -496,9 +496,6 @@ int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
struct kmem_cache *root_cache);
void memcg_free_cache_params(struct kmem_cache *s);
-int memcg_update_cache_size(struct kmem_cache *s, int num_groups);
-void memcg_update_array_size(int num_groups);
-
struct kmem_cache *
__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 095ce8e47a36..c08c9667a1e8 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -120,6 +120,7 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *,
struct kmem_cache *,
const char *);
int kmem_cache_init_memcg_array(struct kmem_cache *, int);
+int kmem_cache_grow_memcg_arrays(int);
#endif
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
@@ -545,8 +546,6 @@ struct memcg_cache_params {
};
};
-int memcg_update_all_caches(int num_memcgs);
-
struct seq_file;
int cache_show(struct kmem_cache *s, struct seq_file *m);
void print_slabinfo_header(struct seq_file *m);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index fa7cbce8f0cf..844258bf0968 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3027,84 +3027,52 @@ int memcg_cache_id(struct mem_cgroup *memcg)
return memcg ? memcg->kmemcg_id : -1;
}
-static size_t memcg_caches_array_size(int num_groups)
+static int memcg_init_cache_id(struct mem_cgroup *memcg)
{
- ssize_t size;
- if (num_groups <= 0)
- return 0;
+ int err = 0;
+ int id, size;
+
+ lockdep_assert_held(&activate_kmem_mutex);
+
+ id = ida_simple_get(&kmem_limited_groups,
+ 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL);
+ if (id < 0)
+ return id;
- size = 2 * num_groups;
+ if (id < memcg_limited_groups_array_size)
+ goto out_setid;
+
+ /*
+ * We don't have enough space for the new id in the arrays that store
+ * per memcg data. Let's try to grow them then.
+ */
+ size = id * 2;
if (size < MEMCG_CACHES_MIN_SIZE)
size = MEMCG_CACHES_MIN_SIZE;
else if (size > MEMCG_CACHES_MAX_SIZE)
size = MEMCG_CACHES_MAX_SIZE;
- return size;
-}
-
-/*
- * We should update the current array size iff all caches updates succeed. This
- * can only be done from the slab side. The slab mutex needs to be held when
- * calling this.
- */
-void memcg_update_array_size(int num)
-{
- if (num > memcg_limited_groups_array_size)
- memcg_limited_groups_array_size = memcg_caches_array_size(num);
-}
-
-int memcg_update_cache_size(struct kmem_cache *s, int num_groups)
-{
- struct memcg_cache_params *cur_params = s->memcg_params;
-
- VM_BUG_ON(!is_root_cache(s));
-
- if (!cur_params)
- return 0;
-
- if (num_groups > memcg_limited_groups_array_size) {
- int i;
- struct memcg_cache_params *new_params;
- ssize_t size = memcg_caches_array_size(num_groups);
-
- size *= sizeof(void *);
- size += offsetof(struct memcg_cache_params, memcg_caches);
-
- new_params = kzalloc(size, GFP_KERNEL);
- if (!new_params)
- return -ENOMEM;
+ mutex_lock(&memcg_slab_mutex);
+ err = kmem_cache_grow_memcg_arrays(size);
+ mutex_unlock(&memcg_slab_mutex);
- new_params->is_root_cache = true;
+ if (err)
+ goto out_rmid;
- /*
- * There is the chance it will be bigger than
- * memcg_limited_groups_array_size, if we failed an allocation
- * in a cache, in which case all caches updated before it, will
- * have a bigger array.
- *
- * But if that is the case, the data after
- * memcg_limited_groups_array_size is certainly unused
- */
- for (i = 0; i < memcg_limited_groups_array_size; i++) {
- if (!cur_params->memcg_caches[i])
- continue;
- new_params->memcg_caches[i] =
- cur_params->memcg_caches[i];
- }
+ /*
+ * Update the arrays' size only after we grew them so that readers
+ * walking over such an array won't get an index out of range provided
+ * they use an appropriate mutex to protect the array's elements.
+ */
+ memcg_limited_groups_array_size = size;
- /*
- * Ideally, we would wait until all caches succeed, and only
- * then free the old one. But this is not worth the extra
- * pointer per-cache we'd have to have for this.
- *
- * It is not a big deal if some caches are left with a size
- * bigger than the others. And all updates will reset this
- * anyway.
- */
- rcu_assign_pointer(s->memcg_params, new_params);
- kfree_rcu(cur_params, rcu_head);
- }
+out_setid:
+ memcg->kmemcg_id = id;
return 0;
+
+out_rmid:
+ ida_simple_remove(&kmem_limited_groups, id);
+ return err;
}
int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
@@ -4949,7 +4917,6 @@ static int __memcg_activate_kmem(struct mem_cgroup *memcg,
unsigned long long limit)
{
int err = 0;
- int memcg_id;
if (memcg_kmem_is_active(memcg))
return 0;
@@ -4979,24 +4946,10 @@ static int __memcg_activate_kmem(struct mem_cgroup *memcg,
if (err)
goto out;
- memcg_id = ida_simple_get(&kmem_limited_groups,
- 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL);
- if (memcg_id < 0) {
- err = memcg_id;
- goto out;
- }
-
- /*
- * Make sure we have enough space for this cgroup in each root cache's
- * memcg_params.
- */
- mutex_lock(&memcg_slab_mutex);
- err = memcg_update_all_caches(memcg_id + 1);
- mutex_unlock(&memcg_slab_mutex);
+ err = memcg_init_cache_id(memcg);
if (err)
- goto out_rmid;
+ goto out;
- memcg->kmemcg_id = memcg_id;
INIT_LIST_HEAD(&memcg->memcg_slab_caches);
/*
@@ -5016,10 +4969,6 @@ static int __memcg_activate_kmem(struct mem_cgroup *memcg,
out:
memcg_resume_kmem_account();
return err;
-
-out_rmid:
- ida_simple_remove(&kmem_limited_groups, memcg_id);
- goto out;
}
static int memcg_activate_kmem(struct mem_cgroup *memcg,
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 44bfc4b1ee09..a98922acfdc6 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -77,10 +77,13 @@ static inline int kmem_cache_sanity_check(const char *name, size_t size)
#endif
#ifdef CONFIG_MEMCG_KMEM
-static int kmem_cache_alloc_memcg_array(struct kmem_cache *s, int nr_entries)
+static int kmem_cache_realloc_memcg_array(struct kmem_cache *s, int nr_entries)
{
+ int i;
size_t size;
- struct memcg_cache_params *new;
+ struct memcg_cache_params *new, *old;
+
+ old = s->memcg_params;
size = offsetof(struct memcg_cache_params, memcg_caches);
size += nr_entries * sizeof(void *);
@@ -90,10 +93,15 @@ static int kmem_cache_alloc_memcg_array(struct kmem_cache *s, int nr_entries)
return -ENOMEM;
new->is_root_cache = true;
+ if (old) {
+ for_each_memcg_cache_index(i)
+ new->memcg_caches[i] = old->memcg_caches[i];
+ }
/* matching rcu_dereference is in cache_from_memcg_idx */
rcu_assign_pointer(s->memcg_params, new);
-
+ if (old)
+ kfree_rcu(old, rcu_head);
return 0;
}
@@ -111,37 +119,35 @@ int kmem_cache_init_memcg_array(struct kmem_cache *s, int nr_entries)
mutex_lock(&slab_mutex);
if (!s->memcg_params)
- ret = kmem_cache_alloc_memcg_array(s, nr_entries);
+ ret = kmem_cache_realloc_memcg_array(s, nr_entries);
mutex_unlock(&slab_mutex);
return ret;
}
-int memcg_update_all_caches(int num_memcgs)
+int kmem_cache_grow_memcg_arrays(int nr_entries)
{
struct kmem_cache *s;
int ret = 0;
- mutex_lock(&slab_mutex);
+ mutex_lock(&slab_mutex);
list_for_each_entry(s, &slab_caches, list) {
if (!is_root_cache(s))
continue;
- ret = memcg_update_cache_size(s, num_memcgs);
+ ret = kmem_cache_realloc_memcg_array(s, nr_entries);
/*
- * See comment in memcontrol.c, memcg_update_cache_size:
- * Instead of freeing the memory, we'll just leave the caches
- * up to this point in an updated state.
+ * We won't shrink the arrays back to the initial size on
+ * failure, because it isn't a big deal if some caches are left
+ * with a size greater than others. Further updates will reset
+ * this anyway.
*/
if (ret)
- goto out;
+ break;
}
-
- memcg_update_array_size(num_memcgs);
-out:
mutex_unlock(&slab_mutex);
return ret;
}
-#endif
+#endif /* CONFIG_MEMCG_KMEM */
/*
* Figure out what the alignment of the objects will be given a set of
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm 4/6] memcg: get rid of memcg_{alloc,free}_cache_params
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
` (2 preceding siblings ...)
2014-04-25 12:33 ` [PATCH -mm 3/6] memcg: cleanup memcg_caches arrays relocation path Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 5/6] memcg: cleanup kmem cache creation/destruction functions naming Vladimir Davydov
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
Let's inline them to memcg_kmem_{create,destroy}_cache to keep the
interface between slab_common.c and memcontrol.c clean.
Note, to properly bail out in memcg_kmem_destroy_cache, I need to know
if the cache was actually destroyed (it won't if it has leaked objects).
That's why I add the return value to kmem_cache_destroy.
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/memcontrol.h | 14 ---------
include/linux/slab.h | 7 +++--
mm/memcontrol.c | 74 +++++++++++++++++++++++---------------------
mm/slab_common.c | 41 +++++++++++++++---------
4 files changed, 69 insertions(+), 67 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6e59393e03f9..3aee79fc7876 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -492,10 +492,6 @@ void __memcg_kmem_uncharge_pages(struct page *page, int order);
int memcg_cache_id(struct mem_cgroup *memcg);
-int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
- struct kmem_cache *root_cache);
-void memcg_free_cache_params(struct kmem_cache *s);
-
struct kmem_cache *
__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
@@ -623,16 +619,6 @@ static inline int memcg_cache_id(struct mem_cgroup *memcg)
return -1;
}
-static inline int memcg_alloc_cache_params(struct mem_cgroup *memcg,
- struct kmem_cache *s, struct kmem_cache *root_cache)
-{
- return 0;
-}
-
-static inline void memcg_free_cache_params(struct kmem_cache *s)
-{
-}
-
static inline struct kmem_cache *
memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
{
diff --git a/include/linux/slab.h b/include/linux/slab.h
index c08c9667a1e8..22cc9cab4279 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -106,6 +106,8 @@
#include <linux/kmemleak.h>
struct mem_cgroup;
+struct memcg_cache_params;
+
/*
* struct kmem_cache related prototypes
*/
@@ -116,13 +118,12 @@ struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
unsigned long,
void (*)(void *));
#ifdef CONFIG_MEMCG_KMEM
-struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *,
- struct kmem_cache *,
+struct kmem_cache *kmem_cache_create_memcg(struct memcg_cache_params *,
const char *);
int kmem_cache_init_memcg_array(struct kmem_cache *, int);
int kmem_cache_grow_memcg_arrays(int);
#endif
-void kmem_cache_destroy(struct kmem_cache *);
+int kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
void kmem_cache_free(struct kmem_cache *, void *);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 844258bf0968..b6ddbe8b4364 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3075,31 +3075,6 @@ out_rmid:
return err;
}
-int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
- struct kmem_cache *root_cache)
-{
- if (!memcg)
- return 0;
-
- s->memcg_params = kzalloc(sizeof(*s->memcg_params), GFP_KERNEL);
- if (!s->memcg_params)
- return -ENOMEM;
-
- s->memcg_params->memcg = memcg;
- s->memcg_params->root_cache = root_cache;
- css_get(&memcg->css);
-
- return 0;
-}
-
-void memcg_free_cache_params(struct kmem_cache *s)
-{
- if (is_root_cache(s))
- return;
- css_put(&s->memcg_params->memcg->css);
- kfree(s->memcg_params);
-}
-
/*
* Prepares the memcg_caches array of the given kmem cache for disposing
* memcgs' copies.
@@ -3128,6 +3103,7 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
struct kmem_cache *root_cache)
{
static char memcg_name[NAME_MAX + 1];
+ struct memcg_cache_params *params;
struct kmem_cache *cachep;
int id;
@@ -3143,16 +3119,26 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
if (cache_from_memcg_idx(root_cache, id))
return;
+ params = kzalloc(sizeof(*params), GFP_KERNEL);
+ if (!params)
+ return;
+
+ params->memcg = memcg;
+ params->root_cache = root_cache;
+
cgroup_name(memcg->css.cgroup, memcg_name, NAME_MAX + 1);
- cachep = kmem_cache_create_memcg(memcg, root_cache, memcg_name);
+ cachep = kmem_cache_create_memcg(params, memcg_name);
/*
* If we could not create a memcg cache, do not complain, because
* that's not critical at all as we can always proceed with the root
* cache.
*/
- if (!cachep)
+ if (!cachep) {
+ kfree(params);
return;
+ }
+ css_get(&memcg->css);
list_add(&cachep->memcg_params->list, &memcg->memcg_slab_caches);
/*
@@ -3166,26 +3152,46 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
root_cache->memcg_params->memcg_caches[id] = cachep;
}
-static void memcg_kmem_destroy_cache(struct kmem_cache *cachep)
+static int memcg_kmem_destroy_cache(struct kmem_cache *cachep)
{
struct kmem_cache *root_cache;
struct mem_cgroup *memcg;
+ struct memcg_cache_params *params;
int id;
lockdep_assert_held(&memcg_slab_mutex);
BUG_ON(is_root_cache(cachep));
- root_cache = cachep->memcg_params->root_cache;
- memcg = cachep->memcg_params->memcg;
+ params = cachep->memcg_params;
+ root_cache = params->root_cache;
+ memcg = params->memcg;
id = memcg_cache_id(memcg);
+ /*
+ * Since memcg_caches arrays can be accessed using only slab_mutex for
+ * protection (e.g. by slabinfo readers), we must clear the cache's
+ * entry before trying to destroy it.
+ */
BUG_ON(root_cache->memcg_params->memcg_caches[id] != cachep);
root_cache->memcg_params->memcg_caches[id] = NULL;
- list_del(&cachep->memcg_params->list);
+ if (kmem_cache_destroy(cachep) != 0) {
+ root_cache->memcg_params->memcg_caches[id] = cachep;
+ return -EBUSY;
+ }
+
+ /*
+ * We delete the cache's memcg params from the memcg's slab caches list
+ * after the cache was destroyed, but that's OK, because the list can
+ * only be accessed under memcg_slab_mutex, which is held during the
+ * whole operation.
+ */
+ list_del(¶ms->list);
+ css_put(&memcg->css);
+ kfree(params);
- kmem_cache_destroy(cachep);
+ return 0;
}
/*
@@ -3230,9 +3236,7 @@ int __kmem_cache_destroy_memcg_children(struct kmem_cache *s)
if (!c)
continue;
- memcg_kmem_destroy_cache(c);
-
- if (cache_from_memcg_idx(s, i))
+ if (memcg_kmem_destroy_cache(c) != 0)
failed++;
}
mutex_unlock(&memcg_slab_mutex);
diff --git a/mm/slab_common.c b/mm/slab_common.c
index a98922acfdc6..cb4e2293ec46 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -179,7 +179,7 @@ unsigned long calculate_alignment(unsigned long flags,
static struct kmem_cache *
do_kmem_cache_create(char *name, size_t object_size, size_t size, size_t align,
unsigned long flags, void (*ctor)(void *),
- struct mem_cgroup *memcg, struct kmem_cache *root_cache)
+ struct memcg_cache_params *memcg_params)
{
struct kmem_cache *s;
int err;
@@ -194,10 +194,9 @@ do_kmem_cache_create(char *name, size_t object_size, size_t size, size_t align,
s->size = size;
s->align = align;
s->ctor = ctor;
-
- err = memcg_alloc_cache_params(memcg, s, root_cache);
- if (err)
- goto out_free_cache;
+#ifdef CONFIG_MEMCG_KMEM
+ s->memcg_params = memcg_params;
+#endif
err = __kmem_cache_create(s, flags);
if (err)
@@ -211,7 +210,6 @@ out:
return s;
out_free_cache:
- memcg_free_cache_params(s);
kfree(s);
goto out;
}
@@ -277,7 +275,7 @@ kmem_cache_create(const char *name, size_t size, size_t align,
s = do_kmem_cache_create(cache_name, size, size,
calculate_alignment(flags, align, size),
- flags, ctor, NULL, NULL);
+ flags, ctor, NULL);
if (IS_ERR(s)) {
err = PTR_ERR(s);
kfree(cache_name);
@@ -307,18 +305,19 @@ EXPORT_SYMBOL(kmem_cache_create);
#ifdef CONFIG_MEMCG_KMEM
/*
* kmem_cache_create_memcg - Create a cache for a memory cgroup.
- * @memcg: The memory cgroup the new cache is for.
- * @root_cache: The parent of the new cache.
+ * @memcg_params: The memcg params to initialize the cache with.
* @memcg_name: The name of the memory cgroup.
*
* This function attempts to create a kmem cache that will serve allocation
* requests going from @memcg to @root_cache. The new cache inherits properties
* from its parent.
*/
-struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg,
- struct kmem_cache *root_cache,
- const char *memcg_name)
+struct kmem_cache *
+kmem_cache_create_memcg(struct memcg_cache_params *memcg_params,
+ const char *memcg_name)
{
+ struct mem_cgroup *memcg = memcg_params->memcg;
+ struct kmem_cache *root_cache = memcg_params->root_cache;
struct kmem_cache *s = NULL;
char *cache_name;
@@ -335,7 +334,7 @@ struct kmem_cache *kmem_cache_create_memcg(struct mem_cgroup *memcg,
s = do_kmem_cache_create(cache_name, root_cache->object_size,
root_cache->size, root_cache->align,
root_cache->flags, root_cache->ctor,
- memcg, root_cache);
+ memcg_params);
if (IS_ERR(s)) {
kfree(cache_name);
s = NULL;
@@ -373,8 +372,17 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
}
#endif /* CONFIG_MEMCG_KMEM */
-void kmem_cache_destroy(struct kmem_cache *s)
+/*
+ * kmem_cache_destroy - Destroy a cache.
+ * @s: The cache to destroy.
+ *
+ * Returns 0 on success. If the cache still has objects, -EBUSY is returned and
+ * a warning is printed to the log.
+ */
+int kmem_cache_destroy(struct kmem_cache *s)
{
+ int ret = 0;
+
get_online_cpus();
get_online_mems();
@@ -384,6 +392,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
if (s->refcount)
goto out_unlock;
+ ret = -EBUSY;
if (kmem_cache_destroy_memcg_children(s) != 0)
goto out_unlock;
@@ -400,9 +409,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
if (s->flags & SLAB_DESTROY_BY_RCU)
rcu_barrier();
- memcg_free_cache_params(s);
kfree(s->name);
kmem_cache_free(kmem_cache, s);
+ ret = 0;
goto out;
out_unlock:
@@ -410,6 +419,8 @@ out_unlock:
out:
put_online_mems();
put_online_cpus();
+
+ return ret;
}
EXPORT_SYMBOL(kmem_cache_destroy);
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm 5/6] memcg: cleanup kmem cache creation/destruction functions naming
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
` (3 preceding siblings ...)
2014-04-25 12:33 ` [PATCH -mm 4/6] memcg: get rid of memcg_{alloc,free}_cache_params Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 12:33 ` [PATCH -mm 6/6] memcg: cleanup kmem_id-related naming Vladimir Davydov
2014-04-27 8:56 ` [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
Current names are rather confusing. Let's try to improve them.
Brief change log:
** old name ** ** new name **
kmem_cache_create_memcg kmem_cache_request_memcg_copy
memcg_kmem_create_cache memcg_copy_kmem_cache
memcg_kmem_destroy_cache memcg_destroy_kmem_cache_copy
__kmem_cache_destroy_memcg_children kmem_cache_destroy_memcg_copies
kmem_cache_destroy_memcg_children kmem_cache_destroy_memcg_array
mem_cgroup_destroy_all_caches memcg_destroy_kmem_cache_copies
create_work memcg_kmem_cache_copy_work
memcg_create_cache_work_func memcg_kmem_cache_copy_work_func
memcg_create_cache_enqueue memcg_schedule_kmem_cache_copy
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/memcontrol.h | 2 +-
include/linux/slab.h | 4 +-
mm/memcontrol.c | 92 +++++++++++++++++++++++++-------------------
mm/slab_common.c | 14 +++----
4 files changed, 62 insertions(+), 50 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 3aee79fc7876..3ee73da2991b 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -498,7 +498,7 @@ __memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
int __memcg_charge_slab(struct kmem_cache *cachep, gfp_t gfp, int order);
void __memcg_uncharge_slab(struct kmem_cache *cachep, int order);
-int __kmem_cache_destroy_memcg_children(struct kmem_cache *s);
+int kmem_cache_destroy_memcg_copies(struct kmem_cache *cachep);
/**
* memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 22cc9cab4279..1f22c6130f1a 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -118,8 +118,8 @@ struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
unsigned long,
void (*)(void *));
#ifdef CONFIG_MEMCG_KMEM
-struct kmem_cache *kmem_cache_create_memcg(struct memcg_cache_params *,
- const char *);
+struct kmem_cache *kmem_cache_request_memcg_copy(struct memcg_cache_params *,
+ const char *);
int kmem_cache_init_memcg_array(struct kmem_cache *, int);
int kmem_cache_grow_memcg_arrays(int);
#endif
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index b6ddbe8b4364..bdd3d373cdca 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3099,8 +3099,12 @@ static int memcg_prepare_kmem_cache(struct kmem_cache *cachep)
return ret;
}
-static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
- struct kmem_cache *root_cache)
+/*
+ * Creates a copy of the given kmem cache to be used for fulfilling allocation
+ * requests coming from the memcg to the cache.
+ */
+static void memcg_copy_kmem_cache(struct mem_cgroup *memcg,
+ struct kmem_cache *root_cache)
{
static char memcg_name[NAME_MAX + 1];
struct memcg_cache_params *params;
@@ -3127,7 +3131,7 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
params->root_cache = root_cache;
cgroup_name(memcg->css.cgroup, memcg_name, NAME_MAX + 1);
- cachep = kmem_cache_create_memcg(params, memcg_name);
+ cachep = kmem_cache_request_memcg_copy(params, memcg_name);
/*
* If we could not create a memcg cache, do not complain, because
* that's not critical at all as we can always proceed with the root
@@ -3152,7 +3156,8 @@ static void memcg_kmem_create_cache(struct mem_cgroup *memcg,
root_cache->memcg_params->memcg_caches[id] = cachep;
}
-static int memcg_kmem_destroy_cache(struct kmem_cache *cachep)
+/* Attempts to destroy a per memcg kmem cache copy. Returns 0 on success. */
+static int memcg_destroy_kmem_cache_copy(struct kmem_cache *cachep)
{
struct kmem_cache *root_cache;
struct mem_cgroup *memcg;
@@ -3225,25 +3230,39 @@ static inline void memcg_resume_kmem_account(void)
current->memcg_kmem_skip_account--;
}
-int __kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+/*
+ * Attempts to destroy all per memcg copies of the given kmem cache. Called on
+ * kmem cache destruction. Returns 0 on success.
+ */
+int kmem_cache_destroy_memcg_copies(struct kmem_cache *cachep)
{
struct kmem_cache *c;
- int i, failed = 0;
+ int i;
+ int ret = 0;
+
+ BUG_ON(!is_root_cache(cachep));
mutex_lock(&memcg_slab_mutex);
for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(s, i);
+ c = cache_from_memcg_idx(cachep, i);
if (!c)
continue;
- if (memcg_kmem_destroy_cache(c) != 0)
- failed++;
+ if (memcg_destroy_kmem_cache_copy(c) != 0)
+ ret = -EBUSY;
}
mutex_unlock(&memcg_slab_mutex);
- return failed;
+ return ret;
}
-static void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
+/*
+ * Attempts to destroy all kmem cache copies corresponding to the given memcg.
+ * Called on css offline.
+ *
+ * XXX: Caches that still have objects on css offline will be leaked. Need to
+ * reparent them instead.
+ */
+static void memcg_destroy_kmem_cache_copies(struct mem_cgroup *memcg)
{
struct kmem_cache *cachep;
struct memcg_cache_params *params, *tmp;
@@ -3256,20 +3275,21 @@ static void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
cachep = memcg_params_to_cache(params);
kmem_cache_shrink(cachep);
if (atomic_read(&cachep->memcg_params->nr_pages) == 0)
- memcg_kmem_destroy_cache(cachep);
+ memcg_destroy_kmem_cache_copy(cachep);
}
mutex_unlock(&memcg_slab_mutex);
}
-struct create_work {
+struct memcg_kmem_cache_copy_work {
struct mem_cgroup *memcg;
struct kmem_cache *cachep;
struct work_struct work;
};
-static void memcg_create_cache_work_func(struct work_struct *w)
+static void memcg_kmem_cache_copy_work_func(struct work_struct *w)
{
- struct create_work *cw = container_of(w, struct create_work, work);
+ struct memcg_kmem_cache_copy_work *cw = container_of(w,
+ struct memcg_kmem_cache_copy_work, work);
struct mem_cgroup *memcg = cw->memcg;
struct kmem_cache *cachep = cw->cachep;
@@ -3277,22 +3297,19 @@ static void memcg_create_cache_work_func(struct work_struct *w)
goto out;
mutex_lock(&memcg_slab_mutex);
- memcg_kmem_create_cache(memcg, cachep);
+ memcg_copy_kmem_cache(memcg, cachep);
mutex_unlock(&memcg_slab_mutex);
out:
css_put(&memcg->css);
kfree(cw);
}
-/*
- * Enqueue the creation of a per-memcg kmem_cache.
- */
-static void __memcg_create_cache_enqueue(struct mem_cgroup *memcg,
- struct kmem_cache *cachep)
+static void __memcg_schedule_kmem_cache_copy(struct mem_cgroup *memcg,
+ struct kmem_cache *cachep)
{
- struct create_work *cw;
+ struct memcg_kmem_cache_copy_work *cw;
- cw = kmalloc(sizeof(struct create_work), GFP_NOWAIT);
+ cw = kmalloc(sizeof(*cw), GFP_NOWAIT);
if (cw == NULL) {
css_put(&memcg->css);
return;
@@ -3300,18 +3317,18 @@ static void __memcg_create_cache_enqueue(struct mem_cgroup *memcg,
cw->memcg = memcg;
cw->cachep = cachep;
+ INIT_WORK(&cw->work, memcg_kmem_cache_copy_work_func);
- INIT_WORK(&cw->work, memcg_create_cache_work_func);
schedule_work(&cw->work);
}
-static void memcg_create_cache_enqueue(struct mem_cgroup *memcg,
- struct kmem_cache *cachep)
+static void memcg_schedule_kmem_cache_copy(struct mem_cgroup *memcg,
+ struct kmem_cache *cachep)
{
/*
* We need to stop accounting when we kmalloc, because if the
* corresponding kmalloc cache is not yet created, the first allocation
- * in __memcg_create_cache_enqueue will recurse.
+ * in __memcg_schedule_kmem_cache_copy will recurse.
*
* However, it is better to enclose the whole function. Depending on
* the debugging options enabled, INIT_WORK(), for instance, can
@@ -3320,7 +3337,7 @@ static void memcg_create_cache_enqueue(struct mem_cgroup *memcg,
* the safest choice is to do it like this, wrapping the whole function.
*/
memcg_stop_kmem_account();
- __memcg_create_cache_enqueue(memcg, cachep);
+ __memcg_schedule_kmem_cache_copy(memcg, cachep);
memcg_resume_kmem_account();
}
@@ -3384,22 +3401,17 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
/*
* If we are in a safe context (can wait, and not in interrupt
- * context), we could be be predictable and return right away.
+ * context), we could be predictable and return right away.
* This would guarantee that the allocation being performed
* already belongs in the new cache.
*
* However, there are some clashes that can arrive from locking.
* For instance, because we acquire the slab_mutex while doing
- * kmem_cache_dup, this means no further allocation could happen
- * with the slab_mutex held.
- *
- * Also, because cache creation issue get_online_cpus(), this
- * creates a lock chain: memcg_slab_mutex -> cpu_hotplug_mutex,
- * that ends up reversed during cpu hotplug. (cpuset allocates
- * a bunch of GFP_KERNEL memory during cpuup). Due to all that,
- * better to defer everything.
+ * kmem_cache_create, this means no further allocation could
+ * happen with the slab_mutex held. So it's better to defer
+ * everything.
*/
- memcg_create_cache_enqueue(memcg, cachep);
+ memcg_schedule_kmem_cache_copy(memcg, cachep);
return cachep;
out:
rcu_read_unlock();
@@ -3523,7 +3535,7 @@ void __memcg_kmem_uncharge_pages(struct page *page, int order)
memcg_uncharge_kmem(memcg, PAGE_SIZE << order);
}
#else
-static inline void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
+static inline void memcg_destroy_kmem_cache_copies(struct mem_cgroup *memcg)
{
}
#endif /* CONFIG_MEMCG_KMEM */
@@ -6358,7 +6370,7 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
css_for_each_descendant_post(iter, css)
mem_cgroup_reparent_charges(mem_cgroup_from_css(iter));
- mem_cgroup_destroy_all_caches(memcg);
+ memcg_destroy_kmem_cache_copies(memcg);
vmpressure_cleanup(&memcg->vmpressure);
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index cb4e2293ec46..36d9b866a3ab 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -304,7 +304,7 @@ EXPORT_SYMBOL(kmem_cache_create);
#ifdef CONFIG_MEMCG_KMEM
/*
- * kmem_cache_create_memcg - Create a cache for a memory cgroup.
+ * kmem_cache_request_memcg_copy - Create a cache for a memory cgroup.
* @memcg_params: The memcg params to initialize the cache with.
* @memcg_name: The name of the memory cgroup.
*
@@ -313,8 +313,8 @@ EXPORT_SYMBOL(kmem_cache_create);
* from its parent.
*/
struct kmem_cache *
-kmem_cache_create_memcg(struct memcg_cache_params *memcg_params,
- const char *memcg_name)
+kmem_cache_request_memcg_copy(struct memcg_cache_params *memcg_params,
+ const char *memcg_name)
{
struct mem_cgroup *memcg = memcg_params->memcg;
struct kmem_cache *root_cache = memcg_params->root_cache;
@@ -349,7 +349,7 @@ out_unlock:
return s;
}
-static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+static int kmem_cache_destroy_memcg_array(struct kmem_cache *s)
{
int rc;
@@ -358,7 +358,7 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
return 0;
mutex_unlock(&slab_mutex);
- rc = __kmem_cache_destroy_memcg_children(s);
+ rc = kmem_cache_destroy_memcg_copies(s);
mutex_lock(&slab_mutex);
if (rc == 0)
@@ -366,7 +366,7 @@ static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
return rc;
}
#else
-static int kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+static int kmem_cache_destroy_memcg_array(struct kmem_cache *s)
{
return 0;
}
@@ -393,7 +393,7 @@ int kmem_cache_destroy(struct kmem_cache *s)
goto out_unlock;
ret = -EBUSY;
- if (kmem_cache_destroy_memcg_children(s) != 0)
+ if (kmem_cache_destroy_memcg_array(s) != 0)
goto out_unlock;
list_del(&s->list);
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm 6/6] memcg: cleanup kmem_id-related naming
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
` (4 preceding siblings ...)
2014-04-25 12:33 ` [PATCH -mm 5/6] memcg: cleanup kmem cache creation/destruction functions naming Vladimir Davydov
@ 2014-04-25 12:33 ` Vladimir Davydov
2014-04-25 13:12 ` [PATCH -mm v1.1] " Vladimir Davydov
2014-04-27 8:56 ` [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
6 siblings, 1 reply; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 12:33 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
The naming of mem_cgroup->kmemcg_id-related functions is rather
inconsistent. We tend to use cache_id as part of their names, but it
isn't quite right, because kmem_id isn't something specific to kmem
caches. It can be used for indexing any array that stores per memcg
data. For instance, we will use it to make list_lru per memcg in the
future. So let's clean up the names and comments related to kmem_id.
Brief change log:
** old name ** ** new name **
mem_cgroup->kmemcg_id mem_cgroup->kmem_id
memcg_init_cache_id() memcg_init_kmem_id()
memcg_cache_id() memcg_kmem_id()
cache_from_memcg_idx() kmem_cache_of_memcg_by_id()
cache_from_memcg_idx(memcg_cache_id()) kmem_cache_of_memcg()
for_each_memcg_cache_index() for_each_possible_memcg_kmem_id()
memcg_limited_groups memcg_kmem_ida
memcg_limited_groups_array_size memcg_nr_kmem_ids_max
MEMCG_CACHES_MIN_SIZE <constant inlined>
MEMCG_CACHES_MAX_SIZE MEMCG_KMEM_ID_MAX + 1
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
include/linux/memcontrol.h | 19 +++-----
mm/memcontrol.c | 110 ++++++++++++++++++++------------------------
mm/slab.c | 4 +-
mm/slab.h | 24 +++++++---
mm/slab_common.c | 10 ++--
mm/slub.c | 10 ++--
6 files changed, 88 insertions(+), 89 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 3ee73da2991b..4080a6418c72 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -458,15 +458,10 @@ static inline void sock_release_memcg(struct sock *sk)
#ifdef CONFIG_MEMCG_KMEM
extern struct static_key memcg_kmem_enabled_key;
-extern int memcg_limited_groups_array_size;
+extern int memcg_nr_kmem_ids_max;
-/*
- * Helper macro to loop through all memcg-specific caches. Callers must still
- * check if the cache is valid (it is either valid or NULL).
- * the slab_mutex must be held when looping through those caches
- */
-#define for_each_memcg_cache_index(_idx) \
- for ((_idx) = 0; (_idx) < memcg_limited_groups_array_size; (_idx)++)
+#define for_each_possible_memcg_kmem_id(id) \
+ for ((id) = 0; (id) < memcg_nr_kmem_ids_max; (id)++)
static inline bool memcg_kmem_enabled(void)
{
@@ -490,7 +485,7 @@ 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_cache_id(struct mem_cgroup *memcg);
+int memcg_kmem_id(struct mem_cgroup *memcg);
struct kmem_cache *
__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
@@ -591,8 +586,8 @@ memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
return __memcg_kmem_get_cache(cachep, gfp);
}
#else
-#define for_each_memcg_cache_index(_idx) \
- for (; NULL; )
+#define for_each_possible_memcg_kmem_id(id) \
+ for ((id) = 0; 0; )
static inline bool memcg_kmem_enabled(void)
{
@@ -614,7 +609,7 @@ memcg_kmem_commit_charge(struct page *page, struct mem_cgroup *memcg, int order)
{
}
-static inline int memcg_cache_id(struct mem_cgroup *memcg)
+static inline int memcg_kmem_id(struct mem_cgroup *memcg)
{
return -1;
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index bdd3d373cdca..076cf986f40c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -357,11 +357,22 @@ struct mem_cgroup {
struct cg_proto tcp_mem;
#endif
#if defined(CONFIG_MEMCG_KMEM)
+ /*
+ * Each kmem-limited memory cgroup has a unique id. We use it for
+ * indexing the arrays that store per cgroup data. An example of such
+ * an array is kmem_cache->memcg_params->memcg_caches.
+ *
+ * We introduce a separate id instead of using cgroup->id to avoid
+ * waste of memory in sparse environments, where we have a lot of
+ * memory cgroups, but only a few of them are kmem-limited.
+ *
+ * For unlimited cgroups kmem_id equals -1.
+ */
+ int kmem_id;
+
/* analogous to slab_common's slab_caches list, but per-memcg;
* protected by memcg_slab_mutex */
struct list_head memcg_slab_caches;
- /* Index in the kmem_cache->memcg_params->memcg_caches array */
- int kmemcg_id;
#endif
int last_scanned_node;
@@ -610,35 +621,25 @@ static void disarm_sock_keys(struct mem_cgroup *memcg)
#endif
#ifdef CONFIG_MEMCG_KMEM
+/* used for mem_cgroup->kmem_id allocations */
+static DEFINE_IDA(memcg_kmem_ida);
+
/*
- * This will be the memcg's index in each cache's ->memcg_params->memcg_caches.
- * The main reason for not using cgroup id for this:
- * this works better in sparse environments, where we have a lot of memcgs,
- * but only a few kmem-limited. Or also, if we have, for instance, 200
- * memcgs, and none but the 200th is kmem-limited, we'd have to have a
- * 200 entry array for that.
- *
- * The current size of the caches array is stored in
- * memcg_limited_groups_array_size. It will double each time we have to
- * increase it.
+ * Max kmem id should be as large as max cgroup id so that we could enable
+ * kmem-accounting for each memory cgroup.
*/
-static DEFINE_IDA(kmem_limited_groups);
-int memcg_limited_groups_array_size;
+#define MEMCG_KMEM_ID_MAX MEM_CGROUP_ID_MAX
/*
- * MIN_SIZE is different than 1, because we would like to avoid going through
- * the alloc/free process all the time. In a small machine, 4 kmem-limited
- * cgroups is a reasonable guess. In the future, it could be a parameter or
- * tunable, but that is strictly not necessary.
+ * We keep the maximal number of kmem ids that may exist in the system in the
+ * memcg_nr_kmem_ids_max variable. We use it for the size of the arrays indexed
+ * by kmem id (see the mem_cgroup->kmem_id definition).
*
- * MAX_SIZE should be as large as the number of cgrp_ids. Ideally, we could get
- * this constant directly from cgroup, but it is understandable that this is
- * better kept as an internal representation in cgroup.c. In any case, the
- * cgrp_id space is not getting any smaller, and we don't have to necessarily
- * increase ours as well if it increases.
+ * If a newly allocated kmem id is greater or equal to memcg_nr_kmem_ids_max,
+ * we double it and reallocate the arrays so that they have enough space to
+ * store data for the new cgroup.
*/
-#define MEMCG_CACHES_MIN_SIZE 4
-#define MEMCG_CACHES_MAX_SIZE MEM_CGROUP_ID_MAX
+int memcg_nr_kmem_ids_max;
/*
* A lot of the calls to the cache allocation functions are expected to be
@@ -653,7 +654,7 @@ static void disarm_kmem_keys(struct mem_cgroup *memcg)
{
if (memcg_kmem_is_active(memcg)) {
static_key_slow_dec(&memcg_kmem_enabled_key);
- ida_simple_remove(&kmem_limited_groups, memcg->kmemcg_id);
+ ida_simple_remove(&memcg_kmem_ida, memcg->kmem_id);
}
/*
* This check can't live in kmem destruction function,
@@ -2930,11 +2931,8 @@ static inline bool memcg_can_account_kmem(struct mem_cgroup *memcg)
*/
static struct kmem_cache *memcg_params_to_cache(struct memcg_cache_params *p)
{
- struct kmem_cache *cachep;
-
VM_BUG_ON(p->is_root_cache);
- cachep = p->root_cache;
- return cache_from_memcg_idx(cachep, memcg_cache_id(p->memcg));
+ return kmem_cache_of_memcg(p->root_cache, p->memcg);
}
#ifdef CONFIG_SLABINFO
@@ -3017,14 +3015,9 @@ static void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
css_put(&memcg->css);
}
-/*
- * helper for acessing a memcg's index. It will be used as an index in the
- * child cache array in kmem_cache, and also to derive its name. This function
- * will return -1 when this is not a kmem-limited memcg.
- */
-int memcg_cache_id(struct mem_cgroup *memcg)
+int memcg_kmem_id(struct mem_cgroup *memcg)
{
- return memcg ? memcg->kmemcg_id : -1;
+ return memcg ? memcg->kmem_id : -1;
}
static int memcg_init_cache_id(struct mem_cgroup *memcg)
@@ -3034,12 +3027,12 @@ static int memcg_init_cache_id(struct mem_cgroup *memcg)
lockdep_assert_held(&activate_kmem_mutex);
- id = ida_simple_get(&kmem_limited_groups,
- 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL);
+ id = ida_simple_get(&memcg_kmem_ida,
+ 0, MEMCG_KMEM_ID_MAX + 1, GFP_KERNEL);
if (id < 0)
return id;
- if (id < memcg_limited_groups_array_size)
+ if (id < memcg_nr_kmem_ids_max)
goto out_setid;
/*
@@ -3047,10 +3040,10 @@ static int memcg_init_cache_id(struct mem_cgroup *memcg)
* per memcg data. Let's try to grow them then.
*/
size = id * 2;
- if (size < MEMCG_CACHES_MIN_SIZE)
- size = MEMCG_CACHES_MIN_SIZE;
- else if (size > MEMCG_CACHES_MAX_SIZE)
- size = MEMCG_CACHES_MAX_SIZE;
+ if (size < 4)
+ size = 4; /* a good number to start with */
+ if (size > MEMCG_KMEM_ID_MAX + 1)
+ size = MEMCG_KMEM_ID_MAX + 1;
mutex_lock(&memcg_slab_mutex);
err = kmem_cache_grow_memcg_arrays(size);
@@ -3064,14 +3057,14 @@ static int memcg_init_cache_id(struct mem_cgroup *memcg)
* walking over such an array won't get an index out of range provided
* they use an appropriate mutex to protect the array's elements.
*/
- memcg_limited_groups_array_size = size;
+ memcg_nr_kmem_ids_max = size;
out_setid:
- memcg->kmemcg_id = id;
+ memcg->kmem_id = id;
return 0;
out_rmid:
- ida_simple_remove(&kmem_limited_groups, id);
+ ida_simple_remove(&memcg_kmem_ida, id);
return err;
}
@@ -3089,11 +3082,10 @@ static int memcg_prepare_kmem_cache(struct kmem_cache *cachep)
return 0;
/* activate_kmem_mutex guarantees a stable value of
- * memcg_limited_groups_array_size */
+ * memcg_nr_kmem_ids_max */
mutex_lock(&activate_kmem_mutex);
mutex_lock(&memcg_slab_mutex);
- ret = kmem_cache_init_memcg_array(cachep,
- memcg_limited_groups_array_size);
+ ret = kmem_cache_init_memcg_array(cachep, memcg_nr_kmem_ids_max);
mutex_unlock(&memcg_slab_mutex);
mutex_unlock(&activate_kmem_mutex);
return ret;
@@ -3113,14 +3105,14 @@ static void memcg_copy_kmem_cache(struct mem_cgroup *memcg,
lockdep_assert_held(&memcg_slab_mutex);
- id = memcg_cache_id(memcg);
+ id = memcg_kmem_id(memcg);
/*
* Since per-memcg caches are created asynchronously on first
* allocation (see memcg_kmem_get_cache()), several threads can try to
* create the same cache, but only one of them may succeed.
*/
- if (cache_from_memcg_idx(root_cache, id))
+ if (kmem_cache_of_memcg_by_id(root_cache, id))
return;
params = kzalloc(sizeof(*params), GFP_KERNEL);
@@ -3146,8 +3138,8 @@ static void memcg_copy_kmem_cache(struct mem_cgroup *memcg,
list_add(&cachep->memcg_params->list, &memcg->memcg_slab_caches);
/*
- * Since readers won't lock (see cache_from_memcg_idx()), we need a
- * barrier here to ensure nobody will see the kmem_cache partially
+ * Since readers won't lock (see kmem_cache_of_memcg_by_id()), we need
+ * a barrier here to ensure nobody will see the kmem_cache partially
* initialized.
*/
smp_wmb();
@@ -3171,7 +3163,7 @@ static int memcg_destroy_kmem_cache_copy(struct kmem_cache *cachep)
params = cachep->memcg_params;
root_cache = params->root_cache;
memcg = params->memcg;
- id = memcg_cache_id(memcg);
+ id = memcg_kmem_id(memcg);
/*
* Since memcg_caches arrays can be accessed using only slab_mutex for
@@ -3243,8 +3235,8 @@ int kmem_cache_destroy_memcg_copies(struct kmem_cache *cachep)
BUG_ON(!is_root_cache(cachep));
mutex_lock(&memcg_slab_mutex);
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(cachep, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(cachep, i);
if (!c)
continue;
@@ -3388,7 +3380,7 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
if (!memcg_can_account_kmem(memcg))
goto out;
- memcg_cachep = cache_from_memcg_idx(cachep, memcg_cache_id(memcg));
+ memcg_cachep = kmem_cache_of_memcg(cachep, memcg);
if (likely(memcg_cachep)) {
cachep = memcg_cachep;
goto out;
@@ -5693,7 +5685,7 @@ static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
{
int ret;
- memcg->kmemcg_id = -1;
+ memcg->kmem_id = -1;
ret = memcg_propagate_kmem(memcg);
if (ret)
return ret;
diff --git a/mm/slab.c b/mm/slab.c
index 25317fd1daa2..194322a634e2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3844,8 +3844,8 @@ static int do_tune_cpucache(struct kmem_cache *cachep, int limit,
return ret;
VM_BUG_ON(!mutex_is_locked(&slab_mutex));
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(cachep, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(cachep, i);
if (c)
/* return value determined by the parent cache only */
__do_tune_cpucache(c, limit, batchcount, shared, gfp);
diff --git a/mm/slab.h b/mm/slab.h
index ba834860fbfd..61f833c569e7 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -145,11 +145,11 @@ static inline const char *cache_name(struct kmem_cache *s)
* created a memcg's cache is destroyed only along with the root cache, it is
* true if we are going to allocate from the cache or hold a reference to the
* root cache by other means. Otherwise, we should hold either the slab_mutex
- * or the memcg's slab_caches_mutex while calling this function and accessing
- * the returned value.
+ * or the memcg_slab_mutex while calling this function and accessing the
+ * returned value.
*/
static inline struct kmem_cache *
-cache_from_memcg_idx(struct kmem_cache *s, int idx)
+kmem_cache_of_memcg_by_id(struct kmem_cache *s, int id)
{
struct kmem_cache *cachep;
struct memcg_cache_params *params;
@@ -159,18 +159,24 @@ cache_from_memcg_idx(struct kmem_cache *s, int idx)
rcu_read_lock();
params = rcu_dereference(s->memcg_params);
- cachep = params->memcg_caches[idx];
+ cachep = params->memcg_caches[id];
rcu_read_unlock();
/*
* Make sure we will access the up-to-date value. The code updating
* memcg_caches issues a write barrier to match this (see
- * memcg_register_cache()).
+ * memcg_copy_kmem_cache()).
*/
smp_read_barrier_depends();
return cachep;
}
+static inline struct kmem_cache *
+kmem_cache_of_memcg(struct kmem_cache *s, struct mem_cgroup *memcg)
+{
+ return kmem_cache_of_memcg_by_id(s, memcg_kmem_id(memcg));
+}
+
static inline struct kmem_cache *memcg_root_cache(struct kmem_cache *s)
{
if (is_root_cache(s))
@@ -214,7 +220,13 @@ static inline const char *cache_name(struct kmem_cache *s)
}
static inline struct kmem_cache *
-cache_from_memcg_idx(struct kmem_cache *s, int idx)
+kmem_cache_of_memcg_by_id(struct kmem_cache *s, int id)
+{
+ return NULL;
+}
+
+static inline struct kmem_cache *
+kmem_cache_of_memcg(struct kmem_cache *s, struct mem_cgroup *memcg)
{
return NULL;
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 36d9b866a3ab..beaaaecd35f4 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -94,11 +94,11 @@ static int kmem_cache_realloc_memcg_array(struct kmem_cache *s, int nr_entries)
new->is_root_cache = true;
if (old) {
- for_each_memcg_cache_index(i)
+ for_each_possible_memcg_kmem_id(i)
new->memcg_caches[i] = old->memcg_caches[i];
}
- /* matching rcu_dereference is in cache_from_memcg_idx */
+ /* matching rcu_dereference is in kmem_cache_of_memcg_by_id */
rcu_assign_pointer(s->memcg_params, new);
if (old)
kfree_rcu(old, rcu_head);
@@ -327,7 +327,7 @@ kmem_cache_request_memcg_copy(struct memcg_cache_params *memcg_params,
mutex_lock(&slab_mutex);
cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
- memcg_cache_id(memcg), memcg_name);
+ memcg_kmem_id(memcg), memcg_name);
if (!cache_name)
goto out_unlock;
@@ -748,8 +748,8 @@ memcg_accumulate_slabinfo(struct kmem_cache *s, struct slabinfo *info)
if (!is_root_cache(s))
return;
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(s, i);
if (!c)
continue;
diff --git a/mm/slub.c b/mm/slub.c
index aa30932c5190..006e6bfe257c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3772,8 +3772,8 @@ __kmem_cache_alias(const char *name, size_t size, size_t align,
s->object_size = max(s->object_size, (int)size);
s->inuse = max_t(int, s->inuse, ALIGN(size, sizeof(void *)));
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(s, i);
if (!c)
continue;
c->object_size = s->object_size;
@@ -5062,8 +5062,8 @@ static ssize_t slab_attr_store(struct kobject *kobj,
* directly either failed or succeeded, in which case we loop
* through the descendants with best-effort propagation.
*/
- for_each_memcg_cache_index(i) {
- struct kmem_cache *c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ struct kmem_cache *c = kmem_cache_of_memcg_by_id(s, i);
if (c)
attribute->store(c, buf, len);
}
@@ -5198,7 +5198,7 @@ static char *create_unique_id(struct kmem_cache *s)
#ifdef CONFIG_MEMCG_KMEM
if (!is_root_cache(s))
p += sprintf(p, "-%08d",
- memcg_cache_id(s->memcg_params->memcg));
+ memcg_kmem_id(s->memcg_params->memcg));
#endif
BUG_ON(p > name + ID_STR_LENGTH - 1);
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH -mm v1.1] memcg: cleanup kmem_id-related naming
2014-04-25 12:33 ` [PATCH -mm 6/6] memcg: cleanup kmem_id-related naming Vladimir Davydov
@ 2014-04-25 13:12 ` Vladimir Davydov
0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-25 13:12 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
The naming of mem_cgroup->kmemcg_id-related functions is rather
inconsistent. We tend to use cache_id as part of their names, but it
isn't quite right, because kmem_id isn't something specific to kmem
caches. It can be used for indexing any array that stores per memcg
data. For instance, we will use it to make list_lru per memcg in the
future. So let's clean up the names and comments related to kmem_id.
Brief change log:
** old name ** ** new name **
mem_cgroup->kmemcg_id mem_cgroup->kmem_id
memcg_init_cache_id() memcg_init_kmem_id()
memcg_cache_id() memcg_kmem_id()
cache_from_memcg_idx() kmem_cache_of_memcg_by_id()
cache_from_memcg_idx(memcg_cache_id()) kmem_cache_of_memcg()
for_each_memcg_cache_index() for_each_possible_memcg_kmem_id()
memcg_limited_groups memcg_kmem_ida
memcg_limited_groups_array_size memcg_nr_kmem_ids_max
MEMCG_CACHES_MIN_SIZE <constant inlined>
MEMCG_CACHES_MAX_SIZE MEMCG_KMEM_ID_MAX + 1
Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
---
Changes in v1.1:
- rename memcg_init_cache_id -> memcg_init_kmem_id
(lost that hunk somehow in v1)
include/linux/memcontrol.h | 19 +++-----
mm/memcontrol.c | 114 ++++++++++++++++++++------------------------
mm/slab.c | 4 +-
mm/slab.h | 24 +++++++---
mm/slab_common.c | 10 ++--
mm/slub.c | 10 ++--
6 files changed, 90 insertions(+), 91 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 3ee73da2991b..4080a6418c72 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -458,15 +458,10 @@ static inline void sock_release_memcg(struct sock *sk)
#ifdef CONFIG_MEMCG_KMEM
extern struct static_key memcg_kmem_enabled_key;
-extern int memcg_limited_groups_array_size;
+extern int memcg_nr_kmem_ids_max;
-/*
- * Helper macro to loop through all memcg-specific caches. Callers must still
- * check if the cache is valid (it is either valid or NULL).
- * the slab_mutex must be held when looping through those caches
- */
-#define for_each_memcg_cache_index(_idx) \
- for ((_idx) = 0; (_idx) < memcg_limited_groups_array_size; (_idx)++)
+#define for_each_possible_memcg_kmem_id(id) \
+ for ((id) = 0; (id) < memcg_nr_kmem_ids_max; (id)++)
static inline bool memcg_kmem_enabled(void)
{
@@ -490,7 +485,7 @@ 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_cache_id(struct mem_cgroup *memcg);
+int memcg_kmem_id(struct mem_cgroup *memcg);
struct kmem_cache *
__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
@@ -591,8 +586,8 @@ memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
return __memcg_kmem_get_cache(cachep, gfp);
}
#else
-#define for_each_memcg_cache_index(_idx) \
- for (; NULL; )
+#define for_each_possible_memcg_kmem_id(id) \
+ for ((id) = 0; 0; )
static inline bool memcg_kmem_enabled(void)
{
@@ -614,7 +609,7 @@ memcg_kmem_commit_charge(struct page *page, struct mem_cgroup *memcg, int order)
{
}
-static inline int memcg_cache_id(struct mem_cgroup *memcg)
+static inline int memcg_kmem_id(struct mem_cgroup *memcg)
{
return -1;
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index bdd3d373cdca..7ecbae9b7800 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -357,11 +357,22 @@ struct mem_cgroup {
struct cg_proto tcp_mem;
#endif
#if defined(CONFIG_MEMCG_KMEM)
+ /*
+ * Each kmem-limited memory cgroup has a unique id. We use it for
+ * indexing the arrays that store per cgroup data. An example of such
+ * an array is kmem_cache->memcg_params->memcg_caches.
+ *
+ * We introduce a separate id instead of using cgroup->id to avoid
+ * waste of memory in sparse environments, where we have a lot of
+ * memory cgroups, but only a few of them are kmem-limited.
+ *
+ * For unlimited cgroups kmem_id equals -1.
+ */
+ int kmem_id;
+
/* analogous to slab_common's slab_caches list, but per-memcg;
* protected by memcg_slab_mutex */
struct list_head memcg_slab_caches;
- /* Index in the kmem_cache->memcg_params->memcg_caches array */
- int kmemcg_id;
#endif
int last_scanned_node;
@@ -610,35 +621,25 @@ static void disarm_sock_keys(struct mem_cgroup *memcg)
#endif
#ifdef CONFIG_MEMCG_KMEM
+/* used for mem_cgroup->kmem_id allocations */
+static DEFINE_IDA(memcg_kmem_ida);
+
/*
- * This will be the memcg's index in each cache's ->memcg_params->memcg_caches.
- * The main reason for not using cgroup id for this:
- * this works better in sparse environments, where we have a lot of memcgs,
- * but only a few kmem-limited. Or also, if we have, for instance, 200
- * memcgs, and none but the 200th is kmem-limited, we'd have to have a
- * 200 entry array for that.
- *
- * The current size of the caches array is stored in
- * memcg_limited_groups_array_size. It will double each time we have to
- * increase it.
+ * Max kmem id should be as large as max cgroup id so that we could enable
+ * kmem-accounting for each memory cgroup.
*/
-static DEFINE_IDA(kmem_limited_groups);
-int memcg_limited_groups_array_size;
+#define MEMCG_KMEM_ID_MAX MEM_CGROUP_ID_MAX
/*
- * MIN_SIZE is different than 1, because we would like to avoid going through
- * the alloc/free process all the time. In a small machine, 4 kmem-limited
- * cgroups is a reasonable guess. In the future, it could be a parameter or
- * tunable, but that is strictly not necessary.
+ * We keep the maximal number of kmem ids that may exist in the system in the
+ * memcg_nr_kmem_ids_max variable. We use it for the size of the arrays indexed
+ * by kmem id (see the mem_cgroup->kmem_id definition).
*
- * MAX_SIZE should be as large as the number of cgrp_ids. Ideally, we could get
- * this constant directly from cgroup, but it is understandable that this is
- * better kept as an internal representation in cgroup.c. In any case, the
- * cgrp_id space is not getting any smaller, and we don't have to necessarily
- * increase ours as well if it increases.
+ * If a newly allocated kmem id is greater or equal to memcg_nr_kmem_ids_max,
+ * we double it and reallocate the arrays so that they have enough space to
+ * store data for the new cgroup.
*/
-#define MEMCG_CACHES_MIN_SIZE 4
-#define MEMCG_CACHES_MAX_SIZE MEM_CGROUP_ID_MAX
+int memcg_nr_kmem_ids_max;
/*
* A lot of the calls to the cache allocation functions are expected to be
@@ -653,7 +654,7 @@ static void disarm_kmem_keys(struct mem_cgroup *memcg)
{
if (memcg_kmem_is_active(memcg)) {
static_key_slow_dec(&memcg_kmem_enabled_key);
- ida_simple_remove(&kmem_limited_groups, memcg->kmemcg_id);
+ ida_simple_remove(&memcg_kmem_ida, memcg->kmem_id);
}
/*
* This check can't live in kmem destruction function,
@@ -2930,11 +2931,8 @@ static inline bool memcg_can_account_kmem(struct mem_cgroup *memcg)
*/
static struct kmem_cache *memcg_params_to_cache(struct memcg_cache_params *p)
{
- struct kmem_cache *cachep;
-
VM_BUG_ON(p->is_root_cache);
- cachep = p->root_cache;
- return cache_from_memcg_idx(cachep, memcg_cache_id(p->memcg));
+ return kmem_cache_of_memcg(p->root_cache, p->memcg);
}
#ifdef CONFIG_SLABINFO
@@ -3017,29 +3015,24 @@ static void memcg_uncharge_kmem(struct mem_cgroup *memcg, u64 size)
css_put(&memcg->css);
}
-/*
- * helper for acessing a memcg's index. It will be used as an index in the
- * child cache array in kmem_cache, and also to derive its name. This function
- * will return -1 when this is not a kmem-limited memcg.
- */
-int memcg_cache_id(struct mem_cgroup *memcg)
+int memcg_kmem_id(struct mem_cgroup *memcg)
{
- return memcg ? memcg->kmemcg_id : -1;
+ return memcg ? memcg->kmem_id : -1;
}
-static int memcg_init_cache_id(struct mem_cgroup *memcg)
+static int memcg_init_kmem_id(struct mem_cgroup *memcg)
{
int err = 0;
int id, size;
lockdep_assert_held(&activate_kmem_mutex);
- id = ida_simple_get(&kmem_limited_groups,
- 0, MEMCG_CACHES_MAX_SIZE, GFP_KERNEL);
+ id = ida_simple_get(&memcg_kmem_ida,
+ 0, MEMCG_KMEM_ID_MAX + 1, GFP_KERNEL);
if (id < 0)
return id;
- if (id < memcg_limited_groups_array_size)
+ if (id < memcg_nr_kmem_ids_max)
goto out_setid;
/*
@@ -3047,10 +3040,10 @@ static int memcg_init_cache_id(struct mem_cgroup *memcg)
* per memcg data. Let's try to grow them then.
*/
size = id * 2;
- if (size < MEMCG_CACHES_MIN_SIZE)
- size = MEMCG_CACHES_MIN_SIZE;
- else if (size > MEMCG_CACHES_MAX_SIZE)
- size = MEMCG_CACHES_MAX_SIZE;
+ if (size < 4)
+ size = 4; /* a good number to start with */
+ if (size > MEMCG_KMEM_ID_MAX + 1)
+ size = MEMCG_KMEM_ID_MAX + 1;
mutex_lock(&memcg_slab_mutex);
err = kmem_cache_grow_memcg_arrays(size);
@@ -3064,14 +3057,14 @@ static int memcg_init_cache_id(struct mem_cgroup *memcg)
* walking over such an array won't get an index out of range provided
* they use an appropriate mutex to protect the array's elements.
*/
- memcg_limited_groups_array_size = size;
+ memcg_nr_kmem_ids_max = size;
out_setid:
- memcg->kmemcg_id = id;
+ memcg->kmem_id = id;
return 0;
out_rmid:
- ida_simple_remove(&kmem_limited_groups, id);
+ ida_simple_remove(&memcg_kmem_ida, id);
return err;
}
@@ -3089,11 +3082,10 @@ static int memcg_prepare_kmem_cache(struct kmem_cache *cachep)
return 0;
/* activate_kmem_mutex guarantees a stable value of
- * memcg_limited_groups_array_size */
+ * memcg_nr_kmem_ids_max */
mutex_lock(&activate_kmem_mutex);
mutex_lock(&memcg_slab_mutex);
- ret = kmem_cache_init_memcg_array(cachep,
- memcg_limited_groups_array_size);
+ ret = kmem_cache_init_memcg_array(cachep, memcg_nr_kmem_ids_max);
mutex_unlock(&memcg_slab_mutex);
mutex_unlock(&activate_kmem_mutex);
return ret;
@@ -3113,14 +3105,14 @@ static void memcg_copy_kmem_cache(struct mem_cgroup *memcg,
lockdep_assert_held(&memcg_slab_mutex);
- id = memcg_cache_id(memcg);
+ id = memcg_kmem_id(memcg);
/*
* Since per-memcg caches are created asynchronously on first
* allocation (see memcg_kmem_get_cache()), several threads can try to
* create the same cache, but only one of them may succeed.
*/
- if (cache_from_memcg_idx(root_cache, id))
+ if (kmem_cache_of_memcg_by_id(root_cache, id))
return;
params = kzalloc(sizeof(*params), GFP_KERNEL);
@@ -3146,8 +3138,8 @@ static void memcg_copy_kmem_cache(struct mem_cgroup *memcg,
list_add(&cachep->memcg_params->list, &memcg->memcg_slab_caches);
/*
- * Since readers won't lock (see cache_from_memcg_idx()), we need a
- * barrier here to ensure nobody will see the kmem_cache partially
+ * Since readers won't lock (see kmem_cache_of_memcg_by_id()), we need
+ * a barrier here to ensure nobody will see the kmem_cache partially
* initialized.
*/
smp_wmb();
@@ -3171,7 +3163,7 @@ static int memcg_destroy_kmem_cache_copy(struct kmem_cache *cachep)
params = cachep->memcg_params;
root_cache = params->root_cache;
memcg = params->memcg;
- id = memcg_cache_id(memcg);
+ id = memcg_kmem_id(memcg);
/*
* Since memcg_caches arrays can be accessed using only slab_mutex for
@@ -3243,8 +3235,8 @@ int kmem_cache_destroy_memcg_copies(struct kmem_cache *cachep)
BUG_ON(!is_root_cache(cachep));
mutex_lock(&memcg_slab_mutex);
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(cachep, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(cachep, i);
if (!c)
continue;
@@ -3388,7 +3380,7 @@ struct kmem_cache *__memcg_kmem_get_cache(struct kmem_cache *cachep,
if (!memcg_can_account_kmem(memcg))
goto out;
- memcg_cachep = cache_from_memcg_idx(cachep, memcg_cache_id(memcg));
+ memcg_cachep = kmem_cache_of_memcg(cachep, memcg);
if (likely(memcg_cachep)) {
cachep = memcg_cachep;
goto out;
@@ -4962,7 +4954,7 @@ static int __memcg_activate_kmem(struct mem_cgroup *memcg,
if (err)
goto out;
- err = memcg_init_cache_id(memcg);
+ err = memcg_init_kmem_id(memcg);
if (err)
goto out;
@@ -5693,7 +5685,7 @@ static int memcg_init_kmem(struct mem_cgroup *memcg, struct cgroup_subsys *ss)
{
int ret;
- memcg->kmemcg_id = -1;
+ memcg->kmem_id = -1;
ret = memcg_propagate_kmem(memcg);
if (ret)
return ret;
diff --git a/mm/slab.c b/mm/slab.c
index 25317fd1daa2..194322a634e2 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3844,8 +3844,8 @@ static int do_tune_cpucache(struct kmem_cache *cachep, int limit,
return ret;
VM_BUG_ON(!mutex_is_locked(&slab_mutex));
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(cachep, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(cachep, i);
if (c)
/* return value determined by the parent cache only */
__do_tune_cpucache(c, limit, batchcount, shared, gfp);
diff --git a/mm/slab.h b/mm/slab.h
index ba834860fbfd..61f833c569e7 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -145,11 +145,11 @@ static inline const char *cache_name(struct kmem_cache *s)
* created a memcg's cache is destroyed only along with the root cache, it is
* true if we are going to allocate from the cache or hold a reference to the
* root cache by other means. Otherwise, we should hold either the slab_mutex
- * or the memcg's slab_caches_mutex while calling this function and accessing
- * the returned value.
+ * or the memcg_slab_mutex while calling this function and accessing the
+ * returned value.
*/
static inline struct kmem_cache *
-cache_from_memcg_idx(struct kmem_cache *s, int idx)
+kmem_cache_of_memcg_by_id(struct kmem_cache *s, int id)
{
struct kmem_cache *cachep;
struct memcg_cache_params *params;
@@ -159,18 +159,24 @@ cache_from_memcg_idx(struct kmem_cache *s, int idx)
rcu_read_lock();
params = rcu_dereference(s->memcg_params);
- cachep = params->memcg_caches[idx];
+ cachep = params->memcg_caches[id];
rcu_read_unlock();
/*
* Make sure we will access the up-to-date value. The code updating
* memcg_caches issues a write barrier to match this (see
- * memcg_register_cache()).
+ * memcg_copy_kmem_cache()).
*/
smp_read_barrier_depends();
return cachep;
}
+static inline struct kmem_cache *
+kmem_cache_of_memcg(struct kmem_cache *s, struct mem_cgroup *memcg)
+{
+ return kmem_cache_of_memcg_by_id(s, memcg_kmem_id(memcg));
+}
+
static inline struct kmem_cache *memcg_root_cache(struct kmem_cache *s)
{
if (is_root_cache(s))
@@ -214,7 +220,13 @@ static inline const char *cache_name(struct kmem_cache *s)
}
static inline struct kmem_cache *
-cache_from_memcg_idx(struct kmem_cache *s, int idx)
+kmem_cache_of_memcg_by_id(struct kmem_cache *s, int id)
+{
+ return NULL;
+}
+
+static inline struct kmem_cache *
+kmem_cache_of_memcg(struct kmem_cache *s, struct mem_cgroup *memcg)
{
return NULL;
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 36d9b866a3ab..beaaaecd35f4 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -94,11 +94,11 @@ static int kmem_cache_realloc_memcg_array(struct kmem_cache *s, int nr_entries)
new->is_root_cache = true;
if (old) {
- for_each_memcg_cache_index(i)
+ for_each_possible_memcg_kmem_id(i)
new->memcg_caches[i] = old->memcg_caches[i];
}
- /* matching rcu_dereference is in cache_from_memcg_idx */
+ /* matching rcu_dereference is in kmem_cache_of_memcg_by_id */
rcu_assign_pointer(s->memcg_params, new);
if (old)
kfree_rcu(old, rcu_head);
@@ -327,7 +327,7 @@ kmem_cache_request_memcg_copy(struct memcg_cache_params *memcg_params,
mutex_lock(&slab_mutex);
cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
- memcg_cache_id(memcg), memcg_name);
+ memcg_kmem_id(memcg), memcg_name);
if (!cache_name)
goto out_unlock;
@@ -748,8 +748,8 @@ memcg_accumulate_slabinfo(struct kmem_cache *s, struct slabinfo *info)
if (!is_root_cache(s))
return;
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(s, i);
if (!c)
continue;
diff --git a/mm/slub.c b/mm/slub.c
index aa30932c5190..006e6bfe257c 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3772,8 +3772,8 @@ __kmem_cache_alias(const char *name, size_t size, size_t align,
s->object_size = max(s->object_size, (int)size);
s->inuse = max_t(int, s->inuse, ALIGN(size, sizeof(void *)));
- for_each_memcg_cache_index(i) {
- c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ c = kmem_cache_of_memcg_by_id(s, i);
if (!c)
continue;
c->object_size = s->object_size;
@@ -5062,8 +5062,8 @@ static ssize_t slab_attr_store(struct kobject *kobj,
* directly either failed or succeeded, in which case we loop
* through the descendants with best-effort propagation.
*/
- for_each_memcg_cache_index(i) {
- struct kmem_cache *c = cache_from_memcg_idx(s, i);
+ for_each_possible_memcg_kmem_id(i) {
+ struct kmem_cache *c = kmem_cache_of_memcg_by_id(s, i);
if (c)
attribute->store(c, buf, len);
}
@@ -5198,7 +5198,7 @@ static char *create_unique_id(struct kmem_cache *s)
#ifdef CONFIG_MEMCG_KMEM
if (!is_root_cache(s))
p += sprintf(p, "-%08d",
- memcg_cache_id(s->memcg_params->memcg));
+ memcg_kmem_id(s->memcg_params->memcg));
#endif
BUG_ON(p > name + ID_STR_LENGTH - 1);
--
1.7.10.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows
2014-04-25 12:33 [PATCH -mm 0/6] memcg/kmem: cleanup naming and callflows Vladimir Davydov
` (5 preceding siblings ...)
2014-04-25 12:33 ` [PATCH -mm 6/6] memcg: cleanup kmem_id-related naming Vladimir Davydov
@ 2014-04-27 8:56 ` Vladimir Davydov
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Davydov @ 2014-04-27 8:56 UTC (permalink / raw)
To: akpm; +Cc: mhocko, hannes, glommer, cl, penberg, linux-kernel, linux-mm,
devel
Please ignore this set - I found that something can be improved, will
send v2 soon. Sorry for the noise.
Thanks.
On Fri, Apr 25, 2014 at 04:33:06PM +0400, Vladimir Davydov wrote:
> Hi,
>
> In reply to "[PATCH RFC -mm v2 3/3] memcg, slab: simplify
> synchronization scheme" Johannes wrote:
>
> > I like this patch, but the API names are confusing. Could we fix up
> > that whole thing by any chance?
>
> (see https://lkml.org/lkml/2014/4/18/317)
>
> So this patch set is about cleaning up memcg/kmem naming.
>
> While preparing it I found that some of the ugly-named functions
> constituting interface between memcontrol.c and slab_common.c can be
> neatly got rid of w/o complicating the code. Quite the contrary, w/o
> them call-flows look much simpler, IMO. So the first four patches do not
> rename anything actually - they just rework call-flows in kmem cache
> creation/destruction and memcg_caches arrays relocations paths. Finally,
> patches 5 and 6 clean up the naming.
>
> Reviews are appreciated.
>
> Thanks,
>
> Vladimir Davydov (6):
> memcg: get rid of memcg_create_cache_name
> memcg: allocate memcg_caches array on first per memcg cache creation
> memcg: cleanup memcg_caches arrays relocation path
> memcg: get rid of memcg_{alloc,free}_cache_params
> memcg: cleanup kmem cache creation/destruction functions naming
> memcg: cleanup kmem_id-related naming
>
> include/linux/memcontrol.h | 40 +----
> include/linux/slab.h | 12 +-
> mm/memcontrol.c | 420 ++++++++++++++++++++------------------------
> mm/slab.c | 4 +-
> mm/slab.h | 24 ++-
> mm/slab_common.c | 127 ++++++++++----
> mm/slub.c | 10 +-
> 7 files changed, 320 insertions(+), 317 deletions(-)
>
> --
> 1.7.10.4
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread