[RFC/PATCH 1/2] slub: Break out alloc_kmem_cache_cpus() from kmem_cache_open()

[RFC/PATCH 1/2] slub: Break out alloc_kmem_cache_cpus() from kmem_cache_open()

[Pekka Enberg]

In preparation for sharing per-CPU structures between merged caches, break out
alloc_kmem_cache_cpus() from kmem_cache_open() function.

Cc: Christoph Lameter <cl@linux.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 mm/slub.c |   37 +++++++++++++++++++++----------------
 1 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 94d2a33..cb61024 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2638,11 +2638,7 @@ static int kmem_cache_open(struct kmem_cache *s,
 #endif
 	if (!init_kmem_cache_nodes(s))
 		goto error;
-
-	if (alloc_kmem_cache_cpus(s))
-		return 1;
-
-	free_kmem_cache_nodes(s);
+	return 1;
 error:
 	if (flags & SLAB_PANIC)
 		panic("Cannot create slab %s size=%lu realsize=%u "
@@ -2818,6 +2814,9 @@ static struct kmem_cache *__init create_kmalloc_cache(const char *name,
 								flags, NULL))
 		goto panic;
 
+	if (!alloc_kmem_cache_cpus(s))
+		goto panic;
+
 	list_add(&s->list, &slab_caches);
 	return s;
 
@@ -3239,6 +3238,8 @@ void __init kmem_cache_init(void)
 		sizeof(struct kmem_cache_node),
 		0, SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
 
+	alloc_kmem_cache_cpus(kmem_cache_node);
+
 	hotplug_memory_notifier(slab_memory_callback, SLAB_CALLBACK_PRI);
 
 	/* Able to allocate the per node structures */
@@ -3247,6 +3248,8 @@ void __init kmem_cache_init(void)
 	temp_kmem_cache = kmem_cache;
 	kmem_cache_open(kmem_cache, "kmem_cache", kmem_size,
 		0, SLAB_HWCACHE_ALIGN | SLAB_PANIC, NULL);
+	alloc_kmem_cache_cpus(kmem_cache);
+
 	kmem_cache = kmem_cache_alloc(kmem_cache, GFP_NOWAIT);
 	memcpy(kmem_cache, temp_kmem_cache, kmem_size);
 
@@ -3469,18 +3472,20 @@ struct kmem_cache *kmem_cache_create(const char *name, size_t size,
 
 	s = kmalloc(kmem_size, GFP_KERNEL);
 	if (s) {
-		if (kmem_cache_open(s, n,
-				size, align, flags, ctor)) {
-			list_add(&s->list, &slab_caches);
-			if (sysfs_slab_add(s)) {
-				list_del(&s->list);
-				kfree(n);
-				kfree(s);
-				goto err;
-			}
-			up_write(&slub_lock);
-			return s;
+		if (!kmem_cache_open(s, n, size, align, flags, ctor))
+			goto err_free;
+
+		if (!alloc_kmem_cache_cpus(s))
+			goto err_free;	
+
+		list_add(&s->list, &slab_caches);
+		if (sysfs_slab_add(s)) {
+			list_del(&s->list);
+			goto err_free;
 		}
+		up_write(&slub_lock);
+		return s;
+err_free:
 		kfree(n);
 		kfree(s);
 	}
-- 
1.7.0.4

[RFC/PATCH 2/2] slub: Don't share struct kmem_cache for shared caches

[Pekka Enberg]

This patch changes the slab cache sharing in SLUB to only share 'struct
kmem_cpu_cache' which contains the actual list of slabs and object freelists.
We no longer share 'struct kmem_cache' between merged caches so /proc/slabinfo
statistics work as expected:

  Before:

  # cat /proc/slabinfo | wc -l
  104

  After:

  # cat /proc/slabinfo | wc -l
  185

Cc: Christoph Lameter <cl@linux.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 mm/slub.c |   98 +++++++++++++++++++-----------------------------------------
 1 files changed, 31 insertions(+), 67 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index cb61024..3a8fbca 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2349,6 +2349,11 @@ init_kmem_cache_node(struct kmem_cache_node *n, struct kmem_cache *s)
 #endif
 }
 
+static inline void dup_kmem_cache_cpus(struct kmem_cache *src, struct kmem_cache *dst)
+{
+	dst->cpu_slab = src->cpu_slab;
+}
+
 static inline int alloc_kmem_cache_cpus(struct kmem_cache *s)
 {
 	BUILD_BUG_ON(PERCPU_DYNAMIC_EARLY_SIZE <
@@ -3441,26 +3446,41 @@ static struct kmem_cache *find_mergeable(size_t size,
 struct kmem_cache *kmem_cache_create(const char *name, size_t size,
 		size_t align, unsigned long flags, void (*ctor)(void *))
 {
-	struct kmem_cache *s;
+	struct kmem_cache *s, *parent;
 	char *n;
 
 	if (WARN_ON(!name))
 		return NULL;
 
 	down_write(&slub_lock);
-	s = find_mergeable(size, align, flags, name, ctor);
-	if (s) {
-		s->refcount++;
+	parent = find_mergeable(size, align, flags, name, ctor);
+	if (parent) {
+		n = kstrdup(name, GFP_KERNEL);
+		if (!n)
+			goto err;
+
+		s = kmalloc(kmem_size, GFP_KERNEL);
+		if (!s)
+			goto err_free;
+
+		if (!kmem_cache_open(s, n, size, align, flags, ctor))
+			goto err_free;
+
+		dup_kmem_cache_cpus(parent, s);
+
+		parent->refcount++;
 		/*
 		 * Adjust the object sizes so that we clear
 		 * the complete object on kzalloc.
 		 */
-		s->objsize = max(s->objsize, (int)size);
-		s->inuse = max_t(int, s->inuse, ALIGN(size, sizeof(void *)));
+		parent->objsize = max(parent->objsize, (int)size);
+		parent->inuse = max_t(int, parent->inuse, ALIGN(size, sizeof(void *)));
 
-		if (sysfs_slab_alias(s, name)) {
-			s->refcount--;
-			goto err;
+		list_add(&s->list, &slab_caches);
+		if (sysfs_slab_add(s)) {
+			parent->refcount--;
+			list_del(&s->list);
+			goto err_free;
 		}
 		up_write(&slub_lock);
 		return s;
@@ -4670,68 +4690,17 @@ static const struct kset_uevent_ops slab_uevent_ops = {
 
 static struct kset *slab_kset;
 
-#define ID_STR_LENGTH 64
-
-/* Create a unique string id for a slab cache:
- *
- * Format	:[flags-]size
- */
-static char *create_unique_id(struct kmem_cache *s)
-{
-	char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
-	char *p = name;
-
-	BUG_ON(!name);
-
-	*p++ = ':';
-	/*
-	 * First flags affecting slabcache operations. We will only
-	 * get here for aliasable slabs so we do not need to support
-	 * too many flags. The flags here must cover all flags that
-	 * are matched during merging to guarantee that the id is
-	 * unique.
-	 */
-	if (s->flags & SLAB_CACHE_DMA)
-		*p++ = 'd';
-	if (s->flags & SLAB_RECLAIM_ACCOUNT)
-		*p++ = 'a';
-	if (s->flags & SLAB_DEBUG_FREE)
-		*p++ = 'F';
-	if (!(s->flags & SLAB_NOTRACK))
-		*p++ = 't';
-	if (p != name + 1)
-		*p++ = '-';
-	p += sprintf(p, "%07d", s->size);
-	BUG_ON(p > name + ID_STR_LENGTH - 1);
-	return name;
-}
-
 static int sysfs_slab_add(struct kmem_cache *s)
 {
 	int err;
 	const char *name;
-	int unmergeable;
 
 	if (slab_state < SYSFS)
 		/* Defer until later */
 		return 0;
 
-	unmergeable = slab_unmergeable(s);
-	if (unmergeable) {
-		/*
-		 * Slabcache can never be merged so we can use the name proper.
-		 * This is typically the case for debug situations. In that
-		 * case we can catch duplicate names easily.
-		 */
-		sysfs_remove_link(&slab_kset->kobj, s->name);
-		name = s->name;
-	} else {
-		/*
-		 * Create a unique name for the slab as a target
-		 * for the symlinks.
-		 */
-		name = create_unique_id(s);
-	}
+	sysfs_remove_link(&slab_kset->kobj, s->name);
+	name = s->name;
 
 	s->kobj.kset = slab_kset;
 	err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, name);
@@ -4747,11 +4716,6 @@ static int sysfs_slab_add(struct kmem_cache *s)
 		return err;
 	}
 	kobject_uevent(&s->kobj, KOBJ_ADD);
-	if (!unmergeable) {
-		/* Setup first alias */
-		sysfs_slab_alias(s, s->name);
-		kfree(name);
-	}
 	return 0;
 }
 
-- 
1.7.0.4

Re: [RFC/PATCH 1/2] slub: Break out alloc_kmem_cache_cpus() from kmem_cache_open()

[Christoph Lameter]

On Mon, 25 Apr 2011, Pekka Enberg wrote:

> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2638,11 +2638,7 @@ static int kmem_cache_open(struct kmem_cache *s,
>  #endif
>  	if (!init_kmem_cache_nodes(s))
>  		goto error;
> -
> -	if (alloc_kmem_cache_cpus(s))
> -		return 1;
> -
> -	free_kmem_cache_nodes(s);

free_kmem_cache_nodes() shows up nowhere now. Memory leak?

Re: [RFC/PATCH 2/2] slub: Don't share struct kmem_cache for shared caches

[Christoph Lameter]

On Mon, 25 Apr 2011, Pekka Enberg wrote:

> This patch changes the slab cache sharing in SLUB to only share 'struct
> kmem_cpu_cache' which contains the actual list of slabs and object freelists.
> We no longer share 'struct kmem_cache' between merged caches so /proc/slabinfo
> statistics work as expected:

I am a bit confused. Struct kmem_cache contains per slab
allocation information and all pages have pointers to struct kmem_cache.

So we have pages now on the kmem_cache_cpu list with varying kmem_cache
pointers? How do we figure out which cache a page belongs to? It seems
that a page from one cache can be freed to the freelists of another.

Does this even run right with the debugging enabled?

Re: [RFC/PATCH 2/2] slub: Don't share struct kmem_cache for shared caches

[Pekka Enberg]

On Mon, 25 Apr 2011, Pekka Enberg wrote:
>> This patch changes the slab cache sharing in SLUB to only share 'struct
>> kmem_cpu_cache' which contains the actual list of slabs and object freelists.
>> We no longer share 'struct kmem_cache' between merged caches so /proc/slabinfo
>> statistics work as expected:

On Mon, Apr 25, 2011 at 10:09 PM, Christoph Lameter <cl@linux.com> wrote:
> I am a bit confused. Struct kmem_cache contains per slab
> allocation information and all pages have pointers to struct kmem_cache.
>
> So we have pages now on the kmem_cache_cpu list with varying kmem_cache
> pointers? How do we figure out which cache a page belongs to? It seems
> that a page from one cache can be freed to the freelists of another.

We share the freelist so it goes to the same freelist. However, I
completely forgot about page to struct kmem_cache mapping so stats
won't work properly.

On Mon, Apr 25, 2011 at 10:09 PM, Christoph Lameter <cl@linux.com> wrote:
> Does this even run right with the debugging enabled?

It boots OK to minimal userspace under KVM with slub_debug. But no, I
didn't test it properly yet.

                        Pekka

Re: [RFC/PATCH 2/2] slub: Don't share struct kmem_cache for shared caches

[Christoph Lameter]

On Mon, 25 Apr 2011, Pekka Enberg wrote:

> On Mon, 25 Apr 2011, Pekka Enberg wrote:
> >> This patch changes the slab cache sharing in SLUB to only share 'struct
> >> kmem_cpu_cache' which contains the actual list of slabs and object freelists.
> >> We no longer share 'struct kmem_cache' between merged caches so /proc/slabinfo
> >> statistics work as expected:
>
> On Mon, Apr 25, 2011 at 10:09 PM, Christoph Lameter <cl@linux.com> wrote:
> > I am a bit confused. Struct kmem_cache contains per slab
> > allocation information and all pages have pointers to struct kmem_cache.
> >
> > So we have pages now on the kmem_cache_cpu list with varying kmem_cache
> > pointers? How do we figure out which cache a page belongs to? It seems
> > that a page from one cache can be freed to the freelists of another.
>
> We share the freelist so it goes to the same freelist. However, I
> completely forgot about page to struct kmem_cache mapping so stats
> won't work properly.

How are the freelist shared? You would need to share kmem_cache_node too
to make this work but then you dont have counters per slab anymore.

The counters are in kmem_cache_cpu and kmem_cache_node. So why a
kmem_cache structure for separate stats?

> On Mon, Apr 25, 2011 at 10:09 PM, Christoph Lameter <cl@linux.com> wrote:
> > Does this even run right with the debugging enabled?
>
> It boots OK to minimal userspace under KVM with slub_debug. But no, I
> didn't test it properly yet.

Run a validation of all objects please. That should check the slab headers
for correct pointers back.

Also by having multiple kmem_cache * for a single kmem_cache_cpu/node
structure you end up with an elevated cacheline footprint.