all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH v3 qemu 6/7] add patch to avoid huge snapshot performance regression
Date: Tue, 17 Oct 2023 14:10:11 +0200	[thread overview]
Message-ID: <20231017121012.132636-7-f.ebner@proxmox.com> (raw)
In-Reply-To: <20231017121012.132636-1-f.ebner@proxmox.com>

Taking a snapshot became prohibitively slow because of the
migration_transferred_bytes() call in migration_rate_exceeded() [0].

This also applied to the async snapshot taking in Proxmox VE, so
work around the issue until it is fixed upstream.

[0]: https://gitlab.com/qemu-project/qemu/-/issues/1821

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v3.

 ...-workaround-snapshot-performance-reg.patch | 57 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 debian/patches/extra/0007-migration-states-workaround-snapshot-performance-reg.patch

diff --git a/debian/patches/extra/0007-migration-states-workaround-snapshot-performance-reg.patch b/debian/patches/extra/0007-migration-states-workaround-snapshot-performance-reg.patch
new file mode 100644
index 0000000..8031837
--- /dev/null
+++ b/debian/patches/extra/0007-migration-states-workaround-snapshot-performance-reg.patch
@@ -0,0 +1,57 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Fiona Ebner <f.ebner@proxmox.com>
+Date: Thu, 28 Sep 2023 11:19:14 +0200
+Subject: [PATCH] migration states: workaround snapshot performance regression
+
+Commit 813cd616 ("migration: Use migration_transferred_bytes() to
+calculate rate_limit") introduced a prohibitive performance regression
+when taking a snapshot [0]. The reason turns out to be the flushing
+done by migration_transferred_bytes()
+
+Just use a _noflush version of the relevant function as a workaround
+until upstream fixes the issue. This is inspired by a not-applied
+upstream series [1], but doing the very minimum to avoid the
+regression.
+
+[0]: https://gitlab.com/qemu-project/qemu/-/issues/1821
+[1]: https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg07708.html
+
+Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
+---
+ migration/migration-stats.c | 16 +++++++++++++++-
+ 1 file changed, 15 insertions(+), 1 deletion(-)
+
+diff --git a/migration/migration-stats.c b/migration/migration-stats.c
+index 095d6d75bb..8073c8ebaa 100644
+--- a/migration/migration-stats.c
++++ b/migration/migration-stats.c
+@@ -18,6 +18,20 @@
+ 
+ MigrationAtomicStats mig_stats;
+ 
++/*
++ * Same as migration_transferred_bytes below, but using the _noflush
++ * variant of qemu_file_transferred() to avoid a performance
++ * regression in migration_rate_exceeded().
++ */
++static uint64_t migration_transferred_bytes_noflush(QEMUFile *f)
++{
++    uint64_t multifd = stat64_get(&mig_stats.multifd_bytes);
++    uint64_t qemu_file = qemu_file_transferred_noflush(f);
++
++    trace_migration_transferred_bytes(qemu_file, multifd);
++    return qemu_file + multifd;
++}
++
+ bool migration_rate_exceeded(QEMUFile *f)
+ {
+     if (qemu_file_get_error(f)) {
+@@ -25,7 +39,7 @@ bool migration_rate_exceeded(QEMUFile *f)
+     }
+ 
+     uint64_t rate_limit_start = stat64_get(&mig_stats.rate_limit_start);
+-    uint64_t rate_limit_current = migration_transferred_bytes(f);
++    uint64_t rate_limit_current = migration_transferred_bytes_noflush(f);
+     uint64_t rate_limit_used = rate_limit_current - rate_limit_start;
+     uint64_t rate_limit_max = stat64_get(&mig_stats.rate_limit_max);
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 6d681da..c27c245 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,6 +4,7 @@ extra/0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch
 extra/0004-migration-block-dirty-bitmap-fix-loading-bitmap-when.patch
 extra/0005-hw-ide-reset-cancel-async-DMA-operation-before-reset.patch
 extra/0006-Revert-Revert-graph-lock-Disable-locking-for-now.patch
+extra/0007-migration-states-workaround-snapshot-performance-reg.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.39.2





  parent reply	other threads:[~2023-10-17 12:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-17 12:10 [pve-devel] [PATCH-SERIES v3 qemu] update to QEMU 8.1.2 Fiona Ebner
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 1/7] d/rules: use disable-download option instead of git-submodules=ignore Fiona Ebner
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 2/7] buildsys: fixup submodule target Fiona Ebner
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 3/7] buildsys: use QEMU's keycodemapdb again Fiona Ebner
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 4/7] update submodule and patches to QEMU 8.1.2 Fiona Ebner
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 5/7] add patch to disable graph locking Fiona Ebner
2023-10-17 12:10 ` Fiona Ebner [this message]
2023-10-17 12:10 ` [pve-devel] [PATCH v3 qemu 7/7] d/control: add versioned Breaks for qemu-server <= 8.0.6 Fiona Ebner
2023-10-25 11:21 ` [pve-devel] applied-series: [PATCH-SERIES v3 qemu] update to QEMU 8.1.2 Thomas Lamprecht

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=20231017121012.132636-7-f.ebner@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.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 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.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal