* [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell
@ 2012-04-12 22:34 Mike Snitzer
2012-04-12 22:34 ` [PATCH 2/2] dm thin: use slab_pool for caches Mike Snitzer
2012-04-23 7:24 ` [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Joe Thornber
0 siblings, 2 replies; 6+ messages in thread
From: Mike Snitzer @ 2012-04-12 22:34 UTC (permalink / raw)
To: dm-devel; +Cc: ejt
Add missing mempool_free() to __cell_release_singleton().
This is a pretty significant leak that will accumulate to over 2GB of
leaked memory just from running the full thinp-test-suite. Leak was
known to exist but the kmalloc_pool which was used for prison cell
allocation caused all leaked memory to be attributed to the "size-128"
cache. kmemleak proved useful in identifying the source of this
illusive leak.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm-thin.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/md/dm-thin.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-thin.c
+++ linux-2.6/drivers/md/dm-thin.c
@@ -279,8 +279,10 @@ static void __cell_release(struct cell *
hlist_del(&cell->list);
- bio_list_add(inmates, cell->holder);
- bio_list_merge(inmates, &cell->bios);
+ if (inmates) {
+ bio_list_add(inmates, cell->holder);
+ bio_list_merge(inmates, &cell->bios);
+ }
mempool_free(cell, prison->cell_pool);
}
@@ -303,9 +305,10 @@ static void cell_release(struct cell *ce
*/
static void __cell_release_singleton(struct cell *cell, struct bio *bio)
{
- hlist_del(&cell->list);
BUG_ON(cell->holder != bio);
BUG_ON(!bio_list_empty(&cell->bios));
+
+ __cell_release(cell, NULL);
}
static void cell_release_singleton(struct cell *cell, struct bio *bio)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] dm thin: use slab_pool for caches
2012-04-12 22:34 [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Mike Snitzer
@ 2012-04-12 22:34 ` Mike Snitzer
2012-04-12 22:39 ` [PATCH 2/2 v2] dm thin: use slab mempools with local caches Mike Snitzer
2012-04-23 7:24 ` [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Joe Thornber
1 sibling, 1 reply; 6+ messages in thread
From: Mike Snitzer @ 2012-04-12 22:34 UTC (permalink / raw)
To: dm-devel; +Cc: ejt
Use dedicated caches prefixed with a "dm_" name rather than rely on
generic slab caches.
This will aid in debugging thinp memory leaks should they occur.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm-thin.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 5 deletions(-)
Index: linux-2.6/drivers/md/dm-thin.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-thin.c
+++ linux-2.6/drivers/md/dm-thin.c
@@ -141,6 +141,8 @@ static uint32_t calc_nr_buckets(unsigned
return n;
}
+struct kmem_cache *_cell_cache;
+
/*
* @nr_cells should be the number of cells you want in use _concurrently_.
* Don't confuse it with the number of distinct keys.
@@ -157,8 +159,7 @@ static struct bio_prison *prison_create(
return NULL;
spin_lock_init(&prison->lock);
- prison->cell_pool = mempool_create_kmalloc_pool(nr_cells,
- sizeof(struct cell));
+ prison->cell_pool = mempool_create_slab_pool(nr_cells, _cell_cache);
if (!prison->cell_pool) {
kfree(prison);
return NULL;
@@ -1649,6 +1650,9 @@ static void pool_features_init(struct po
pf->discard_passdown = 1;
}
+struct kmem_cache *_new_mapping_cache;
+struct kmem_cache *_endio_hook_cache;
+
static void __pool_destroy(struct pool *pool)
{
__pool_table_remove(pool);
@@ -1738,7 +1742,7 @@ static struct pool *pool_create(struct m
pool->next_mapping = NULL;
pool->mapping_pool =
- mempool_create_kmalloc_pool(MAPPING_POOL_SIZE, sizeof(struct new_mapping));
+ mempool_create_slab_pool(MAPPING_POOL_SIZE, _new_mapping_cache);
if (!pool->mapping_pool) {
*error = "Error creating pool's mapping mempool";
err_p = ERR_PTR(-ENOMEM);
@@ -1746,7 +1750,7 @@ static struct pool *pool_create(struct m
}
pool->endio_hook_pool =
- mempool_create_kmalloc_pool(ENDIO_HOOK_POOL_SIZE, sizeof(struct endio_hook));
+ mempool_create_slab_pool(ENDIO_HOOK_POOL_SIZE, _endio_hook_cache);
if (!pool->endio_hook_pool) {
*error = "Error creating pool's endio_hook mempool";
err_p = ERR_PTR(-ENOMEM);
@@ -2748,7 +2752,42 @@ static int __init dm_thin_init(void)
r = dm_register_target(&pool_target);
if (r)
- dm_unregister_target(&thin_target);
+ goto bad_pool_target;
+
+ _cell_cache = kmem_cache_create("dm_bio_prison_cell",
+ sizeof(struct cell),
+ __alignof__(struct cell), 0, NULL);
+ if (!_cell_cache) {
+ r = -ENOMEM;
+ goto bad_cell_cache;
+ }
+
+ _new_mapping_cache = kmem_cache_create("dm_thin_new_mapping",
+ sizeof(struct new_mapping),
+ __alignof__(struct new_mapping), 0, NULL);
+ if (!_new_mapping_cache) {
+ r = -ENOMEM;
+ goto bad_new_mapping_cache;
+ }
+
+ _endio_hook_cache = kmem_cache_create("dm_thin_endio_hook",
+ sizeof(struct endio_hook),
+ __alignof__(struct endio_hook), 0, NULL);
+ if (!_endio_hook_cache) {
+ r = -ENOMEM;
+ goto bad_endio_hook_cache;
+ }
+
+ return 0;
+
+bad_endio_hook_cache:
+ kmem_cache_destroy(_new_mapping_cache);
+bad_new_mapping_cache:
+ kmem_cache_destroy(_cell_cache);
+bad_cell_cache:
+ dm_unregister_target(&pool_target);
+bad_pool_target:
+ dm_unregister_target(&thin_target);
return r;
}
@@ -2757,6 +2796,9 @@ static void dm_thin_exit(void)
{
dm_unregister_target(&thin_target);
dm_unregister_target(&pool_target);
+ kmem_cache_destroy(_cell_cache);
+ kmem_cache_destroy(_new_mapping_cache);
+ kmem_cache_destroy(_endio_hook_cache);
}
module_init(dm_thin_init);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2 v2] dm thin: use slab mempools with local caches
2012-04-12 22:34 ` [PATCH 2/2] dm thin: use slab_pool for caches Mike Snitzer
@ 2012-04-12 22:39 ` Mike Snitzer
0 siblings, 0 replies; 6+ messages in thread
From: Mike Snitzer @ 2012-04-12 22:39 UTC (permalink / raw)
To: dm-devel; +Cc: ejt
Use dedicated caches prefixed with a "dm_" name rather than rely on
kmalloc mempools backed by generic slab caches.
This will aid in debugging thinp memory leaks should they occur.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
drivers/md/dm-thin.c | 52 +++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 47 insertions(+), 5 deletions(-)
v2: tweaked subject and header some, no code changes.
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 301db0f..57d40b1 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -141,6 +141,8 @@ static uint32_t calc_nr_buckets(unsigned nr_cells)
return n;
}
+struct kmem_cache *_cell_cache;
+
/*
* @nr_cells should be the number of cells you want in use _concurrently_.
* Don't confuse it with the number of distinct keys.
@@ -157,8 +159,7 @@ static struct bio_prison *prison_create(unsigned nr_cells)
return NULL;
spin_lock_init(&prison->lock);
- prison->cell_pool = mempool_create_kmalloc_pool(nr_cells,
- sizeof(struct cell));
+ prison->cell_pool = mempool_create_slab_pool(nr_cells, _cell_cache);
if (!prison->cell_pool) {
kfree(prison);
return NULL;
@@ -1649,6 +1650,9 @@ static void pool_features_init(struct pool_features *pf)
pf->discard_passdown = 1;
}
+struct kmem_cache *_new_mapping_cache;
+struct kmem_cache *_endio_hook_cache;
+
static void __pool_destroy(struct pool *pool)
{
__pool_table_remove(pool);
@@ -1738,7 +1742,7 @@ static struct pool *pool_create(struct mapped_device *pool_md,
pool->next_mapping = NULL;
pool->mapping_pool =
- mempool_create_kmalloc_pool(MAPPING_POOL_SIZE, sizeof(struct new_mapping));
+ mempool_create_slab_pool(MAPPING_POOL_SIZE, _new_mapping_cache);
if (!pool->mapping_pool) {
*error = "Error creating pool's mapping mempool";
err_p = ERR_PTR(-ENOMEM);
@@ -1746,7 +1750,7 @@ static struct pool *pool_create(struct mapped_device *pool_md,
}
pool->endio_hook_pool =
- mempool_create_kmalloc_pool(ENDIO_HOOK_POOL_SIZE, sizeof(struct endio_hook));
+ mempool_create_slab_pool(ENDIO_HOOK_POOL_SIZE, _endio_hook_cache);
if (!pool->endio_hook_pool) {
*error = "Error creating pool's endio_hook mempool";
err_p = ERR_PTR(-ENOMEM);
@@ -2748,7 +2752,42 @@ static int __init dm_thin_init(void)
r = dm_register_target(&pool_target);
if (r)
- dm_unregister_target(&thin_target);
+ goto bad_pool_target;
+
+ _cell_cache = kmem_cache_create("dm_bio_prison_cell",
+ sizeof(struct cell),
+ __alignof__(struct cell), 0, NULL);
+ if (!_cell_cache) {
+ r = -ENOMEM;
+ goto bad_cell_cache;
+ }
+
+ _new_mapping_cache = kmem_cache_create("dm_thin_new_mapping",
+ sizeof(struct new_mapping),
+ __alignof__(struct new_mapping), 0, NULL);
+ if (!_new_mapping_cache) {
+ r = -ENOMEM;
+ goto bad_new_mapping_cache;
+ }
+
+ _endio_hook_cache = kmem_cache_create("dm_thin_endio_hook",
+ sizeof(struct endio_hook),
+ __alignof__(struct endio_hook), 0, NULL);
+ if (!_endio_hook_cache) {
+ r = -ENOMEM;
+ goto bad_endio_hook_cache;
+ }
+
+ return 0;
+
+bad_endio_hook_cache:
+ kmem_cache_destroy(_new_mapping_cache);
+bad_new_mapping_cache:
+ kmem_cache_destroy(_cell_cache);
+bad_cell_cache:
+ dm_unregister_target(&pool_target);
+bad_pool_target:
+ dm_unregister_target(&thin_target);
return r;
}
@@ -2757,6 +2796,9 @@ static void dm_thin_exit(void)
{
dm_unregister_target(&thin_target);
dm_unregister_target(&pool_target);
+ kmem_cache_destroy(_cell_cache);
+ kmem_cache_destroy(_new_mapping_cache);
+ kmem_cache_destroy(_endio_hook_cache);
}
module_init(dm_thin_init);
--
1.7.4.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell
2012-04-12 22:34 [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Mike Snitzer
2012-04-12 22:34 ` [PATCH 2/2] dm thin: use slab_pool for caches Mike Snitzer
@ 2012-04-23 7:24 ` Joe Thornber
2012-04-23 10:02 ` Alasdair G Kergon
1 sibling, 1 reply; 6+ messages in thread
From: Joe Thornber @ 2012-04-23 7:24 UTC (permalink / raw)
To: Mike Snitzer; +Cc: dm-devel, ejt
On Thu, Apr 12, 2012 at 06:34:34PM -0400, Mike Snitzer wrote:
> Add missing mempool_free() to __cell_release_singleton().
>
> This is a pretty significant leak that will accumulate to over 2GB of
> leaked memory just from running the full thinp-test-suite.
When was this introduced? With agk's tweaks for 3.4?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell
2012-04-23 10:02 ` Alasdair G Kergon
@ 2012-04-23 9:26 ` Joe Thornber
0 siblings, 0 replies; 6+ messages in thread
From: Joe Thornber @ 2012-04-23 9:26 UTC (permalink / raw)
To: ejt, dm-devel
On Mon, Apr 23, 2012 at 11:02:46AM +0100, Alasdair G Kergon wrote:
> On Mon, Apr 23, 2012 at 08:24:08AM +0100, Joe Thornber wrote:
> > On Thu, Apr 12, 2012 at 06:34:34PM -0400, Mike Snitzer wrote:
> > > Add missing mempool_free() to __cell_release_singleton().
> > >
> > > This is a pretty significant leak that will accumulate to over 2GB of
> > > leaked memory just from running the full thinp-test-suite.
> > When was this introduced? With agk's tweaks for 3.4?
>
> I think it was here:
> http://www.redhat.com/archives/dm-devel/2012-March/msg00080.html
Yep, this should call mempool_release:
@@ -305,22 +310,45 @@ static void cell_release(struct cell *cell, struct bio_list *bios)
* bio may be in the cell. This function releases the cell, and also does
* a sanity check.
*/
+static void __cell_release_singleton(struct cell *cell, struct bio *bio)
+{
+ hlist_del(&cell->list);
+ BUG_ON(cell->holder != bio);
+ BUG_ON(!bio_list_empty(&cell->bios));
+}
+
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell
2012-04-23 7:24 ` [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Joe Thornber
@ 2012-04-23 10:02 ` Alasdair G Kergon
2012-04-23 9:26 ` Joe Thornber
0 siblings, 1 reply; 6+ messages in thread
From: Alasdair G Kergon @ 2012-04-23 10:02 UTC (permalink / raw)
To: ejt; +Cc: dm-devel
On Mon, Apr 23, 2012 at 08:24:08AM +0100, Joe Thornber wrote:
> On Thu, Apr 12, 2012 at 06:34:34PM -0400, Mike Snitzer wrote:
> > Add missing mempool_free() to __cell_release_singleton().
> >
> > This is a pretty significant leak that will accumulate to over 2GB of
> > leaked memory just from running the full thinp-test-suite.
> When was this introduced? With agk's tweaks for 3.4?
I think it was here:
http://www.redhat.com/archives/dm-devel/2012-March/msg00080.html
Localised fix:
http://people.redhat.com/agk/patches/linux/editing/dm-thin-reinstate-missing-mempool_free-in-cell_release_singleton.patch
Alasdair
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-23 10:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-12 22:34 [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Mike Snitzer
2012-04-12 22:34 ` [PATCH 2/2] dm thin: use slab_pool for caches Mike Snitzer
2012-04-12 22:39 ` [PATCH 2/2 v2] dm thin: use slab mempools with local caches Mike Snitzer
2012-04-23 7:24 ` [PATCH 1/2] dm thin: fix memory leak of singleton bio-prison cell Joe Thornber
2012-04-23 10:02 ` Alasdair G Kergon
2012-04-23 9:26 ` Joe Thornber
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.