All of lore.kernel.org
 help / color / mirror / Atom feed
* Huge memory leak in virtio, see kvm-Bugs-2989366
@ 2010-04-20 22:29 Leszek Urbanski
  2010-04-20 22:51 ` Alexander Graf
  2010-04-21  1:58 ` Ryan Harper
  0 siblings, 2 replies; 18+ messages in thread
From: Leszek Urbanski @ 2010-04-20 22:29 UTC (permalink / raw)
  To: kvm

Hi,

this is a follow-up to bug 2989366:

https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2989366&group_id=180599

after extensive debugging with the guys on #kvm it turns out that the leak is
in the qemu-kvm userland process, in virtio-blk.

A summary of my setup is described in the bug report above.

The affected guests have a common load profile: frequent sequential I/O on
large (~ 2 GB) files.

I tried switching off or changing almost all options in my qemu command
line and the only option that makes a difference is -drive if=virtio.

When an affected guest is run with virtio drives the qemu-kvm process starts
leaking immediately after startup and grows (for the most heavily leaking
guests) by ~ 1 GB RSS for every ten hours (and keeps growing until OOM).

With -drive if=ide or scsi, it doesn't leak at all.

A diff of /proc/<pid>/maps of an affected qemu-kvm at startup and after
1.5 hrs:

-039b9000-5ccd0000 rw-p 00000000 00:00 0
+039b9000-65803000 rw-p 00000000 00:00 0

(a heap leak?)

I'm willing to debug further. The problem is 100% reproducible.


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-20 22:29 Huge memory leak in virtio, see kvm-Bugs-2989366 Leszek Urbanski
@ 2010-04-20 22:51 ` Alexander Graf
  2010-04-21  7:53   ` Leszek Urbanski
  2010-04-21  1:58 ` Ryan Harper
  1 sibling, 1 reply; 18+ messages in thread
From: Alexander Graf @ 2010-04-20 22:51 UTC (permalink / raw)
  To: Leszek Urbanski; +Cc: kvm


On 21.04.2010, at 00:29, Leszek Urbanski wrote:

> Hi,
> 
> this is a follow-up to bug 2989366:
> 
> https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2989366&group_id=180599
> 
> after extensive debugging with the guys on #kvm it turns out that the leak is
> in the qemu-kvm userland process, in virtio-blk.
> 
> A summary of my setup is described in the bug report above.
> 
> The affected guests have a common load profile: frequent sequential I/O on
> large (~ 2 GB) files.
> 
> I tried switching off or changing almost all options in my qemu command
> line and the only option that makes a difference is -drive if=virtio.
> 
> When an affected guest is run with virtio drives the qemu-kvm process starts
> leaking immediately after startup and grows (for the most heavily leaking
> guests) by ~ 1 GB RSS for every ten hours (and keeps growing until OOM).
> 
> With -drive if=ide or scsi, it doesn't leak at all.
> 
> A diff of /proc/<pid>/maps of an affected qemu-kvm at startup and after
> 1.5 hrs:
> 
> -039b9000-5ccd0000 rw-p 00000000 00:00 0
> +039b9000-65803000 rw-p 00000000 00:00 0
> 
> (a heap leak?)
> 
> I'm willing to debug further. The problem is 100% reproducible.

Certainly something malloc()'ed. It'd be great to send this through valgrind. Thanks to KVM the guest still runs natively, so the slowdown isn't _that_ bug through it. I'm also not a valgrind expert, but IIRC there was a separate memory allocation module.


Alex


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-20 22:29 Huge memory leak in virtio, see kvm-Bugs-2989366 Leszek Urbanski
  2010-04-20 22:51 ` Alexander Graf
@ 2010-04-21  1:58 ` Ryan Harper
  2010-04-21  6:08   ` Michael Tokarev
  1 sibling, 1 reply; 18+ messages in thread
From: Ryan Harper @ 2010-04-21  1:58 UTC (permalink / raw)
  To: Leszek Urbanski; +Cc: kvm

