linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Pekka Enberg <penberg@cs.helsinki.fi>
To: Christoph Lameter <clameter@sgi.com>
Cc: Matt Mackall <mpm@selenic.com>,
	linux-mm@kvack.org, David Miller <davem@davemloft.net>,
	acme <acme@redhat.com>
Subject: Re: Subject: Slab allocators: Remove kmem_cache_name() to fix invalid frees
Date: Wed, 28 May 2008 22:48:05 +0300	[thread overview]
Message-ID: <483DB6F5.2030101@cs.helsinki.fi> (raw)
In-Reply-To: <Pine.LNX.4.64.0805281121100.32755@schroedinger.engr.sgi.com>

Christoph Lameter wrote:
> A draft patch to allow anonymous caches follows. Allowing duplicate names
> would also be possible. Just need to deal with sysfs. Maybe generate a
> _x at the end?

Or a "cache-" prefix. I don't think we ought to allow duplicate names, 
though.

> Subject: slub: Support anonymous slabs
> 
> Slabs really do not need to have a name so one could pass NULL as a name. 
> The name is only relevant for sysfs support. There we need a unique name.
> Use the address of the kmem_cache structure as the name.
> 
> [Would output "" if debugging is one]
> 
> 
> Signed-off-by: Christoph Lameter <clameter@sgi.com>
> 
> ---
>  mm/slub.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c	2008-05-28 11:16:24.000000000 -0700
> +++ linux-2.6/mm/slub.c	2008-05-28 11:17:33.000000000 -0700
> @@ -4310,6 +4310,7 @@ static int sysfs_slab_add(struct kmem_ca
>  	int err;
>  	const char *name;
>  	int unmergeable;
> +	char buf[20];
>  
>  	if (slab_state < SYSFS)
>  		/* Defer until later */
> @@ -4322,8 +4323,13 @@ static int sysfs_slab_add(struct kmem_ca
>  		 * 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);
> +		if (s->name)
> +			sysfs_remove_link(&slab_kset->kobj, s->name);
>  		name = s->name;
> +		if (!name) {
> +			sprintf(buf, "%p", s);
> +			name = buf;
> +		}

Perhaps add a comment here explaining why we're doing this?

Other than that, looks good to me. Please re-send with proper changelog 
from the original patch.

		Pekka

--
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>

  reply	other threads:[~2008-05-28 19:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-28 17:40 Subject: Slab allocators: Remove kmem_cache_name() to fix invalid frees Christoph Lameter
2008-05-28 17:51 ` Matt Mackall
2008-05-28 17:58   ` Christoph Lameter
2008-05-28 18:23   ` Christoph Lameter
2008-05-28 19:48     ` Pekka Enberg [this message]
2008-05-28 20:32 ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=483DB6F5.2030101@cs.helsinki.fi \
    --to=penberg@cs.helsinki.fi \
    --cc=acme@redhat.com \
    --cc=clameter@sgi.com \
    --cc=davem@davemloft.net \
    --cc=linux-mm@kvack.org \
    --cc=mpm@selenic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).