The following changes since commit a082fab9d259473a9d5d53307cf83b1223301181: Merge tag 'pull-ppc-20221117' of https://gitlab.com/danielhb/qemu into staging (2022-11-17 12:39:38 -0500) are available in the Git repository at: https://gitlab.com/juan.quintela/qemu.git tags/next-pull-request for you to fetch changes up to b5280437a7f49cf617cdd99bbbe2c7bd1652408b: migration: Block migration comment or code is wrong (2022-11-21 11:58:10 +0100) ---------------------------------------------------------------- Migration PULL request (take 3) Hi Drop everything that is not a bug fix: - fixes by peter - fix comment on block creation (me) - fix return values from qio_channel_block() Please, apply. (take 1) It includes: - Leonardo fix for zero_copy flush - Fiona fix for return value of readv/writev - Peter Xu cleanups - Peter Xu preempt patches - Patches ready from zero page (me) - AVX2 support (ling) - fix for slow networking and reordering of first packets (manish) Please, apply. ---------------------------------------------------------------- Fiona Ebner (1): migration/channel-block: fix return value for qio_channel_block_{readv,writev} Juan Quintela (1): migration: Block migration comment or code is wrong Leonardo Bras (1): migration/multifd/zero-copy: Create helper function for flushing Peter Xu (5): migration: Fix possible infinite loop of ram save process migration: Fix race on qemu_file_shutdown() migration: Disallow postcopy preempt to be used with compress migration: Use non-atomic ops for clear log bitmap migration: Disable multifd explicitly with compression include/exec/ram_addr.h | 11 +++++----- include/exec/ramblock.h | 3 +++ include/qemu/bitmap.h | 1 + migration/block.c | 4 ++-- migration/channel-block.c | 6 ++++-- migration/migration.c | 18 ++++++++++++++++ migration/multifd.c | 30 ++++++++++++++++---------- migration/qemu-file.c | 27 ++++++++++++++++++++--- migration/ram.c | 27 ++++++++++++++--------- util/bitmap.c | 45 +++++++++++++++++++++++++++++++++++++++ 10 files changed, 139 insertions(+), 33 deletions(-) -- 2.38.1 From: Fiona Ebner <f.ebner@proxmox.com> in the error case. The documentation in include/io/channel.h states that -1 or QIO_CHANNEL_ERR_BLOCK should be returned upon error. Simply passing along the return value from the bdrv-functions has the potential to confuse the call sides. Non-blocking mode is not implemented currently, so -1 it is. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/channel-block.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/migration/channel-block.c b/migration/channel-block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/channel-block.c +++ b/migration/channel-block.c @@ -XXX,XX +XXX,XX @@ qio_channel_block_readv(QIOChannel *ioc, qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov); ret = bdrv_readv_vmstate(bioc->bs, &qiov, bioc->offset); if (ret < 0) { - return ret; + error_setg_errno(errp, -ret, "bdrv_readv_vmstate failed"); + return -1; } bioc->offset += qiov.size; @@ -XXX,XX +XXX,XX @@ qio_channel_block_writev(QIOChannel *ioc, qemu_iovec_init_external(&qiov, (struct iovec *)iov, niov); ret = bdrv_writev_vmstate(bioc->bs, &qiov, bioc->offset); if (ret < 0) { - return ret; + error_setg_errno(errp, -ret, "bdrv_writev_vmstate failed"); + return -1; } bioc->offset += qiov.size; -- 2.38.1 From: Leonardo Bras <leobras@redhat.com> Move flushing code from multifd_send_sync_main() to a new helper, and call it in multifd_send_sync_main(). Signed-off-by: Leonardo Bras <leobras@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/multifd.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index XXXXXXX..XXXXXXX 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -XXX,XX +XXX,XX @@ void multifd_save_cleanup(void) multifd_send_state = NULL; } +static int multifd_zero_copy_flush(QIOChannel *c) +{ + int ret; + Error *err = NULL; + + ret = qio_channel_flush(c, &err); + if (ret < 0) { + error_report_err(err); + return -1; + } + if (ret == 1) { + dirty_sync_missed_zero_copy(); + } + + return ret; +} + int multifd_send_sync_main(QEMUFile *f) { int i; @@ -XXX,XX +XXX,XX @@ int multifd_send_sync_main(QEMUFile *f) qemu_mutex_unlock(&p->mutex); qemu_sem_post(&p->sem); - if (flush_zero_copy && p->c) { - int ret; - Error *err = NULL; - - ret = qio_channel_flush(p->c, &err); - if (ret < 0) { - error_report_err(err); - return -1; - } else if (ret == 1) { - dirty_sync_missed_zero_copy(); - } + if (flush_zero_copy && p->c && (multifd_zero_copy_flush(p->c) < 0)) { + return -1; } } for (i = 0; i < migrate_multifd_channels(); i++) { -- 2.38.1 From: Peter Xu <peterx@redhat.com> When starting ram saving procedure (especially at the completion phase), always set last_seen_block to non-NULL to make sure we can always correctly detect the case where "we've migrated all the dirty pages". Then we'll guarantee both last_seen_block and pss.block will be valid always before the loop starts. See the comment in the code for some details. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/ram.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static int ram_find_and_save_block(RAMState *rs) return pages; } + /* + * Always keep last_seen_block/last_page valid during this procedure, + * because find_dirty_block() relies on these values (e.g., we compare + * last_seen_block with pss.block to see whether we searched all the + * ramblocks) to detect the completion of migration. Having NULL value + * of last_seen_block can conditionally cause below loop to run forever. + */ + if (!rs->last_seen_block) { + rs->last_seen_block = QLIST_FIRST_RCU(&ram_list.blocks); + rs->last_page = 0; + } + pss.block = rs->last_seen_block; pss.page = rs->last_page; pss.complete_round = false; - if (!pss.block) { - pss.block = QLIST_FIRST_RCU(&ram_list.blocks); - } - do { again = true; found = get_queued_page(rs, &pss); -- 2.38.1 From: Peter Xu <peterx@redhat.com> In qemu_file_shutdown(), there's a possible race if with current order of operation. There're two major things to do: (1) Do real shutdown() (e.g. shutdown() syscall on socket) (2) Update qemufile's last_error We must do (2) before (1) otherwise there can be a race condition like: page receiver other thread ------------- ------------ qemu_get_buffer() do shutdown() returns 0 (buffer all zero) (meanwhile we didn't check this retcode) try to detect IO error last_error==NULL, IO okay install ALL-ZERO page set last_error --> guest crash! To fix this, we can also check retval of qemu_get_buffer(), but not all APIs can be properly checked and ultimately we still need to go back to qemu_file_get_error(). E.g. qemu_get_byte() doesn't return error. Maybe some day a rework of qemufile API is really needed, but for now keep using qemu_file_get_error() and fix it by not allowing that race condition to happen. Here shutdown() is indeed special because the last_error was emulated. For real -EIO errors it'll always be set when e.g. sendmsg() error triggers so we won't miss those ones, only shutdown() is a bit tricky here. Cc: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/qemu-file.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/migration/qemu-file.c b/migration/qemu-file.c index XXXXXXX..XXXXXXX 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -XXX,XX +XXX,XX @@ int qemu_file_shutdown(QEMUFile *f) int ret = 0; f->shutdown = true; + + /* + * We must set qemufile error before the real shutdown(), otherwise + * there can be a race window where we thought IO all went though + * (because last_error==NULL) but actually IO has already stopped. + * + * If without correct ordering, the race can happen like this: + * + * page receiver other thread + * ------------- ------------ + * qemu_get_buffer() + * do shutdown() + * returns 0 (buffer all zero) + * (we didn't check this retcode) + * try to detect IO error + * last_error==NULL, IO okay + * install ALL-ZERO page + * set last_error + * --> guest crash! + */ + if (!f->last_error) { + qemu_file_set_error(f, -EIO); + } + if (!qio_channel_has_feature(f->ioc, QIO_CHANNEL_FEATURE_SHUTDOWN)) { return -ENOSYS; @@ -XXX,XX +XXX,XX @@ int qemu_file_shutdown(QEMUFile *f) ret = -EIO; } - if (!f->last_error) { - qemu_file_set_error(f, -EIO); - } return ret; } -- 2.38.1 From: Peter Xu <peterx@redhat.com> The preempt mode requires the capability to assign channel for each of the page, while the compression logic will currently assign pages to different compress thread/local-channel so potentially they're incompatible. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ static bool migrate_caps_check(bool *cap_list, error_setg(errp, "Postcopy preempt requires postcopy-ram"); return false; } + + /* + * Preempt mode requires urgent pages to be sent in separate + * channel, OTOH compression logic will disorder all pages into + * different compression channels, which is not compatible with the + * preempt assumptions on channel assignments. + */ + if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) { + error_setg(errp, "Postcopy preempt not compatible with compress"); + return false; + } } return true; -- 2.38.1 From: Peter Xu <peterx@redhat.com> Since we already have bitmap_mutex to protect either the dirty bitmap or the clear log bitmap, we don't need atomic operations to set/clear/test on the clear log bitmap. Switching all ops from atomic to non-atomic versions, meanwhile touch up the comments to show which lock is in charge. Introduced non-atomic version of bitmap_test_and_clear_atomic(), mostly the same as the atomic version but simplified a few places, e.g. dropped the "old_bits" variable, and also the explicit memory barriers. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- include/exec/ram_addr.h | 11 +++++----- include/exec/ramblock.h | 3 +++ include/qemu/bitmap.h | 1 + util/bitmap.c | 45 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -XXX,XX +XXX,XX @@ static inline long clear_bmap_size(uint64_t pages, uint8_t shift) } /** - * clear_bmap_set: set clear bitmap for the page range + * clear_bmap_set: set clear bitmap for the page range. Must be with + * bitmap_mutex held. * * @rb: the ramblock to operate on * @start: the start page number @@ -XXX,XX +XXX,XX @@ static inline void clear_bmap_set(RAMBlock *rb, uint64_t start, { uint8_t shift = rb->clear_bmap_shift; - bitmap_set_atomic(rb->clear_bmap, start >> shift, - clear_bmap_size(npages, shift)); + bitmap_set(rb->clear_bmap, start >> shift, clear_bmap_size(npages, shift)); } /** - * clear_bmap_test_and_clear: test clear bitmap for the page, clear if set + * clear_bmap_test_and_clear: test clear bitmap for the page, clear if set. + * Must be with bitmap_mutex held. * * @rb: the ramblock to operate on * @page: the page number to check @@ -XXX,XX +XXX,XX @@ static inline bool clear_bmap_test_and_clear(RAMBlock *rb, uint64_t page) { uint8_t shift = rb->clear_bmap_shift; - return bitmap_test_and_clear_atomic(rb->clear_bmap, page >> shift, 1); + return bitmap_test_and_clear(rb->clear_bmap, page >> shift, 1); } static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset) diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h index XXXXXXX..XXXXXXX 100644 --- a/include/exec/ramblock.h +++ b/include/exec/ramblock.h @@ -XXX,XX +XXX,XX @@ struct RAMBlock { * and split clearing of dirty bitmap on the remote node (e.g., * KVM). The bitmap will be set only when doing global sync. * + * It is only used during src side of ram migration, and it is + * protected by the global ram_state.bitmap_mutex. + * * NOTE: this bitmap is different comparing to the other bitmaps * in that one bit can represent multiple guest pages (which is * decided by the `clear_bmap_shift' variable below). On diff --git a/include/qemu/bitmap.h b/include/qemu/bitmap.h index XXXXXXX..XXXXXXX 100644 --- a/include/qemu/bitmap.h +++ b/include/qemu/bitmap.h @@ -XXX,XX +XXX,XX @@ void bitmap_set(unsigned long *map, long i, long len); void bitmap_set_atomic(unsigned long *map, long i, long len); void bitmap_clear(unsigned long *map, long start, long nr); bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr); +bool bitmap_test_and_clear(unsigned long *map, long start, long nr); void bitmap_copy_and_clear_atomic(unsigned long *dst, unsigned long *src, long nr); unsigned long bitmap_find_next_zero_area(unsigned long *map, diff --git a/util/bitmap.c b/util/bitmap.c index XXXXXXX..XXXXXXX 100644 --- a/util/bitmap.c +++ b/util/bitmap.c @@ -XXX,XX +XXX,XX @@ void bitmap_clear(unsigned long *map, long start, long nr) } } +bool bitmap_test_and_clear(unsigned long *map, long start, long nr) +{ + unsigned long *p = map + BIT_WORD(start); + const long size = start + nr; + int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG); + unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start); + bool dirty = false; + + assert(start >= 0 && nr >= 0); + + /* First word */ + if (nr - bits_to_clear > 0) { + if ((*p) & mask_to_clear) { + dirty = true; + } + *p &= ~mask_to_clear; + nr -= bits_to_clear; + bits_to_clear = BITS_PER_LONG; + p++; + } + + /* Full words */ + if (bits_to_clear == BITS_PER_LONG) { + while (nr >= BITS_PER_LONG) { + if (*p) { + dirty = true; + *p = 0; + } + nr -= BITS_PER_LONG; + p++; + } + } + + /* Last word */ + if (nr) { + mask_to_clear &= BITMAP_LAST_WORD_MASK(size); + if ((*p) & mask_to_clear) { + dirty = true; + } + *p &= ~mask_to_clear; + } + + return dirty; +} + bool bitmap_test_and_clear_atomic(unsigned long *map, long start, long nr) { unsigned long *p = map + BIT_WORD(start); -- 2.38.1 From: Peter Xu <peterx@redhat.com> Multifd thread model does not work for compression, explicitly disable it. Note that previuosly even we can enable both of them, nothing will go wrong, because the compression code has higher priority so multifd feature will just be ignored. Now we'll fail even earlier at config time so the user should be aware of the consequence better. Note that there can be a slight chance of breaking existing users, but let's assume they're not majority and not serious users, or they should have found that multifd is not working already. With that, we can safely drop the check in ram_save_target_page() for using multifd, because when multifd=on then compression=off, then the removed check on save_page_use_compression() will also always return false too. Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com> --- migration/migration.c | 7 +++++++ migration/ram.c | 11 +++++------ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -XXX,XX +XXX,XX @@ static bool migrate_caps_check(bool *cap_list, } } + if (cap_list[MIGRATION_CAPABILITY_MULTIFD]) { + if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) { + error_setg(errp, "Multifd is not compatible with compress"); + return false; + } + } + return true; } diff --git a/migration/ram.c b/migration/ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -XXX,XX +XXX,XX @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss) } /* - * Do not use multifd for: - * 1. Compression as the first page in the new block should be posted out - * before sending the compressed page - * 2. In postcopy as one whole host page should be placed + * Do not use multifd in postcopy as one whole host page should be + * placed. Meanwhile postcopy requires atomic update of pages, so even + * if host page size == guest page size the dest guest during run may + * still see partially copied pages which is data corruption. */ - if (!save_page_use_compression(rs) && migrate_use_multifd() - && !migration_in_postcopy()) { + if (migrate_use_multifd() && !migration_in_postcopy()) { return ram_save_multifd_page(rs, block, offset); } -- 2.38.1 And it appears that what is wrong is the code. During bulk stage we need to make sure that some block is dirty, but no games with max_size at all. Signed-off-by: Juan Quintela <quintela@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- migration/block.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migration/block.c b/migration/block.c index XXXXXXX..XXXXXXX 100644 --- a/migration/block.c +++ b/migration/block.c @@ -XXX,XX +XXX,XX @@ static void block_save_pending(QEMUFile *f, void *opaque, uint64_t max_size, blk_mig_unlock(); /* Report at least one block pending during bulk phase */ - if (pending <= max_size && !block_mig_state.bulk_completed) { - pending = max_size + BLK_MIG_BLOCK_SIZE; + if (!pending && !block_mig_state.bulk_completed) { + pending = BLK_MIG_BLOCK_SIZE; } trace_migration_block_save_pending(pending); -- 2.38.1 The following changes since commit 30e8a06b64aa58a3990ba39cb5d09531e7d265e0: Merge tag 'net-pull-request' of https://github.com/jasowang/qemu into staging (2026-06-29 17:41:42 +0200) are available in the Git repository at: https://gitlab.com/peterx/qemu.git tags/next-pull-request for you to fetch changes up to 60f2281450c4b4e9a16922fd69c66486a8207abb: migration: clean up postcopy blocktime presentation (2026-07-17 12:33:42 -0400) ---------------------------------------------------------------- Migration pull for rc1 - Bin's cleanup on migration blocktime (partly) - Yujun's fix on print format ---------------------------------------------------------------- Bin Guo (2): migration: fix ineffective overflow assert in postcopy blocktime migration: clean up postcopy blocktime presentation yujun (1): migration: Fix invalid %ud format and trace arg typo migration/migration-hmp-cmds.c | 2 +- migration/postcopy-ram.c | 14 ++++++-------- migration/trace-events | 6 +++--- 3 files changed, 10 insertions(+), 12 deletions(-) -- 2.54.0 From: yujun <yujun@kylinos.cn> Standard printf has no %ud conversion; glibc treats it as %u followed by a literal 'd', so postcopy userfaultfd error messages printed event values like "5d" instead of "5". The same typo existed in two postcopy discard trace format strings. Also rename the misspelled ram_save_iterate_big_wait() trace argument milliconds to milliseconds. Fixes: 00fa4fc85b ("postcopy: Allow registering of fd handler") Signed-off-by: yujun <yujun@kylinos.cn> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20260630031324.43453-1-yujun@kylinos.cn Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/postcopy-ram.c | 4 ++-- migration/trace-events | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -XXX,XX +XXX,XX @@ static void *postcopy_ram_fault_thread(void *opaque) } } if (msg.event != UFFD_EVENT_PAGEFAULT) { - error_report("%s: Read unexpected event %ud from userfaultfd", + error_report("%s: Read unexpected event %u from userfaultfd", __func__, msg.event); continue; /* It's not a page fault, shouldn't happen */ } @@ -XXX,XX +XXX,XX @@ retry: } } if (msg.event != UFFD_EVENT_PAGEFAULT) { - error_report("%s: Read unexpected event %ud " + error_report("%s: Read unexpected event %u " "from userfaultfd (shared)", __func__, msg.event); continue; /* It's not a page fault, shouldn't happen */ diff --git a/migration/trace-events b/migration/trace-events index XXXXXXX..XXXXXXX 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -XXX,XX +XXX,XX @@ loadvm_postcopy_handle_run(void) "" loadvm_postcopy_handle_resume(void) "" loadvm_postcopy_ram_handle_discard(void) "" loadvm_postcopy_ram_handle_discard_end(void) "" -loadvm_postcopy_ram_handle_discard_header(const char *ramid, uint16_t len) "%s: %ud" +loadvm_postcopy_ram_handle_discard_header(const char *ramid, uint16_t len) "%s: %u" loadvm_process_command(const char *s, uint16_t len) "com=%s len=%d" loadvm_process_command_ping(uint32_t val) "0x%x" loadvm_approve_switchover(unsigned int switchover_ack_pending_num) "Switchover ack pending num=%u" postcopy_ram_listen_thread_exit(void) "" postcopy_ram_listen_thread_start(void) "" qemu_savevm_send_postcopy_advise(void) "" -qemu_savevm_send_postcopy_ram_discard(const char *id, uint16_t len) "%s: %ud" +qemu_savevm_send_postcopy_ram_discard(const char *id, uint16_t len) "%s: %u" savevm_command_send(uint16_t command, uint16_t len) "com=0x%x len=%d" savevm_section_start(const char *id, unsigned int section_id) "%s, section_id %u" savevm_section_end(const char *id, unsigned int section_id, int ret) "%s, section_id %u -> %d" @@ -XXX,XX +XXX,XX @@ colo_flush_ram_cache_begin(uint64_t dirty_pages) "dirty_pages %" PRIu64 colo_flush_ram_cache_end(void) "" save_xbzrle_page_skipping(void) "" save_xbzrle_page_overflow(void) "" -ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations" +ram_save_iterate_big_wait(uint64_t milliseconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations" ram_load_start(void) "" ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64 ram_write_tracking_ramblock_start(const char *block_id, size_t page_size, void *addr, size_t length) "%s: page_size: %zu addr: %p length: %zu" -- 2.54.0 From: Bin Guo <guobin@linux.alibaba.com> vcpu_faults_current[] is uint8_t. The overflow assert was checked after the post-increment, so 255 would wrap to 0 and the assert would pass silently. Move the check before the increment and use < 255. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Link: https://lore.kernel.org/r/20260716101952.65329-2-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/postcopy-ram.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -XXX,XX +XXX,XX @@ void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, /* * Account how many concurrent faults on this vCPU we trapped. See * comments above vcpu_faults_current[] on why it can be more than one. + * + * vcpu_faults_current[] is uint8_t, so assert before incrementing to + * catch overflow before it wraps. */ + assert(dc->vcpu_faults_current[cpu] < 255); if (dc->vcpu_faults_current[cpu]++ == 0) { dc->smp_cpus_down++; /* @@ -XXX,XX +XXX,XX @@ void mark_postcopy_blocktime_begin(uintptr_t addr, uint32_t ptid, */ dc->last_begin = current; } - - /* Making sure it won't overflow - it really should never! */ - assert(dc->vcpu_faults_current[cpu] <= 255); } else { /* * For non-vCPU thread faults, we don't care about tid or cpu index -- 2.54.0 From: Bin Guo <guobin@linux.alibaba.com> Small cleanups for the postcopy blocktime fine-grained tracking feature: * Remove a redundant memset() on latency_buckets after g_new0(). * Use singular "Latency" in the HMP label for postcopy non-vCPU latency, consistent with other single-value labels. Signed-off-by: Bin Guo <guobin@linux.alibaba.com> Link: https://lore.kernel.org/r/20260716101952.65329-4-guobin@linux.alibaba.com Signed-off-by: Peter Xu <peterx@redhat.com> --- migration/migration-hmp-cmds.c | 2 +- migration/postcopy-ram.c | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index XXXXXXX..XXXXXXX 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -XXX,XX +XXX,XX @@ static void migration_dump_blocktime(Monitor *mon, MigrationInfo *info) } if (info->has_postcopy_non_vcpu_latency) { - monitor_printf(mon, "Postcopy non-vCPU Latencies (ns): %" PRIu64 "\n", + monitor_printf(mon, "Postcopy non-vCPU Latency (ns): %" PRIu64 "\n", info->postcopy_non_vcpu_latency); } diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c index XXXXXXX..XXXXXXX 100644 --- a/migration/postcopy-ram.c +++ b/migration/postcopy-ram.c @@ -XXX,XX +XXX,XX @@ static struct PostcopyBlocktimeContext *blocktime_context_new(void) unsigned int smp_cpus = ms->smp.cpus; PostcopyBlocktimeContext *ctx = g_new0(PostcopyBlocktimeContext, 1); - /* Initialize all counters to be zeros */ - memset(ctx->latency_buckets, 0, sizeof(ctx->latency_buckets)); - ctx->vcpu_blocktime_total = g_new0(uint64_t, smp_cpus); ctx->vcpu_faults_count = g_new0(uint64_t, smp_cpus); ctx->vcpu_faults_current = g_new0(uint8_t, smp_cpus); -- 2.54.0