From patchwork Tue Jun 8 07:33:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1489142 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=RUbvYxaO; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Fzhp51wYrz9sW6 for ; Tue, 8 Jun 2021 17:34:36 +1000 (AEST) Received: from localhost ([::1]:35158 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lqWFu-000371-8d for incoming@patchwork.ozlabs.org; Tue, 08 Jun 2021 03:34:34 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51646) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFK-00033r-OH for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:33:58 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:38636) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFG-00056a-An for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:33:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623137633; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=jLsc5GbcLneb2563Wriv6NoNTNHHrrCGWGrRkTihdOc=; b=RUbvYxaOXHPolZk3NFdPyrY6FztYxVG4yuOC9iRj0p5P9JqKE0QRXnYFRNG4BGKPbypybw QwxEjMrNhi+s08AVrajgqEeJNOaIKMuelSsiZ9g167tqyW6004Oh4zZckAav+5CZlon6H+ Aw5r2vT5EvNFa57tpsgOanADbpaTLv8= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-269-Rf9p69omP7qT7lS6QvVcHA-1; Tue, 08 Jun 2021 03:33:51 -0400 X-MC-Unique: Rf9p69omP7qT7lS6QvVcHA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8A94B9126B; Tue, 8 Jun 2021 07:33:50 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id 62E8160CCF; Tue, 8 Jun 2021 07:33:48 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH v3 1/5] block-copy: streamline choice of copy_range vs. read/write Date: Tue, 8 Jun 2021 09:33:40 +0200 Message-Id: <20210608073344.53637-2-eesposit@redhat.com> In-Reply-To: <20210608073344.53637-1-eesposit@redhat.com> References: <20210608073344.53637-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -29 X-Spam_score: -3.0 X-Spam_bar: --- X-Spam_report: (-3.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.2, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Emanuele Giuseppe Esposito , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" From: Paolo Bonzini Put the logic to determine the copy size in a separate function, so that there is a simple state machine for the possible methods of copying data from one BlockDriverState to the other. Use .method instead of .copy_range as in-out argument, and include also .zeroes as an additional copy method. While at it, store the common computation of block_copy_max_transfer into a new field of BlockCopyState, and make sure that we always obey max_transfer; that's more efficient even for the COPY_RANGE_READ_WRITE case. Signed-off-by: Emanuele Giuseppe Esposito Signed-off-by: Paolo Bonzini --- block/block-copy.c | 171 ++++++++++++++++++++++----------------------- 1 file changed, 85 insertions(+), 86 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 943e30b7e6..d58051288b 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -28,6 +28,14 @@ #define BLOCK_COPY_MAX_WORKERS 64 #define BLOCK_COPY_SLICE_TIME 100000000ULL /* ns */ +typedef enum { + COPY_READ_WRITE_CLUSTER, + COPY_READ_WRITE, + COPY_WRITE_ZEROES, + COPY_RANGE_SMALL, + COPY_RANGE_FULL +} BlockCopyMethod; + static coroutine_fn int block_copy_task_entry(AioTask *task); typedef struct BlockCopyCallState { @@ -64,8 +72,7 @@ typedef struct BlockCopyTask { BlockCopyCallState *call_state; int64_t offset; int64_t bytes; - bool zeroes; - bool copy_range; + BlockCopyMethod method; QLIST_ENTRY(BlockCopyTask) list; CoQueue wait_queue; /* coroutines blocked on this task */ } BlockCopyTask; @@ -86,8 +93,8 @@ typedef struct BlockCopyState { BdrvDirtyBitmap *copy_bitmap; int64_t in_flight_bytes; int64_t cluster_size; - bool use_copy_range; - int64_t copy_size; + BlockCopyMethod method; + int64_t max_transfer; uint64_t len; QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ QLIST_HEAD(, BlockCopyCallState) calls; @@ -149,6 +156,24 @@ static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset, return true; } +static int64_t block_copy_chunk_size(BlockCopyState *s) +{ + switch (s->method) { + case COPY_READ_WRITE_CLUSTER: + return s->cluster_size; + case COPY_READ_WRITE: + case COPY_RANGE_SMALL: + return MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER), + s->max_transfer); + case COPY_RANGE_FULL: + return MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE), + s->max_transfer); + default: + /* Cannot have COPY_WRITE_ZEROES here. */ + abort(); + } +} + /* * Search for the first dirty area in offset/bytes range and create task at * the beginning of it. @@ -158,8 +183,9 @@ static BlockCopyTask *block_copy_task_create(BlockCopyState *s, int64_t offset, int64_t bytes) { BlockCopyTask *task; - int64_t max_chunk = MIN_NON_ZERO(s->copy_size, call_state->max_chunk); + int64_t max_chunk = block_copy_chunk_size(s); + max_chunk = MIN_NON_ZERO(max_chunk, call_state->max_chunk); if (!bdrv_dirty_bitmap_next_dirty_area(s->copy_bitmap, offset, offset + bytes, max_chunk, &offset, &bytes)) @@ -183,7 +209,7 @@ static BlockCopyTask *block_copy_task_create(BlockCopyState *s, .call_state = call_state, .offset = offset, .bytes = bytes, - .copy_range = s->use_copy_range, + .method = s->method, }; qemu_co_queue_init(&task->wait_queue); QLIST_INSERT_HEAD(&s->tasks, task, list); @@ -267,28 +293,27 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, .len = bdrv_dirty_bitmap_size(copy_bitmap), .write_flags = write_flags, .mem = shres_create(BLOCK_COPY_MAX_MEM), + .max_transfer = QEMU_ALIGN_DOWN(block_copy_max_transfer(source, target) + , cluster_size), }; - if (block_copy_max_transfer(source, target) < cluster_size) { + if (s->max_transfer < cluster_size) { /* * copy_range does not respect max_transfer. We don't want to bother * with requests smaller than block-copy cluster size, so fallback to * buffered copying (read and write respect max_transfer on their * behalf). */ - s->use_copy_range = false; - s->copy_size = cluster_size; + s->method = COPY_READ_WRITE_CLUSTER; } else if (write_flags & BDRV_REQ_WRITE_COMPRESSED) { /* Compression supports only cluster-size writes and no copy-range. */ - s->use_copy_range = false; - s->copy_size = cluster_size; + s->method = COPY_READ_WRITE_CLUSTER; } else { /* * We enable copy-range, but keep small copy_size, until first * successful copy_range (look at block_copy_do_copy). */ - s->use_copy_range = use_copy_range; - s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER); + s->method = use_copy_range ? COPY_RANGE_SMALL : COPY_READ_WRITE; } ratelimit_init(&s->rate_limit); @@ -343,17 +368,14 @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool, * * No sync here: nor bitmap neighter intersecting requests handling, only copy. * - * @copy_range is an in-out argument: if *copy_range is false, copy_range is not - * done. If *copy_range is true, copy_range is attempted. If the copy_range - * attempt fails, the function falls back to the usual read+write and - * *copy_range is set to false. *copy_range and zeroes must not be true - * simultaneously. - * + * @method is an in-out argument, so that copy_range can be either extended to + * a full-size buffer or disabled if the copy_range attempt fails. The output + * value of @method should be used for subsequent tasks. * Returns 0 on success. */ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, int64_t offset, int64_t bytes, - bool zeroes, bool *copy_range, + BlockCopyMethod *method, bool *error_is_read) { int ret; @@ -367,9 +389,9 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, assert(offset + bytes <= s->len || offset + bytes == QEMU_ALIGN_UP(s->len, s->cluster_size)); assert(nbytes < INT_MAX); - assert(!(*copy_range && zeroes)); - if (zeroes) { + switch (*method) { + case COPY_WRITE_ZEROES: ret = bdrv_co_pwrite_zeroes(s->target, offset, nbytes, s->write_flags & ~BDRV_REQ_WRITE_COMPRESSED); if (ret < 0) { @@ -377,89 +399,67 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, *error_is_read = false; } return ret; - } - if (*copy_range) { + case COPY_RANGE_SMALL: + case COPY_RANGE_FULL: ret = bdrv_co_copy_range(s->source, offset, s->target, offset, nbytes, 0, s->write_flags); - if (ret < 0) { - trace_block_copy_copy_range_fail(s, offset, ret); - *copy_range = false; - /* Fallback to read+write with allocated buffer */ - } else { + if (ret >= 0) { + /* Successful copy-range, increase copy_size. */ + *method = COPY_RANGE_FULL; return 0; } - } - - /* - * In case of failed copy_range request above, we may proceed with buffered - * request larger than BLOCK_COPY_MAX_BUFFER. Still, further requests will - * be properly limited, so don't care too much. Moreover the most likely - * case (copy_range is unsupported for the configuration, so the very first - * copy_range request fails) is handled by setting large copy_size only - * after first successful copy_range. - */ - bounce_buffer = qemu_blockalign(s->source->bs, nbytes); + trace_block_copy_copy_range_fail(s, offset, ret); + *method = COPY_READ_WRITE; + /* Fall through to read+write with allocated buffer */ - ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0); - if (ret < 0) { - trace_block_copy_read_fail(s, offset, ret); - *error_is_read = true; - goto out; - } + default: + /* + * In case of failed copy_range request above, we may proceed with + * buffered request larger than BLOCK_COPY_MAX_BUFFER. + * Still, further requests will be properly limited, so don't care too + * much. Moreover the most likely case (copy_range is unsupported for + * the configuration, so the very first copy_range request fails) + * is handled by setting large copy_size only after first successful + * copy_range. + */ - ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer, - s->write_flags); - if (ret < 0) { - trace_block_copy_write_fail(s, offset, ret); - *error_is_read = false; - goto out; - } + bounce_buffer = qemu_blockalign(s->source->bs, nbytes); -out: - qemu_vfree(bounce_buffer); + ret = bdrv_co_pread(s->source, offset, nbytes, bounce_buffer, 0); + if (ret < 0) { + trace_block_copy_read_fail(s, offset, ret); + *error_is_read = true; + goto out; + } - return ret; -} + ret = bdrv_co_pwrite(s->target, offset, nbytes, bounce_buffer, + s->write_flags); + if (ret < 0) { + trace_block_copy_write_fail(s, offset, ret); + *error_is_read = false; + goto out; + } -static void block_copy_handle_copy_range_result(BlockCopyState *s, - bool is_success) -{ - if (!s->use_copy_range) { - /* already disabled */ - return; +out: + qemu_vfree(bounce_buffer); } - if (is_success) { - /* - * Successful copy-range. Now increase copy_size. copy_range - * does not respect max_transfer (it's a TODO), so we factor - * that in here. - */ - s->copy_size = - MIN(MAX(s->cluster_size, BLOCK_COPY_MAX_COPY_RANGE), - QEMU_ALIGN_DOWN(block_copy_max_transfer(s->source, - s->target), - s->cluster_size)); - } else { - /* Copy-range failed, disable it. */ - s->use_copy_range = false; - s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER); - } + return ret; } static coroutine_fn int block_copy_task_entry(AioTask *task) { BlockCopyTask *t = container_of(task, BlockCopyTask, task); + BlockCopyState *s = t->s; bool error_is_read = false; - bool copy_range = t->copy_range; + BlockCopyMethod method = t->method; int ret; - ret = block_copy_do_copy(t->s, t->offset, t->bytes, t->zeroes, - ©_range, &error_is_read); - if (t->copy_range) { - block_copy_handle_copy_range_result(t->s, copy_range); + ret = block_copy_do_copy(s, t->offset, t->bytes, &method, &error_is_read); + if (s->method == t->method) { + s->method = method; } if (ret < 0) { if (!t->call_state->ret) { @@ -642,8 +642,7 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) continue; } if (ret & BDRV_BLOCK_ZERO) { - task->zeroes = true; - task->copy_range = false; + task->method = COPY_WRITE_ZEROES; } if (!call_state->ignore_ratelimit) { From patchwork Tue Jun 8 07:33:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1489144 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=aM1idbmL; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Fzhqn63CXz9s24 for ; Tue, 8 Jun 2021 17:36:05 +1000 (AEST) Received: from localhost ([::1]:42090 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lqWHL-0007li-RF for incoming@patchwork.ozlabs.org; Tue, 08 Jun 2021 03:36:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51644) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFJ-00031p-VZ for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:33:57 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:58111) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFH-00059C-VJ for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:33:57 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623137635; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=UO3E0kVVljCJWgtb0Y3sT341JalvuhmXACZvnO6bPOU=; b=aM1idbmLjGSKjK5RM2m9eD5wD5vOPmn1Y23omNTYab1Moa0BfiVIv8FewAlnwRlEgBzNLU 35/JOSbPMMucGxaounOHwrqhQ4RbWyLc/3sYWEPKLrgRNjV8ne0OJpxSGGU5Wa2Des5enL rgaIVzTkNw0Zr4WfbZIkEeli8Gsxud4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-507-cNZYhFmKP1GMuBxvcZDKzA-1; Tue, 08 Jun 2021 03:33:54 -0400 X-MC-Unique: cNZYhFmKP1GMuBxvcZDKzA-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 24EFE108C24A; Tue, 8 Jun 2021 07:33:53 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id F0870608BA; Tue, 8 Jun 2021 07:33:50 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH v3 2/5] block-copy: improve comments of BlockCopyTask and BlockCopyState types and functions Date: Tue, 8 Jun 2021 09:33:41 +0200 Message-Id: <20210608073344.53637-3-eesposit@redhat.com> In-Reply-To: <20210608073344.53637-1-eesposit@redhat.com> References: <20210608073344.53637-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=216.205.24.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -29 X-Spam_score: -3.0 X-Spam_bar: --- X-Spam_report: (-3.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.2, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, 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.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Emanuele Giuseppe Esposito , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" As done in BlockCopyCallState, categorize BlockCopyTask and BlockCopyState in IN, State and OUT fields. This is just to understand which field has to be protected with a lock. .sleep_state is handled in the series "coroutine: new sleep/wake API" and thus here left as TODO. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 47 ++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index d58051288b..b3533a3003 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -56,25 +56,33 @@ typedef struct BlockCopyCallState { QLIST_ENTRY(BlockCopyCallState) list; /* State */ - int ret; bool finished; - QemuCoSleep sleep; - bool cancelled; + QemuCoSleep sleep; /* TODO: protect API with a lock */ /* OUT parameters */ + bool cancelled; bool error_is_read; + int ret; } BlockCopyCallState; typedef struct BlockCopyTask { AioTask task; + /* + * IN parameters. Initialized in block_copy_task_create() + * and never changed. + */ BlockCopyState *s; BlockCopyCallState *call_state; int64_t offset; - int64_t bytes; - BlockCopyMethod method; - QLIST_ENTRY(BlockCopyTask) list; + int64_t bytes; /* only re-set in task_shrink, before running the task */ + BlockCopyMethod method; /* initialized in block_copy_dirty_clusters() */ + + /* State */ CoQueue wait_queue; /* coroutines blocked on this task */ + + /* To reference all call states from BlockCopyState */ + QLIST_ENTRY(BlockCopyTask) list; } BlockCopyTask; static int64_t task_end(BlockCopyTask *task) @@ -90,15 +98,25 @@ typedef struct BlockCopyState { */ BdrvChild *source; BdrvChild *target; - BdrvDirtyBitmap *copy_bitmap; + + /* State */ int64_t in_flight_bytes; - int64_t cluster_size; BlockCopyMethod method; - int64_t max_transfer; - uint64_t len; QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ QLIST_HEAD(, BlockCopyCallState) calls; + /* State fields that use a thread-safe API */ + BdrvDirtyBitmap *copy_bitmap; + ProgressMeter *progress; + SharedResource *mem; + RateLimit rate_limit; + /* + * IN parameters. Initialized in block_copy_state_new() + * and never changed. + */ + int64_t cluster_size; + int64_t max_transfer; + uint64_t len; BdrvRequestFlags write_flags; /* @@ -114,14 +132,11 @@ typedef struct BlockCopyState { * In this case, block_copy() will query the source’s allocation status, * skip unallocated regions, clear them in the copy_bitmap, and invoke * block_copy_reset_unallocated() every time it does. + * + * This field is set in backup_run() before coroutines are run, + * therefore is an IN. */ bool skip_unallocated; - - ProgressMeter *progress; - - SharedResource *mem; - - RateLimit rate_limit; } BlockCopyState; static BlockCopyTask *find_conflicting_task(BlockCopyState *s, From patchwork Tue Jun 8 07:33:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1489146 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=D5q7jbQt; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FzhsH6BW7z9s24 for ; Tue, 8 Jun 2021 17:37:23 +1000 (AEST) Received: from localhost ([::1]:46958 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lqWIb-0002cx-Sv for incoming@patchwork.ozlabs.org; Tue, 08 Jun 2021 03:37:21 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51670) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFM-00039R-HA for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:00 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:55860) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFK-0005Bl-Sj for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623137637; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vI/v1vJhwHUv3ayb0ieVDpXi6kMQ53a/rw7SR748b9U=; b=D5q7jbQtvQ5srXIJZtImn0xASwaPUU3uZuihyh53sXMvH0zLJhQctY5eDJJ0uqKrSqtlPz n5skDHOov8I4PjrPnhbyRvflD9w5SSfdlYaBn/7AOTSgTtlCb/sCviZvHZOhWOVsW3RpFW Z6wRmR3ezqBQo853H+lHHCOO3ZF650k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-272-lXIGh390MK-W0dio3Sl3RQ-1; Tue, 08 Jun 2021 03:33:56 -0400 X-MC-Unique: lXIGh390MK-W0dio3Sl3RQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B649080F042; Tue, 8 Jun 2021 07:33:55 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id 84A91608BA; Tue, 8 Jun 2021 07:33:53 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH v3 3/5] block-copy: move progress_set_remaining in block_copy_task_end Date: Tue, 8 Jun 2021 09:33:42 +0200 Message-Id: <20210608073344.53637-4-eesposit@redhat.com> In-Reply-To: <20210608073344.53637-1-eesposit@redhat.com> References: <20210608073344.53637-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -29 X-Spam_score: -3.0 X-Spam_bar: --- X-Spam_report: (-3.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.2, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, 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.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Emanuele Giuseppe Esposito , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Moving this function in task_end ensures to update the progress anyways, even if there is an error. It also helps in next patch, allowing task_end to have only one critical section. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index b3533a3003..e2adb5b2ea 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -263,6 +263,9 @@ static void coroutine_fn block_copy_task_end(BlockCopyTask *task, int ret) bdrv_set_dirty_bitmap(task->s->copy_bitmap, task->offset, task->bytes); } QLIST_REMOVE(task, list); + progress_set_remaining(task->s->progress, + bdrv_get_dirty_count(task->s->copy_bitmap) + + task->s->in_flight_bytes); qemu_co_queue_restart_all(&task->wait_queue); } @@ -647,9 +650,6 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) } if (s->skip_unallocated && !(ret & BDRV_BLOCK_ALLOCATED)) { block_copy_task_end(task, 0); - progress_set_remaining(s->progress, - bdrv_get_dirty_count(s->copy_bitmap) + - s->in_flight_bytes); trace_block_copy_skip_range(s, task->offset, task->bytes); offset = task_end(task); bytes = end - offset; From patchwork Tue Jun 8 07:33:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1489143 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gXtedwGF; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Fzhp80m11z9s24 for ; Tue, 8 Jun 2021 17:34:40 +1000 (AEST) Received: from localhost ([::1]:35604 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lqWFy-0003PM-1y for incoming@patchwork.ozlabs.org; Tue, 08 Jun 2021 03:34:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51710) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFP-0003KL-QO for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:03 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:60311) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFN-0005Dx-ID for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:03 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623137640; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rmZTZjRes0RYYTkV41IuOafzd0h5qFCup42BWDpIB1s=; b=gXtedwGFhC6Tdu1T25xqy6ZwtBwGKXY1BDG16TDoy9MqaRLTxfLtKiXiSj6UYm6Nl/MTaW e0n5IxZpZvUGtlG46Yg2i7yDpv70+vAjxuvSjDeE1B1uj67UmcythFQbCyt3MmX3zfUsYv Izh7dO5yFNo2/qyvs0aW/7slpcNRQPY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-440-JTsUYWZ2OkWyOMtYbdtoBw-1; Tue, 08 Jun 2021 03:33:59 -0400 X-MC-Unique: JTsUYWZ2OkWyOMtYbdtoBw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8D66D9126F; Tue, 8 Jun 2021 07:33:58 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id 331F9608BA; Tue, 8 Jun 2021 07:33:55 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH v3 4/5] block-copy: add a CoMutex Date: Tue, 8 Jun 2021 09:33:43 +0200 Message-Id: <20210608073344.53637-5-eesposit@redhat.com> In-Reply-To: <20210608073344.53637-1-eesposit@redhat.com> References: <20210608073344.53637-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -29 X-Spam_score: -3.0 X-Spam_bar: --- X-Spam_report: (-3.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.2, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, 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.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Emanuele Giuseppe Esposito , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Add a CoMutex to protect concurrent access of block-copy data structures. This mutex also protects .copy_bitmap, because its thread-safe API does not prevent it from assigning two tasks to the same bitmap region. .finished, .cancelled and reads to .ret and .error_is_read will be protected in the following patch, because are used also outside coroutines. Also set block_copy_task_create as coroutine_fn because: 1) it is static and only invoked by coroutine functions 2) this patch introduces and uses a CoMutex lock there Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 82 ++++++++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index e2adb5b2ea..56f62913e4 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -61,6 +61,7 @@ typedef struct BlockCopyCallState { /* OUT parameters */ bool cancelled; + /* Fields protected by lock in BlockCopyState */ bool error_is_read; int ret; } BlockCopyCallState; @@ -78,7 +79,7 @@ typedef struct BlockCopyTask { int64_t bytes; /* only re-set in task_shrink, before running the task */ BlockCopyMethod method; /* initialized in block_copy_dirty_clusters() */ - /* State */ + /* State. Protected by lock in BlockCopyState */ CoQueue wait_queue; /* coroutines blocked on this task */ /* To reference all call states from BlockCopyState */ @@ -99,7 +100,8 @@ typedef struct BlockCopyState { BdrvChild *source; BdrvChild *target; - /* State */ + /* State. Protected by lock */ + CoMutex lock; int64_t in_flight_bytes; BlockCopyMethod method; QLIST_HEAD(, BlockCopyTask) tasks; /* All tasks from all block-copy calls */ @@ -139,8 +141,10 @@ typedef struct BlockCopyState { bool skip_unallocated; } BlockCopyState; -static BlockCopyTask *find_conflicting_task(BlockCopyState *s, - int64_t offset, int64_t bytes) +/* Called with lock held */ +static BlockCopyTask *find_conflicting_task_locked(BlockCopyState *s, + int64_t offset, + int64_t bytes) { BlockCopyTask *t; @@ -160,18 +164,22 @@ static BlockCopyTask *find_conflicting_task(BlockCopyState *s, static bool coroutine_fn block_copy_wait_one(BlockCopyState *s, int64_t offset, int64_t bytes) { - BlockCopyTask *task = find_conflicting_task(s, offset, bytes); + BlockCopyTask *task; + + QEMU_LOCK_GUARD(&s->lock); + task = find_conflicting_task_locked(s, offset, bytes); if (!task) { return false; } - qemu_co_queue_wait(&task->wait_queue, NULL); + qemu_co_queue_wait(&task->wait_queue, &s->lock); return true; } -static int64_t block_copy_chunk_size(BlockCopyState *s) +/* Called with lock held */ +static int64_t block_copy_chunk_size_locked(BlockCopyState *s) { switch (s->method) { case COPY_READ_WRITE_CLUSTER: @@ -193,14 +201,16 @@ static int64_t block_copy_chunk_size(BlockCopyState *s) * Search for the first dirty area in offset/bytes range and create task at * the beginning of it. */ -static BlockCopyTask *block_copy_task_create(BlockCopyState *s, - BlockCopyCallState *call_state, - int64_t offset, int64_t bytes) +static coroutine_fn BlockCopyTask *block_copy_task_create(BlockCopyState *s, + BlockCopyCallState *call_state, + int64_t offset, int64_t bytes) { BlockCopyTask *task; - int64_t max_chunk = block_copy_chunk_size(s); + int64_t max_chunk; - max_chunk = MIN_NON_ZERO(max_chunk, call_state->max_chunk); + QEMU_LOCK_GUARD(&s->lock); + max_chunk = MIN_NON_ZERO(block_copy_chunk_size_locked(s), + call_state->max_chunk); if (!bdrv_dirty_bitmap_next_dirty_area(s->copy_bitmap, offset, offset + bytes, max_chunk, &offset, &bytes)) @@ -212,7 +222,7 @@ static BlockCopyTask *block_copy_task_create(BlockCopyState *s, bytes = QEMU_ALIGN_UP(bytes, s->cluster_size); /* region is dirty, so no existent tasks possible in it */ - assert(!find_conflicting_task(s, offset, bytes)); + assert(!find_conflicting_task_locked(s, offset, bytes)); bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); s->in_flight_bytes += bytes; @@ -248,16 +258,19 @@ static void coroutine_fn block_copy_task_shrink(BlockCopyTask *task, assert(new_bytes > 0 && new_bytes < task->bytes); - task->s->in_flight_bytes -= task->bytes - new_bytes; - bdrv_set_dirty_bitmap(task->s->copy_bitmap, - task->offset + new_bytes, task->bytes - new_bytes); - - task->bytes = new_bytes; - qemu_co_queue_restart_all(&task->wait_queue); + WITH_QEMU_LOCK_GUARD(&task->s->lock) { + task->s->in_flight_bytes -= task->bytes - new_bytes; + bdrv_set_dirty_bitmap(task->s->copy_bitmap, + task->offset + new_bytes, + task->bytes - new_bytes); + task->bytes = new_bytes; + qemu_co_queue_restart_all(&task->wait_queue); + } } static void coroutine_fn block_copy_task_end(BlockCopyTask *task, int ret) { + QEMU_LOCK_GUARD(&task->s->lock); task->s->in_flight_bytes -= task->bytes; if (ret < 0) { bdrv_set_dirty_bitmap(task->s->copy_bitmap, task->offset, task->bytes); @@ -335,6 +348,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target, } ratelimit_init(&s->rate_limit); + qemu_co_mutex_init(&s->lock); QLIST_INIT(&s->tasks); QLIST_INIT(&s->calls); @@ -390,6 +404,8 @@ static coroutine_fn int block_copy_task_run(AioTaskPool *pool, * a full-size buffer or disabled if the copy_range attempt fails. The output * value of @method should be used for subsequent tasks. * Returns 0 on success. + * + * Called with lock held. */ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, int64_t offset, int64_t bytes, @@ -476,16 +492,20 @@ static coroutine_fn int block_copy_task_entry(AioTask *task) int ret; ret = block_copy_do_copy(s, t->offset, t->bytes, &method, &error_is_read); - if (s->method == t->method) { - s->method = method; - } - if (ret < 0) { - if (!t->call_state->ret) { - t->call_state->ret = ret; - t->call_state->error_is_read = error_is_read; + + WITH_QEMU_LOCK_GUARD(&t->s->lock) { + if (s->method == t->method) { + s->method = method; + } + + if (ret < 0) { + if (!t->call_state->ret) { + t->call_state->ret = ret; + t->call_state->error_is_read = error_is_read; + } + } else { + progress_work_done(t->s->progress, t->bytes); } - } else { - progress_work_done(t->s->progress, t->bytes); } co_put_to_shres(t->s->mem, t->bytes); block_copy_task_end(t, ret); @@ -587,10 +607,12 @@ int64_t block_copy_reset_unallocated(BlockCopyState *s, bytes = clusters * s->cluster_size; if (!ret) { + qemu_co_mutex_lock(&s->lock); bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes); progress_set_remaining(s->progress, bdrv_get_dirty_count(s->copy_bitmap) + s->in_flight_bytes); + qemu_co_mutex_unlock(&s->lock); } *count = bytes; @@ -729,7 +751,9 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) { int ret; + qemu_co_mutex_lock(&call_state->s->lock); QLIST_INSERT_HEAD(&call_state->s->calls, call_state, list); + qemu_co_mutex_unlock(&call_state->s->lock); do { ret = block_copy_dirty_clusters(call_state); @@ -756,7 +780,9 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) call_state->cb(call_state->cb_opaque); } + qemu_co_mutex_lock(&call_state->s->lock); QLIST_REMOVE(call_state, list); + qemu_co_mutex_unlock(&call_state->s->lock); return ret; } From patchwork Tue Jun 8 07:33:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 1489145 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: 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=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=R6EyrcYO; dkim-atps=neutral Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Fzhqv1z2pz9s24 for ; Tue, 8 Jun 2021 17:36:11 +1000 (AEST) Received: from localhost ([::1]:42750 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lqWHR-0008G8-9o for incoming@patchwork.ozlabs.org; Tue, 08 Jun 2021 03:36:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:51740) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFS-0003Vc-Ls for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:06 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:28945) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lqWFQ-0005GI-Ot for qemu-devel@nongnu.org; Tue, 08 Jun 2021 03:34:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1623137644; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3kh0KqmdCWNV7x5VvloIjB4t8//cjeCMhlFWcIJ19HY=; b=R6EyrcYOgP97TASXnXjdO59nHKwgNm/xgCK0t89GlwEySTaj7jFhgVf3T1vYEwMPry5XId W0+rCKVqPmzbCs5tByB00GMjo+reN2ShfRLRvfASUia76n0ZFG/FMor/8b/Jzfb37m1dP/ Le3sUxeW4vAX1zaIP7r9u6zWrFMNB38= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-160-vn1hwRGYPzKIhsFDNHCRrQ-1; Tue, 08 Jun 2021 03:34:02 -0400 X-MC-Unique: vn1hwRGYPzKIhsFDNHCRrQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 3B5FB107ACCA; Tue, 8 Jun 2021 07:34:01 +0000 (UTC) Received: from localhost.localdomain (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id EC604608BA; Tue, 8 Jun 2021 07:33:58 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Subject: [PATCH v3 5/5] block-copy: atomic .cancelled and .finished fields in BlockCopyCallState Date: Tue, 8 Jun 2021 09:33:44 +0200 Message-Id: <20210608073344.53637-6-eesposit@redhat.com> In-Reply-To: <20210608073344.53637-1-eesposit@redhat.com> References: <20210608073344.53637-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eesposit@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Received-SPF: pass client-ip=170.10.133.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -29 X-Spam_score: -3.0 X-Spam_bar: --- X-Spam_report: (-3.0 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.2, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Emanuele Giuseppe Esposito , Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini , John Snow Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" By adding acquire/release pairs, we ensure that .ret and .error_is_read fields are written by block_copy_dirty_clusters before .finished is true. The atomic here are necessary because the fields are concurrently modified also outside coroutines. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-copy.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/block/block-copy.c b/block/block-copy.c index 56f62913e4..55b6ce6a57 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -56,11 +56,11 @@ typedef struct BlockCopyCallState { QLIST_ENTRY(BlockCopyCallState) list; /* State */ - bool finished; + bool finished; /* atomic */ QemuCoSleep sleep; /* TODO: protect API with a lock */ /* OUT parameters */ - bool cancelled; + bool cancelled; /* atomic */ /* Fields protected by lock in BlockCopyState */ bool error_is_read; int ret; @@ -648,7 +648,8 @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(bytes, s->cluster_size)); - while (bytes && aio_task_pool_status(aio) == 0 && !call_state->cancelled) { + while (bytes && aio_task_pool_status(aio) == 0 && + !qatomic_read(&call_state->cancelled)) { BlockCopyTask *task; int64_t status_bytes; @@ -758,7 +759,7 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) do { ret = block_copy_dirty_clusters(call_state); - if (ret == 0 && !call_state->cancelled) { + if (ret == 0 && !qatomic_read(&call_state->cancelled)) { ret = block_copy_wait_one(call_state->s, call_state->offset, call_state->bytes); } @@ -772,9 +773,9 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) * 2. We have waited for some intersecting block-copy request * It may have failed and produced new dirty bits. */ - } while (ret > 0 && !call_state->cancelled); + } while (ret > 0 && !qatomic_read(&call_state->cancelled)); - call_state->finished = true; + qatomic_store_release(&call_state->finished, true); if (call_state->cb) { call_state->cb(call_state->cb_opaque); @@ -837,35 +838,37 @@ void block_copy_call_free(BlockCopyCallState *call_state) return; } - assert(call_state->finished); + assert(qatomic_load_acquire(&call_state->finished)); g_free(call_state); } bool block_copy_call_finished(BlockCopyCallState *call_state) { - return call_state->finished; + return qatomic_load_acquire(&call_state->finished); } bool block_copy_call_succeeded(BlockCopyCallState *call_state) { - return call_state->finished && !call_state->cancelled && - call_state->ret == 0; + return qatomic_load_acquire(&call_state->finished) && + !qatomic_read(&call_state->cancelled) && + call_state->ret == 0; } bool block_copy_call_failed(BlockCopyCallState *call_state) { - return call_state->finished && !call_state->cancelled && - call_state->ret < 0; + return qatomic_load_acquire(&call_state->finished) && + !qatomic_read(&call_state->cancelled) && + call_state->ret < 0; } bool block_copy_call_cancelled(BlockCopyCallState *call_state) { - return call_state->cancelled; + return qatomic_read(&call_state->cancelled); } int block_copy_call_status(BlockCopyCallState *call_state, bool *error_is_read) { - assert(call_state->finished); + assert(qatomic_load_acquire(&call_state->finished)); if (error_is_read) { *error_is_read = call_state->error_is_read; } @@ -874,7 +877,7 @@ int block_copy_call_status(BlockCopyCallState *call_state, bool *error_is_read) void block_copy_call_cancel(BlockCopyCallState *call_state) { - call_state->cancelled = true; + qatomic_set(&call_state->cancelled, true); block_copy_kick(call_state); }