| Message ID | 20201226103347.868-1-gaojinhao@huawei.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix memory leak of some device state in migration | expand |
Patchew URL: https://patchew.org/QEMU/20201226103347.868-1-gaojinhao@huawei.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20201226103347.868-1-gaojinhao@huawei.com
Subject: [PATCH 0/8] Fix memory leak of some device state in migration
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20201226103347.868-1-gaojinhao@huawei.com -> patchew/20201226103347.868-1-gaojinhao@huawei.com
Switched to a new branch 'test'
0a34e05 dbus-vmstate: Fix memory leak of dbus_vmstate
1e7ce1c tpm_emulator: Fix memory leak of vmstate_tpm_emulator
e5e20f9 vmbus: Fix memory leak of vmstate_vmbus_chan_req
0cfbfe6 savevm: Fix memory leak of vmstate_configuration
0d767d2 spapr_pci: Fix memory leak of vmstate_spapr_pci
729d773 spapr: Fix memory leak of vmstate_spapr_event_entry
bd9decd virtio-net: Fix memory leak of vmstate_virtio_net_rss
a7debfa vmbus: Fix memory leak of vmstate_gpadl
=== OUTPUT BEGIN ===
1/8 Checking commit a7debfa33be6 (vmbus: Fix memory leak of vmstate_gpadl)
ERROR: spaces required around that '=' (ctx:WxV)
#31: FILE: hw/hyperv/vmbus.c:528:
+ gpadl->num_gfns =0;
^
total: 1 errors, 0 warnings, 21 lines checked
Patch 1/8 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/8 Checking commit bd9decdc8d81 (virtio-net: Fix memory leak of vmstate_virtio_net_rss)
3/8 Checking commit 729d7739521c (spapr: Fix memory leak of vmstate_spapr_event_entry)
4/8 Checking commit 0d767d2f6f68 (spapr_pci: Fix memory leak of vmstate_spapr_pci)
5/8 Checking commit 0cfbfe6f37d9 (savevm: Fix memory leak of vmstate_configuration)
6/8 Checking commit e5e20f91e7ae (vmbus: Fix memory leak of vmstate_vmbus_chan_req)
7/8 Checking commit 1e7ce1c84602 (tpm_emulator: Fix memory leak of vmstate_tpm_emulator)
8/8 Checking commit 0a34e0598fb9 (dbus-vmstate: Fix memory leak of dbus_vmstate)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20201226103347.868-1-gaojinhao@huawei.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On Sat, Dec 26, 2020 at 06:33:39PM +0800, g00517791 wrote: > From: Jinhao Gao <gaojinhao@huawei.com> > > For some device state having some fields of VMS_ALLOC flag, they don't > free memory allocated for the fields in vmstate_save_state and vmstate > _load_state. We add funcs or sentences of free memory before allocation > of memory or after load of memory to avoid memory leak. > Isn't there a way to handle it centrally? IIUC the issue is repeated loads in case a load fails, right? So can't we do something along the lines of: Signed-off-by: Michael S. Tsirkin <mst@redhat.com> 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); } }
Thank you for you review. I will modify patches according to your opinion. Jinhao Gao -----Original Message----- From: Michael S. Tsirkin [mailto:mst@redhat.com] Sent: 2020年12月27日 21:20 To: gaojinhao <gaojinhao@huawei.com> Cc: qemu-devel@nongnu.org; qemu-ppc@nongnu.org; Marc-André Lureau <marcandre.lureau@redhat.com>; Stefan Berger <stefanb@linux.vnet.ibm.com>; Jason Wang <jasowang@redhat.com>; David Gibson <david@gibson.dropbear.id.au>; Greg Kurz <groug@kaod.org>; Juan Quintela <quintela@redhat.com>; Dr . David Alan Gilbert <dgilbert@redhat.com>; Wanghaibin (D) <wanghaibin.wang@huawei.com>; zhukeqian <zhukeqian1@huawei.com> Subject: Re: [PATCH 0/8] Fix memory leak of some device state in migration On Sat, Dec 26, 2020 at 06:33:39PM +0800, g00517791 wrote: > From: Jinhao Gao <gaojinhao@huawei.com> > > For some device state having some fields of VMS_ALLOC flag, they don't > free memory allocated for the fields in vmstate_save_state and vmstate > _load_state. We add funcs or sentences of free memory before > allocation of memory or after load of memory to avoid memory leak. > Isn't there a way to handle it centrally? IIUC the issue is repeated loads in case a load fails, right? So can't we do something along the lines of: Signed-off-by: Michael S. Tsirkin <mst@redhat.com> 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); } } -- MST
From: Jinhao Gao <gaojinhao@huawei.com> For some device state having some fields of VMS_ALLOC flag, they don't free memory allocated for the fields in vmstate_save_state and vmstate _load_state. We add funcs or sentences of free memory before allocation of memory or after load of memory to avoid memory leak. Jinhao Gao (8): vmbus: Fix memory leak of vmstate_gpadl virtio-net: Fix memory leak of vmstate_virtio_net_rss spapr: Fix memory leak of vmstate_spapr_event_entry spapr_pci: Fix memory leak of vmstate_spapr_pci savevm: Fix memory leak of vmstate_configuration vmbus: Fix memory leak of vmstate_vmbus_chan_req tpm_emulator: Fix memory leak of vmstate_tpm_emulator dbus-vmstate: Fix memory leak of dbus_vmstate backends/dbus-vmstate.c | 11 +++++++++++ backends/tpm/tpm_emulator.c | 13 +++++++++++++ hw/hyperv/vmbus.c | 22 ++++++++++++++++++++++ hw/net/virtio-net.c | 11 +++++++++++ hw/ppc/spapr.c | 12 ++++++++++++ hw/ppc/spapr_pci.c | 11 +++++++++++ migration/savevm.c | 31 +++++++++++++++++++++++++++---- 7 files changed, 107 insertions(+), 4 deletions(-)