* Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced
@ 2011-10-23 9:21 Michael Leun
2011-10-23 9:52 ` Linus Torvalds
0 siblings, 1 reply; 6+ messages in thread
From: Michael Leun @ 2011-10-23 9:21 UTC (permalink / raw)
To: LKML, dm-devel, Linus Torvalds, Mikulas Patocka
Hi,
How to reproduce:
lvcreate -L30G -n testsnap vg1 # of course substitute VG as appropriate
dd if=/dev/zero of=/dev/vg1/testsnap bs=2M # to make things clear
lvcreate -L15G -s /dev/vg1/test -n testsnap
dd if=/dev/zero of=/dev/vg1/testsnap bs=2M &
watch free
I noticed roughly 1GB memory vanishing from free / -/+ buffers/cache
per 2GB copied.
Bisecting yielded a6e50b409d3f9e0833e69c3c9cca822e8fa4adbb, reverting
that from git master cures the memory leak
--
MfG,
Michael Leun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced
2011-10-23 9:21 Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced Michael Leun
@ 2011-10-23 9:52 ` Linus Torvalds
2011-10-23 13:44 ` Alasdair G Kergon
2011-10-25 17:45 ` [PATCH] check for mempool memory leaks Mikulas Patocka
0 siblings, 2 replies; 6+ messages in thread
From: Linus Torvalds @ 2011-10-23 9:52 UTC (permalink / raw)
To: Alasdair G Kergon; +Cc: Michael Leun, LKML, dm-devel, Mikulas Patocka
Making sure that Alasdair sees this too (hopefully he picked up on it
from dm-devel, but best send things directly too)
Mikulas, Alasdair? I see the mempool_free() for the "master job", what
about everything else? Does the dm_kcopyd_prepare_callback() perhaps
need to do a
job->master_job = job;
or similar?
Linus
On Sun, Oct 23, 2011 at 12:21 PM, Michael Leun <ml@newton.leun.net> wrote:
> Hi,
>
> How to reproduce:
>
> lvcreate -L30G -n testsnap vg1 # of course substitute VG as appropriate
> dd if=/dev/zero of=/dev/vg1/testsnap bs=2M # to make things clear
> lvcreate -L15G -s /dev/vg1/test -n testsnap
> dd if=/dev/zero of=/dev/vg1/testsnap bs=2M &
> watch free
>
> I noticed roughly 1GB memory vanishing from free / -/+ buffers/cache
> per 2GB copied.
>
> Bisecting yielded a6e50b409d3f9e0833e69c3c9cca822e8fa4adbb, reverting
> that from git master cures the memory leak
>
> --
> MfG,
>
> Michael Leun
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced
2011-10-23 9:52 ` Linus Torvalds
@ 2011-10-23 13:44 ` Alasdair G Kergon
2011-10-23 14:30 ` Michael Leun
2011-10-25 17:45 ` [PATCH] check for mempool memory leaks Mikulas Patocka
1 sibling, 1 reply; 6+ messages in thread
From: Alasdair G Kergon @ 2011-10-23 13:44 UTC (permalink / raw)
To: Linus Torvalds, Michael Leun
Cc: Alasdair G Kergon, LKML, dm-devel, Mikulas Patocka
On Sun, Oct 23, 2011 at 12:52:06PM +0300, Linus Torvalds wrote:
> Mikulas, Alasdair? I see the mempool_free() for the "master job", what
> about everything else? Does the dm_kcopyd_prepare_callback() perhaps
> need to do a
>
> job->master_job = job;
>
> or similar?
If you look at the other place that performs that mempool_alloc, it's indeed
present there.
Michael - can you test if this solves the problem?
Alasdair
--- a/drivers/md/dm-kcopyd.c
+++ b/drivers/md/dm-kcopyd.c
@@ -628,6 +628,7 @@ void *dm_kcopyd_prepare_callback(struct dm_kcopyd_client *kc,
job->kc = kc;
job->fn = fn;
job->context = context;
+ job->master_job = job;
atomic_inc(&kc->nr_jobs);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced
2011-10-23 13:44 ` Alasdair G Kergon
@ 2011-10-23 14:30 ` Michael Leun
2011-10-23 20:19 ` [dm-devel] " Alasdair G Kergon
0 siblings, 1 reply; 6+ messages in thread
From: Michael Leun @ 2011-10-23 14:30 UTC (permalink / raw)
To: Alasdair G Kergon; +Cc: Linus Torvalds, LKML, dm-devel, Mikulas Patocka
On Sun, 23 Oct 2011 14:44:49 +0100
Alasdair G Kergon <agk@redhat.com> wrote:
> On Sun, Oct 23, 2011 at 12:52:06PM +0300, Linus Torvalds wrote:
> > Mikulas, Alasdair? I see the mempool_free() for the "master job",
> > what about everything else? Does the dm_kcopyd_prepare_callback()
> > perhaps need to do a
> >
> > job->master_job = job;
> >
> > or similar?
>
> If you look at the other place that performs that mempool_alloc, it's
> indeed present there.
>
> Michael - can you test if this solves the problem?
>
> Alasdair
>
> --- a/drivers/md/dm-kcopyd.c
> +++ b/drivers/md/dm-kcopyd.c
> @@ -628,6 +628,7 @@ void *dm_kcopyd_prepare_callback(struct
> dm_kcopyd_client *kc, job->kc = kc;
> job->fn = fn;
> job->context = context;
> + job->master_job = job;
>
> atomic_inc(&kc->nr_jobs);
>
>
Yup, it indeed does.
--
MfG,
Michael Leun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [dm-devel] Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced
2011-10-23 14:30 ` Michael Leun
@ 2011-10-23 20:19 ` Alasdair G Kergon
0 siblings, 0 replies; 6+ messages in thread
From: Alasdair G Kergon @ 2011-10-23 20:19 UTC (permalink / raw)
To: Linus Torvalds, Michael Leun
Cc: Alasdair G Kergon, Mikulas Patocka, dm-devel, LKML
On Sun, Oct 23, 2011 at 04:30:42PM +0200, Michael Leun wrote:
> On Sun, 23 Oct 2011 14:44:49 +0100
> Alasdair G Kergon <agk@redhat.com> wrote:
> > Michael - can you test if this solves the problem?
OK, let's go with this patch then.
Linus, you can pull it from here:
http://people.redhat.com/agk/git/linux-dm for-linus
Alasdair G Kergon (1):
dm kcopyd: fix job_pool leak
drivers/md/dm-kcopyd.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] check for mempool memory leaks
2011-10-23 9:52 ` Linus Torvalds
2011-10-23 13:44 ` Alasdair G Kergon
@ 2011-10-25 17:45 ` Mikulas Patocka
1 sibling, 0 replies; 6+ messages in thread
From: Mikulas Patocka @ 2011-10-25 17:45 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alasdair G Kergon, Michael Leun, LKML, dm-devel
On Sun, 23 Oct 2011, Linus Torvalds wrote:
> Making sure that Alasdair sees this too (hopefully he picked up on it
> from dm-devel, but best send things directly too)
>
> Mikulas, Alasdair? I see the mempool_free() for the "master job", what
> about everything else? Does the dm_kcopyd_prepare_callback() perhaps
> need to do a
>
> job->master_job = job;
>
> or similar?
>
> Linus
>
> On Sun, Oct 23, 2011 at 12:21 PM, Michael Leun <ml@newton.leun.net> wrote:
> > Hi,
> >
> > How to reproduce:
> >
> > lvcreate -L30G -n testsnap vg1 # of course substitute VG as appropriate
> > dd if=/dev/zero of=/dev/vg1/testsnap bs=2M # to make things clear
> > lvcreate -L15G -s /dev/vg1/test -n testsnap
> > dd if=/dev/zero of=/dev/vg1/testsnap bs=2M &
> > watch free
> >
> > I noticed roughly 1GB memory vanishing from free / -/+ buffers/cache
> > per 2GB copied.
> >
> > Bisecting yielded a6e50b409d3f9e0833e69c3c9cca822e8fa4adbb, reverting
> > that from git master cures the memory leak
> >
> > --
> > MfG,
> >
> > Michael Leun
> >
> >
>
BTW. would you like to accept this patch that enables tracking the number
of objects allocated in a mempool and reporting a warning? So that such
bugs could be automatically detected in the future?
---
Introduce an option DEBUG_MEMPOOL. It will check that a mempool is empty
when destroying the mempool.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
include/linux/mempool.h | 6 ++++++
lib/Kconfig.debug | 8 ++++++++
mm/mempool.c | 25 +++++++++++++++++++++++--
3 files changed, 37 insertions(+), 2 deletions(-)
Index: linux-3.1-fast/lib/Kconfig.debug
===================================================================
--- linux-3.1-fast.orig/lib/Kconfig.debug 2011-10-25 00:31:56.000000000 +0200
+++ linux-3.1-fast/lib/Kconfig.debug 2011-10-25 01:18:27.000000000 +0200
@@ -411,6 +411,14 @@ config SLUB_STATS
out which slabs are relevant to a particular load.
Try running: slabinfo -DA
+config DEBUG_MEMPOOL
+ bool "Check memory leaks in mempools"
+ depends on DEBUG_KERNEL
+ help
+ Enable debugging memory leaks in mempools. This options makes
+ the kernel count the number of objects allocated in a mempool
+ and check if this number is zero when the mempool is destroyed.
+
config DEBUG_KMEMLEAK
bool "Kernel memory leak detector"
depends on DEBUG_KERNEL && EXPERIMENTAL && !MEMORY_HOTPLUG && \
Index: linux-3.1-fast/mm/mempool.c
===================================================================
--- linux-3.1-fast.orig/mm/mempool.c 2011-10-25 00:39:34.000000000 +0200
+++ linux-3.1-fast/mm/mempool.c 2011-10-25 01:24:03.000000000 +0200
@@ -71,6 +71,9 @@ mempool_t *mempool_create_node(int min_n
kfree(pool);
return NULL;
}
+#ifdef CONFIG_DEBUG_MEMPOOL
+ atomic_long_set(&pool->nr_allocated, 0);
+#endif
spin_lock_init(&pool->lock);
pool->min_nr = min_nr;
pool->pool_data = pool_data;
@@ -183,7 +186,13 @@ EXPORT_SYMBOL(mempool_resize);
void mempool_destroy(mempool_t *pool)
{
/* Check for outstanding elements */
- BUG_ON(pool->curr_nr != pool->min_nr);
+#ifdef CONFIG_DEBUG_MEMPOOL
+ WARN(atomic_long_read(&pool->nr_allocated),
+ "%ld objects leaked in mempool",
+ atomic_long_read(&pool->nr_allocated));
+#else
+ WARN_ON(pool->curr_nr != pool->min_nr);
+#endif
free_pool(pool);
}
EXPORT_SYMBOL(mempool_destroy);
@@ -217,13 +226,20 @@ void * mempool_alloc(mempool_t *pool, gf
repeat_alloc:
element = pool->alloc(gfp_temp, pool->pool_data);
- if (likely(element != NULL))
+ if (likely(element != NULL)) {
+#ifdef CONFIG_DEBUG_MEMPOOL
+ atomic_long_inc(&pool->nr_allocated);
+#endif
return element;
+ }
spin_lock_irqsave(&pool->lock, flags);
if (likely(pool->curr_nr)) {
element = remove_element(pool);
spin_unlock_irqrestore(&pool->lock, flags);
+#ifdef CONFIG_DEBUG_MEMPOOL
+ atomic_long_inc(&pool->nr_allocated);
+#endif
return element;
}
spin_unlock_irqrestore(&pool->lock, flags);
@@ -265,6 +281,11 @@ void mempool_free(void *element, mempool
if (unlikely(element == NULL))
return;
+#ifdef CONFIG_DEBUG_MEMPOOL
+ BUG_ON(!atomic_long_read(&pool->nr_allocated));
+ atomic_long_dec(&pool->nr_allocated);
+#endif
+
smp_mb();
if (pool->curr_nr < pool->min_nr) {
spin_lock_irqsave(&pool->lock, flags);
Index: linux-3.1-fast/include/linux/mempool.h
===================================================================
--- linux-3.1-fast.orig/include/linux/mempool.h 2011-10-25 01:12:13.000000000 +0200
+++ linux-3.1-fast/include/linux/mempool.h 2011-10-25 01:18:51.000000000 +0200
@@ -5,6 +5,9 @@
#define _LINUX_MEMPOOL_H
#include <linux/wait.h>
+#ifdef CONFIG_DEBUG_MEMPOOLS
+#include <linux/atomic.h>
+#endif
struct kmem_cache;
@@ -12,6 +15,9 @@ typedef void * (mempool_alloc_t)(gfp_t g
typedef void (mempool_free_t)(void *element, void *pool_data);
typedef struct mempool_s {
+#ifdef CONFIG_DEBUG_MEMPOOL
+ atomic_long_t nr_allocated;
+#endif
spinlock_t lock;
int min_nr; /* nr of elements at *elements */
int curr_nr; /* Current nr of elements at *elements */
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-25 17:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-23 9:21 Bisected: Massive memory leak in dm-snapshot in 3.1 development introduced Michael Leun
2011-10-23 9:52 ` Linus Torvalds
2011-10-23 13:44 ` Alasdair G Kergon
2011-10-23 14:30 ` Michael Leun
2011-10-23 20:19 ` [dm-devel] " Alasdair G Kergon
2011-10-25 17:45 ` [PATCH] check for mempool memory leaks Mikulas Patocka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome