[{"id":3675529,"web_url":"http://patchwork.ozlabs.org/comment/3675529/","msgid":"<8734136atk.fsf@suse.de>","list_archive_url":null,"date":"2026-04-09T22:14:15","subject":"Re: [PATCH 09/14] migration: Move iteration counter out of RAM","submitter":{"id":85343,"url":"http://patchwork.ozlabs.org/api/people/85343/","name":"Fabiano Rosas","email":"farosas@suse.de"},"content":"Peter Xu writes:\n\n> It used to hide in RAM dirty sync path. Now with more modules being able\n> to slow sync on dirty information, keeping it there may not be good anymore\n> because it's not RAM's own concept for iterations: all modules should\n> follow.\n>\n> More importantly, mgmt may try to query dirty info (to make policy\n> decisions like adjusting downtime) by listening to iteration count changes\n> via QMP events. So we must make sure the boost of iterations only happens\n> _after_ the dirty sync operations with whatever form (RAM's dirty bitmap\n> sync, or VFIO's different ioctls to fetch latest dirty info from kernel).\n>\n> Move this to core migration path to manage, together with the event\n> generation, so that it can be well ordered with the sync operations for all\n> modules.\n>\n> This brings a good side effect that we should have an old issue regarding\n> to cpu_throttle_dirty_sync_timer_tick() which can randomly boost iteration\n> counts (because it invokes sync ops). Now it won't, which is actually the\n> right behavior.\n>\n\nGood.\n\n> Said that, we have code (not only QEMU, but likely mgmt too) assuming the\n> 1st iteration will always shows dirty count to 1. Make it initialized with\n> 1 this time, because we'll miss the dirty sync for setup() on boosting this\n> counter now.\n>\n> Reviewed-by: Hyman Huang \n> Reviewed-by: Prasad Pandit \n> Reviewed-by: Juraj Marcin \n> Signed-off-by: Peter Xu \n> ---\n> migration/migration-stats.h | 3 ++-\n> migration/migration.c | 29 ++++++++++++++++++++++++++---\n> migration/ram.c | 6 ------\n> 3 files changed, 28 insertions(+), 10 deletions(-)\n>\n> diff --git a/migration/migration-stats.h b/migration/migration-stats.h\n> index 1153520f7a..326ddb0088 100644\n> --- a/migration/migration-stats.h\n> +++ b/migration/migration-stats.h\n> @@ -43,7 +43,8 @@ typedef struct {\n> */\n> uint64_t dirty_pages_rate;\n> /*\n> - * Number of times we have synchronized guest bitmaps.\n> + * Number of times we have synchronized guest bitmaps. This always\n> + * starts from 1 for the 1st iteration.\n> */\n> uint64_t dirty_sync_count;\n> /*\n> diff --git a/migration/migration.c b/migration/migration.c\n> index a9ee3360e1..57d91ad9e3 100644\n> --- a/migration/migration.c\n> +++ b/migration/migration.c\n> @@ -1654,10 +1654,15 @@ int migrate_init(MigrationState *s, Error **errp)\n> s->threshold_size = 0;\n> s->switchover_acked = false;\n> s->rdma_migration = false;\n> +\n> /*\n> - * set mig_stats memory to zero for a new migration\n> + * set mig_stats memory to zero for a new migration.. except the\n> + * iteration counter, which we want to make sure it returns 1 for the\n> + * first iteration.\n> */\n> memset(&mig_stats, 0, sizeof(mig_stats));\n> + mig_stats.dirty_sync_count = 1;\n> +\n> migration_reset_vfio_bytes_transferred();\n> \n> s->postcopy_package_loaded = false;\n> @@ -3230,10 +3235,28 @@ static bool migration_iteration_next_ready(MigrationState *s,\n> static void migration_iteration_go_next(MigPendingData *pending)\n> {\n> /*\n> - * Do a slow sync will achieve this. TODO: move RAM iteration code\n> - * into the core layer.\n> + * Do a slow sync first before boosting the iteration count.\n> */\n> qemu_savevm_query_pending(pending, true);\n> +\n> + /*\n> + * Boost dirty sync count to reflect we finished one iteration.\n> + *\n> + * NOTE: we need to make sure when this happens (together with the\n> + * event sent below) all modules have slow-synced the pending data\n> + * above. That means a write mem barrier, but qatomic_add() should be\n> + * enough.\n> + *\n> + * It's because a mgmt could wait on the iteration event to query again\n> + * on pending data for policy changes (e.g. downtime adjustments). The\n> + * ordering will make sure the query will fetch the latest results from\n> + * all the modules.\n> + */\n> + qatomic_add(&mig_stats.dirty_sync_count, 1);\n> +\n> + if (migrate_events()) {\n> + qapi_event_send_migration_pass(mig_stats.dirty_sync_count);\n> + }\n> }\n> \n> /*\n> diff --git a/migration/ram.c b/migration/ram.c\n> index e5b7217bf5..686162643d 100644\n> --- a/migration/ram.c\n> +++ b/migration/ram.c\n> @@ -1136,8 +1136,6 @@ static void migration_bitmap_sync(RAMState *rs, bool last_stage)\n> RAMBlock *block;\n> int64_t end_time;\n> \n> - qatomic_add(&mig_stats.dirty_sync_count, 1);\n> -\n> if (!rs->time_last_bitmap_sync) {\n> rs->time_last_bitmap_sync = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);\n\nTangent: aren't these updates to time_last_bitmap_sync racy with the\nsync from the throttle code?\n\n> }\n> @@ -1172,10 +1170,6 @@ static void migration_bitmap_sync(RAMState *rs, bool last_stage)\n> rs->num_dirty_pages_period = 0;\n> rs->bytes_xfer_prev = migration_transferred_bytes();\n> }\n> - if (migrate_events()) {\n> - uint64_t generation = qatomic_read(&mig_stats.dirty_sync_count);\n> - qapi_event_send_migration_pass(generation);\n> - }\n> }\n> \n> void migration_bitmap_sync_precopy(bool last_stage)","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256\n header.s=susede2_rsa header.b=PdMP5iUO;\n\tdkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=lDkdHtVz;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.a=rsa-sha256 header.s=susede2_rsa header.b=PdMP5iUO;\n\tdkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=lDkdHtVz;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org\n (client-ip=209.51.188.17; helo=lists.gnu.org;\n envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n receiver=patchwork.ozlabs.org)","smtp-out1.suse.de;\n\tnone"],"Received":["from lists.gnu.org (lists1p.gnu.org [209.51.188.17])\n\t(using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fsDkg2N2cz1xtJ\n\tfor ; Fri, 10 Apr 2026 08:14:51 +1000 (AEST)","from localhost ([::1] helo=lists1p.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.90_1)\n\t(envelope-from )\n\tid 1wAxdn-0004NK-If; Thu, 09 Apr 2026 18:14:23 -0400","from eggs.gnu.org ([2001:470:142:3::10])\n by lists1p.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)\n (Exim 4.90_1) (envelope-from ) id 1wAxdl-0004N0-Uz\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 18:14:22 -0400","from smtp-out1.suse.de ([195.135.223.130])\n by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128)\n (Exim 4.90_1) (envelope-from ) id 1wAxdk-0002Gf-5M\n for qemu-devel@nongnu.org; Thu, 09 Apr 2026 18:14:21 -0400","from imap1.dmz-prg2.suse.org (unknown [10.150.64.97])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n (No client certificate requested)\n by smtp-out1.suse.de (Postfix) with ESMTPS id 532CD6A7CF;\n Thu, 9 Apr 2026 22:14:18 +0000 (UTC)","from imap1.dmz-prg2.suse.org (localhost [127.0.0.1])\n (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n (No client certificate requested)\n by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id CC7804A0B3;\n Thu, 9 Apr 2026 22:14:17 +0000 (UTC)","from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167])\n by imap1.dmz-prg2.suse.org with ESMTPSA id 9/qwJbkk2GnFOwAAD6G6ig\n (envelope-from ); Thu, 09 Apr 2026 22:14:17 +0000"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n t=1775772858;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=7j4UjjEo9IqtLM1/EXu0CYuyLZlGrCapR+WK40xCRBM=;\n b=PdMP5iUOkEhuREtQ0iL526s1Q3lWkNSNx0m1slWSxVFIJyPR+hCDPf1utDUyLNzYS4fO/y\n 6ZyG/GrAVdbRSoTIAFqvfRz1AS4IQ15Ou886WS+01VQjXidNfptRe+zYqQKbki1eR8ebd4\n vk8diV1qklnN/PkF0TykfRrNIiAyEGQ=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_ed25519; t=1775772858;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=7j4UjjEo9IqtLM1/EXu0CYuyLZlGrCapR+WK40xCRBM=;\n b=lDkdHtVzKNHkaCVD24CphGeF91Z0GIJtE2L5NdhTk+dqn1Cswigv2oHTKabb2np0jWVJeM\n T/hT6w30VikuWxAg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n t=1775772858;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=7j4UjjEo9IqtLM1/EXu0CYuyLZlGrCapR+WK40xCRBM=;\n b=PdMP5iUOkEhuREtQ0iL526s1Q3lWkNSNx0m1slWSxVFIJyPR+hCDPf1utDUyLNzYS4fO/y\n 6ZyG/GrAVdbRSoTIAFqvfRz1AS4IQ15Ou886WS+01VQjXidNfptRe+zYqQKbki1eR8ebd4\n vk8diV1qklnN/PkF0TykfRrNIiAyEGQ=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_ed25519; t=1775772858;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n mime-version:mime-version:content-type:content-type:\n in-reply-to:in-reply-to:references:references;\n bh=7j4UjjEo9IqtLM1/EXu0CYuyLZlGrCapR+WK40xCRBM=;\n b=lDkdHtVzKNHkaCVD24CphGeF91Z0GIJtE2L5NdhTk+dqn1Cswigv2oHTKabb2np0jWVJeM\n T/hT6w30VikuWxAg=="],"From":"Fabiano Rosas ","To":"Peter Xu , qemu-devel@nongnu.org","Cc":"\"Maciej S . Szmigiero\" , =?utf-8?q?Daniel_P_?=\n\t=?utf-8?q?=2E_Berrang=C3=A9?= ,\n Zhiyi Guo , Juraj Marcin ,\n Peter Xu , Prasad Pandit ,\n Avihai Horon , Kirti Wankhede ,\n\t=?utf-8?q?C=C3=A9dric?= Le Goater ,\n Joao Martins ,\n Markus Armbruster , Alex Williamson ,\n Hyman Huang , Prasad Pandit ","Subject":"Re: [PATCH 09/14] migration: Move iteration counter out of RAM","In-Reply-To":"<20260408165559.157108-10-peterx@redhat.com>","References":"<20260408165559.157108-1-peterx@redhat.com>\n <20260408165559.157108-10-peterx@redhat.com>","Date":"Thu, 09 Apr 2026 19:14:15 -0300","Message-ID":"<8734136atk.fsf@suse.de>","MIME-Version":"1.0","Content-Type":"text/plain","X-Spam-Score":"-4.30","X-Spamd-Result":"default: False [-4.30 / 50.00]; BAYES_HAM(-3.00)[100.00%];\n NEURAL_HAM_LONG(-1.00)[-1.000];\n NEURAL_HAM_SHORT(-0.20)[-1.000]; MIME_GOOD(-0.10)[text/plain];\n ARC_NA(0.00)[]; MISSING_XM_UA(0.00)[]; RCVD_TLS_ALL(0.00)[];\n MIME_TRACE(0.00)[0:+]; RCVD_VIA_SMTP_AUTH(0.00)[];\n RCPT_COUNT_TWELVE(0.00)[16];\n FUZZY_RATELIMITED(0.00)[rspamd.com];\n MID_RHS_MATCH_FROM(0.00)[];\n DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[];\n FROM_EQ_ENVFROM(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[];\n RCVD_COUNT_TWO(0.00)[2];\n DBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:helo,suse.de:mid]","Received-SPF":"pass client-ip=195.135.223.130; envelope-from=farosas@suse.de;\n helo=smtp-out1.suse.de","X-Spam_score_int":"-43","X-Spam_score":"-4.4","X-Spam_bar":"----","X-Spam_report":"(-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,\n DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1,\n RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,\n RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001,\n SPF_PASS=-0.001 autolearn=ham autolearn_force=no","X-Spam_action":"no action","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"qemu development ","List-Unsubscribe":",\n ","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org"}}]