* Leszek Urbanski <tygrys@moo.pl> [2010-04-20 17:37]:
> Hi,
> 
> this is a follow-up to bug 2989366:
> 
> https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2989366&group_id=180599
> 
> after extensive debugging with the guys on #kvm it turns out that the leak is
> in the qemu-kvm userland process, in virtio-blk.
> 
> A summary of my setup is described in the bug report above.
> 
> The affected guests have a common load profile: frequent sequential I/O on
> large (~ 2 GB) files.
> 
> I tried switching off or changing almost all options in my qemu command
> line and the only option that makes a difference is -drive if=virtio.
> 
> When an affected guest is run with virtio drives the qemu-kvm process starts
> leaking immediately after startup and grows (for the most heavily leaking
> guests) by ~ 1 GB RSS for every ten hours (and keeps growing until OOM).
> 
> With -drive if=ide or scsi, it doesn't leak at all.
> 
> A diff of /proc/<pid>/maps of an affected qemu-kvm at startup and after
> 1.5 hrs:
> 
> -039b9000-5ccd0000 rw-p 00000000 00:00 0
> +039b9000-65803000 rw-p 00000000 00:00 0
> 
> (a heap leak?)
> 
> I'm willing to debug further. The problem is 100% reproducible.

Is that qemu-kvm 0.12.3 compiled from source? or using the distro
package?

If you drop the -smp 4 part, you could also try plain qemu to eliminate
if there was a qemu-kvm merge issue. 

Also, if you switch to a different guest do you still see the same leak?
This should help determine if the virtio-blk front end is part of the
issue.


-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-21  1:58 ` Ryan Harper
@ 2010-04-21  6:08   ` Michael Tokarev
  2010-04-21 13:29     ` Leszek Urbanski
  2010-04-21 14:32     ` Ryan Harper
  0 siblings, 2 replies; 18+ messages in thread
From: Michael Tokarev @ 2010-04-21  6:08 UTC (permalink / raw)
  To: Ryan Harper; +Cc: Leszek Urbanski, kvm

21.04.2010 05:58, Ryan Harper wrote:
> * Leszek Urbanski<tygrys@moo.pl>  [2010-04-20 17:37]:
>> Hi,
>>
>> this is a follow-up to bug 2989366:
>>
>> https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2989366&group_id=180599
>>
>> after extensive debugging with the guys on #kvm it turns out that the leak is
>> in the qemu-kvm userland process, in virtio-blk.
[]
> Is that qemu-kvm 0.12.3 compiled from source? or using the distro
> package?

