| Message ID | 20211124100617.19786-22-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Migration: Transmit and detect zero pages in the multifd threads | expand |
* Juan Quintela (quintela@redhat.com) wrote: > This patch adds counters and similar. Logic will be added on the > following patch. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > migration/multifd.h | 13 ++++++++++++- > migration/multifd.c | 22 +++++++++++++++++++--- > migration/trace-events | 2 +- > 3 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/migration/multifd.h b/migration/multifd.h > index 39e55d7f05..973315b545 100644 > --- a/migration/multifd.h > +++ b/migration/multifd.h > @@ -49,7 +49,10 @@ typedef struct { > /* size of the next packet that contains pages */ > uint32_t next_packet_size; > uint64_t packet_num; > - uint64_t unused[4]; /* Reserved for future use */ > + /* zero pages */ > + uint32_t zero_pages; Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? > + uint32_t unused32[1]; /* Reserved for future use */ > + uint64_t unused64[3]; /* Reserved for future use */ > char ramblock[256]; > uint64_t offset[]; > } __attribute__((packed)) MultiFDPacket_t; > @@ -117,6 +120,10 @@ typedef struct { > ram_addr_t *normal; > /* num of non zero pages */ > uint32_t normal_num; > + /* Pages that are zero */ > + ram_addr_t *zero; > + /* num of zero pages */ > + uint32_t zero_num; > /* used for compression methods */ > void *data; > } MultiFDSendParams; > @@ -162,6 +169,10 @@ typedef struct { > ram_addr_t *normal; > /* num of non zero pages */ > uint32_t normal_num; > + /* Pages that are zero */ > + ram_addr_t *zero; > + /* num of zero pages */ > + uint32_t zero_num; > /* used for de-compression methods */ > void *data; > } MultiFDRecvParams; > diff --git a/migration/multifd.c b/migration/multifd.c > index d1ab823f98..2e4dffd6c6 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) > packet->normal_pages = cpu_to_be32(p->normal_num); > packet->next_packet_size = cpu_to_be32(p->next_packet_size); > packet->packet_num = cpu_to_be64(p->packet_num); > + packet->zero_pages = cpu_to_be32(p->zero_num); > > if (p->pages->block) { > strncpy(packet->ramblock, p->pages->block->idstr, 256); > @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) > p->next_packet_size = be32_to_cpu(packet->next_packet_size); > p->packet_num = be64_to_cpu(packet->packet_num); > > - if (p->normal_num == 0) { > + p->zero_num = be32_to_cpu(packet->zero_pages); > + if (p->zero_num > packet->pages_alloc - p->normal_num) { > + error_setg(errp, "multifd: received packet " > + "with %d zero pages and expected maximum pages are %d", > + p->normal_num, packet->pages_alloc - p->zero_num) ; should that be p->zero_num, packet->pages_alloc - p->normal_num ? (and be %u) Dave > + return -1; > + } > + > + if (p->normal_num == 0 && p->zero_num == 0) { > return 0; > } > > @@ -550,6 +559,8 @@ void multifd_save_cleanup(void) > p->iov = NULL; > g_free(p->normal); > p->normal = NULL; > + g_free(p->zero); > + p->zero = NULL; > multifd_send_state->ops->send_cleanup(p, &local_err); > if (local_err) { > migrate_set_error(migrate_get_current(), local_err); > @@ -638,6 +649,7 @@ static void *multifd_send_thread(void *opaque) > uint32_t flags = p->flags; > p->iovs_num = 1; > p->normal_num = 0; > + p->zero_num = 0; > > for (int i = 0; i < p->pages->num; i++) { > p->normal[p->normal_num] = p->pages->offset[i]; > @@ -659,8 +671,8 @@ static void *multifd_send_thread(void *opaque) > p->pages->block = NULL; > qemu_mutex_unlock(&p->mutex); > > - trace_multifd_send(p->id, packet_num, p->normal_num, flags, > - p->next_packet_size); > + trace_multifd_send(p->id, packet_num, p->normal_num, p->zero_num, > + flags, p->next_packet_size); > > p->iov[0].iov_len = p->packet_len; > p->iov[0].iov_base = p->packet; > @@ -910,6 +922,7 @@ int multifd_save_setup(Error **errp) > /* We need one extra place for the packet header */ > p->iov = g_new0(struct iovec, page_count + 1); > p->normal = g_new0(ram_addr_t, page_count); > + p->zero = g_new0(ram_addr_t, page_count); > socket_send_channel_create(multifd_new_send_channel_async, p); > } > > @@ -1011,6 +1024,8 @@ int multifd_load_cleanup(Error **errp) > p->iov = NULL; > g_free(p->normal); > p->normal = NULL; > + g_free(p->zero); > + p->zero = NULL; > multifd_recv_state->ops->recv_cleanup(p); > } > qemu_sem_destroy(&multifd_recv_state->sem_sync); > @@ -1150,6 +1165,7 @@ int multifd_load_setup(Error **errp) > p->name = g_strdup_printf("multifdrecv_%d", i); > p->iov = g_new0(struct iovec, page_count); > p->normal = g_new0(ram_addr_t, page_count); > + p->zero = g_new0(ram_addr_t, page_count); > } > > for (i = 0; i < thread_count; i++) { > diff --git a/migration/trace-events b/migration/trace-events > index af8dee9af0..608decbdcc 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -124,7 +124,7 @@ multifd_recv_sync_main_wait(uint8_t id) "channel %d" > multifd_recv_terminate_threads(bool error) "error %d" > multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %d packets %" PRIu64 " pages %" PRIu64 > multifd_recv_thread_start(uint8_t id) "%d" > -multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " normal pages %d flags 0x%x next packet size %d" > +multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t zero, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " normal pages %d zero pages %d flags 0x%x next packet size %d" > multifd_send_error(uint8_t id) "channel %d" > multifd_send_sync_main(long packet_num) "packet num %ld" > multifd_send_sync_main_signal(uint8_t id) "channel %d" > -- > 2.33.1 >
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> This patch adds counters and similar. Logic will be added on the >> following patch. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> --- >> migration/multifd.h | 13 ++++++++++++- >> migration/multifd.c | 22 +++++++++++++++++++--- >> migration/trace-events | 2 +- >> 3 files changed, 32 insertions(+), 5 deletions(-) >> >> diff --git a/migration/multifd.h b/migration/multifd.h >> index 39e55d7f05..973315b545 100644 >> --- a/migration/multifd.h >> +++ b/migration/multifd.h >> @@ -49,7 +49,10 @@ typedef struct { >> /* size of the next packet that contains pages */ >> uint32_t next_packet_size; >> uint64_t packet_num; >> - uint64_t unused[4]; /* Reserved for future use */ >> + /* zero pages */ >> + uint32_t zero_pages; > > Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? I *have* to also add the flag. I was waiting for 7.0 to get out, because I still have to do the compatibility bits. Otherwise you can't migrate to an old multifd version. > >> + uint32_t unused32[1]; /* Reserved for future use */ >> + uint64_t unused64[3]; /* Reserved for future use */ >> char ramblock[256]; >> uint64_t offset[]; >> } __attribute__((packed)) MultiFDPacket_t; >> @@ -117,6 +120,10 @@ typedef struct { >> ram_addr_t *normal; >> /* num of non zero pages */ >> uint32_t normal_num; >> + /* Pages that are zero */ >> + ram_addr_t *zero; >> + /* num of zero pages */ >> + uint32_t zero_num; >> /* used for compression methods */ >> void *data; >> } MultiFDSendParams; >> @@ -162,6 +169,10 @@ typedef struct { >> ram_addr_t *normal; >> /* num of non zero pages */ >> uint32_t normal_num; >> + /* Pages that are zero */ >> + ram_addr_t *zero; >> + /* num of zero pages */ >> + uint32_t zero_num; >> /* used for de-compression methods */ >> void *data; >> } MultiFDRecvParams; >> diff --git a/migration/multifd.c b/migration/multifd.c >> index d1ab823f98..2e4dffd6c6 100644 >> --- a/migration/multifd.c >> +++ b/migration/multifd.c >> @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) >> packet->normal_pages = cpu_to_be32(p->normal_num); >> packet->next_packet_size = cpu_to_be32(p->next_packet_size); >> packet->packet_num = cpu_to_be64(p->packet_num); >> + packet->zero_pages = cpu_to_be32(p->zero_num); >> >> if (p->pages->block) { >> strncpy(packet->ramblock, p->pages->block->idstr, 256); >> @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) >> p->next_packet_size = be32_to_cpu(packet->next_packet_size); >> p->packet_num = be64_to_cpu(packet->packet_num); >> >> - if (p->normal_num == 0) { >> + p->zero_num = be32_to_cpu(packet->zero_pages); >> + if (p->zero_num > packet->pages_alloc - p->normal_num) { >> + error_setg(errp, "multifd: received packet " >> + "with %d zero pages and expected maximum pages are %d", >> + p->normal_num, packet->pages_alloc - p->zero_num) ; > > should that be p->zero_num, packet->pages_alloc - p->normal_num ? > (and be %u) Copy and paste error. You are right on both cases. Thanks.
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > * Juan Quintela (quintela@redhat.com) wrote: > >> This patch adds counters and similar. Logic will be added on the > >> following patch. > >> > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> --- > >> migration/multifd.h | 13 ++++++++++++- > >> migration/multifd.c | 22 +++++++++++++++++++--- > >> migration/trace-events | 2 +- > >> 3 files changed, 32 insertions(+), 5 deletions(-) > >> > >> diff --git a/migration/multifd.h b/migration/multifd.h > >> index 39e55d7f05..973315b545 100644 > >> --- a/migration/multifd.h > >> +++ b/migration/multifd.h > >> @@ -49,7 +49,10 @@ typedef struct { > >> /* size of the next packet that contains pages */ > >> uint32_t next_packet_size; > >> uint64_t packet_num; > >> - uint64_t unused[4]; /* Reserved for future use */ > >> + /* zero pages */ > >> + uint32_t zero_pages; > > > > Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? > > I *have* to also add the flag. I meant can't you add a flag to say that this whole packet is zero pages and then you only need one counter. Dave > I was waiting for 7.0 to get out, because I still have to do the > compatibility bits. Otherwise you can't migrate to an old multifd version. > > > > >> + uint32_t unused32[1]; /* Reserved for future use */ > >> + uint64_t unused64[3]; /* Reserved for future use */ > >> char ramblock[256]; > >> uint64_t offset[]; > >> } __attribute__((packed)) MultiFDPacket_t; > >> @@ -117,6 +120,10 @@ typedef struct { > >> ram_addr_t *normal; > >> /* num of non zero pages */ > >> uint32_t normal_num; > >> + /* Pages that are zero */ > >> + ram_addr_t *zero; > >> + /* num of zero pages */ > >> + uint32_t zero_num; > >> /* used for compression methods */ > >> void *data; > >> } MultiFDSendParams; > >> @@ -162,6 +169,10 @@ typedef struct { > >> ram_addr_t *normal; > >> /* num of non zero pages */ > >> uint32_t normal_num; > >> + /* Pages that are zero */ > >> + ram_addr_t *zero; > >> + /* num of zero pages */ > >> + uint32_t zero_num; > >> /* used for de-compression methods */ > >> void *data; > >> } MultiFDRecvParams; > >> diff --git a/migration/multifd.c b/migration/multifd.c > >> index d1ab823f98..2e4dffd6c6 100644 > >> --- a/migration/multifd.c > >> +++ b/migration/multifd.c > >> @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) > >> packet->normal_pages = cpu_to_be32(p->normal_num); > >> packet->next_packet_size = cpu_to_be32(p->next_packet_size); > >> packet->packet_num = cpu_to_be64(p->packet_num); > >> + packet->zero_pages = cpu_to_be32(p->zero_num); > >> > >> if (p->pages->block) { > >> strncpy(packet->ramblock, p->pages->block->idstr, 256); > >> @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) > >> p->next_packet_size = be32_to_cpu(packet->next_packet_size); > >> p->packet_num = be64_to_cpu(packet->packet_num); > >> > >> - if (p->normal_num == 0) { > >> + p->zero_num = be32_to_cpu(packet->zero_pages); > >> + if (p->zero_num > packet->pages_alloc - p->normal_num) { > >> + error_setg(errp, "multifd: received packet " > >> + "with %d zero pages and expected maximum pages are %d", > >> + p->normal_num, packet->pages_alloc - p->zero_num) ; > > > > should that be p->zero_num, packet->pages_alloc - p->normal_num ? > > (and be %u) > > Copy and paste error. You are right on both cases. > > Thanks. >
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: >> > * Juan Quintela (quintela@redhat.com) wrote: >> >> This patch adds counters and similar. Logic will be added on the >> >> following patch. >> >> >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> >> --- >> >> migration/multifd.h | 13 ++++++++++++- >> >> migration/multifd.c | 22 +++++++++++++++++++--- >> >> migration/trace-events | 2 +- >> >> 3 files changed, 32 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/migration/multifd.h b/migration/multifd.h >> >> index 39e55d7f05..973315b545 100644 >> >> --- a/migration/multifd.h >> >> +++ b/migration/multifd.h >> >> @@ -49,7 +49,10 @@ typedef struct { >> >> /* size of the next packet that contains pages */ >> >> uint32_t next_packet_size; >> >> uint64_t packet_num; >> >> - uint64_t unused[4]; /* Reserved for future use */ >> >> + /* zero pages */ >> >> + uint32_t zero_pages; >> > >> > Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? >> >> I *have* to also add the flag. > > I meant can't you add a flag to say that this whole packet is zero pages > and then you only need one counter. No, in general packets are going to transmit *both*, zero pages and normal pages. It depends on the content that one receives. Later, Juan. > Dave > >> I was waiting for 7.0 to get out, because I still have to do the >> compatibility bits. Otherwise you can't migrate to an old multifd version. >> >> > >> >> + uint32_t unused32[1]; /* Reserved for future use */ >> >> + uint64_t unused64[3]; /* Reserved for future use */ >> >> char ramblock[256]; >> >> uint64_t offset[]; >> >> } __attribute__((packed)) MultiFDPacket_t; >> >> @@ -117,6 +120,10 @@ typedef struct { >> >> ram_addr_t *normal; >> >> /* num of non zero pages */ >> >> uint32_t normal_num; >> >> + /* Pages that are zero */ >> >> + ram_addr_t *zero; >> >> + /* num of zero pages */ >> >> + uint32_t zero_num; >> >> /* used for compression methods */ >> >> void *data; >> >> } MultiFDSendParams; >> >> @@ -162,6 +169,10 @@ typedef struct { >> >> ram_addr_t *normal; >> >> /* num of non zero pages */ >> >> uint32_t normal_num; >> >> + /* Pages that are zero */ >> >> + ram_addr_t *zero; >> >> + /* num of zero pages */ >> >> + uint32_t zero_num; >> >> /* used for de-compression methods */ >> >> void *data; >> >> } MultiFDRecvParams; >> >> diff --git a/migration/multifd.c b/migration/multifd.c >> >> index d1ab823f98..2e4dffd6c6 100644 >> >> --- a/migration/multifd.c >> >> +++ b/migration/multifd.c >> >> @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) >> >> packet->normal_pages = cpu_to_be32(p->normal_num); >> >> packet->next_packet_size = cpu_to_be32(p->next_packet_size); >> >> packet->packet_num = cpu_to_be64(p->packet_num); >> >> + packet->zero_pages = cpu_to_be32(p->zero_num); >> >> >> >> if (p->pages->block) { >> >> strncpy(packet->ramblock, p->pages->block->idstr, 256); >> >> @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) >> >> p->next_packet_size = be32_to_cpu(packet->next_packet_size); >> >> p->packet_num = be64_to_cpu(packet->packet_num); >> >> >> >> - if (p->normal_num == 0) { >> >> + p->zero_num = be32_to_cpu(packet->zero_pages); >> >> + if (p->zero_num > packet->pages_alloc - p->normal_num) { >> >> + error_setg(errp, "multifd: received packet " >> >> + "with %d zero pages and expected maximum pages are %d", >> >> + p->normal_num, packet->pages_alloc - p->zero_num) ; >> > >> > should that be p->zero_num, packet->pages_alloc - p->normal_num ? >> > (and be %u) >> >> Copy and paste error. You are right on both cases. >> >> Thanks. >>
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > > * Juan Quintela (quintela@redhat.com) wrote: > >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > >> > * Juan Quintela (quintela@redhat.com) wrote: > >> >> This patch adds counters and similar. Logic will be added on the > >> >> following patch. > >> >> > >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > >> >> --- > >> >> migration/multifd.h | 13 ++++++++++++- > >> >> migration/multifd.c | 22 +++++++++++++++++++--- > >> >> migration/trace-events | 2 +- > >> >> 3 files changed, 32 insertions(+), 5 deletions(-) > >> >> > >> >> diff --git a/migration/multifd.h b/migration/multifd.h > >> >> index 39e55d7f05..973315b545 100644 > >> >> --- a/migration/multifd.h > >> >> +++ b/migration/multifd.h > >> >> @@ -49,7 +49,10 @@ typedef struct { > >> >> /* size of the next packet that contains pages */ > >> >> uint32_t next_packet_size; > >> >> uint64_t packet_num; > >> >> - uint64_t unused[4]; /* Reserved for future use */ > >> >> + /* zero pages */ > >> >> + uint32_t zero_pages; > >> > > >> > Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? > >> > >> I *have* to also add the flag. > > > > I meant can't you add a flag to say that this whole packet is zero pages > > and then you only need one counter. > > No, in general packets are going to transmit *both*, zero pages and > normal pages. It depends on the content that one receives. OK, I'd wondered if it was just easier to send two packets; but fine. Dave > Later, Juan. > > > Dave > > > >> I was waiting for 7.0 to get out, because I still have to do the > >> compatibility bits. Otherwise you can't migrate to an old multifd version. > >> > >> > > >> >> + uint32_t unused32[1]; /* Reserved for future use */ > >> >> + uint64_t unused64[3]; /* Reserved for future use */ > >> >> char ramblock[256]; > >> >> uint64_t offset[]; > >> >> } __attribute__((packed)) MultiFDPacket_t; > >> >> @@ -117,6 +120,10 @@ typedef struct { > >> >> ram_addr_t *normal; > >> >> /* num of non zero pages */ > >> >> uint32_t normal_num; > >> >> + /* Pages that are zero */ > >> >> + ram_addr_t *zero; > >> >> + /* num of zero pages */ > >> >> + uint32_t zero_num; > >> >> /* used for compression methods */ > >> >> void *data; > >> >> } MultiFDSendParams; > >> >> @@ -162,6 +169,10 @@ typedef struct { > >> >> ram_addr_t *normal; > >> >> /* num of non zero pages */ > >> >> uint32_t normal_num; > >> >> + /* Pages that are zero */ > >> >> + ram_addr_t *zero; > >> >> + /* num of zero pages */ > >> >> + uint32_t zero_num; > >> >> /* used for de-compression methods */ > >> >> void *data; > >> >> } MultiFDRecvParams; > >> >> diff --git a/migration/multifd.c b/migration/multifd.c > >> >> index d1ab823f98..2e4dffd6c6 100644 > >> >> --- a/migration/multifd.c > >> >> +++ b/migration/multifd.c > >> >> @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) > >> >> packet->normal_pages = cpu_to_be32(p->normal_num); > >> >> packet->next_packet_size = cpu_to_be32(p->next_packet_size); > >> >> packet->packet_num = cpu_to_be64(p->packet_num); > >> >> + packet->zero_pages = cpu_to_be32(p->zero_num); > >> >> > >> >> if (p->pages->block) { > >> >> strncpy(packet->ramblock, p->pages->block->idstr, 256); > >> >> @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) > >> >> p->next_packet_size = be32_to_cpu(packet->next_packet_size); > >> >> p->packet_num = be64_to_cpu(packet->packet_num); > >> >> > >> >> - if (p->normal_num == 0) { > >> >> + p->zero_num = be32_to_cpu(packet->zero_pages); > >> >> + if (p->zero_num > packet->pages_alloc - p->normal_num) { > >> >> + error_setg(errp, "multifd: received packet " > >> >> + "with %d zero pages and expected maximum pages are %d", > >> >> + p->normal_num, packet->pages_alloc - p->zero_num) ; > >> > > >> > should that be p->zero_num, packet->pages_alloc - p->normal_num ? > >> > (and be %u) > >> > >> Copy and paste error. You are right on both cases. > >> > >> Thanks. > >> >
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Juan Quintela (quintela@redhat.com) wrote: >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: >> > * Juan Quintela (quintela@redhat.com) wrote: >> >> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: >> >> > * Juan Quintela (quintela@redhat.com) wrote: >> >> >> This patch adds counters and similar. Logic will be added on the >> >> >> following patch. >> >> >> >> >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> >> >> --- >> >> >> migration/multifd.h | 13 ++++++++++++- >> >> >> migration/multifd.c | 22 +++++++++++++++++++--- >> >> >> migration/trace-events | 2 +- >> >> >> 3 files changed, 32 insertions(+), 5 deletions(-) >> >> >> >> >> >> diff --git a/migration/multifd.h b/migration/multifd.h >> >> >> index 39e55d7f05..973315b545 100644 >> >> >> --- a/migration/multifd.h >> >> >> +++ b/migration/multifd.h >> >> >> @@ -49,7 +49,10 @@ typedef struct { >> >> >> /* size of the next packet that contains pages */ >> >> >> uint32_t next_packet_size; >> >> >> uint64_t packet_num; >> >> >> - uint64_t unused[4]; /* Reserved for future use */ >> >> >> + /* zero pages */ >> >> >> + uint32_t zero_pages; >> >> > >> >> > Had you considered just adding a flag, MULTIFD_FLAG_ZERO to the packet? >> >> >> >> I *have* to also add the flag. >> > >> > I meant can't you add a flag to say that this whole packet is zero pages >> > and then you only need one counter. >> >> No, in general packets are going to transmit *both*, zero pages and >> normal pages. It depends on the content that one receives. > > OK, I'd wondered if it was just easier to send two packets; but fine. Zero pages travel for free. To have initial packets to the same size, we always send an array of 128 offsets in the packet (I am speaking x86_64 here). And as we receive an array of 128 pages, we have space there for the zero pages, no need of a different packet at all. Later, Juan.
diff --git a/migration/multifd.h b/migration/multifd.h index 39e55d7f05..973315b545 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -49,7 +49,10 @@ typedef struct { /* size of the next packet that contains pages */ uint32_t next_packet_size; uint64_t packet_num; - uint64_t unused[4]; /* Reserved for future use */ + /* zero pages */ + uint32_t zero_pages; + uint32_t unused32[1]; /* Reserved for future use */ + uint64_t unused64[3]; /* Reserved for future use */ char ramblock[256]; uint64_t offset[]; } __attribute__((packed)) MultiFDPacket_t; @@ -117,6 +120,10 @@ typedef struct { ram_addr_t *normal; /* num of non zero pages */ uint32_t normal_num; + /* Pages that are zero */ + ram_addr_t *zero; + /* num of zero pages */ + uint32_t zero_num; /* used for compression methods */ void *data; } MultiFDSendParams; @@ -162,6 +169,10 @@ typedef struct { ram_addr_t *normal; /* num of non zero pages */ uint32_t normal_num; + /* Pages that are zero */ + ram_addr_t *zero; + /* num of zero pages */ + uint32_t zero_num; /* used for de-compression methods */ void *data; } MultiFDRecvParams; diff --git a/migration/multifd.c b/migration/multifd.c index d1ab823f98..2e4dffd6c6 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -265,6 +265,7 @@ static void multifd_send_fill_packet(MultiFDSendParams *p) packet->normal_pages = cpu_to_be32(p->normal_num); packet->next_packet_size = cpu_to_be32(p->next_packet_size); packet->packet_num = cpu_to_be64(p->packet_num); + packet->zero_pages = cpu_to_be32(p->zero_num); if (p->pages->block) { strncpy(packet->ramblock, p->pages->block->idstr, 256); @@ -327,7 +328,15 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) p->next_packet_size = be32_to_cpu(packet->next_packet_size); p->packet_num = be64_to_cpu(packet->packet_num); - if (p->normal_num == 0) { + p->zero_num = be32_to_cpu(packet->zero_pages); + if (p->zero_num > packet->pages_alloc - p->normal_num) { + error_setg(errp, "multifd: received packet " + "with %d zero pages and expected maximum pages are %d", + p->normal_num, packet->pages_alloc - p->zero_num) ; + return -1; + } + + if (p->normal_num == 0 && p->zero_num == 0) { return 0; } @@ -550,6 +559,8 @@ void multifd_save_cleanup(void) p->iov = NULL; g_free(p->normal); p->normal = NULL; + g_free(p->zero); + p->zero = NULL; multifd_send_state->ops->send_cleanup(p, &local_err); if (local_err) { migrate_set_error(migrate_get_current(), local_err); @@ -638,6 +649,7 @@ static void *multifd_send_thread(void *opaque) uint32_t flags = p->flags; p->iovs_num = 1; p->normal_num = 0; + p->zero_num = 0; for (int i = 0; i < p->pages->num; i++) { p->normal[p->normal_num] = p->pages->offset[i]; @@ -659,8 +671,8 @@ static void *multifd_send_thread(void *opaque) p->pages->block = NULL; qemu_mutex_unlock(&p->mutex); - trace_multifd_send(p->id, packet_num, p->normal_num, flags, - p->next_packet_size); + trace_multifd_send(p->id, packet_num, p->normal_num, p->zero_num, + flags, p->next_packet_size); p->iov[0].iov_len = p->packet_len; p->iov[0].iov_base = p->packet; @@ -910,6 +922,7 @@ int multifd_save_setup(Error **errp) /* We need one extra place for the packet header */ p->iov = g_new0(struct iovec, page_count + 1); p->normal = g_new0(ram_addr_t, page_count); + p->zero = g_new0(ram_addr_t, page_count); socket_send_channel_create(multifd_new_send_channel_async, p); } @@ -1011,6 +1024,8 @@ int multifd_load_cleanup(Error **errp) p->iov = NULL; g_free(p->normal); p->normal = NULL; + g_free(p->zero); + p->zero = NULL; multifd_recv_state->ops->recv_cleanup(p); } qemu_sem_destroy(&multifd_recv_state->sem_sync); @@ -1150,6 +1165,7 @@ int multifd_load_setup(Error **errp) p->name = g_strdup_printf("multifdrecv_%d", i); p->iov = g_new0(struct iovec, page_count); p->normal = g_new0(ram_addr_t, page_count); + p->zero = g_new0(ram_addr_t, page_count); } for (i = 0; i < thread_count; i++) { diff --git a/migration/trace-events b/migration/trace-events index af8dee9af0..608decbdcc 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -124,7 +124,7 @@ multifd_recv_sync_main_wait(uint8_t id) "channel %d" multifd_recv_terminate_threads(bool error) "error %d" multifd_recv_thread_end(uint8_t id, uint64_t packets, uint64_t pages) "channel %d packets %" PRIu64 " pages %" PRIu64 multifd_recv_thread_start(uint8_t id) "%d" -multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " normal pages %d flags 0x%x next packet size %d" +multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t zero, uint32_t flags, uint32_t next_packet_size) "channel %d packet_num %" PRIu64 " normal pages %d zero pages %d flags 0x%x next packet size %d" multifd_send_error(uint8_t id) "channel %d" multifd_send_sync_main(long packet_num) "packet num %ld" multifd_send_sync_main_signal(uint8_t id) "channel %d"
This patch adds counters and similar. Logic will be added on the following patch. Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/multifd.h | 13 ++++++++++++- migration/multifd.c | 22 +++++++++++++++++++--- migration/trace-events | 2 +- 3 files changed, 32 insertions(+), 5 deletions(-)