From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Maciej S . Szmigiero" <mail@maciej.szmigiero.name>,
peterx@redhat.com, "Cédric Le Goater" <clg@redhat.com>,
"Avihai Horon" <avihaih@nvidia.com>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Fabiano Rosas" <farosas@suse.de>,
"Prasad Pandit" <ppandit@redhat.com>
Subject: [PATCH v2 7/7] migration/multifd: Document the reason to sync for save_setup()
Date: Thu, 5 Dec 2024 19:58:34 -0500 [thread overview]
Message-ID: <20241206005834.1050905-8-peterx@redhat.com> (raw)
In-Reply-To: <20241206005834.1050905-1-peterx@redhat.com>
It's not straightforward to see why src QEMU needs to sync multifd during
setup() phase. After all, there's no page queued at that point.
For old QEMUs, there's a solid reason: EOS requires it to work. While it's
clueless on the new QEMUs which do not take EOS message as sync requests.
One will figure that out only when this is conditionally removed. In fact,
the author did try it out. Logically we could still avoid doing this on
new machine types, however that needs a separate compat field and that can
be an overkill in some trivial overhead in setup() phase.
Let's instead document it completely, to avoid someone else tries this
again and do the debug one more time, or anyone confused on why this ever
existed.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 5d4bdefe69..ddee703585 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3036,6 +3036,33 @@ static int ram_save_setup(QEMUFile *f, void *opaque, Error **errp)
migration_ops->ram_save_target_page = ram_save_target_page_legacy;
}
+ /*
+ * This operation is unfortunate..
+ *
+ * For legacy QEMUs using per-section sync
+ * =======================================
+ *
+ * This must exist because the EOS below requires the SYNC messages
+ * per-channel to work.
+ *
+ * For modern QEMUs using per-round sync
+ * =====================================
+ *
+ * Logically this sync is not needed (because we know there's nothing
+ * in the multifd queue yet!). However as a side effect, this makes
+ * sure the dest side won't receive any data before it properly reaches
+ * ram_load_precopy().
+ *
+ * Without this sync, src QEMU can send data too soon so that dest may
+ * not have been ready to receive it (e.g., rb->receivedmap may be
+ * uninitialized, for example).
+ *
+ * Logically "wait for recv setup ready" shouldn't need to involve src
+ * QEMU at all, however to be compatible with old QEMUs, let's stick
+ * with this. Fortunately the overhead is low to sync during setup
+ * because the VM is running, so at least it's not accounted as part of
+ * downtime.
+ */
bql_unlock();
ret = multifd_ram_flush_and_sync(f);
bql_lock();
--
2.47.0
next prev parent reply other threads:[~2024-12-06 1:00 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-06 0:58 [PATCH v2 0/7] migration/multifd: Some VFIO / postcopy preparations on flush Peter Xu
2024-12-06 0:58 ` [PATCH v2 1/7] migration/multifd: Further remove the SYNC on complete Peter Xu
2024-12-06 13:17 ` Fabiano Rosas
2024-12-06 14:40 ` Peter Xu
2024-12-06 0:58 ` [PATCH v2 2/7] migration/multifd: Allow to sync with sender threads only Peter Xu
2024-12-06 13:26 ` Fabiano Rosas
2024-12-06 14:50 ` Peter Xu
2024-12-06 15:00 ` Fabiano Rosas
2024-12-06 0:58 ` [PATCH v2 3/7] migration/ram: Move RAM_SAVE_FLAG* into ram.h Peter Xu
2024-12-06 13:43 ` Fabiano Rosas
2024-12-06 15:03 ` Peter Xu
2024-12-06 15:10 ` Fabiano Rosas
2024-12-06 15:46 ` Peter Xu
2024-12-06 16:58 ` Fabiano Rosas
2024-12-06 0:58 ` [PATCH v2 4/7] migration/multifd: Unify RAM_SAVE_FLAG_MULTIFD_FLUSH messages Peter Xu
2024-12-06 14:12 ` Fabiano Rosas
2024-12-06 0:58 ` [PATCH v2 5/7] migration/multifd: Remove sync processing on postcopy Peter Xu
2024-12-06 14:19 ` Fabiano Rosas
2024-12-06 0:58 ` [PATCH v2 6/7] migration/multifd: Cleanup src flushes on condition check Peter Xu
2024-12-06 14:18 ` Fabiano Rosas
2024-12-06 15:13 ` Peter Xu
2024-12-06 0:58 ` Peter Xu [this message]
2024-12-06 14:40 ` [PATCH v2 7/7] migration/multifd: Document the reason to sync for save_setup() Fabiano Rosas
2024-12-06 15:36 ` Peter Xu
2024-12-06 17:01 ` Fabiano Rosas
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=20241206005834.1050905-8-peterx@redhat.com \
--to=peterx@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=avihaih@nvidia.com \
--cc=clg@redhat.com \
--cc=farosas@suse.de \
--cc=mail@maciej.szmigiero.name \
--cc=ppandit@redhat.com \
--cc=qemu-devel@nongnu.org \
/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).