From: gaojinhao <gaojinhao@huawei.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Stefan Berger" <stefanb@linux.vnet.ibm.com>,
"Greg Kurz" <groug@kaod.org>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"Juan Quintela" <quintela@redhat.com>,
"qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>,
"Wanghaibin (D)" <wanghaibin.wang@huawei.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
zhukeqian <zhukeqian1@huawei.com>,
"David Gibson" <david@gibson.dropbear.id.au>
Subject: RE: [PATCH v3 3/3] vmstate: Fix memory leak in vmstate_handle_alloc()
Date: Mon, 8 Feb 2021 11:46:29 +0000 [thread overview]
Message-ID: <7562293f0ee140b9b2a20b170c657207@huawei.com> (raw)
In-Reply-To: <20210208105232.GA3033@work-vm>
Hi, Dave,
I will check the code about vmstate_spapr_tce_table to figure out the problem of seg fault.
Thank you for your check.
Jinhao Gao
-----Original Message-----
From: Dr. David Alan Gilbert [mailto:dgilbert@redhat.com]
Sent: 2021年2月8日 18:53
To: gaojinhao <gaojinhao@huawei.com>
Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; Michael S . Tsirkin <mst@redhat.com>; David Gibson <david@gibson.dropbear.id.au>; Greg Kurz <groug@kaod.org>; Marc-André Lureau <marcandre.lureau@redhat.com>; Stefan Berger <stefanb@linux.vnet.ibm.com>; Jason Wang <jasowang@redhat.com>; Juan Quintela <quintela@redhat.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; zhukeqian <zhukeqian1@huawei.com>
Subject: Re: [PATCH v3 3/3] vmstate: Fix memory leak in vmstate_handle_alloc()
* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Jinhao Gao (gaojinhao@huawei.com) wrote:
> > Some memory allocated for fields having a flag of VMS_ALLOC in
> > SaveState may not free before VM load vmsd in migration. So we
> > pre-free memory before allocation in vmstate_handle_alloc() to avoid memleaks.
> >
> > Reported-by: Euler Robot <euler.robot@huawei.com>
> > Signed-off-by: Jinhao Gao <gaojinhao@huawei.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> Yes, I think that's OK; it's actually pretty rare for this to happen;
> normally inwards migrations either succeed or fail and exit; doing
> multiple loads from snapshots is valid and I guess COLO hits this as well.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
I'm having to unqueue this because it's triggering a seg fault on Power in iotest 267 (just run make check).
#2 0x0000000116d0d4c8 in vmstate_handle_alloc (opaque=<optimized out>, field=0x11799e0c8 <__compound_literal.1+312>, ptr=0x1001f8f14b0) at ../qemu/migration/vmstate.c:73
#3 0x0000000116d0d4c8 in vmstate_load_state (f=0x1001f6d0000, vmsd=0x117928730 <vmstate_spapr_tce_table>, opaque=0x1001f8f1400, version_id=<optimized out>) at ../qemu/migration/vmstate.c:122
#4 0x0000000116fb4a4c in vmstate_load (f=0x1001f6d0000, se=0x1001fc7bc40) at ../qemu/migration/savevm.c:910
#5 0x0000000116fb5010 in qemu_loadvm_section_start_full (f=f@entry=0x1001f6d0000, mis=<optimized out>) at ../qemu/migration/savevm.c:2433
It's the mig_nb_table that Power is doing some special handling with; so it needs some more checking before we can fix this.
Dave
> > ---
> > migration/vmstate.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/migration/vmstate.c b/migration/vmstate.c index
> > e9d2aef66b..873f76739f 100644
> > --- a/migration/vmstate.c
> > +++ b/migration/vmstate.c
> > @@ -70,6 +70,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field,
> > gsize size = vmstate_size(opaque, field);
> > size *= vmstate_n_elems(opaque, field);
> > if (size) {
> > + g_free(*(void **)ptr);
> > *(void **)ptr = g_malloc(size);
> > }
> > }
> > --
> > 2.23.0
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
next prev parent reply other threads:[~2021-02-08 18:19 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-31 6:10 [PATCH v3 0/3] Fix memory leak of some device state in migration Jinhao Gao
2020-12-31 6:10 ` [PATCH v3 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci Jinhao Gao
2020-12-31 6:10 ` [PATCH v3 2/3] savevm: Fix memory leak of vmstate_configuration Jinhao Gao
2020-12-31 6:10 ` [PATCH v3 3/3] vmstate: Fix memory leak in vmstate_handle_alloc() Jinhao Gao
2021-01-05 11:18 ` Dr. David Alan Gilbert
2021-01-06 5:46 ` gaojinhao
2021-02-08 10:52 ` Dr. David Alan Gilbert
2021-02-08 11:46 ` gaojinhao [this message]
2021-02-04 14:26 ` [PATCH v3 0/3] Fix memory leak of some device state in migration Dr. David Alan Gilbert
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=7562293f0ee140b9b2a20b170c657207@huawei.com \
--to=gaojinhao@huawei.com \
--cc=david@gibson.dropbear.id.au \
--cc=dgilbert@redhat.com \
--cc=groug@kaod.org \
--cc=jasowang@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=quintela@redhat.com \
--cc=stefanb@linux.vnet.ibm.com \
--cc=wanghaibin.wang@huawei.com \
--cc=zhukeqian1@huawei.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;
as well as URLs for NNTP newsgroup(s).