* [pve-devel] [PATCH qemu 1/2] refresh patch context @ 2023-08-14 9:21 Fiona Ebner 2023-08-14 9:21 ` [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing Fiona Ebner 2023-08-16 9:59 ` [pve-devel] applied-series: [PATCH qemu 1/2] refresh patch context Fiona Ebner 0 siblings, 2 replies; 4+ messages in thread From: Fiona Ebner @ 2023-08-14 9:21 UTC (permalink / raw) To: pve-devel Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> --- ...-PVE-add-savevm-async-for-background-state-snapshots.patch | 2 +- .../pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch | 2 +- .../pve/0027-PVE-Backup-add-vma-backup-format-code.patch | 4 ++-- .../pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch index 762ba9b..0c98142 100644 --- a/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch +++ b/debian/patches/pve/0017-PVE-add-savevm-async-for-background-state-snapshots.patch @@ -139,7 +139,7 @@ index 8a142fc7a9..a7824b5266 100644 'threadinfo.c', diff --git a/migration/savevm-async.c b/migration/savevm-async.c new file mode 100644 -index 0000000000..ac1fac6378 +index 0000000000..aa2017d496 --- /dev/null +++ b/migration/savevm-async.c @@ -0,0 +1,533 @@ diff --git a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch index 98cd3ce..f5f6f7b 100644 --- a/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch +++ b/debian/patches/pve/0018-PVE-add-optional-buffer-size-to-QEMUFile.patch @@ -192,7 +192,7 @@ index 9d0155a2a1..cc06240e8d 100644 int qemu_fclose(QEMUFile *f); diff --git a/migration/savevm-async.c b/migration/savevm-async.c -index ac1fac6378..ea3b2f36a6 100644 +index aa2017d496..b97f2c4f14 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -380,7 +380,7 @@ void qmp_savevm_start(const char *statefile, Error **errp) diff --git a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch index 3c3ac54..5ee84e1 100644 --- a/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch +++ b/debian/patches/pve/0027-PVE-Backup-add-vma-backup-format-code.patch @@ -1735,7 +1735,7 @@ index 0000000000..ac7da237d0 +} diff --git a/vma.c b/vma.c new file mode 100644 -index 0000000000..304f02bc84 +index 0000000000..c76ecefa0f --- /dev/null +++ b/vma.c @@ -0,0 +1,878 @@ @@ -2619,7 +2619,7 @@ index 0000000000..304f02bc84 +} diff --git a/vma.h b/vma.h new file mode 100644 -index 0000000000..1b62859165 +index 0000000000..86d2873aa5 --- /dev/null +++ b/vma.h @@ -0,0 +1,150 @@ diff --git a/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch b/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch index 4a1b1b8..f5fa200 100644 --- a/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch +++ b/debian/patches/pve/0045-savevm-async-don-t-hold-BQL-during-setup.patch @@ -13,7 +13,7 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> 1 file changed, 2 deletions(-) diff --git a/migration/savevm-async.c b/migration/savevm-async.c -index ea3b2f36a6..dd7744ab66 100644 +index b97f2c4f14..87ea0573d3 100644 --- a/migration/savevm-async.c +++ b/migration/savevm-async.c @@ -403,10 +403,8 @@ void qmp_savevm_start(const char *statefile, Error **errp) -- 2.39.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing 2023-08-14 9:21 [pve-devel] [PATCH qemu 1/2] refresh patch context Fiona Ebner @ 2023-08-14 9:21 ` Fiona Ebner 2023-08-14 12:55 ` Wolfgang Bumiller 2023-08-16 9:59 ` [pve-devel] applied-series: [PATCH qemu 1/2] refresh patch context Fiona Ebner 1 sibling, 1 reply; 4+ messages in thread From: Fiona Ebner @ 2023-08-14 9:21 UTC (permalink / raw) To: pve-devel Reported in the community forum [0]. By default, there can be large amounts of memory left assigned to the QEMU process after backup. Likely because of fragmentation, it's necessary to explicitly call malloc_trim() to tell glibc that it shouldn't keep all that memory resident for the process. QEMU itself already does a malloc_trim() in the RCU thread, but that code path might not be reached (or not for a long time) under usual operation. The value of 4 MiB for the argument was also copied from there. Example with the following configuration: > agent: 1 > boot: order=scsi0 > cores: 4 > cpu: x86-64-v2-AES > ide2: none,media=cdrom > memory: 1024 > name: backup-mem > net0: virtio=DA:58:18:26:59:9F,bridge=vmbr0,firewall=1 > numa: 0 > ostype: l26 > scsi0: rbd:base-107-disk-0/vm-106-disk-1,size=4302M > scsihw: virtio-scsi-pci > smbios1: uuid=b2d4511e-8d01-44f1-afd6-9581b30c24a6 > sockets: 2 > startup: order=2 > virtio0: lvmthin:vm-106-disk-1,iothread=1,size=1G > virtio1: lvmthin:vm-106-disk-2,iothread=1,size=1G > virtio2: lvmthin:vm-106-disk-3,iothread=1,size=1G > vmgenid: 0a1d8751-5e02-449d-977e-c0160e900231 Before the change: > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > VmRSS: 370948 kB > root@pve8a1 ~ # vzdump 106 --storage pbs > (...) > INFO: Backup job finished successfully > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > VmRSS: 2114964 kB After the change: > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > VmRSS: 398788 kB > root@pve8a1 ~ # vzdump 106 --storage pbs > (...) > INFO: Backup job finished successfully > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > VmRSS: 424356 kB [0]: https://forum.proxmox.com/threads/131339/ Co-diagnosed-by: Friedrich Weber <f.weber@proxmox.com> Co-diagnosed-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> --- ...ckup-Proxmox-backup-patches-for-QEMU.patch | 23 +++++++++++++++---- ...igrate-dirty-bitmap-state-via-savevm.patch | 4 ++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch index 3753eff..d873601 100644 --- a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch +++ b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch @@ -79,7 +79,8 @@ Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> adapt for new job lock mechanism replacing AioContext locks adapt to QAPI changes improve canceling - allow passing max-workers setting] + allow passing max-workers setting + use malloc_trim after backup] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> --- block/meson.build | 5 + @@ -92,11 +93,11 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> monitor/hmp-cmds.c | 72 +++ proxmox-backup-client.c | 146 +++++ proxmox-backup-client.h | 60 ++ - pve-backup.c | 1097 ++++++++++++++++++++++++++++++++ + pve-backup.c | 1109 ++++++++++++++++++++++++++++++++ qapi/block-core.json | 226 +++++++ qapi/common.json | 13 + qapi/machine.json | 15 +- - 14 files changed, 1711 insertions(+), 13 deletions(-) + 14 files changed, 1723 insertions(+), 13 deletions(-) create mode 100644 proxmox-backup-client.c create mode 100644 proxmox-backup-client.h create mode 100644 pve-backup.c @@ -587,10 +588,10 @@ index 0000000000..8cbf645b2c +#endif /* PROXMOX_BACKUP_CLIENT_H */ diff --git a/pve-backup.c b/pve-backup.c new file mode 100644 -index 0000000000..dd72ee0ed6 +index 0000000000..10ca8a0b1d --- /dev/null +++ b/pve-backup.c -@@ -0,0 +1,1097 @@ +@@ -0,0 +1,1109 @@ +#include "proxmox-backup-client.h" +#include "vma.h" + @@ -605,6 +606,10 @@ index 0000000000..dd72ee0ed6 +#include "qapi/qmp/qerror.h" +#include "qemu/cutils.h" + ++#if defined(CONFIG_MALLOC_TRIM) ++#include <malloc.h> ++#endif ++ +#include <proxmox-backup-qemu.h> + +/* PVE backup state and related function */ @@ -869,6 +874,14 @@ index 0000000000..dd72ee0ed6 + backup_state.stat.end_time = time(NULL); + backup_state.stat.finishing = false; + qemu_mutex_unlock(&backup_state.stat.lock); ++ ++#if defined(CONFIG_MALLOC_TRIM) ++ /* ++ * Try to reclaim memory for buffers (and, in case of PBS, Rust futures), etc. ++ * Won't happen by default if there is fragmentation. ++ */ ++ malloc_trim(4 * 1024 * 1024); ++#endif +} + +static void coroutine_fn pvebackup_co_complete_stream(void *opaque) diff --git a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch index 9437869..7a906e9 100644 --- a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch +++ b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch @@ -175,10 +175,10 @@ index 0000000000..887e998b9e + NULL); +} diff --git a/pve-backup.c b/pve-backup.c -index dd72ee0ed6..cb5312fff3 100644 +index 10ca8a0b1d..0a5ce2cab8 100644 --- a/pve-backup.c +++ b/pve-backup.c -@@ -1090,6 +1090,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) +@@ -1102,6 +1102,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) ret->pbs_library_version = g_strdup(proxmox_backup_qemu_version()); ret->pbs_dirty_bitmap = true; ret->pbs_dirty_bitmap_savevm = true; -- 2.39.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing 2023-08-14 9:21 ` [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing Fiona Ebner @ 2023-08-14 12:55 ` Wolfgang Bumiller 0 siblings, 0 replies; 4+ messages in thread From: Wolfgang Bumiller @ 2023-08-14 12:55 UTC (permalink / raw) To: Fiona Ebner; +Cc: pve-devel On Mon, Aug 14, 2023 at 11:21:33AM +0200, Fiona Ebner wrote: > Reported in the community forum [0]. By default, there can be large > amounts of memory left assigned to the QEMU process after backup. > Likely because of fragmentation, it's necessary to explicitly call > malloc_trim() to tell glibc that it shouldn't keep all that memory > resident for the process. > > QEMU itself already does a malloc_trim() in the RCU thread, but that > code path might not be reached (or not for a long time) under usual > operation. The value of 4 MiB for the argument was also copied from > there. > > Example with the following configuration: > > agent: 1 > > boot: order=scsi0 > > cores: 4 > > cpu: x86-64-v2-AES > > ide2: none,media=cdrom > > memory: 1024 > > name: backup-mem > > net0: virtio=DA:58:18:26:59:9F,bridge=vmbr0,firewall=1 > > numa: 0 > > ostype: l26 > > scsi0: rbd:base-107-disk-0/vm-106-disk-1,size=4302M > > scsihw: virtio-scsi-pci > > smbios1: uuid=b2d4511e-8d01-44f1-afd6-9581b30c24a6 > > sockets: 2 > > startup: order=2 > > virtio0: lvmthin:vm-106-disk-1,iothread=1,size=1G > > virtio1: lvmthin:vm-106-disk-2,iothread=1,size=1G > > virtio2: lvmthin:vm-106-disk-3,iothread=1,size=1G > > vmgenid: 0a1d8751-5e02-449d-977e-c0160e900231 > > Before the change: > > > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > > VmRSS: 370948 kB > > root@pve8a1 ~ # vzdump 106 --storage pbs > > (...) > > INFO: Backup job finished successfully > > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > > VmRSS: 2114964 kB > > After the change: > > > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > > VmRSS: 398788 kB > > root@pve8a1 ~ # vzdump 106 --storage pbs > > (...) > > INFO: Backup job finished successfully > > root@pve8a1 ~ # grep VmRSS /proc/$(cat /var/run/qemu-server/106.pid)/status > > VmRSS: 424356 kB > > [0]: https://forum.proxmox.com/threads/131339/ > > Co-diagnosed-by: Friedrich Weber <f.weber@proxmox.com> > Co-diagnosed-by: Dominik Csapak <d.csapak@proxmox.com> > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Both patches Acked-by: Wolfgang Bumiller <w.bumiller@proxmox.com> > --- > ...ckup-Proxmox-backup-patches-for-QEMU.patch | 23 +++++++++++++++---- > ...igrate-dirty-bitmap-state-via-savevm.patch | 4 ++-- > 2 files changed, 20 insertions(+), 7 deletions(-) > > diff --git a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch > index 3753eff..d873601 100644 > --- a/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch > +++ b/debian/patches/pve/0030-PVE-Backup-Proxmox-backup-patches-for-QEMU.patch > @@ -79,7 +79,8 @@ Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> > adapt for new job lock mechanism replacing AioContext locks > adapt to QAPI changes > improve canceling > - allow passing max-workers setting] > + allow passing max-workers setting > + use malloc_trim after backup] > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> > --- > block/meson.build | 5 + > @@ -92,11 +93,11 @@ Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> > monitor/hmp-cmds.c | 72 +++ > proxmox-backup-client.c | 146 +++++ > proxmox-backup-client.h | 60 ++ > - pve-backup.c | 1097 ++++++++++++++++++++++++++++++++ > + pve-backup.c | 1109 ++++++++++++++++++++++++++++++++ > qapi/block-core.json | 226 +++++++ > qapi/common.json | 13 + > qapi/machine.json | 15 +- > - 14 files changed, 1711 insertions(+), 13 deletions(-) > + 14 files changed, 1723 insertions(+), 13 deletions(-) > create mode 100644 proxmox-backup-client.c > create mode 100644 proxmox-backup-client.h > create mode 100644 pve-backup.c > @@ -587,10 +588,10 @@ index 0000000000..8cbf645b2c > +#endif /* PROXMOX_BACKUP_CLIENT_H */ > diff --git a/pve-backup.c b/pve-backup.c > new file mode 100644 > -index 0000000000..dd72ee0ed6 > +index 0000000000..10ca8a0b1d > --- /dev/null > +++ b/pve-backup.c > -@@ -0,0 +1,1097 @@ > +@@ -0,0 +1,1109 @@ > +#include "proxmox-backup-client.h" > +#include "vma.h" > + > @@ -605,6 +606,10 @@ index 0000000000..dd72ee0ed6 > +#include "qapi/qmp/qerror.h" > +#include "qemu/cutils.h" > + > ++#if defined(CONFIG_MALLOC_TRIM) > ++#include <malloc.h> > ++#endif > ++ > +#include <proxmox-backup-qemu.h> > + > +/* PVE backup state and related function */ > @@ -869,6 +874,14 @@ index 0000000000..dd72ee0ed6 > + backup_state.stat.end_time = time(NULL); > + backup_state.stat.finishing = false; > + qemu_mutex_unlock(&backup_state.stat.lock); > ++ > ++#if defined(CONFIG_MALLOC_TRIM) > ++ /* > ++ * Try to reclaim memory for buffers (and, in case of PBS, Rust futures), etc. > ++ * Won't happen by default if there is fragmentation. > ++ */ > ++ malloc_trim(4 * 1024 * 1024); > ++#endif > +} > + > +static void coroutine_fn pvebackup_co_complete_stream(void *opaque) > diff --git a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch > index 9437869..7a906e9 100644 > --- a/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch > +++ b/debian/patches/pve/0034-PVE-Migrate-dirty-bitmap-state-via-savevm.patch > @@ -175,10 +175,10 @@ index 0000000000..887e998b9e > + NULL); > +} > diff --git a/pve-backup.c b/pve-backup.c > -index dd72ee0ed6..cb5312fff3 100644 > +index 10ca8a0b1d..0a5ce2cab8 100644 > --- a/pve-backup.c > +++ b/pve-backup.c > -@@ -1090,6 +1090,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) > +@@ -1102,6 +1102,7 @@ ProxmoxSupportStatus *qmp_query_proxmox_support(Error **errp) > ret->pbs_library_version = g_strdup(proxmox_backup_qemu_version()); > ret->pbs_dirty_bitmap = true; > ret->pbs_dirty_bitmap_savevm = true; > -- > 2.39.2 > > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [pve-devel] applied-series: [PATCH qemu 1/2] refresh patch context 2023-08-14 9:21 [pve-devel] [PATCH qemu 1/2] refresh patch context Fiona Ebner 2023-08-14 9:21 ` [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing Fiona Ebner @ 2023-08-16 9:59 ` Fiona Ebner 1 sibling, 0 replies; 4+ messages in thread From: Fiona Ebner @ 2023-08-16 9:59 UTC (permalink / raw) To: pve-devel applied both patches with Wolfgang's Acked-by ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-08-16 10:00 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-14 9:21 [pve-devel] [PATCH qemu 1/2] refresh patch context Fiona Ebner 2023-08-14 9:21 ` [pve-devel] [PATCH qemu 2/2] backup: trim heap after finishing Fiona Ebner 2023-08-14 12:55 ` Wolfgang Bumiller 2023-08-16 9:59 ` [pve-devel] applied-series: [PATCH qemu 1/2] refresh patch context Fiona Ebner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox