On Tue, 2022-08-02 at 08:39 +0200, Juan Quintela wrote: > This implements the zero page dection and handling. > > Signed-off-by: Juan Quintela <quint...@redhat.com> > > --- > > Add comment for offset (dave) > Use local variables for offset/block to have shorter lines > --- > migration/multifd.h | 5 +++++ > migration/multifd.c | 41 +++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 44 insertions(+), 2 deletions(-) > > diff --git a/migration/multifd.h b/migration/multifd.h > index a1b852200d..5931de6f86 100644 > --- a/migration/multifd.h > +++ b/migration/multifd.h > @@ -52,6 +52,11 @@ typedef struct { > uint32_t unused32[1]; /* Reserved for future use */ > uint64_t unused64[3]; /* Reserved for future use */ > char ramblock[256]; > + /* > + * This array contains the pointers to: > + * - normal pages (initial normal_pages entries) > + * - zero pages (following zero_pages entries) > + */ > uint64_t offset[]; > } __attribute__((packed)) MultiFDPacket_t; > > diff --git a/migration/multifd.c b/migration/multifd.c > index 4473d9f834..89811619d8 100644 > --- a/migration/multifd.c > +++ b/migration/multifd.c > @@ -11,6 +11,7 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/cutils.h" > #include "qemu/rcu.h" > #include "exec/target_page.h" > #include "sysemu/sysemu.h" > @@ -275,6 +276,12 @@ static void multifd_send_fill_packet(MultiFDSendParams > *p) > > packet->offset[i] = cpu_to_be64(temp); > } > + for (i = 0; i < p->zero_num; i++) { > + /* there are architectures where ram_addr_t is 32 bit */ > + uint64_t temp = p->zero[i]; > + > + packet->offset[p->normal_num + i] = cpu_to_be64(temp); > + } > } > > static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp) > @@ -358,6 +365,18 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams > *p, Error **errp) > p->normal[i] = offset; > } > > + for (i = 0; i < p->zero_num; i++) { > + uint64_t offset = be64_to_cpu(packet->offset[p->normal_num + i]); > + > + if (offset > (block->used_length - p->page_size)) { > + error_setg(errp, "multifd: offset too long %" PRIu64 > + " (max " RAM_ADDR_FMT ")", > + offset, block->used_length); > + return -1; > + } > + p->zero[i] = offset; > + } > + > return 0; > }
IIUC ram_addr_t is supposed to be the address size for the architecture, mainly being 32 or 64 bits. So packet->offset[i] is always u64, and p->zero[i] possibly being u32 or u64. Since both local variables and packet->offset[i] are 64-bit, there is no issue. But on 'p->zero[i] = offset' we can have 'u32 = u64', and this should raise a warning (or am I missing something?). > > @@ -648,6 +667,8 @@ static void *multifd_send_thread(void *opaque) > { > MultiFDSendParams *p = opaque; > Error *local_err = NULL; > + /* qemu older than 7.0 don't understand zero page on multifd channel */ > + bool use_zero_page = migrate_use_multifd_zero_page(); > int ret = 0; > bool use_zero_copy_send = migrate_use_zero_copy_send(); > > @@ -670,6 +691,7 @@ static void *multifd_send_thread(void *opaque) > qemu_mutex_lock(&p->mutex); > > if (p->pending_job) { > + RAMBlock *rb = p->pages->block; > uint64_t packet_num = p->packet_num; > p->flags = 0; > if (p->sync_needed) { > @@ -688,8 +710,16 @@ static void *multifd_send_thread(void *opaque) > } > > for (int i = 0; i < p->pages->num; i++) { > - p->normal[p->normal_num] = p->pages->offset[i]; > - p->normal_num++; > + uint64_t offset = p->pages->offset[i]; > + if (use_zero_page && > + buffer_is_zero(rb->host + offset, p->page_size)) { > + p->zero[p->zero_num] = offset; Same here. > + p->zero_num++; > + ram_release_page(rb->idstr, offset); > + } else { > + p->normal[p->normal_num] = offset; Same here? (p->normal[i] can also be u32) > + p->normal_num++; > + } > } > > if (p->normal_num) { > @@ -1152,6 +1182,13 @@ static void *multifd_recv_thread(void *opaque) > } > } > > + for (int i = 0; i < p->zero_num; i++) { > + void *page = p->host + p->zero[i]; > + if (!buffer_is_zero(page, p->page_size)) { > + memset(page, 0, p->page_size); > + } > + } > + > if (sync_needed) { > qemu_sem_post(&multifd_recv_state->sem_sync); > qemu_sem_wait(&p->sem_sync);