Re: [PATCH RFC 23/26] migration/multifd: Device state transfer support - send side

2024-05-06 Thread Maciej S. Szmigiero 

On 29.04.2024 22:04, Peter Xu wrote:

On Tue, Apr 16, 2024 at 04:43:02PM +0200, Maciej S. Szmigiero wrote:

+bool multifd_queue_page(RAMBlock *block, ram_addr_t offset)
+{
+g_autoptr(GMutexLocker) locker = NULL;
+
+/*
+ * Device state submissions for shared channels can come
+ * from multiple threads and conflict with page submissions
+ * with respect to multifd_send_state access.
+ */
+if (!multifd_send_state->device_state_dedicated_channels) {
+locker = g_mutex_locker_new(_send_state->queue_job_mutex);


Haven't read the rest, but suggest to stick with QemuMutex for the whole
patchset, as that's what we use in the rest migration code, along with
QEMU_LOCK_GUARD().



Ack, from a quick scan of QEMU thread sync primitives it seems that
QemuMutex with QemuLockable and QemuCond should fulfill the
requirements to replace GMutex, GMutexLocker and GCond.

Thanks,
Maciej

Re: [PATCH RFC 23/26] migration/multifd: Device state transfer support - send side

2024-04-29 Thread Peter Xu 
On Tue, Apr 16, 2024 at 04:43:02PM +0200, Maciej S. Szmigiero wrote:
> +bool multifd_queue_page(RAMBlock *block, ram_addr_t offset)
> +{
> +g_autoptr(GMutexLocker) locker = NULL;
> +
> +/*
> + * Device state submissions for shared channels can come
> + * from multiple threads and conflict with page submissions
> + * with respect to multifd_send_state access.
> + */
> +if (!multifd_send_state->device_state_dedicated_channels) {
> +locker = g_mutex_locker_new(_send_state->queue_job_mutex);

Haven't read the rest, but suggest to stick with QemuMutex for the whole
patchset, as that's what we use in the rest migration code, along with
QEMU_LOCK_GUARD().

> +}
> +
> +return multifd_queue_page_locked(block, offset);
> +}

-- 
Peter Xu

[PATCH RFC 23/26] migration/multifd: Device state transfer support - send side

2024-04-16 Thread Maciej S. Szmigiero 
From: "Maciej S. Szmigiero" 

A new function multifd_queue_device_state() is provided for device to queue
its state for transmission via a multifd channel.

Signed-off-by: Maciej S. Szmigiero 
---
 include/migration/misc.h |   4 +
 migration/multifd-zlib.c |   2 +-
 migration/multifd-zstd.c |   2 +-
 migration/multifd.c  | 244 ++-
 migration/multifd.h  |  30 +++--
 5 files changed, 244 insertions(+), 38 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index c9e200f4eb8f..25968e31247b 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -117,4 +117,8 @@ bool migration_in_bg_snapshot(void);
 /* migration/block-dirty-bitmap.c */
 void dirty_bitmap_mig_init(void);
 
+/* migration/multifd.c */
+int multifd_queue_device_state(char *idstr, uint32_t instance_id,
+   char *data, size_t len);
+
 #endif
diff --git a/migration/multifd-zlib.c b/migration/multifd-zlib.c
index 99821cd4d5ef..e20c1de6033d 100644
--- a/migration/multifd-zlib.c
+++ b/migration/multifd-zlib.c
@@ -177,7 +177,7 @@ static int zlib_send_prepare(MultiFDSendParams *p, Error 
**errp)
 
 out:
 p->flags |= MULTIFD_FLAG_ZLIB;
-multifd_send_fill_packet(p);
+multifd_send_fill_packet_ram(p);
 return 0;
 }
 
diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 02112255adcc..37cebd006921 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -166,7 +166,7 @@ static int zstd_send_prepare(MultiFDSendParams *p, Error 
**errp)
 
 out:
 p->flags |= MULTIFD_FLAG_ZSTD;
-multifd_send_fill_packet(p);
+multifd_send_fill_packet_ram(p);
 return 0;
 }
 
diff --git a/migration/multifd.c b/migration/multifd.c
index 878ff7d9f9f0..d8ce01539a05 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -12,6 +12,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
+#include "qemu/iov.h"
 #include "qemu/rcu.h"
 #include "exec/target_page.h"
 #include "sysemu/sysemu.h"
@@ -20,6 +21,7 @@
 #include "qapi/error.h"
 #include "channel.h"
 #include "file.h"
+#include "migration/misc.h"
 #include "migration.h"
 #include "migration-stats.h"
 #include "savevm.h"
@@ -50,9 +52,17 @@ typedef struct {
 } __attribute__((packed)) MultiFDInit_t;
 
 struct {
+/*
+ * Are there some device state dedicated channels (true) or
+ * should device state be sent via any available channel (false)?
+ */
+bool device_state_dedicated_channels;
+GMutex queue_job_mutex;
+
 MultiFDSendParams *params;
-/* array of pages to sent */
+/* array of pages or device state to be sent */
 MultiFDPages_t *pages;
+MultiFDDeviceState_t *device_state;
 /*
  * Global number of generated multifd packets.
  *
@@ -169,7 +179,7 @@ static void multifd_send_prepare_iovs(MultiFDSendParams *p)
 }
 
 /**
- * nocomp_send_prepare: prepare date to be able to send
+ * nocomp_send_prepare_ram: prepare RAM data for sending
  *
  * For no compression we just have to calculate the size of the
  * packet.
@@ -179,7 +189,7 @@ static void multifd_send_prepare_iovs(MultiFDSendParams *p)
  * @p: Params for the channel that we are using
  * @errp: pointer to an error
  */
-static int nocomp_send_prepare(MultiFDSendParams *p, Error **errp)
+static int nocomp_send_prepare_ram(MultiFDSendParams *p, Error **errp)
 {
 bool use_zero_copy_send = migrate_zero_copy_send();
 int ret;
@@ -198,13 +208,13 @@ static int nocomp_send_prepare(MultiFDSendParams *p, 
Error **errp)
  * Only !zerocopy needs the header in IOV; zerocopy will
  * send it separately.
  */
-multifd_send_prepare_header(p);
+multifd_send_prepare_header_ram(p);
 }
 
 multifd_send_prepare_iovs(p);
 p->flags |= MULTIFD_FLAG_NOCOMP;
 
-multifd_send_fill_packet(p);
+multifd_send_fill_packet_ram(p);
 
 if (use_zero_copy_send) {
 /* Send header first, without zerocopy */
@@ -218,6 +228,59 @@ static int nocomp_send_prepare(MultiFDSendParams *p, Error 
**errp)
 return 0;
 }
 
+static void multifd_send_fill_packet_device_state(MultiFDSendParams *p)
+{
+MultiFDPacketDeviceState_t *packet = p->packet_device_state;
+
+packet->hdr.flags = cpu_to_be32(p->flags);
+strncpy(packet->idstr, p->device_state->idstr, sizeof(packet->idstr));
+packet->instance_id = cpu_to_be32(p->device_state->instance_id);
+packet->next_packet_size = cpu_to_be32(p->next_packet_size);
+}
+
+/**
+ * nocomp_send_prepare_device_state: prepare device state data for sending
+ *
+ * Returns 0 for success or -1 for error
+ *
+ * @p: Params for the channel that we are using
+ * @errp: pointer to an error
+ */
+static int nocomp_send_prepare_device_state(MultiFDSendParams *p,
+Error **errp)
+{
+assert(!multifd_send_state->device_state_dedicated_channels ||
+   p->is_device_state_dedicated);
+
+