From: Glauber Costa <glommer@parallels.com>
To: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>,
linux-mm@kvack.org, David Rientjes <rientjes@google.com>,
Matt Mackall <mpm@selenic.com>, Joonsoo Kim <js1304@gmail.com>
Subject: Re: Common [13/20] Extract a common function for kmem_cache_destroy
Date: Tue, 31 Jul 2012 18:16:35 +0400 [thread overview]
Message-ID: <5017E8C3.1040004@parallels.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1207310910580.32295@router.home>
On 07/31/2012 06:12 PM, Christoph Lameter wrote:
> On Tue, 31 Jul 2012, Glauber Costa wrote:
>
>> Problem is that you are now allocating objects from kmem_cache with
>> kmem_cache_alloc, but freeing it with kfree - and in multiple locations.
>
> Why would this be an issue"?
I believe consistency wins here. Since the kmalloc cache can be
different in many ways from the normal caches in their paths, we should
use the corresponding free functions for those. But perhaps I shouldn't
even have mentioned that, since this is, as I explained below, the real
root issue, and confused the report...
>> In particular, after the whole series is applied, you will have a call
>> to "kfree(s)" in sysfs_slab_remove() that is called from
>> kmem_cache_shutdown(), and later on kmem_cache_free(kmem_cache, s) from
>> the destruction common code -> a double free.
>
> I will look at that but I have already reworked the patches a couple of
> times since then. I hope to be able to post an updated series against
> upstream at the end of the week (before the next conference).
>
Unfortunately, that wasn't the only problem as well. I am not yet able
to pinpoint the correct source, but we're handling cache deletion very
poorly after this series.
Since you said you had reworked this, I'll just stop looking for now.
But would you please make sure that this following use case is well
tested before you send?
1) After machine is up, create a bogus cache
2) free that cache right away.
3) Create two more caches.
The creation of the second cache fails, because
kmem_cache_alloc(kmem_cache, x) returns bad values. Those bad values can
take multiple forms, but the most common is a value that is equal to an
already assigned value.
I am creating caches for the following objects to demonstrate that:
struct bgb {
struct dentry d;
int a;
int b;
int c;
};
struct bgb2 {
struct dentry d;
struct inode i;
int a;
int b;
int c;
};
But this shouldn't matter at all, I am just posting so you can rule out
any size or merging related issue.
--
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>
next prev parent reply other threads:[~2012-07-31 14:19 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-01 19:52 Common [00/20] Sl[auo]b: Common code rework V4 Christoph Lameter
2012-06-01 19:52 ` Common [01/20] [slob] Define page struct fields used in mm_types.h Christoph Lameter
2012-06-01 19:52 ` Common [03/20] [slob] Remove various small accessors Christoph Lameter
2012-06-01 19:52 ` Common [04/20] [slab] Use page struct fields instead of casting Christoph Lameter
2012-06-01 19:52 ` Common [05/20] [slab] Remove some accessors Christoph Lameter
2012-06-01 19:52 ` Common [06/20] Extract common fields from struct kmem_cache Christoph Lameter
2012-06-01 19:52 ` Common [07/20] [slab] Get rid of obj_size macro Christoph Lameter
2012-06-01 19:52 ` Common [08/20] Extract common code for kmem_cache_create() Christoph Lameter
2012-06-01 19:52 ` Common [09/20] Common definition for boot state of the slab allocators Christoph Lameter
2012-06-01 19:52 ` Common [10/20] Use a common mutex definition Christoph Lameter
2012-06-01 19:52 ` Common [11/20] Move kmem_cache_create mutex handling to common code Christoph Lameter
2012-06-01 19:52 ` Common [13/20] Extract a common function for kmem_cache_destroy Christoph Lameter
2012-07-31 12:01 ` Glauber Costa
2012-07-31 14:12 ` Christoph Lameter
2012-07-31 14:16 ` Glauber Costa [this message]
2012-07-31 14:42 ` Christoph Lameter
2012-07-31 14:47 ` Glauber Costa
2012-07-31 16:30 ` Christoph Lameter
2012-07-31 16:41 ` Glauber Costa
2012-07-31 16:52 ` Christoph Lameter
2012-06-01 19:52 ` Common [14/20] Always use the name "kmem_cache" for the slab cache with the kmem_cache structure Christoph Lameter
2012-06-01 19:53 ` Common [16/20] Get rid of __kmem_cache_destroy Christoph Lameter
2012-06-01 19:53 ` Common [17/20] Move duping of slab name to slab_common.c Christoph Lameter
2012-06-01 19:53 ` Common [18/20] Do slab aliasing call from common code Christoph Lameter
2012-06-01 19:53 ` Common [19/20] Allocate kmem_cache structure in slab_common.c Christoph Lameter
2012-06-01 19:53 ` Common [20/20] Common alignment code Christoph Lameter
2012-06-13 15:24 Common [00/20] Sl[auo]b: Common code rework V5 (for merge) Christoph Lameter
2012-06-13 15:25 ` Common [13/20] Extract a common function for kmem_cache_destroy Christoph Lameter
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=5017E8C3.1040004@parallels.com \
--to=glommer@parallels.com \
--cc=cl@linux.com \
--cc=js1304@gmail.com \
--cc=linux-mm@kvack.org \
--cc=mpm@selenic.com \
--cc=penberg@kernel.org \
--cc=rientjes@google.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