Message ID | 20201226103347.868-1-gaojinhao@huawei.com |
---|---|
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(-)