From patchwork Thu Oct 12 14:06:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847482 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=oDV/3yxq; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=uIb+a9pm; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s0l5R0Pz1yqj for ; Fri, 13 Oct 2023 01:07:51 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwLb-0008Sc-Lz; Thu, 12 Oct 2023 10:07:32 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLA-0008LN-VX for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:06 -0400 Received: from smtp-out1.suse.de ([2001:67c:2178:6::1c]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwL5-0008ED-8F for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:02 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 0BE6C21870; Thu, 12 Oct 2023 14:06:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119617; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q4IScuvXsh8y/0/HKeuwuCl6yb+TmB3/Lm9GVpyLb4k=; b=oDV/3yxqOplJmtC0CXR7zx3d2i7Mp/AgSB1+GSYWWhEdL/Ih7zqx0U9bgEEwvnbgO3X1EW J2AyMV6uQpgYoeWwJgiB2SeSpY/Z417NeASDFDcDw87BG7wMelAfxkGG5PAiiXuOWP8j8V oYVXTH8aEl+NJ3t4x97xUiAI2cLmec0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119617; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q4IScuvXsh8y/0/HKeuwuCl6yb+TmB3/Lm9GVpyLb4k=; b=uIb+a9pmfqdeKkHtByeY0hDt/Hd92f3p4Rhzmle27Xszd3W18HsInVXEXV2z1SvxusLBAF DVpfjx1TS3B+CTBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 75155139ED; Thu, 12 Oct 2023 14:06:55 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id oGDRD3/9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:06:55 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 1/6] migration/multifd: Remove channels_ready semaphore Date: Thu, 12 Oct 2023 11:06:46 -0300 Message-Id: <20231012140651.13122-2-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out1.suse.de; none X-Spam-Score: 0.90 X-Spamd-Result: default: False [0.90 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[] Received-SPF: pass client-ip=2001:67c:2178:6::1c; envelope-from=farosas@suse.de; 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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The channels_ready semaphore is a global variable not linked to any single multifd channel. Waiting on it only means that "some" channel has become ready to send data. Since we need to address the channels by index (multifd_send_state->params[i]), that information adds nothing of value. The channel being addressed is not necessarily the one that just released the semaphore. The only usage of this semaphore that makes sense is to wait for it in a loop that iterates for the number of channels. That could mean: all channels have been setup and are operational OR all channels have finished their work and are idle. Currently all code that waits on channels_ready is redundant. There is always a subsequent lock or semaphore that does the actual data protection/synchronization. - at multifd_send_pages: Waiting on channels_ready doesn't mean the 'next_channel' is ready, it could be any other channel. So there are already cases where this code runs as if no semaphore was there. Waiting outside of the loop is also incorrect because if the current channel already has a pending_job, then it will loop into the next one without waiting the semaphore and the count will be greater than zero at the end of the execution. Checking that "any" channel is ready as a proxy for all channels being ready would work, but it's not what the code is doing and not really needed because the channel lock and 'sem' would be enough. - at multifd_send_sync: This usage is correct, but it is made redundant by the wait on sem_sync. What this piece of code is doing is making sure all channels have sent the SYNC packet and became idle afterwards. Signed-off-by: Fabiano Rosas --- migration/multifd.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 0f6b203877..e26f5f246d 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -362,8 +362,6 @@ struct { MultiFDPages_t *pages; /* global number of generated multifd packets */ uint64_t packet_num; - /* send channels ready */ - QemuSemaphore channels_ready; /* * Have we already run terminate threads. There is a race when it * happens that we got one error while we are exiting. @@ -403,7 +401,6 @@ static int multifd_send_pages(QEMUFile *f) return -1; } - qemu_sem_wait(&multifd_send_state->channels_ready); /* * next_channel can remain from a previous migration that was * using more channels, so ensure it doesn't overflow if the @@ -554,7 +551,6 @@ void multifd_save_cleanup(void) error_free(local_err); } } - qemu_sem_destroy(&multifd_send_state->channels_ready); g_free(multifd_send_state->params); multifd_send_state->params = NULL; multifd_pages_clear(multifd_send_state->pages); @@ -630,7 +626,6 @@ int multifd_send_sync_main(QEMUFile *f) for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; - qemu_sem_wait(&multifd_send_state->channels_ready); trace_multifd_send_sync_main_wait(p->id); qemu_sem_wait(&p->sem_sync); @@ -664,7 +659,6 @@ static void *multifd_send_thread(void *opaque) p->num_packets = 1; while (true) { - qemu_sem_post(&multifd_send_state->channels_ready); qemu_sem_wait(&p->sem); if (qatomic_read(&multifd_send_state->exiting)) { @@ -759,7 +753,6 @@ out: */ if (ret != 0) { qemu_sem_post(&p->sem_sync); - qemu_sem_post(&multifd_send_state->channels_ready); } qemu_mutex_lock(&p->mutex); @@ -796,7 +789,6 @@ static void multifd_tls_outgoing_handshake(QIOTask *task, * is not created, and then tell who pay attention to me. */ p->quit = true; - qemu_sem_post(&multifd_send_state->channels_ready); qemu_sem_post(&p->sem_sync); } } @@ -874,7 +866,6 @@ static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, { migrate_set_error(migrate_get_current(), err); /* Error happen, we need to tell who pay attention to me */ - qemu_sem_post(&multifd_send_state->channels_ready); qemu_sem_post(&p->sem_sync); /* * Although multifd_send_thread is not created, but main migration @@ -919,7 +910,6 @@ int multifd_save_setup(Error **errp) multifd_send_state = g_malloc0(sizeof(*multifd_send_state)); multifd_send_state->params = g_new0(MultiFDSendParams, thread_count); multifd_send_state->pages = multifd_pages_init(page_count); - qemu_sem_init(&multifd_send_state->channels_ready, 0); qatomic_set(&multifd_send_state->exiting, 0); multifd_send_state->ops = multifd_ops[migrate_multifd_compression()]; From patchwork Thu Oct 12 14:06:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847484 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=vkQWg1Lc; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=93okaGq1; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s121xcbz1yqj for ; Fri, 13 Oct 2023 01:08:06 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwLi-0000Ky-F3; Thu, 12 Oct 2023 10:07:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLA-0008LO-VK for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:06 -0400 Received: from smtp-out1.suse.de ([195.135.220.28]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwL6-0008EK-E8 for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:03 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id E304C21860; Thu, 12 Oct 2023 14:06:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119618; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=82J1vnQaM/EdCBU+c/qhxC6JWBgn08RqMNZW4Q/6FH4=; b=vkQWg1Lc9zSkCgqUlwka+WknzITqPB1/ehXDUC17b+0vDHp77i6wBtl8vbmUV7sFkUAOKo nVqZ52Y3kjCDMz4u2o3/5EMupGuGLkLmxsmWsM6ktoTV7qSeBxOWqruN13ZznNysA4LR28 BE7Sd4+sCQ3MVJxEbGk5Q8L7+tuMig0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119618; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=82J1vnQaM/EdCBU+c/qhxC6JWBgn08RqMNZW4Q/6FH4=; b=93okaGq17N+H2WdiRrc1ENp484TQPsk7n/mbvu+KWYXnzoJjwfjxN++em1HLe0DgGTin4R 6PWLex5wQB6CmbBw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 6A46E139ED; Thu, 12 Oct 2023 14:06:57 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 8EqZDYH9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:06:57 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 2/6] migration/multifd: Stop checking p->quit in multifd_send_thread Date: Thu, 12 Oct 2023 11:06:47 -0300 Message-Id: <20231012140651.13122-3-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out1.suse.de; none X-Spam-Score: 0.88 X-Spamd-Result: default: False [0.88 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-0.02)[51.65%] Received-SPF: pass client-ip=195.135.220.28; envelope-from=farosas@suse.de; 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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We don't need to check p->quit in the multifd_send_thread() because it is shadowed by the 'exiting' flag. Ever since that flag was added p->quit became obsolete as a way to stop the thread. Since p->quit is set at multifd_send_terminate_threads() under the p->mutex lock, the thread will only see it once it loops, so 'exiting' will always be seen first. Note that setting p->quit at multifd_send_terminate_threads() still makes sense because we need a way to inform multifd_send_pages() that the channel has stopped. Signed-off-by: Fabiano Rosas Reviewed-by: Juan Quintela --- migration/multifd.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index e26f5f246d..92ae61a50f 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -731,9 +731,6 @@ static void *multifd_send_thread(void *opaque) if (flags & MULTIFD_FLAG_SYNC) { qemu_sem_post(&p->sem_sync); } - } else if (p->quit) { - qemu_mutex_unlock(&p->mutex); - break; } else { qemu_mutex_unlock(&p->mutex); /* sometimes there are spurious wakeups */ From patchwork Thu Oct 12 14:06:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847485 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=nrXQXwUZ; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=knmpAnX8; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s1f3tfMz1yqj for ; Fri, 13 Oct 2023 01:08:38 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwM4-0000qP-3j; Thu, 12 Oct 2023 10:08:00 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLG-0008NQ-5E for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:11 -0400 Received: from smtp-out2.suse.de ([2001:67c:2178:6::1d]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwL9-0008EW-V2 for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:08 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id DE1691F74C; Thu, 12 Oct 2023 14:07:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119620; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VqP+ho12n+Y5hPTN0+FsvdUNCjTSc5IPV+iWy1nErzs=; b=nrXQXwUZI6YL6ngTRXd+H7dGLnDBIVfiHvl9PqbDpzCeZCLM4ifvrdUnDyt/MDVd0FwQ3m GY7sMF6BdOBITHNqUIPP9cAm8apjDshtx+tlptkfdOHkyrKPmavP9NudGKmwuljJv17xVG A0O3mjG0PlaXEoNp986OOih8FcUFbTc= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119620; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VqP+ho12n+Y5hPTN0+FsvdUNCjTSc5IPV+iWy1nErzs=; b=knmpAnX8kyfuV1hXJjHURLDUraf2xIMSomWjG8Kmj4Nf8jVZN4t/p6dSJiZyDgD69BsQcv RNT02rYXDAHe1LCQ== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 5BF04139ED; Thu, 12 Oct 2023 14:06:59 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 8NzsCYP9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:06:59 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 3/6] migration/multifd: Decouple control flow from the SYNC packet Date: Thu, 12 Oct 2023 11:06:48 -0300 Message-Id: <20231012140651.13122-4-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out2.suse.de; none X-Spam-Score: -2.10 X-Spamd-Result: default: False [-2.10 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] Received-SPF: pass client-ip=2001:67c:2178:6::1d; envelope-from=farosas@suse.de; helo=smtp-out2.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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We currently use the 'sem_sync' semaphore on the sending side: 1) to know when the multifd_send_thread() has finished sending the MULTIFD_FLAG_SYNC packet; This is unnecessary. Multifd sends packets one by one and completion is already bound by the 'sem' semaphore. The SYNC packet has nothing special that would require it to have a separate semaphore on the sending side. 2) to wait for the multifd threads to finish before cleaning up; This happens because multifd_send_sync_main() blocks ram_save_complete() from finishing until the semaphore is posted. This is surprising and not documented. Clarify the above situation by renaming 'sem_sync' to 'sem_done' and making the #2 usage the main one. Post to 'sem_sync' only when there's no more pending_jobs. Signed-off-by: Fabiano Rosas --- I remove the parts about the receiving side. I wasn't sure about them and we don't need to mix the two. Potentially we need the sem_sync on the recv to ensure all channels wait before becoming available to read once again after a FLUSH. --- migration/multifd.c | 76 ++++++++++++++++++++++++------------------ migration/multifd.h | 4 +-- migration/trace-events | 2 +- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 92ae61a50f..94f4ae5ff8 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -533,7 +533,7 @@ void multifd_save_cleanup(void) p->c = NULL; qemu_mutex_destroy(&p->mutex); qemu_sem_destroy(&p->sem); - qemu_sem_destroy(&p->sem_sync); + qemu_sem_destroy(&p->sem_done); g_free(p->name); p->name = NULL; multifd_pages_clear(p->pages); @@ -591,6 +591,44 @@ int multifd_send_sync_main(QEMUFile *f) } } + for (i = 0; i < migrate_multifd_channels(); i++) { + MultiFDSendParams *p = &multifd_send_state->params[i]; + + trace_multifd_send_sync_main_signal(p->id); + + qemu_mutex_lock(&p->mutex); + + if (p->quit) { + error_report("%s: channel %d has already quit", __func__, i); + qemu_mutex_unlock(&p->mutex); + return -1; + } + + p->packet_num = multifd_send_state->packet_num++; + p->flags |= MULTIFD_FLAG_SYNC; + p->pending_job++; + qemu_mutex_unlock(&p->mutex); + qemu_sem_post(&p->sem); + } + + /* wait for all channels to be idle */ + for (i = 0; i < migrate_multifd_channels(); i++) { + MultiFDSendParams *p = &multifd_send_state->params[i]; + + /* + * Even idle channels will wait for p->sem at the top of the + * loop. + */ + qemu_sem_post(&p->sem); + + trace_multifd_send_wait(migrate_multifd_channels() - i); + qemu_sem_wait(&p->sem_done); + + qemu_mutex_lock(&p->mutex); + assert(!p->pending_job || p->quit); + qemu_mutex_unlock(&p->mutex); + } + /* * When using zero-copy, it's necessary to flush the pages before any of * the pages can be sent again, so we'll make sure the new version of the @@ -601,34 +639,11 @@ int multifd_send_sync_main(QEMUFile *f) * to be less frequent, e.g. only after we finished one whole scanning of * all the dirty bitmaps. */ - flush_zero_copy = migrate_zero_copy_send(); for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; - trace_multifd_send_sync_main_signal(p->id); - - qemu_mutex_lock(&p->mutex); - - if (p->quit) { - error_report("%s: channel %d has already quit", __func__, i); - qemu_mutex_unlock(&p->mutex); - return -1; - } - - p->packet_num = multifd_send_state->packet_num++; - p->flags |= MULTIFD_FLAG_SYNC; - p->pending_job++; - qemu_mutex_unlock(&p->mutex); - qemu_sem_post(&p->sem); - } - for (i = 0; i < migrate_multifd_channels(); i++) { - MultiFDSendParams *p = &multifd_send_state->params[i]; - - trace_multifd_send_sync_main_wait(p->id); - qemu_sem_wait(&p->sem_sync); - if (flush_zero_copy && p->c && (multifd_zero_copy_flush(p->c) < 0)) { return -1; } @@ -728,12 +743,9 @@ static void *multifd_send_thread(void *opaque) p->pending_job--; qemu_mutex_unlock(&p->mutex); - if (flags & MULTIFD_FLAG_SYNC) { - qemu_sem_post(&p->sem_sync); - } } else { qemu_mutex_unlock(&p->mutex); - /* sometimes there are spurious wakeups */ + qemu_sem_post(&p->sem_done); } } @@ -749,7 +761,7 @@ out: * who pay attention to me. */ if (ret != 0) { - qemu_sem_post(&p->sem_sync); + qemu_sem_post(&p->sem_done); } qemu_mutex_lock(&p->mutex); @@ -786,7 +798,7 @@ static void multifd_tls_outgoing_handshake(QIOTask *task, * is not created, and then tell who pay attention to me. */ p->quit = true; - qemu_sem_post(&p->sem_sync); + qemu_sem_post(&p->sem_done); } } @@ -863,7 +875,7 @@ static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, { migrate_set_error(migrate_get_current(), err); /* Error happen, we need to tell who pay attention to me */ - qemu_sem_post(&p->sem_sync); + qemu_sem_post(&p->sem_done); /* * Although multifd_send_thread is not created, but main migration * thread need to judge whether it is running, so we need to mark @@ -915,7 +927,7 @@ int multifd_save_setup(Error **errp) qemu_mutex_init(&p->mutex); qemu_sem_init(&p->sem, 0); - qemu_sem_init(&p->sem_sync, 0); + qemu_sem_init(&p->sem_done, 0); p->quit = false; p->pending_job = 0; p->id = i; diff --git a/migration/multifd.h b/migration/multifd.h index a835643b48..71bd66974d 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -90,8 +90,8 @@ typedef struct { /* sem where to wait for more work */ QemuSemaphore sem; - /* syncs main thread and channels */ - QemuSemaphore sem_sync; + /* channel is done transmitting until more pages are queued */ + QemuSemaphore sem_done; /* this mutex protects the following parameters */ QemuMutex mutex; diff --git a/migration/trace-events b/migration/trace-events index ee9c8f4d63..3aef79a951 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -137,7 +137,7 @@ multifd_send(uint8_t id, uint64_t packet_num, uint32_t normal, uint32_t flags, u multifd_send_error(uint8_t id) "channel %u" multifd_send_sync_main(long packet_num) "packet num %ld" multifd_send_sync_main_signal(uint8_t id) "channel %u" -multifd_send_sync_main_wait(uint8_t id) "channel %u" +multifd_send_wait(uint8_t n) "waiting for %u channels to finish sending" multifd_send_terminate_threads(bool error) "error %d" multifd_send_thread_end(uint8_t id, uint64_t packets, uint64_t normal_pages) "channel %u packets %" PRIu64 " normal pages %" PRIu64 multifd_send_thread_start(uint8_t id) "%u" From patchwork Thu Oct 12 14:06:49 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847480 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=OQ0p2Lna; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=ZxHgu9ox; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s0c3Rxpz1yqj for ; Fri, 13 Oct 2023 01:07:44 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwLj-0000Mh-1Z; Thu, 12 Oct 2023 10:07:39 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLE-0008NE-K6 for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:09 -0400 Received: from smtp-out1.suse.de ([195.135.220.28]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwLA-0008Ej-Jx for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:08 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id BD5B121878; Thu, 12 Oct 2023 14:07:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119622; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Oz4n+qj236YL1a8qzoYEdHstGZkz+mRU8otmuN/uam0=; b=OQ0p2LnazH7o6j9d2wDB+av+p03ARYymqi8eMvJjhLaF10D8wwwVGGdEOV7NX2oWv4dyWz rmT6sbv/+SN6MXzBbLkI7VjQPVusdAFLqSONsnJ9Fo21FEJyEV4DQHjJj6AOMPPFlkTozh 5Hmgd4ffjnRoUDz8b+gCrCn1lHTi78k= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119622; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Oz4n+qj236YL1a8qzoYEdHstGZkz+mRU8otmuN/uam0=; b=ZxHgu9oxtfTNq11MXbcrf8nhPuYaKUzeBW0whSxBON4x5uDBg8zG9Q2jQDMUf5waDK4vwI 5YTCPEjiEhE2EIAA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 453A4139ED; Thu, 12 Oct 2023 14:07:01 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id wFdmBIX9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:07:01 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 4/6] migration/multifd: Extract sem_done waiting into a function Date: Thu, 12 Oct 2023 11:06:49 -0300 Message-Id: <20231012140651.13122-5-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out1.suse.de; none X-Spam-Score: 0.22 X-Spamd-Result: default: False [0.22 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-0.68)[83.12%] Received-SPF: pass client-ip=195.135.220.28; envelope-from=farosas@suse.de; 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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org This helps document the intent of the loop via the function name and we can reuse this in the future. Signed-off-by: Fabiano Rosas --- migration/multifd.c | 47 +++++++++++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 17 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 94f4ae5ff8..4ffa67339c 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -576,6 +576,35 @@ static int multifd_zero_copy_flush(QIOChannel *c) return ret; } +static void multifd_send_wait(void) +{ + int i; + + /* wait for all channels to be idle */ + for (i = 0; i < migrate_multifd_channels(); i++) { + MultiFDSendParams *p = &multifd_send_state->params[i]; + + /* + * Even idle channels will wait for p->sem at the top of the + * loop. + */ + qemu_sem_post(&p->sem); + + trace_multifd_send_wait(migrate_multifd_channels() - i); + qemu_sem_wait(&p->sem_done); + + qemu_mutex_lock(&p->mutex); + assert(!p->pending_job || p->quit); + qemu_mutex_unlock(&p->mutex); + } + + /* + * All channels went idle and have no more jobs. Unless we send + * them more work, we're good to allow any cleanup code to run at + * this point. + */ +} + int multifd_send_sync_main(QEMUFile *f) { int i; @@ -611,23 +640,7 @@ int multifd_send_sync_main(QEMUFile *f) qemu_sem_post(&p->sem); } - /* wait for all channels to be idle */ - for (i = 0; i < migrate_multifd_channels(); i++) { - MultiFDSendParams *p = &multifd_send_state->params[i]; - - /* - * Even idle channels will wait for p->sem at the top of the - * loop. - */ - qemu_sem_post(&p->sem); - - trace_multifd_send_wait(migrate_multifd_channels() - i); - qemu_sem_wait(&p->sem_done); - - qemu_mutex_lock(&p->mutex); - assert(!p->pending_job || p->quit); - qemu_mutex_unlock(&p->mutex); - } + multifd_send_wait(); /* * When using zero-copy, it's necessary to flush the pages before any of From patchwork Thu Oct 12 14:06:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847483 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=a7+T/PhM; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=oxm+FYsF; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s0n4ZVqz1yqj for ; Fri, 13 Oct 2023 01:07:53 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwLm-0000WU-Mo; Thu, 12 Oct 2023 10:07:42 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLE-0008NL-Ux for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:09 -0400 Received: from smtp-out2.suse.de ([195.135.220.29]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwLB-0008F4-RR for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:08 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A8D331F898; Thu, 12 Oct 2023 14:07:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119624; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DJwOf25AFLkGxrZXTydCy3mSdIODrn/xWE+T4mZJn4A=; b=a7+T/PhM/Xf2uuLiW0hiZd1lTouZD8ut6rpWQzb/iuCIZHX/srid0i6VIUqM8FN9d8UTCd 08sRyIeGC1wIQBX5BI4u+P1M4Li1EWQeIj8LAerrnfa15ngLUIu4myd/dibgB4TJikRHG5 W7jjF2B1OOaNFYWKe7KgyAJGIRiHL7k= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119624; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=DJwOf25AFLkGxrZXTydCy3mSdIODrn/xWE+T4mZJn4A=; b=oxm+FYsF+mC+rv2FoA5M8MBRvxMWDwKVH5nwMB0riGB5Qmu6P8GZ2BLHJ8Dh63oeTlSdHd vZ/8lI2kxq2wWbAg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 319CD139ED; Thu, 12 Oct 2023 14:07:02 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id sEA9O4b9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:07:02 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 5/6] migration/multifd: Stop setting 'quit' outside of channels Date: Thu, 12 Oct 2023 11:06:50 -0300 Message-Id: <20231012140651.13122-6-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out2.suse.de; none X-Spam-Score: 0.90 X-Spamd-Result: default: False [0.90 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-0.00)[30.55%] Received-SPF: pass client-ip=195.135.220.29; envelope-from=farosas@suse.de; helo=smtp-out2.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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We shouldn't really be touching channel state from outside the channels except for the transfer of pages. Move the setting of p->quit into the channel. Keep posting the 'sem' from multifd_send_terminate_threads() so any channel waiting on the semaphore will unblock and see the 'exiting' flag and quit by itself. Signed-off-by: Fabiano Rosas --- migration/multifd.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index 4ffa67339c..b7ba3fe0e6 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -497,13 +497,11 @@ static void multifd_send_terminate_threads(Error *err) for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; - qemu_mutex_lock(&p->mutex); - p->quit = true; + /* kick the channel if it was waiting for work */ qemu_sem_post(&p->sem); if (p->c) { qio_channel_shutdown(p->c, QIO_CHANNEL_SHUTDOWN_BOTH, NULL); } - qemu_mutex_unlock(&p->mutex); } } @@ -690,6 +688,9 @@ static void *multifd_send_thread(void *opaque) qemu_sem_wait(&p->sem); if (qatomic_read(&multifd_send_state->exiting)) { + qemu_mutex_lock(&p->mutex); + p->quit = true; + qemu_mutex_unlock(&p->mutex); break; } qemu_mutex_lock(&p->mutex); @@ -765,6 +766,11 @@ static void *multifd_send_thread(void *opaque) out: if (local_err) { trace_multifd_send_error(p->id); + + qemu_mutex_lock(&p->mutex); + p->quit = true; + qemu_mutex_unlock(&p->mutex); + multifd_send_terminate_threads(local_err); error_free(local_err); } From patchwork Thu Oct 12 14:06:51 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fabiano Rosas X-Patchwork-Id: 1847486 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256 header.s=susede2_rsa header.b=iUS/ly2k; dkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=PYRFIJYy; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=nongnu.org (client-ip=209.51.188.17; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=patchwork.ozlabs.org) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S5s1s4ptwz1yqj for ; Fri, 13 Oct 2023 01:08:49 +1100 (AEDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qqwM5-0000xu-3x; Thu, 12 Oct 2023 10:08:01 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qqwLG-0008Ns-32 for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:11 -0400 Received: from smtp-out2.suse.de ([195.135.220.29]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qqwLE-0008FM-2f for qemu-devel@nongnu.org; Thu, 12 Oct 2023 10:07:09 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 971121F896; Thu, 12 Oct 2023 14:07:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1697119626; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=++Is/2mFziUP/EUP+aPGrjXPSq6ScRThkQMj6DrB2oc=; b=iUS/ly2k+F/WDgx53vDoj2n0UdPzCP5IBI+aLLr6ZSm4hT0PkQaiJwPoMsNEX+vDFx0c/F HWO8QXpQ+S6HM+L4JQmpwFggE7ijpxjocscSU1odFg4oqSF8ldu4x6rJaTdGlYY+iO879v S6nOBBo+68TfdQufVmyGa79Y5hKDVII= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1697119626; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=++Is/2mFziUP/EUP+aPGrjXPSq6ScRThkQMj6DrB2oc=; b=PYRFIJYyjc7uzhJaC+II9ppeRcUNG01hju5QCwdKQ073iwRHa3liinWhGtFg6NwbFmH/H/ YxbgfGkteLisggCg== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 204B9139ED; Thu, 12 Oct 2023 14:07:04 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id EFEmN4j9J2U5CgAAMHmgww (envelope-from ); Thu, 12 Oct 2023 14:07:04 +0000 From: Fabiano Rosas To: qemu-devel@nongnu.org Cc: Juan Quintela , Peter Xu , Leonardo Bras , Elena Ufimtseva Subject: [RFC PATCH v2 6/6] migration/multifd: Bring back the 'ready' semaphore Date: Thu, 12 Oct 2023 11:06:51 -0300 Message-Id: <20231012140651.13122-7-farosas@suse.de> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20231012140651.13122-1-farosas@suse.de> References: <20231012140651.13122-1-farosas@suse.de> MIME-Version: 1.0 Authentication-Results: smtp-out2.suse.de; none X-Spam-Score: -2.10 X-Spamd-Result: default: False [-2.10 / 50.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; R_MISSING_CHARSET(2.50)[]; MIME_GOOD(-0.10)[text/plain]; BROKEN_CONTENT_TYPE(1.50)[]; RCPT_COUNT_FIVE(0.00)[5]; NEURAL_HAM_LONG(-3.00)[-1.000]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MID_CONTAINS_FROM(1.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; BAYES_HAM(-3.00)[100.00%] Received-SPF: pass client-ip=195.135.220.29; envelope-from=farosas@suse.de; helo=smtp-out2.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, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, 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: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Bring back the 'ready' semaphore, but this time make it per-channel, so that we can do true lockstep switching of MultiFDPages without taking the channel lock. Drop the channel lock as it now becomes useless. The rules for accessing the MultiFDSendParams are: - between sem and sem_done/ready if we're the channel qemu_sem_post(&p->ready); qemu_sem_wait(&p->sem); qemu_sem_post(&p->sem_done); - between ready and sem if we're not the channel qemu_sem_wait(&p->ready); qemu_sem_post(&p->sem); Signed-off-by: Fabiano Rosas --- One issue I can see with this is that we might now have to wait at multifd_send_pages() if a channel takes too long to send it's packet and come back to p->ready. We would need to find a way of ignoring a slow channel and skipping ahead to the next one in line. --- migration/multifd.c | 45 +++++++++++++-------------------------------- migration/multifd.h | 5 ++--- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/migration/multifd.c b/migration/multifd.c index b7ba3fe0e6..7fa7bc33fd 100644 --- a/migration/multifd.c +++ b/migration/multifd.c @@ -410,10 +410,10 @@ static int multifd_send_pages(QEMUFile *f) for (i = next_channel;; i = (i + 1) % migrate_multifd_channels()) { p = &multifd_send_state->params[i]; - qemu_mutex_lock(&p->mutex); + qemu_sem_wait(&p->ready); + if (p->quit) { error_report("%s: channel %d has already quit!", __func__, i); - qemu_mutex_unlock(&p->mutex); return -1; } if (!p->pending_job) { @@ -421,7 +421,6 @@ static int multifd_send_pages(QEMUFile *f) next_channel = (i + 1) % migrate_multifd_channels(); break; } - qemu_mutex_unlock(&p->mutex); } assert(!p->pages->num); assert(!p->pages->block); @@ -429,7 +428,6 @@ static int multifd_send_pages(QEMUFile *f) p->packet_num = multifd_send_state->packet_num++; multifd_send_state->pages = p->pages; p->pages = pages; - qemu_mutex_unlock(&p->mutex); qemu_sem_post(&p->sem); return 1; @@ -529,9 +527,9 @@ void multifd_save_cleanup(void) } socket_send_channel_destroy(p->c); p->c = NULL; - qemu_mutex_destroy(&p->mutex); qemu_sem_destroy(&p->sem); qemu_sem_destroy(&p->sem_done); + qemu_sem_destroy(&p->ready); g_free(p->name); p->name = NULL; multifd_pages_clear(p->pages); @@ -586,14 +584,12 @@ static void multifd_send_wait(void) * Even idle channels will wait for p->sem at the top of the * loop. */ + qemu_sem_wait(&p->ready); qemu_sem_post(&p->sem); trace_multifd_send_wait(migrate_multifd_channels() - i); qemu_sem_wait(&p->sem_done); - - qemu_mutex_lock(&p->mutex); assert(!p->pending_job || p->quit); - qemu_mutex_unlock(&p->mutex); } /* @@ -621,20 +617,17 @@ int multifd_send_sync_main(QEMUFile *f) for (i = 0; i < migrate_multifd_channels(); i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; + qemu_sem_wait(&p->ready); trace_multifd_send_sync_main_signal(p->id); - qemu_mutex_lock(&p->mutex); - if (p->quit) { error_report("%s: channel %d has already quit", __func__, i); - qemu_mutex_unlock(&p->mutex); return -1; } p->packet_num = multifd_send_state->packet_num++; p->flags |= MULTIFD_FLAG_SYNC; p->pending_job++; - qemu_mutex_unlock(&p->mutex); qemu_sem_post(&p->sem); } @@ -685,15 +678,14 @@ static void *multifd_send_thread(void *opaque) p->num_packets = 1; while (true) { + qemu_sem_post(&p->ready); qemu_sem_wait(&p->sem); if (qatomic_read(&multifd_send_state->exiting)) { - qemu_mutex_lock(&p->mutex); p->quit = true; - qemu_mutex_unlock(&p->mutex); + qemu_sem_post(&p->sem_done); break; } - qemu_mutex_lock(&p->mutex); if (p->pending_job) { uint64_t packet_num = p->packet_num; @@ -714,7 +706,6 @@ static void *multifd_send_thread(void *opaque) if (p->normal_num) { ret = multifd_send_state->ops->send_prepare(p, &local_err); if (ret != 0) { - qemu_mutex_unlock(&p->mutex); break; } } @@ -725,7 +716,6 @@ static void *multifd_send_thread(void *opaque) p->total_normal_pages += p->normal_num; p->pages->num = 0; p->pages->block = NULL; - qemu_mutex_unlock(&p->mutex); trace_multifd_send(p->id, packet_num, p->normal_num, flags, p->next_packet_size); @@ -753,12 +743,9 @@ static void *multifd_send_thread(void *opaque) stat64_add(&mig_stats.multifd_bytes, p->next_packet_size); stat64_add(&mig_stats.transferred, p->next_packet_size); - qemu_mutex_lock(&p->mutex); p->pending_job--; - qemu_mutex_unlock(&p->mutex); } else { - qemu_mutex_unlock(&p->mutex); qemu_sem_post(&p->sem_done); } } @@ -766,11 +753,8 @@ static void *multifd_send_thread(void *opaque) out: if (local_err) { trace_multifd_send_error(p->id); - - qemu_mutex_lock(&p->mutex); p->quit = true; - qemu_mutex_unlock(&p->mutex); - + qemu_sem_post(&p->ready); multifd_send_terminate_threads(local_err); error_free(local_err); } @@ -780,12 +764,10 @@ out: * who pay attention to me. */ if (ret != 0) { - qemu_sem_post(&p->sem_done); + p->quit = true; + qemu_sem_post(&p->ready); } - - qemu_mutex_lock(&p->mutex); p->running = false; - qemu_mutex_unlock(&p->mutex); rcu_unregister_thread(); migration_threads_remove(thread); @@ -817,7 +799,7 @@ static void multifd_tls_outgoing_handshake(QIOTask *task, * is not created, and then tell who pay attention to me. */ p->quit = true; - qemu_sem_post(&p->sem_done); + qemu_sem_post(&p->ready); } } @@ -893,14 +875,13 @@ static void multifd_new_send_channel_cleanup(MultiFDSendParams *p, QIOChannel *ioc, Error *err) { migrate_set_error(migrate_get_current(), err); - /* Error happen, we need to tell who pay attention to me */ - qemu_sem_post(&p->sem_done); /* * Although multifd_send_thread is not created, but main migration * thread need to judge whether it is running, so we need to mark * its status. */ p->quit = true; + qemu_sem_post(&p->ready); object_unref(OBJECT(ioc)); error_free(err); } @@ -944,9 +925,9 @@ int multifd_save_setup(Error **errp) for (i = 0; i < thread_count; i++) { MultiFDSendParams *p = &multifd_send_state->params[i]; - qemu_mutex_init(&p->mutex); qemu_sem_init(&p->sem, 0); qemu_sem_init(&p->sem_done, 0); + qemu_sem_init(&p->ready, 0); p->quit = false; p->pending_job = 0; p->id = i; diff --git a/migration/multifd.h b/migration/multifd.h index 71bd66974d..6bb10b07aa 100644 --- a/migration/multifd.h +++ b/migration/multifd.h @@ -93,8 +93,8 @@ typedef struct { /* channel is done transmitting until more pages are queued */ QemuSemaphore sem_done; - /* this mutex protects the following parameters */ - QemuMutex mutex; + QemuSemaphore ready; + /* is this channel thread running */ bool running; /* should this thread finish */ @@ -209,4 +209,3 @@ typedef struct { void multifd_register_ops(int method, MultiFDMethods *ops); #endif -