linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Matt Mackall <mpm@selenic.com>
Cc: b32542@freescale.com, linux-mm@kvack.org,
	Christoph Lameter <cl@linux-foundation.org>,
	Pekka Enberg <penberg@cs.helsinki.fi>,
	tytso@mit.edu, linux-kernel@vger.kernel.org,
	Zeng Zhaoming <zengzm.kernel@gmail.com>
Subject: Re: [PATCH] slub: operate cache name memory same to slab and slob
Date: Sat, 20 Nov 2010 16:55:22 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.00.1011201650120.10618@chino.kir.corp.google.com> (raw)
In-Reply-To: <1290119265.26343.814.camel@calx>

On Thu, 18 Nov 2010, Matt Mackall wrote:

> > The leak in ext4_mb_init() above is because it is using kstrdup() to 
> > allocate the string itself and then on destroy uses kmem_cache_name() to 
> > attain the slub allocator's pointer to the name, not the memory the ext4 
> > layer allocated itself.
> 
> And Pekka says:
> 
> > The kstrdup() is there because of SLUB cache merging. See commit 
> > 84c1cf62465e2fb0a692620dcfeb52323ab03d48 ("SLUB: Fix merged slab 
> > cache names") for details.
> 
> I see. So we can either:
> 
> - force anyone using dynamically-allocated names to track their own damn
> pointer
> - implement kstrdup in the other allocators and fix all callers (the
> bulk of which use static names!)
> - eliminate dynamically-allocated names (mostly useless when we start
> merging slabs!)
> - add an indirection layer for slub that holds the unmerged details
> - stop pretending we track slab names and show only generic names based
> on size in /proc
> 

I agree that we should force each user to track its own memory, and this 
is really what the issue is about (it doesn't matter if that memory is the 
cache's name).  This particular issue is an ext4 memory leak and not the 
responsibility of any allocator.

> kmem_cache_name() is also a highly suspect function in a
> post-merged-slabs kernel. As ext4 is the only user in the kernel, and it
> got it wrong, perhaps it's time to rip it out.
> 

Yes, I think kmem_cache_name() should be removed since it shouldn't be 
used for anything other than the internal slabinfo/slabtop display as the 
slub allocator actually specifies in include/linux/slub_def.h.  The only 
user is ext4 to track this dynamically allocated pointer, so we can 
eliminate it if we leave it to track its own memory allocations (a slab 
allocator shouldn't be carrying a metadata payload).

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2010-11-21  0:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-18  3:00 b32542
2010-11-18 21:15 ` Matt Mackall
2010-11-18 21:36   ` David Rientjes
2010-11-18 22:27     ` Matt Mackall
2010-11-19 14:38       ` Zeng Zhaoming
2010-11-21  0:55       ` David Rientjes [this message]
2010-11-18 21:41   ` Pekka Enberg

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=alpine.DEB.2.00.1011201650120.10618@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=b32542@freescale.com \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mpm@selenic.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=tytso@mit.edu \
    --cc=zengzm.kernel@gmail.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