(i'm not the OP, but we talked with him on irc about the issue)

It's a debian package of qemu-kvm.  There are a couple of cosmetic
and unrelated patches applied to it, with one important to fix the
large iovecs issue.  See

  http://git.debian.org/?p=collab-maint/qemu-kvm.git;a=tree;f=debian/patches;h=25ae313db327faa0559016e40fa6161018eb49f4;hb=caa82cbb176403e88128b4fe2698ff192ea10891

for the complete set of patches in there (it's debian/patches
directory in http://git.debian.org/?p=collab-maint/qemu-kvm.git ,
in v0.12.3+dfsg-4 branch).  The only interesting patch in there
is the avoid_creating_too_large_iovecs_in_multiwrite_merge.patch
one, the rest are not relevant.

> If you drop the -smp 4 part, you could also try plain qemu to eliminate
> if there was a qemu-kvm merge issue.

So basically, upstream qemu now works as good
as qemu-kvm for non-smp guests?

> Also, if you switch to a different guest do you still see the same leak?
> This should help determine if the virtio-blk front end is part of the
> issue.

There are only a few guests which are affected.  So far it is not
really clear what differs them from others: a reinstall of a new
guest with the same components and doing same functions will not
necessary show the leak.

Thanks!

/mjt

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-20 22:51 ` Alexander Graf
@ 2010-04-21  7:53   ` Leszek Urbanski
  2010-04-21  8:25     ` Avi Kivity
  0 siblings, 1 reply; 18+ messages in thread
From: Leszek Urbanski @ 2010-04-21  7:53 UTC (permalink / raw)
  To: Alexander Graf; +Cc: kvm

<08761DCA-B8FA-41C8-A839-52E09BC06824@suse.de>; from Alexander Graf on Wed, Apr 21, 2010 at 00:51:21 +0200

> > -039b9000-5ccd0000 rw-p 00000000 00:00 0
> > +039b9000-65803000 rw-p 00000000 00:00 0
> > 
> > (a heap leak?)
> > 
> > I'm willing to debug further. The problem is 100% reproducible.
> 
> Certainly something malloc()'ed. It'd be great to send this through valgrind. Thanks to KVM the guest still runs natively, so the slowdown isn't _that_ bug through it. I'm also not a valgrind expert, but IIRC there was a separate memory allocation module.

Unfortunately, KVM guests with virtio won't even finish booting with valgrind. 
The guest's kernel complains about I/O errors in the first sectors of the
virtio root device.


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-21  7:53   ` Leszek Urbanski
@ 2010-04-21  8:25     ` Avi Kivity
  0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2010-04-21  8:25 UTC (permalink / raw)
  To: Leszek Urbanski; +Cc: Alexander Graf, kvm

On 04/21/2010 10:53 AM, Leszek Urbanski wrote:
> <08761DCA-B8FA-41C8-A839-52E09BC06824@suse.de>; from Alexander Graf on Wed, Apr 21, 2010 at 00:51:21 +0200
>
>    
>>> -039b9000-5ccd0000 rw-p 00000000 00:00 0
>>> +039b9000-65803000 rw-p 00000000 00:00 0
>>>
>>> (a heap leak?)
>>>
>>> I'm willing to debug further. The problem is 100% reproducible.
>>>        
>> Certainly something malloc()'ed. It'd be great to send this through valgrind. Thanks to KVM the guest still runs natively, so the slowdown isn't _that_ bug through it. I'm also not a valgrind expert, but IIRC there was a separate memory allocation module.
>>      
> Unfortunately, KVM guests with virtio won't even finish booting with valgrind.
> The guest's kernel complains about I/O errors in the first sectors of the
> virtio root device.
>
>    

You can try glibc's malloc tracing, see libc's mtrace().

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-21  6:08   ` Michael Tokarev
@ 2010-04-21 13:29     ` Leszek Urbanski
  2010-04-21 18:39       ` Stefan Hajnoczi
  2010-04-21 14:32     ` Ryan Harper
  1 sibling, 1 reply; 18+ messages in thread
From: Leszek Urbanski @ 2010-04-21 13:29 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Ryan Harper, kvm

<4BCE9654.2030604@msgid.tls.msk.ru>; from Michael Tokarev on Wed, Apr 21, 2010 at 10:08:20 +0400
> 
> There are only a few guests which are affected.  So far it is not
> really clear what differs them from others: a reinstall of a new
> guest with the same components and doing same functions will not
> necessary show the leak.

I checked further and I can reproduce it on every guest with a simple:

while true ; do dd if=/dev/urandom of=file bs=10M count=200 ; cat file > /dev/null ; done

It will leak slowly.

-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-21  6:08   ` Michael Tokarev
  2010-04-21 13:29     ` Leszek Urbanski
@ 2010-04-21 14:32     ` Ryan Harper
  2010-04-21 18:27       ` [PATCH] block: Free iovec arrays allocated by multiwrite_merge() Stefan Hajnoczi
  1 sibling, 1 reply; 18+ messages in thread
From: Ryan Harper @ 2010-04-21 14:32 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Ryan Harper, Leszek Urbanski, kvm

* Michael Tokarev <mjt@tls.msk.ru> [2010-04-21 01:08]:
> 21.04.2010 05:58, Ryan Harper wrote:
> >* Leszek Urbanski<tygrys@moo.pl>  [2010-04-20 17:37]:
> >>Hi,
> >>
> >>this is a follow-up to bug 2989366:
> >>
> >>https://sourceforge.net/tracker/?func=detail&atid=893831&aid=2989366&group_id=180599
> >>
> >>after extensive debugging with the guys on #kvm it turns out that the 
> >>leak is
> >>in the qemu-kvm userland process, in virtio-blk.
> []
> >Is that qemu-kvm 0.12.3 compiled from source? or using the distro
> >package?
> 
> (i'm not the OP, but we talked with him on irc about the issue)
> 
> It's a debian package of qemu-kvm.  There are a couple of cosmetic
> and unrelated patches applied to it, with one important to fix the
> large iovecs issue.  See
> 
>  http://git.debian.org/?p=collab-maint/qemu-kvm.git;a=tree;f=debian/patches;h=25ae313db327faa0559016e40fa6161018eb49f4;hb=caa82cbb176403e88128b4fe2698ff192ea10891
> 
> for the complete set of patches in there (it's debian/patches
> directory in http://git.debian.org/?p=collab-maint/qemu-kvm.git ,
> in v0.12.3+dfsg-4 branch).  The only interesting patch in there
> is the avoid_creating_too_large_iovecs_in_multiwrite_merge.patch
> one, the rest are not relevant.
> 
> >If you drop the -smp 4 part, you could also try plain qemu to eliminate
> >if there was a qemu-kvm merge issue.
> 
> So basically, upstream qemu now works as good
> as qemu-kvm for non-smp guests?

Not all of qemu-kvm features are upstream in qemu, but upstream qemu
supports kvm enough that it's worth testing to see if can't reproduce
there should provide some insight as to where the bug might be.  

> 
> >Also, if you switch to a different guest do you still see the same leak?
> >This should help determine if the virtio-blk front end is part of the
> >issue.
> 
> There are only a few guests which are affected.  So far it is not
> really clear what differs them from others: a reinstall of a new
> guest with the same components and doing same functions will not
> necessary show the leak.

That is troublesome.  Are you saying if I install  lenny guest and do
the dd; cat I might not reproduce it?


> 
> Thanks!
> 
> /mjt
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 14:32     ` Ryan Harper
@ 2010-04-21 18:27       ` Stefan Hajnoczi
  2010-04-21 18:35         ` Ryan Harper
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2010-04-21 18:27 UTC (permalink / raw)
  To: kvm; +Cc: Leszek Urbanski, Michael Tokarev, Ryan Harper, Stefan Hajnoczi

A new iovec array is allocated when creating a merged write request.
This patch ensures that the iovec array is deleted in addition to its
qiov owner.

Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index e891544..2d31474 100644
--- a/block.c
+++ b/block.c
@@ -1731,6 +1731,9 @@ static void multiwrite_user_cb(MultiwriteCB *mcb)
 
     for (i = 0; i < mcb->num_callbacks; i++) {
         mcb->callbacks[i].cb(mcb->callbacks[i].opaque, mcb->error);
+        if (mcb->callbacks[i].free_qiov) {
+            qemu_iovec_destroy(mcb->callbacks[i].free_qiov);
+        }
         qemu_free(mcb->callbacks[i].free_qiov);
         qemu_vfree(mcb->callbacks[i].free_buf);
     }
-- 
1.7.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 18:27       ` [PATCH] block: Free iovec arrays allocated by multiwrite_merge() Stefan Hajnoczi
@ 2010-04-21 18:35         ` Ryan Harper
  2010-04-21 18:43           ` Brian Jackson
  2010-04-21 19:59           ` Leszek Urbanski
  0 siblings, 2 replies; 18+ messages in thread
From: Ryan Harper @ 2010-04-21 18:35 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kvm, Leszek Urbanski, Michael Tokarev, Ryan Harper

* Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> [2010-04-21 13:27]:
> A new iovec array is allocated when creating a merged write request.
> This patch ensures that the iovec array is deleted in addition to its
> qiov owner.

Nice catch.  Send this to qemu-devel and Avi and merge into qemu-kvm
once it's commited there.

> 
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index e891544..2d31474 100644
> --- a/block.c
> +++ b/block.c
> @@ -1731,6 +1731,9 @@ static void multiwrite_user_cb(MultiwriteCB *mcb)
> 
>      for (i = 0; i < mcb->num_callbacks; i++) {
>          mcb->callbacks[i].cb(mcb->callbacks[i].opaque, mcb->error);
> +        if (mcb->callbacks[i].free_qiov) {
> +            qemu_iovec_destroy(mcb->callbacks[i].free_qiov);
> +        }
>          qemu_free(mcb->callbacks[i].free_qiov);
>          qemu_vfree(mcb->callbacks[i].free_buf);
>      }
> -- 
> 1.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Huge memory leak in virtio, see kvm-Bugs-2989366
  2010-04-21 13:29     ` Leszek Urbanski
@ 2010-04-21 18:39       ` Stefan Hajnoczi
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Hajnoczi @ 2010-04-21 18:39 UTC (permalink / raw)
  To: Leszek Urbanski; +Cc: Michael Tokarev, Ryan Harper, kvm

Leszek,
Please try the qemu-kvm.git patch I have sent called "block: Free
iovec arrays allocated by multiwrite_merge()" to confirm that it fixes
the leak.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 18:35         ` Ryan Harper
@ 2010-04-21 18:43           ` Brian Jackson
  2010-04-21 19:59           ` Leszek Urbanski
  1 sibling, 0 replies; 18+ messages in thread
From: Brian Jackson @ 2010-04-21 18:43 UTC (permalink / raw)
  To: Ryan Harper; +Cc: Stefan Hajnoczi, kvm, Leszek Urbanski, Michael Tokarev

On Wednesday 21 April 2010 13:35:36 Ryan Harper wrote:
> * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> [2010-04-21 13:27]:
> > A new iovec array is allocated when creating a merged write request.
> > This patch ensures that the iovec array is deleted in addition to its
> > qiov owner.
> 
> Nice catch.  Send this to qemu-devel and Avi and merge into qemu-kvm
> once it's commited there.


And tag for the stable release that should be coming soon?


> 
> > Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> > ---
> >  block.c |    3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index e891544..2d31474 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -1731,6 +1731,9 @@ static void multiwrite_user_cb(MultiwriteCB *mcb)
> >
> >      for (i = 0; i < mcb->num_callbacks; i++) {
> >          mcb->callbacks[i].cb(mcb->callbacks[i].opaque, mcb->error);
> > +        if (mcb->callbacks[i].free_qiov) {
> > +            qemu_iovec_destroy(mcb->callbacks[i].free_qiov);
> > +        }
> >          qemu_free(mcb->callbacks[i].free_qiov);
> >          qemu_vfree(mcb->callbacks[i].free_buf);
> >      }
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 18:35         ` Ryan Harper
  2010-04-21 18:43           ` Brian Jackson
@ 2010-04-21 19:59           ` Leszek Urbanski
  2010-04-21 20:03             ` Ryan Harper
  1 sibling, 1 reply; 18+ messages in thread
From: Leszek Urbanski @ 2010-04-21 19:59 UTC (permalink / raw)
  To: kvm; +Cc: Stefan Hajnoczi, Michael Tokarev, Avi Kivity, Ryan Harper

<20100421183536.GG24351@us.ibm.com>; from Ryan Harper on Wed, Apr 21, 2010 at 13:35:36 -0500
> * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> [2010-04-21 13:27]:
> > A new iovec array is allocated when creating a merged write request.
> > This patch ensures that the iovec array is deleted in addition to its
> > qiov owner.
> 
> Nice catch.  Send this to qemu-devel and Avi and merge into qemu-kvm
> once it's commited there.

Nice catch indeed.

Debugging with mtrace() also pointed to the iovec code as the culprit.

It's been running for 45 minutes with this patch applied - no leaks. Thanks!


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 19:59           ` Leszek Urbanski
@ 2010-04-21 20:03             ` Ryan Harper
  2010-04-21 20:13               ` Leszek Urbanski
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Harper @ 2010-04-21 20:03 UTC (permalink / raw)
  To: Leszek Urbanski
  Cc: kvm, Stefan Hajnoczi, Michael Tokarev, Avi Kivity, Ryan Harper

* Leszek Urbanski <tygrys@moo.pl> [2010-04-21 14:59]:
> <20100421183536.GG24351@us.ibm.com>; from Ryan Harper on Wed, Apr 21, 2010 at 13:35:36 -0500
> > * Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> [2010-04-21 13:27]:
> > > A new iovec array is allocated when creating a merged write request.
> > > This patch ensures that the iovec array is deleted in addition to its
> > > qiov owner.
> > 
> > Nice catch.  Send this to qemu-devel and Avi and merge into qemu-kvm
> > once it's commited there.
> 
> Nice catch indeed.
> 
> Debugging with mtrace() also pointed to the iovec code as the culprit.

I've not used mtrace before, could you dump your command invocation for
the list?  I know other's would be glad to see an example with kvm

-- 
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 20:03             ` Ryan Harper
@ 2010-04-21 20:13               ` Leszek Urbanski
  2010-04-25 12:36                 ` Avi Kivity
  0 siblings, 1 reply; 18+ messages in thread
From: Leszek Urbanski @ 2010-04-21 20:13 UTC (permalink / raw)
  To: Ryan Harper; +Cc: kvm

<20100421200358.GH24351@us.ibm.com>; from Ryan Harper on Wed, Apr 21, 2010 at 15:03:58 -0500

> > Debugging with mtrace() also pointed to the iovec code as the culprit.
> 
> I've not used mtrace before, could you dump your command invocation for
> the list?  I know other's would be glad to see an example with kvm

#include <mcheck.c>, put mtrace() and muntrace() around the code in main()
in vl.c

export MALLOC_TRACE=/some/file

then run qemu with your usual options. After powering off the guest,
run "mtrace /path/to/qemu-binary /some/file" - it's a perl script that makes
the output more human readable.

It's a little tricky, though - remember that it will see most allocations
occurring in qemu-malloc.c - it only traces explicit glibc malloc() calls.


-- 
Leszek "Tygrys" Urbanski, SCSA, SCNA
 "Unix-to-Unix Copy Program;" said PDP-1. "You will never find a more
  wretched hive of bugs and flamers. We must be cautious." -- DECWARS
     http://cygnus.moo.pl/ -- Cygnus High Altitude Balloon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-21 20:13               ` Leszek Urbanski
@ 2010-04-25 12:36                 ` Avi Kivity
  2010-04-25 13:27                   ` Stefan Hajnoczi
  0 siblings, 1 reply; 18+ messages in thread
From: Avi Kivity @ 2010-04-25 12:36 UTC (permalink / raw)
  To: Leszek Urbanski; +Cc: Ryan Harper, kvm

On 04/21/2010 11:13 PM, Leszek Urbanski wrote:
> <20100421200358.GH24351@us.ibm.com>; from Ryan Harper on Wed, Apr 21, 2010 at 15:03:58 -0500
>
>    
>>> Debugging with mtrace() also pointed to the iovec code as the culprit.
>>>        
>> I've not used mtrace before, could you dump your command invocation for
>> the list?  I know other's would be glad to see an example with kvm
>>      
> #include<mcheck.c>, put mtrace() and muntrace() around the code in main()
> in vl.c
>
> export MALLOC_TRACE=/some/file
>
> then run qemu with your usual options. After powering off the guest,
> run "mtrace /path/to/qemu-binary /some/file" - it's a perl script that makes
> the output more human readable.
>
> It's a little tricky, though - remember that it will see most allocations
> occurring in qemu-malloc.c - it only traces explicit glibc malloc() calls.
>    

Does it show only the immediate caller, without a complete stack trace?

If so, that sucks.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-25 12:36                 ` Avi Kivity
@ 2010-04-25 13:27                   ` Stefan Hajnoczi
  2010-04-25 13:35                     ` Avi Kivity
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Hajnoczi @ 2010-04-25 13:27 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Leszek Urbanski, Ryan Harper, kvm, Stefan Hajnoczi

From: Stefan Hajnoczi <stefanha@gmail.com>

The MALLOC_TRACE output didn't look useful when I tried it either.

Instead I used the following to find origin of the leak.  Still very basic but
works better with qemu_malloc() and friends.

This is just a hack but I wanted to share it in case someone finds it useful in
the future.

---
 Makefile.objs |    2 +-
 leakcheck.c   |   17 +++++++++++++++
 leakcheck.py  |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 osdep.c       |    7 +++++-
 qemu-malloc.c |   26 +++++++++++++++++++----
 5 files changed, 108 insertions(+), 7 deletions(-)
 create mode 100644 leakcheck.c
 create mode 100755 leakcheck.py

diff --git a/Makefile.objs b/Makefile.objs
index 59ec879..82a4fac 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -7,7 +7,7 @@ qobject-obj-y += qerror.o
 #######################################################################
 # block-obj-y is code used by both qemu system emulation and qemu-img
 
-block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
+block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o leakcheck.o
 block-obj-y += nbd.o block.o aio.o aes.o osdep.o
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
diff --git a/leakcheck.c b/leakcheck.c
new file mode 100644
index 0000000..a5fa51a
--- /dev/null
+++ b/leakcheck.c
@@ -0,0 +1,17 @@
+#include <stdio.h>
+
+static FILE *fp;
+
+extern void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1);
+
+void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1)
+{
+	if (!fp) {
+		fp = fopen("/tmp/leakcheck.log", "w");
+		if (!fp) {
+			return;
+		}
+	}
+
+	fprintf(fp, "%c %p %p %zd %p\n", action, old_addr, addr, size, ret1);
+}
diff --git a/leakcheck.py b/leakcheck.py
new file mode 100755
index 0000000..64b1a1b
--- /dev/null
+++ b/leakcheck.py
@@ -0,0 +1,63 @@
+#!/usr/bin/env python
+import sys
+
+class Event(object):
+    def __init__(self, num, action, old_addr, addr, size, ret_addr):
+        self.num = num
+        self.action = action
+        self.old_addr = old_addr
+        self.addr = addr
+        self.size = size
+        self.ret_addr = ret_addr
+
+    def __str__(self):
+        return '%d %s %s %s %s %s' % (self.num, self.action, self.old_addr, self.addr, self.size, self.ret_addr)
+
+def malloc(event):
+    if event.addr in allocs:
+        sys.stderr.write('malloc returned duplicate address from %s\n' % event)
+    allocs[event.addr] = event
+
+def free(event):
+    if event.addr == '(nil)':
+        return
+    if event.addr not in allocs:
+        sys.stderr.write('free of unallocated address from %s\n' % event)
+        return
+    malloc_event = allocs[event.addr]
+    del allocs[event.addr]
+    if (malloc_event.action in 'msz' and event.action == 'f') or \
+       (malloc_event.action == 'a' and event.action == 'v'):
+        return
+    sys.stderr.write('mismatched actions for %s and %s\n' % (malloc_event, event))
+
+def realloc(event):
+    free(Event(event.num, 'f', event.old_addr, '(nil)', 0, event.ret_addr))
+    malloc(Event(event.num, 'm', '(nil)', event.addr, event.size, event.ret_addr))
+
+allocs = {}
+watermark = 0
+event_num = 0
+for line in sys.stdin:
+    event_num += 1
+
+    cmd = line.strip()
+    if cmd == 'watermark':
+        watermark = event_num
+        continue
+
+    action, old_addr, addr, size, ret_addr = cmd.split()
+    event = Event(event_num, action, old_addr, addr, size, ret_addr)
+    if action in 'amsz':
+        malloc(event)
+    elif action in 'fv':
+        free(event)
+    elif action == 'r':
+        realloc(event)
+    else:
+        sys.stderr.write('invalid action "%c"\n' % action)
+        sys.exit(1)
+
+for event in sorted(allocs.itervalues(), key=lambda e: e.num):
+    if event.num > watermark:
+        print event
diff --git a/osdep.c b/osdep.c
index 8a710e7..40788e5 100644
--- a/osdep.c
+++ b/osdep.c
@@ -95,6 +95,8 @@ void qemu_vfree(void *ptr)
 
 #else
 
+extern void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1);
+
 void *qemu_memalign(size_t alignment, size_t size)
 {
 #if defined(_POSIX_C_SOURCE) && !defined(__sun__)
@@ -110,7 +112,9 @@ void *qemu_memalign(size_t alignment, size_t size)
 #elif defined(CONFIG_BSD)
     return oom_check(valloc(size));
 #else
-    return oom_check(memalign(alignment, size));
+    void *p = oom_check(memalign(alignment, size));
+    leakcheck_log('a', NULL, p, size, __builtin_return_address(0));
+    return p;
 #endif
 }
 
@@ -126,6 +130,7 @@ void *qemu_vmalloc(size_t size)
 
 void qemu_vfree(void *ptr)
 {
+    leakcheck_log('v', NULL, ptr, 0, __builtin_return_address(0));
     free(ptr);
 }
 
diff --git a/qemu-malloc.c b/qemu-malloc.c
index 6cdc5de..bf832f2 100644
--- a/qemu-malloc.c
+++ b/qemu-malloc.c
@@ -24,6 +24,8 @@
 #include "qemu-common.h"
 #include <stdlib.h>
 
+extern void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1);
+
 static void *oom_check(void *ptr)
 {
     if (ptr == NULL) {
@@ -39,6 +41,7 @@ void *get_mmap_addr(unsigned long size)
 
 void qemu_free(void *ptr)
 {
+    leakcheck_log('f', NULL, ptr, 0, __builtin_return_address(0));
     free(ptr);
 }
 
@@ -51,7 +54,7 @@ static int allow_zero_malloc(void)
 #endif
 }
 
-void *qemu_malloc(size_t size)
+static void *qemu_malloc_common(size_t size)
 {
     if (!size && !allow_zero_malloc()) {
         abort();
@@ -59,19 +62,30 @@ void *qemu_malloc(size_t size)
     return oom_check(malloc(size ? size : 1));
 }
 
+void *qemu_malloc(size_t size)
+{
+    void *p = qemu_malloc_common(size);
+    leakcheck_log('m', NULL, p, size, __builtin_return_address(0));
+    return p;
+}
+
 void *qemu_realloc(void *ptr, size_t size)
 {
     if (!size && !allow_zero_malloc()) {
         abort();
     }
-    return oom_check(realloc(ptr, size ? size : 1));
+    size = size ? size : 1;
+    void *p = oom_check(realloc(ptr, size));
+    leakcheck_log('r', ptr, p, size, __builtin_return_address(0));
+    return p;
 }
 
 void *qemu_mallocz(size_t size)
 {
     void *ptr;
-    ptr = qemu_malloc(size);
+    ptr = qemu_malloc_common(size);
     memset(ptr, 0, size);
+    leakcheck_log('z', NULL, ptr, size, __builtin_return_address(0));
     return ptr;
 }
 
@@ -79,8 +93,9 @@ char *qemu_strdup(const char *str)
 {
     char *ptr;
     size_t len = strlen(str);
-    ptr = qemu_malloc(len + 1);
+    ptr = qemu_malloc_common(len + 1);
     memcpy(ptr, str, len + 1);
+    leakcheck_log('s', NULL, ptr, len + 1, __builtin_return_address(0));
     return ptr;
 }
 
@@ -93,8 +108,9 @@ char *qemu_strndup(const char *str, size_t size)
         size = end - str;
     }
 
-    new = qemu_malloc(size + 1);
+    new = qemu_malloc_common(size + 1);
     new[size] = 0;
 
+    leakcheck_log('s', NULL, new, size + 1, __builtin_return_address(0));
     return memcpy(new, str, size);
 }
-- 
1.7.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] block: Free iovec arrays allocated by multiwrite_merge()
  2010-04-25 13:27                   ` Stefan Hajnoczi
@ 2010-04-25 13:35                     ` Avi Kivity
  0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2010-04-25 13:35 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: Leszek Urbanski, Ryan Harper, kvm, Stefan Hajnoczi

On 04/25/2010 04:27 PM, Stefan Hajnoczi wrote:
> From: Stefan Hajnoczi<stefanha@gmail.com>
>
> The MALLOC_TRACE output didn't look useful when I tried it either.
>
> Instead I used the following to find origin of the leak.  Still very basic but
> works better with qemu_malloc() and friends.
>
> This is just a hack but I wanted to share it in case someone finds it useful in
> the future.
>
> new file mode 100644
> index 0000000..a5fa51a
> --- /dev/null
> +++ b/leakcheck.c
> @@ -0,0 +1,17 @@
> +#include<stdio.h>
> +
> +static FILE *fp;
> +
> +extern void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1);
> +
> +void leakcheck_log(char action, void *old_addr, void *addr, size_t size, void *ret1)
> +{
> +	if (!fp) {
> +		fp = fopen("/tmp/leakcheck.log", "w");
> +		if (!fp) {
> +			return;
> +		}
> +	}
>    

use backtrace(3) or backtrace_symbols(3), or even better, an external tool?

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2010-04-25 13:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-20 22:29 Huge memory leak in virtio, see kvm-Bugs-2989366 Leszek Urbanski
2010-04-20 22:51 ` Alexander Graf
2010-04-21  7:53   ` Leszek Urbanski
2010-04-21  8:25     ` Avi Kivity
2010-04-21  1:58 ` Ryan Harper
2010-04-21  6:08   ` Michael Tokarev
2010-04-21 13:29     ` Leszek Urbanski
2010-04-21 18:39       ` Stefan Hajnoczi
2010-04-21 14:32     ` Ryan Harper
2010-04-21 18:27       ` [PATCH] block: Free iovec arrays allocated by multiwrite_merge() Stefan Hajnoczi
2010-04-21 18:35         ` Ryan Harper
2010-04-21 18:43           ` Brian Jackson
2010-04-21 19:59           ` Leszek Urbanski
2010-04-21 20:03             ` Ryan Harper
2010-04-21 20:13               ` Leszek Urbanski
2010-04-25 12:36                 ` Avi Kivity
2010-04-25 13:27                   ` Stefan Hajnoczi
2010-04-25 13:35                     ` Avi Kivity

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.