Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Tue, 24 Apr 2007, Neil Brown wrote:

> kobject_set_name actually takes a format and arbitrary args and uses
> vsnprintf, so it has to make it's own copy.

Ok then this should be fine...

SLAB: Fix sysfs directory handling

This fixes the problem that SLUB does not track the names of aliased
slabs by changing the way that SLUB manages the files in /sys/slab.

If the slab that is being operated on is not mergeable (usually the
case if we are debugging) then do not create any aliases. If an alias
exists that we conflict with then remove it before creating the
directory for the unmergeable slab. If there is a true slab cache there
and not an alias then we fail since there is a true duplication of
slab cache names. So debugging allows the detection of slab name
duplication as usual.

If the slab is mergeable then we create a directory with a unique name
created from the slab size, slab options and the pointer to the kmem_cache
structure (disambiguation). All names referring to the slabs will
then be created as symlinks to that unique name. These symlinks are
not going to be removed on kmem_cache_destroy() since we only carry
a counter for the number of aliases. If a new symlink is created
then it may just replace an existing one. This means that one can create
a gazillion slabs with the same name (if they all refer to mergeable
caches). It will only increase the alias count. So we have the potential
of not detecting duplicate slab names (there is actually no harm
done by doing that). We will detect the duplications as
as soon as debugging is enabled because we will then no longer
generate symlinks and special unique names.

Signed-off-by: Christoph Lameter <[EMAIL PROTECTED]>

Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-23 13:08:41.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-23 18:05:16.0 -0700
@@ -3307,16 +3307,68 @@ static struct kset_uevent_ops slab_ueven
 
 decl_subsys(slab, _ktype, _uevent_ops);
 
+#define ID_STR_LENGTH 64
+
+/* Create a unique string id for a slab cache:
+ * format
+ * :[flags-]size:[memory address of kmemcache]
+ */
+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 */
+   if (s->flags & SLAB_CACHE_DMA)
+   *p++ = 'd';
+   if (s->flags & SLAB_RECLAIM_ACCOUNT)
+   *p++ = 'a';
+   if (s->flags & SLAB_DESTROY_BY_RCU)
+   *p++ = 'r';\
+   /* Debug flags */
+   if (s->flags & SLAB_RED_ZONE)
+   *p++ = 'Z';
+   if (s->flags & SLAB_POISON)
+   *p++ = 'P';
+   if (s->flags & SLAB_STORE_USER)
+   *p++ = 'U';
+   if (p != name + 1)
+   *p++ = '-';
+   p += sprintf(p,"%07d:0x%p" ,s->size, s);
+   BUG_ON(p > name + ID_STR_LENGTH - 1);
+   return name;
+}
+
 static int sysfs_slab_add(struct kmem_cache *s)
 {
int err;
+   const char *name;
 
if (slab_state < SYSFS)
/* Defer until later */
return 0;
 
+   if (s->flags & SLUB_NEVER_MERGE) {
+   /*
+* 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(_subsys.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);
+
kobj_set_kset_s(s, slab_subsys);
-   kobject_set_name(>kobj, s->name);
+   kobject_set_name(>kobj, name);
kobject_init(>kobj);
err = kobject_add(>kobj);
if (err)
@@ -3326,6 +3378,10 @@ static int sysfs_slab_add(struct kmem_ca
if (err)
return err;
kobject_uevent(>kobj, KOBJ_ADD);
+   if (!(s->flags & SLUB_NEVER_MERGE)) {
+   sysfs_slab_alias(s, s->name);
+   kfree(name);
+   }
return 0;
 }
 
@@ -3351,9 +3407,14 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
+   if (slab_state == SYSFS) {
+   /*
+* If we have a leftover link then remove it.
+*/
+   sysfs_remove_link(_subsys.kset.kobj, name);
return sysfs_create_link(_subsys.kset.kobj,
>kobj, name);
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body 

Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Monday April 23, [EMAIL PROTECTED] wrote:
> On Tue, 24 Apr 2007, Neil Brown wrote:
> 
> > On Monday April 23, [EMAIL PROTECTED] wrote:
> > > Would this work? Contains a solution somewhat along the lines of your 
> > > thoughts on the subject.
> > > 
> > 
> > Concept seems sound.
> > Code needs a kfree of the name returned by create_unique_id, and I
> > think ID_STR_LENGTH needs to be at least 34.
> 
> Sysfs copies the string?

kobject_set_name copies the string, either into a small char array in
the kobject, or into kmalloced space.
kobject_set_name actually takes a format and arbitrary args and uses
vsnprintf, so it has to make it's own copy.

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Tue, 24 Apr 2007, Neil Brown wrote:

> On Monday April 23, [EMAIL PROTECTED] wrote:
> > Would this work? Contains a solution somewhat along the lines of your 
> > thoughts on the subject.
> > 
> 
> Concept seems sound.
> Code needs a kfree of the name returned by create_unique_id, and I
> think ID_STR_LENGTH needs to be at least 34.

Sysfs copies the string?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Monday April 23, [EMAIL PROTECTED] wrote:
> Would this work? Contains a solution somewhat along the lines of your 
> thoughts on the subject.
> 

Concept seems sound.
Code needs a kfree of the name returned by create_unique_id, and I
think ID_STR_LENGTH needs to be at least 34.
Maybe that should be allocated on the stack in sysfs_slab_add, rather
than using kmalloc/free.

Thanks,
NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
Would this work? Contains a solution somewhat along the lines of your 
thoughts on the subject.


SLAB: Fix sysfs directory handling

This fixes the problem that SLUB does not track the names of aliased
slabs by changing the way that SLUB manages the files in /sys/slab.

If the slab that is being created is not mergeable (usually the
case if we are debugging) then do not create any aliases. If an alias
exists that we conflict with then remove it before creating the
directory for the unmergeable slab. If there is a true slab cache there
and not an alias then we fail since there is a true duplication of
slab cache names. So debugging allows the detection of slab name
duplication as usual.

If the slab is mergeable then we create a directory with a unique name
created from the slab size, slab options and the pointer to the kmem_cache
structure (disambiguation). All names referring to the slabs will
then be created as symlinks to that unique name. These symlinks are
not going to be removed on kmem_cache_destroy() since we only carry
a counter for the number of aliases. If a new symlink is created
then it may just replace an existing one. This means that one can create
a gazillion slabs with the same name (if they all refer to mergeable
caches). Doing so will only increase the alias count. So we have the 
potential of not detecting duplicate slab names (SLUB works fine
with duplicate slab names anyways). We will detect the duplications when
debugging is enabled because we will then no longer generate symlinks and 
special unique names.

Signed-off-by: Christoph Lameter <[EMAIL PROTECTED]>

Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-23 13:08:41.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-23 13:09:10.0 -0700
@@ -3307,16 +3307,66 @@ static struct kset_uevent_ops slab_ueven
 
 decl_subsys(slab, _ktype, _uevent_ops);
 
+#define ID_STR_LENGTH 30
+
+/* Create a unique string id for a slab cache:
+ * format
+ * :[flags-]size:[memory address of kmemcache]
+ */
+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 */
+   if (s->flags & SLAB_CACHE_DMA)
+   *p++ = 'd';
+   if (s->flags & SLAB_RECLAIM_ACCOUNT)
+   *p++ = 'a';
+   if (s->flags & SLAB_DESTROY_BY_RCU)
+   *p++ = 'r';
+   /* Debug flags */
+   if (s->flags & SLAB_RED_ZONE)
+   *p++ = 'Z';
+   if (s->flags & SLAB_POISON)
+   *p++ = 'P';
+   if (s->flags & SLAB_STORE_USER)
+   *p++ = 'U';
+   if (p != name + 1)
+   *p++ = '-';
+   p += sprintf(p,"%06d:0x%p" ,s->size, s);
+   BUG_ON(p > name + ID_STR_LENGTH - 1);
+   return name;
+}
+
 static int sysfs_slab_add(struct kmem_cache *s)
 {
int err;
+   const char *name;
 
if (slab_state < SYSFS)
/* Defer until later */
return 0;
-
+   if (s->flags & SLUB_NEVER_MERGE) {
+   /*
+* 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(_subsys.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);
kobj_set_kset_s(s, slab_subsys);
-   kobject_set_name(>kobj, s->name);
+   kobject_set_name(>kobj, name);
kobject_init(>kobj);
err = kobject_add(>kobj);
if (err)
@@ -3326,6 +3376,8 @@ static int sysfs_slab_add(struct kmem_ca
if (err)
return err;
kobject_uevent(>kobj, KOBJ_ADD);
+   if (!(s->flags & SLUB_NEVER_MERGE))
+   sysfs_slab_alias(s, s->name);
return 0;
 }
 
@@ -3351,9 +3403,14 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
+   if (slab_state == SYSFS) {
+   /*
+* If we have a leftover link then remove it.
+*/
+   sysfs_remove_link(_subsys.kset.kobj, name);
return sysfs_create_link(_subsys.kset.kobj,
>kobj, name);
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  

Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Mon, 23 Apr 2007, Neil Brown wrote:

> Another option might be to name each cache actually created with a
> unique name, and then create a symlink for each cache that was asked
> for (whether it was created or whether a pre-existing cache was used).
> Then being lazy about deletion shouldn't be a problem.

You may be right.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Thursday April 19, [EMAIL PROTECTED] wrote:
> 
> Right. Sigh. But there is no user of the symlinks.
> 
> I could drop the symlinks completely. Just do not track what names a cache 
> aliases to?
> 

Suppose I have a kmem_cache which at different times has different
sizes (like, for example, the cache used for 'stripe_head' in
md/raid5.c.  If I 'grow' and array, I resize that kmem_cache).

Suppose that the first time I create it, it has size X, and then
someone else creates a cache with size X.  The second cache will become
an alias for the first.  So when the first is destroyed, the name
says. 

Now I try to create another cache with the same name (it is serving
the same purpose) but with a different size.  It will need to create a
new cache, but the name is still in use.  Bang!

I really think you need to return a different handle to each
kmem_cache_create call so that you know which cache is being deleted
so you can mangle names correctly.

Another option might be to name each cache actually created with a
unique name, and then create a symlink for each cache that was asked
for (whether it was created or whether a pre-existing cache was used).
Then being lazy about deletion shouldn't be a problem.

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Thursday April 19, [EMAIL PROTECTED] wrote:
 
 Right. Sigh. But there is no user of the symlinks.
 
 I could drop the symlinks completely. Just do not track what names a cache 
 aliases to?
 

Suppose I have a kmem_cache which at different times has different
sizes (like, for example, the cache used for 'stripe_head' in
md/raid5.c.  If I 'grow' and array, I resize that kmem_cache).

Suppose that the first time I create it, it has size X, and then
someone else creates a cache with size X.  The second cache will become
an alias for the first.  So when the first is destroyed, the name
says. 

Now I try to create another cache with the same name (it is serving
the same purpose) but with a different size.  It will need to create a
new cache, but the name is still in use.  Bang!

I really think you need to return a different handle to each
kmem_cache_create call so that you know which cache is being deleted
so you can mangle names correctly.

Another option might be to name each cache actually created with a
unique name, and then create a symlink for each cache that was asked
for (whether it was created or whether a pre-existing cache was used).
Then being lazy about deletion shouldn't be a problem.

NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Mon, 23 Apr 2007, Neil Brown wrote:

 Another option might be to name each cache actually created with a
 unique name, and then create a symlink for each cache that was asked
 for (whether it was created or whether a pre-existing cache was used).
 Then being lazy about deletion shouldn't be a problem.

You may be right.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
Would this work? Contains a solution somewhat along the lines of your 
thoughts on the subject.


SLAB: Fix sysfs directory handling

This fixes the problem that SLUB does not track the names of aliased
slabs by changing the way that SLUB manages the files in /sys/slab.

If the slab that is being created is not mergeable (usually the
case if we are debugging) then do not create any aliases. If an alias
exists that we conflict with then remove it before creating the
directory for the unmergeable slab. If there is a true slab cache there
and not an alias then we fail since there is a true duplication of
slab cache names. So debugging allows the detection of slab name
duplication as usual.

If the slab is mergeable then we create a directory with a unique name
created from the slab size, slab options and the pointer to the kmem_cache
structure (disambiguation). All names referring to the slabs will
then be created as symlinks to that unique name. These symlinks are
not going to be removed on kmem_cache_destroy() since we only carry
a counter for the number of aliases. If a new symlink is created
then it may just replace an existing one. This means that one can create
a gazillion slabs with the same name (if they all refer to mergeable
caches). Doing so will only increase the alias count. So we have the 
potential of not detecting duplicate slab names (SLUB works fine
with duplicate slab names anyways). We will detect the duplications when
debugging is enabled because we will then no longer generate symlinks and 
special unique names.

Signed-off-by: Christoph Lameter [EMAIL PROTECTED]

Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-23 13:08:41.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-23 13:09:10.0 -0700
@@ -3307,16 +3307,66 @@ static struct kset_uevent_ops slab_ueven
 
 decl_subsys(slab, slab_ktype, slab_uevent_ops);
 
+#define ID_STR_LENGTH 30
+
+/* Create a unique string id for a slab cache:
+ * format
+ * :[flags-]size:[memory address of kmemcache]
+ */
+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 */
+   if (s-flags  SLAB_CACHE_DMA)
+   *p++ = 'd';
+   if (s-flags  SLAB_RECLAIM_ACCOUNT)
+   *p++ = 'a';
+   if (s-flags  SLAB_DESTROY_BY_RCU)
+   *p++ = 'r';
+   /* Debug flags */
+   if (s-flags  SLAB_RED_ZONE)
+   *p++ = 'Z';
+   if (s-flags  SLAB_POISON)
+   *p++ = 'P';
+   if (s-flags  SLAB_STORE_USER)
+   *p++ = 'U';
+   if (p != name + 1)
+   *p++ = '-';
+   p += sprintf(p,%06d:0x%p ,s-size, s);
+   BUG_ON(p  name + ID_STR_LENGTH - 1);
+   return name;
+}
+
 static int sysfs_slab_add(struct kmem_cache *s)
 {
int err;
+   const char *name;
 
if (slab_state  SYSFS)
/* Defer until later */
return 0;
-
+   if (s-flags  SLUB_NEVER_MERGE) {
+   /*
+* 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_subsys.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);
kobj_set_kset_s(s, slab_subsys);
-   kobject_set_name(s-kobj, s-name);
+   kobject_set_name(s-kobj, name);
kobject_init(s-kobj);
err = kobject_add(s-kobj);
if (err)
@@ -3326,6 +3376,8 @@ static int sysfs_slab_add(struct kmem_ca
if (err)
return err;
kobject_uevent(s-kobj, KOBJ_ADD);
+   if (!(s-flags  SLUB_NEVER_MERGE))
+   sysfs_slab_alias(s, s-name);
return 0;
 }
 
@@ -3351,9 +3403,14 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
+   if (slab_state == SYSFS) {
+   /*
+* If we have a leftover link then remove it.
+*/
+   sysfs_remove_link(slab_subsys.kset.kobj, name);
return sysfs_create_link(slab_subsys.kset.kobj,
s-kobj, name);
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  

Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Monday April 23, [EMAIL PROTECTED] wrote:
 Would this work? Contains a solution somewhat along the lines of your 
 thoughts on the subject.
 

Concept seems sound.
Code needs a kfree of the name returned by create_unique_id, and I
think ID_STR_LENGTH needs to be at least 34.
Maybe that should be allocated on the stack in sysfs_slab_add, rather
than using kmalloc/free.

Thanks,
NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Tue, 24 Apr 2007, Neil Brown wrote:

 On Monday April 23, [EMAIL PROTECTED] wrote:
  Would this work? Contains a solution somewhat along the lines of your 
  thoughts on the subject.
  
 
 Concept seems sound.
 Code needs a kfree of the name returned by create_unique_id, and I
 think ID_STR_LENGTH needs to be at least 34.

Sysfs copies the string?

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Neil Brown
On Monday April 23, [EMAIL PROTECTED] wrote:
 On Tue, 24 Apr 2007, Neil Brown wrote:
 
  On Monday April 23, [EMAIL PROTECTED] wrote:
   Would this work? Contains a solution somewhat along the lines of your 
   thoughts on the subject.
   
  
  Concept seems sound.
  Code needs a kfree of the name returned by create_unique_id, and I
  think ID_STR_LENGTH needs to be at least 34.
 
 Sysfs copies the string?

kobject_set_name copies the string, either into a small char array in
the kobject, or into kmalloced space.
kobject_set_name actually takes a format and arbitrary args and uses
vsnprintf, so it has to make it's own copy.

NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-23 Thread Christoph Lameter
On Tue, 24 Apr 2007, Neil Brown wrote:

 kobject_set_name actually takes a format and arbitrary args and uses
 vsnprintf, so it has to make it's own copy.

Ok then this should be fine...

SLAB: Fix sysfs directory handling

This fixes the problem that SLUB does not track the names of aliased
slabs by changing the way that SLUB manages the files in /sys/slab.

If the slab that is being operated on is not mergeable (usually the
case if we are debugging) then do not create any aliases. If an alias
exists that we conflict with then remove it before creating the
directory for the unmergeable slab. If there is a true slab cache there
and not an alias then we fail since there is a true duplication of
slab cache names. So debugging allows the detection of slab name
duplication as usual.

If the slab is mergeable then we create a directory with a unique name
created from the slab size, slab options and the pointer to the kmem_cache
structure (disambiguation). All names referring to the slabs will
then be created as symlinks to that unique name. These symlinks are
not going to be removed on kmem_cache_destroy() since we only carry
a counter for the number of aliases. If a new symlink is created
then it may just replace an existing one. This means that one can create
a gazillion slabs with the same name (if they all refer to mergeable
caches). It will only increase the alias count. So we have the potential
of not detecting duplicate slab names (there is actually no harm
done by doing that). We will detect the duplications as
as soon as debugging is enabled because we will then no longer
generate symlinks and special unique names.

Signed-off-by: Christoph Lameter [EMAIL PROTECTED]

Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-23 13:08:41.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-23 18:05:16.0 -0700
@@ -3307,16 +3307,68 @@ static struct kset_uevent_ops slab_ueven
 
 decl_subsys(slab, slab_ktype, slab_uevent_ops);
 
+#define ID_STR_LENGTH 64
+
+/* Create a unique string id for a slab cache:
+ * format
+ * :[flags-]size:[memory address of kmemcache]
+ */
+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 */
+   if (s-flags  SLAB_CACHE_DMA)
+   *p++ = 'd';
+   if (s-flags  SLAB_RECLAIM_ACCOUNT)
+   *p++ = 'a';
+   if (s-flags  SLAB_DESTROY_BY_RCU)
+   *p++ = 'r';\
+   /* Debug flags */
+   if (s-flags  SLAB_RED_ZONE)
+   *p++ = 'Z';
+   if (s-flags  SLAB_POISON)
+   *p++ = 'P';
+   if (s-flags  SLAB_STORE_USER)
+   *p++ = 'U';
+   if (p != name + 1)
+   *p++ = '-';
+   p += sprintf(p,%07d:0x%p ,s-size, s);
+   BUG_ON(p  name + ID_STR_LENGTH - 1);
+   return name;
+}
+
 static int sysfs_slab_add(struct kmem_cache *s)
 {
int err;
+   const char *name;
 
if (slab_state  SYSFS)
/* Defer until later */
return 0;
 
+   if (s-flags  SLUB_NEVER_MERGE) {
+   /*
+* 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_subsys.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);
+
kobj_set_kset_s(s, slab_subsys);
-   kobject_set_name(s-kobj, s-name);
+   kobject_set_name(s-kobj, name);
kobject_init(s-kobj);
err = kobject_add(s-kobj);
if (err)
@@ -3326,6 +3378,10 @@ static int sysfs_slab_add(struct kmem_ca
if (err)
return err;
kobject_uevent(s-kobj, KOBJ_ADD);
+   if (!(s-flags  SLUB_NEVER_MERGE)) {
+   sysfs_slab_alias(s, s-name);
+   kfree(name);
+   }
return 0;
 }
 
@@ -3351,9 +3407,14 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
+   if (slab_state == SYSFS) {
+   /*
+* If we have a leftover link then remove it.
+*/
+   sysfs_remove_link(slab_subsys.kset.kobj, name);
return sysfs_create_link(slab_subsys.kset.kobj,
s-kobj, name);
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a 

Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
Another approach drop the symlinks completely. Just 
write a message to the syslog informing the user that we
created an alias. If debugging is off then the user would have to consult
the syslog to find aliases.


Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-19 22:46:20.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-19 22:48:26.0 -0700
@@ -158,11 +158,9 @@ LIST_HEAD(slab_caches);
 
 #ifdef CONFIG_SYSFS
 static int sysfs_slab_add(struct kmem_cache *);
-static int sysfs_slab_alias(struct kmem_cache *, const char *);
 static void sysfs_slab_remove(struct kmem_cache *);
 #else
 static int sysfs_slab_add(struct kmem_cache *s) { return 0; }
-static int sysfs_slab_alias(struct kmem_cache *s, const char *p) { return 0; }
 static void sysfs_slab_remove(struct kmem_cache *s) {}
 #endif
 
@@ -2324,8 +2322,8 @@ struct kmem_cache *kmem_cache_create(con
 */
s->objsize = max(s->objsize, (int)size);
s->inuse = max_t(int, s->inuse, ALIGN(size, sizeof(void *)));
-   if (sysfs_slab_alias(s, name))
-   goto err;
+   printk(KERN_INFO "SLUB: %s is an alias of %s\n",
+   name, s->name);
} else {
s = kmalloc(kmem_size, GFP_KERNEL);
if (s && kmem_cache_open(s, GFP_KERNEL, name,
@@ -3335,37 +,6 @@ static void sysfs_slab_remove(struct kme
kobject_del(>kobj);
 }
 
-/*
- * Need to buffer aliases during bootup until sysfs becomes
- * available lest we loose that information.
- */
-struct saved_alias {
-   struct kmem_cache *s;
-   const char *name;
-   struct saved_alias *next;
-};
-
-struct saved_alias *alias_list;
-
-static int sysfs_slab_alias(struct kmem_cache *s, const char *name)
-{
-   struct saved_alias *al;
-
-   if (slab_state == SYSFS)
-   return sysfs_create_link(_subsys.kset.kobj,
-   >kobj, name);
-
-   al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
-   if (!al)
-   return -ENOMEM;
-
-   al->s = s;
-   al->name = name;
-   al->next = alias_list;
-   alias_list = al;
-   return 0;
-}
-
 int __init slab_sysfs_init(void)
 {
int err;
@@ -3378,15 +3345,6 @@ int __init slab_sysfs_init(void)
 
finish_bootstrap();
 
-   while (alias_list) {
-   struct saved_alias *al = alias_list;
-
-   alias_list = alias_list->next;
-   err = sysfs_slab_alias(al->s, al->name);
-   BUG_ON(err);
-   kfree(al);
-   }
-
resiliency_test();
return 0;
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
On Fri, 20 Apr 2007, Neil Brown wrote:

> On Thursday April 19, [EMAIL PROTECTED] wrote:
> > On Fri, 20 Apr 2007, Neil Brown wrote:
> > 
> > > Not sure how best to fix this one kmem_cache_destroy currently
> > > doesn't know which alias is being destroyed.
> > 
> > The aliases are there for decorative purposes when running without
> > debugging. If one switches on debugging then it matters but then the
> > symlinks are not created since there will be no aliases.
> > 
> > I guess we can ignore the problem?
> 
> Maybe
> But then if we create the same cache with a different size, we might
> need to create a directory in sysfs, but there is already a symlink
> there... 
> It doesn't feel very clean.

Right. Sigh. But there is no user of the symlinks.

I could drop the symlinks completely. Just do not track what names a cache 
aliases to?


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Neil Brown
On Thursday April 19, [EMAIL PROTECTED] wrote:
> On Fri, 20 Apr 2007, Neil Brown wrote:
> 
> > Not sure how best to fix this one kmem_cache_destroy currently
> > doesn't know which alias is being destroyed.
> 
> The aliases are there for decorative purposes when running without
> debugging. If one switches on debugging then it matters but then the
> symlinks are not created since there will be no aliases.
> 
> I guess we can ignore the problem?

Maybe
But then if we create the same cache with a different size, we might
need to create a directory in sysfs, but there is already a symlink
there... 
It doesn't feel very clean.

NeilBrown
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
On Fri, 20 Apr 2007, Neil Brown wrote:

> Not sure how best to fix this one kmem_cache_destroy currently
> doesn't know which alias is being destroyed.

The aliases are there for decorative purposes when running without
debugging. If one switches on debugging then it matters but then the
symlinks are not created since there will be no aliases.

I guess we can ignore the problem?


Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-19 22:13:28.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-19 22:15:31.0 -0700
@@ -3351,9 +3351,19 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
-   return sysfs_create_link(_subsys.kset.kobj,
+   if (slab_state == SYSFS) {
+   int rc;
+
+   /*
+* Aliases are there mainly for decorative purposes
+* and we have no way of removing them properly.
+* Creating a link may fail due to the symlink remaining.
+* f.e. module unloading and loading.
+*/
+   rc = sysfs_create_link(_subsys.kset.kobj,
>kobj, name);
+   return 0;
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
On Fri, 20 Apr 2007, Neil Brown wrote:

 Not sure how best to fix this one kmem_cache_destroy currently
 doesn't know which alias is being destroyed.

The aliases are there for decorative purposes when running without
debugging. If one switches on debugging then it matters but then the
symlinks are not created since there will be no aliases.

I guess we can ignore the problem?


Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-19 22:13:28.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-19 22:15:31.0 -0700
@@ -3351,9 +3351,19 @@ static int sysfs_slab_alias(struct kmem_
 {
struct saved_alias *al;
 
-   if (slab_state == SYSFS)
-   return sysfs_create_link(slab_subsys.kset.kobj,
+   if (slab_state == SYSFS) {
+   int rc;
+
+   /*
+* Aliases are there mainly for decorative purposes
+* and we have no way of removing them properly.
+* Creating a link may fail due to the symlink remaining.
+* f.e. module unloading and loading.
+*/
+   rc = sysfs_create_link(slab_subsys.kset.kobj,
s-kobj, name);
+   return 0;
+   }
 
al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
if (!al)
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Neil Brown
On Thursday April 19, [EMAIL PROTECTED] wrote:
 On Fri, 20 Apr 2007, Neil Brown wrote:
 
  Not sure how best to fix this one kmem_cache_destroy currently
  doesn't know which alias is being destroyed.
 
 The aliases are there for decorative purposes when running without
 debugging. If one switches on debugging then it matters but then the
 symlinks are not created since there will be no aliases.
 
 I guess we can ignore the problem?

Maybe
But then if we create the same cache with a different size, we might
need to create a directory in sysfs, but there is already a symlink
there... 
It doesn't feel very clean.

NeilBrown
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
On Fri, 20 Apr 2007, Neil Brown wrote:

 On Thursday April 19, [EMAIL PROTECTED] wrote:
  On Fri, 20 Apr 2007, Neil Brown wrote:
  
   Not sure how best to fix this one kmem_cache_destroy currently
   doesn't know which alias is being destroyed.
  
  The aliases are there for decorative purposes when running without
  debugging. If one switches on debugging then it matters but then the
  symlinks are not created since there will be no aliases.
  
  I guess we can ignore the problem?
 
 Maybe
 But then if we create the same cache with a different size, we might
 need to create a directory in sysfs, but there is already a symlink
 there... 
 It doesn't feel very clean.

Right. Sigh. But there is no user of the symlinks.

I could drop the symlinks completely. Just do not track what names a cache 
aliases to?


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: SLUB: kmem_cache_destroy doesn't - version 2.

2007-04-19 Thread Christoph Lameter
Another approach drop the symlinks completely. Just 
write a message to the syslog informing the user that we
created an alias. If debugging is off then the user would have to consult
the syslog to find aliases.


Index: linux-2.6.21-rc6/mm/slub.c
===
--- linux-2.6.21-rc6.orig/mm/slub.c 2007-04-19 22:46:20.0 -0700
+++ linux-2.6.21-rc6/mm/slub.c  2007-04-19 22:48:26.0 -0700
@@ -158,11 +158,9 @@ LIST_HEAD(slab_caches);
 
 #ifdef CONFIG_SYSFS
 static int sysfs_slab_add(struct kmem_cache *);
-static int sysfs_slab_alias(struct kmem_cache *, const char *);
 static void sysfs_slab_remove(struct kmem_cache *);
 #else
 static int sysfs_slab_add(struct kmem_cache *s) { return 0; }
-static int sysfs_slab_alias(struct kmem_cache *s, const char *p) { return 0; }
 static void sysfs_slab_remove(struct kmem_cache *s) {}
 #endif
 
@@ -2324,8 +2322,8 @@ struct kmem_cache *kmem_cache_create(con
 */
s-objsize = max(s-objsize, (int)size);
s-inuse = max_t(int, s-inuse, ALIGN(size, sizeof(void *)));
-   if (sysfs_slab_alias(s, name))
-   goto err;
+   printk(KERN_INFO SLUB: %s is an alias of %s\n,
+   name, s-name);
} else {
s = kmalloc(kmem_size, GFP_KERNEL);
if (s  kmem_cache_open(s, GFP_KERNEL, name,
@@ -3335,37 +,6 @@ static void sysfs_slab_remove(struct kme
kobject_del(s-kobj);
 }
 
-/*
- * Need to buffer aliases during bootup until sysfs becomes
- * available lest we loose that information.
- */
-struct saved_alias {
-   struct kmem_cache *s;
-   const char *name;
-   struct saved_alias *next;
-};
-
-struct saved_alias *alias_list;
-
-static int sysfs_slab_alias(struct kmem_cache *s, const char *name)
-{
-   struct saved_alias *al;
-
-   if (slab_state == SYSFS)
-   return sysfs_create_link(slab_subsys.kset.kobj,
-   s-kobj, name);
-
-   al = kmalloc(sizeof(struct saved_alias), GFP_KERNEL);
-   if (!al)
-   return -ENOMEM;
-
-   al-s = s;
-   al-name = name;
-   al-next = alias_list;
-   alias_list = al;
-   return 0;
-}
-
 int __init slab_sysfs_init(void)
 {
int err;
@@ -3378,15 +3345,6 @@ int __init slab_sysfs_init(void)
 
finish_bootstrap();
 
-   while (alias_list) {
-   struct saved_alias *al = alias_list;
-
-   alias_list = alias_list-next;
-   err = sysfs_slab_alias(al-s, al-name);
-   BUG_ON(err);
-   kfree(al);
-   }
-
resiliency_test();
return 0;
 }
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/