On Mon, Jul 22, 2024 at 02:59:12PM -0300, Fabiano Rosas wrote:
> While we cannot yet disentangle the multifd packet from page data, we
> can make the code a bit cleaner by setting the page-related fields in
> a separate function.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> migration/multifd.c | 97 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 61 insertions(+), 36 deletions(-)
>
> diff --git a/migration/multifd.c b/migration/multifd.c
> index fcdb12e04f..d25b8658b2 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -409,65 +409,62 @@ static int multifd_recv_initial_packet(QIOChannel *c,
> Error **errp)
> return msg.id;
> }
>
> -void multifd_send_fill_packet(MultiFDSendParams *p)
> +static void multifd_ram_fill_packet(MultiFDSendParams *p)
> {
> MultiFDPacket_t *packet = p->packet;
> MultiFDPages_t *pages = &p->data->u.ram;
> - uint64_t packet_num;
> uint32_t zero_num = pages->num - pages->normal_num;
> - int i;
>
> - packet->flags = cpu_to_be32(p->flags);
> packet->pages_alloc = cpu_to_be32(pages->allocated);
> packet->normal_pages = cpu_to_be32(pages->normal_num);
> packet->zero_pages = cpu_to_be32(zero_num);
> - packet->next_packet_size = cpu_to_be32(p->next_packet_size);
Definitely good intention, but I had a feeling that this will need to be
reorganized again when Maciej reworks on top, due to the fact that
next_packet_size will be ram-private field, simply because it's defined
after pages_alloc and normal_pages...
E.g., see:
https://lore.kernel.org/r/41dedaf2c9abebb5e45f88c052daa26320715a92.1718717584.git.maciej.szmigi...@oracle.com
Where the new MultiFDPacketHdr_t cannot include next_packet_size (even if
VFIO will need that too..).
typedef struct {
uint32_t magic;
uint32_t version;
uint32_t flags;
} __attribute__((packed)) MultiFDPacketHdr_t;
So _maybe_ it's easier we drop this patch and leave that part to Maciej to
identify which is common and which is arm/vfio specific. No strong
opinions here.
> -
> - packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
> - packet->packet_num = cpu_to_be64(packet_num);
>
> if (pages->block) {
> strncpy(packet->ramblock, pages->block->idstr, 256);
> }
>
> - for (i = 0; i < pages->num; i++) {
> + for (int i = 0; i < pages->num; i++) {
> /* there are architectures where ram_addr_t is 32 bit */
> uint64_t temp = pages->offset[i];
>
> packet->offset[i] = cpu_to_be64(temp);
> }
>
> - p->packets_sent++;
> p->total_normal_pages += pages->normal_num;
> p->total_zero_pages += zero_num;
> +}
>
> - trace_multifd_send(p->id, packet_num, pages->normal_num, zero_num,
> +void multifd_send_fill_packet(MultiFDSendParams *p)
> +{
> + MultiFDPacket_t *packet = p->packet;
> + uint64_t packet_num;
> +
> + memset(packet, 0, p->packet_len);
> +
> + packet->magic = cpu_to_be32(MULTIFD_MAGIC);
> + packet->version = cpu_to_be32(MULTIFD_VERSION);
> +
> + packet->flags = cpu_to_be32(p->flags);
> + packet->next_packet_size = cpu_to_be32(p->next_packet_size);
> +
> + packet_num = qatomic_fetch_inc(&multifd_send_state->packet_num);
> + packet->packet_num = cpu_to_be64(packet_num);
> +
> + p->packets_sent++;
> +
> + multifd_ram_fill_packet(p);
> +
> + trace_multifd_send(p->id, packet_num,
> + be32_to_cpu(packet->normal_pages),
> + be32_to_cpu(packet->zero_pages),
> p->flags, p->next_packet_size);
> }
>
> -static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
> +static int multifd_ram_unfill_packet(MultiFDRecvParams *p, Error **errp)
> {
> MultiFDPacket_t *packet = p->packet;
> int i;
>
> - packet->magic = be32_to_cpu(packet->magic);
> - if (packet->magic != MULTIFD_MAGIC) {
> - error_setg(errp, "multifd: received packet "
> - "magic %x and expected magic %x",
> - packet->magic, MULTIFD_MAGIC);
> - return -1;
> - }
> -
> - packet->version = be32_to_cpu(packet->version);
> - if (packet->version != MULTIFD_VERSION) {
> - error_setg(errp, "multifd: received packet "
> - "version %u and expected version %u",
> - packet->version, MULTIFD_VERSION);
> - return -1;
> - }
> -
> - p->flags = be32_to_cpu(packet->flags);
> -
> packet->pages_alloc = be32_to_cpu(packet->pages_alloc);
> /*
> * If we received a packet that is 100 times bigger than expected
> @@ -496,15 +493,9 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
> *p, Error **errp)
> return -1;
> }
>
> - p->next_packet_size = be32_to_cpu(packet->next_packet_size);
> - p->packet_num = be64_to_cpu(packet->packet_num);
> - p->packets_recved++;
> p->total_normal_pages += p->normal_num;
> p->total_zero_pages += p->zero_num;
>
> - trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
> - p->flags, p->next_packet_size);
> -
> if (p->normal_num == 0 && p->zero_num == 0) {
> return 0;
> }
> @@ -546,6 +537,40 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
> *p, Error **errp)
> return 0;
> }
>
> +static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
> +{
> + MultiFDPacket_t *packet = p->packet;
> + int ret = 0;
> +
> + packet->magic = be32_to_cpu(packet->magic);
> + if (packet->magic != MULTIFD_MAGIC) {
> + error_setg(errp, "multifd: received packet "
> + "magic %x and expected magic %x",
> + packet->magic, MULTIFD_MAGIC);
> + return -1;
> + }
> +
> + packet->version = be32_to_cpu(packet->version);
> + if (packet->version != MULTIFD_VERSION) {
> + error_setg(errp, "multifd: received packet "
> + "version %u and expected version %u",
> + packet->version, MULTIFD_VERSION);
> + return -1;
> + }
> +
> + p->flags = be32_to_cpu(packet->flags);
> + p->next_packet_size = be32_to_cpu(packet->next_packet_size);
> + p->packet_num = be64_to_cpu(packet->packet_num);
> + p->packets_recved++;
> +
> + ret = multifd_ram_unfill_packet(p, errp);
> +
> + trace_multifd_recv(p->id, p->packet_num, p->normal_num, p->zero_num,
> + p->flags, p->next_packet_size);
> +
> + return ret;
> +}
> +
> static bool multifd_send_should_exit(void)
> {
> return qatomic_read(&multifd_send_state->exiting);
> --
> 2.35.3
>
--
Peter Xu
