From patchwork Wed Nov 16 13:48:31 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045257 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 933DFC4332F for ; Wed, 16 Nov 2022 13:59:54 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImu-0000jX-N2; Wed, 16 Nov 2022 08:49:12 -0500 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 1ovImo-0000dR-0z for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:06 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImg-0007yy-H4 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:05 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606536; h=from:from:reply-to:subject:subject: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=bO0L22eyG3dTt+qnpvoRbb4k6Fdg1bFh/jyiE6IeO78=; b=WVpo/yTd/Ow5FVqlNUyviXlS+3RNmW4UeiLZPCJj1icdrGlV6Boi1eR+bfbfUXIoqx3Jrd kP3EvTmR7bMN4qwUbJxccwCOL6R0gQuRNwavh/2kZYudovjHwzjnXIOxRjCfHTeqe9AXlU CawcigL0tEUroio9+je+H15UsGD4HYQ= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-142-MTcMNPJHNn282VpFKnw22g-1; Wed, 16 Nov 2022 08:48:52 -0500 X-MC-Unique: MTcMNPJHNn282VpFKnw22g-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7E1DA3850E85; Wed, 16 Nov 2022 13:48:52 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2D54F4A9265; Wed, 16 Nov 2022 13:48:52 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org Subject: [PATCH 01/20] block: introduce a lock to protect graph operations Date: Wed, 16 Nov 2022 08:48:31 -0500 Message-Id: <20221116134850.3051419-2-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Paolo Bonzini block layer graph operations are always run under BQL in the main loop. This is proved by the assertion qemu_in_main_thread() and its wrapper macro GLOBAL_STATE_CODE. However, there are also concurrent coroutines running in other iothreads that always try to traverse the graph. Currently this is protected (among various other things) by the AioContext lock, but once this is removed we need to make sure that reads do not happen while modifying the graph. We distinguish between writer (main loop, under BQL) that modifies the graph, and readers (all other coroutines running in various AioContext), that go through the graph edges, reading ->parents and->children. The writer (main loop) has an "exclusive" access, so it first waits for current read to finish, and then prevents incoming ones from entering while it has the exclusive access. The readers (coroutines in multiple AioContext) are free to access the graph as long the writer is not modifying the graph. In case it is, they go in a CoQueue and sleep until the writer is done. If a coroutine changes AioContext, the counter in the original and new AioContext are left intact, since the writer does not care where is the reader, but only if there is one. As a result, some AioContexts might have a negative reader count, to balance the positive count of the AioContext that took the lock. This also means that when an AioContext is deleted it may have a nonzero reader count. In that case we transfer the count to a global shared counter so that the writer is always aware of all readers. Co-developed-with: Emanuele Giuseppe Esposito Signed-off-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- block/graph-lock.c | 221 +++++++++++++++++++++++++++++++++++++ block/meson.build | 1 + include/block/aio.h | 9 ++ include/block/block_int.h | 1 + include/block/graph-lock.h | 129 ++++++++++++++++++++++ 5 files changed, 361 insertions(+) create mode 100644 block/graph-lock.c create mode 100644 include/block/graph-lock.h diff --git a/block/graph-lock.c b/block/graph-lock.c new file mode 100644 index 0000000000..b608a89d7c --- /dev/null +++ b/block/graph-lock.c @@ -0,0 +1,221 @@ +/* + * Graph lock: rwlock to protect block layer graph manipulations (add/remove + * edges and nodes) + * + * Copyright (c) 2022 Red Hat + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ + +#include "qemu/osdep.h" +#include "qemu/main-loop.h" +#include "block/graph-lock.h" +#include "block/block.h" +#include "block/block_int.h" + +/* Protects the list of aiocontext and orphaned_reader_count */ +static QemuMutex aio_context_list_lock; + +/* Written and read with atomic operations. */ +static int has_writer; + +/* + * A reader coroutine could move from an AioContext to another. + * If this happens, there is no problem from the point of view of + * counters. The problem is that the total count becomes + * unbalanced if one of the two AioContexts gets deleted. + * The count of readers must remain correct, so the AioContext's + * balance is transferred to this glboal variable. + * Protected by aio_context_list_lock. + */ +static uint32_t orphaned_reader_count; + +/* Queue of readers waiting for the writer to finish */ +static CoQueue reader_queue; + +/* + * List of AioContext. This list ensures that each AioContext + * can safely modify only its own counter, avoid reading/writing + * others and thus improving performances by avoiding cacheline bounces. + */ +static QTAILQ_HEAD(, AioContext) aio_context_list = + QTAILQ_HEAD_INITIALIZER(aio_context_list); + +static void __attribute__((__constructor__)) bdrv_init_graph_lock(void) +{ + qemu_mutex_init(&aio_context_list_lock); + qemu_co_queue_init(&reader_queue); +} + +void register_aiocontext(AioContext *ctx) +{ + QEMU_LOCK_GUARD(&aio_context_list_lock); + assert(ctx->reader_count == 0); + QTAILQ_INSERT_TAIL(&aio_context_list, ctx, next_aio); +} + +void unregister_aiocontext(AioContext *ctx) +{ + QEMU_LOCK_GUARD(&aio_context_list_lock); + orphaned_reader_count += ctx->reader_count; + QTAILQ_REMOVE(&aio_context_list, ctx, next_aio); +} + +static uint32_t reader_count(void) +{ + AioContext *ctx; + uint32_t rd; + + QEMU_LOCK_GUARD(&aio_context_list_lock); + + /* rd can temporarly be negative, but the total will *always* be >= 0 */ + rd = orphaned_reader_count; + QTAILQ_FOREACH(ctx, &aio_context_list, next_aio) { + rd += qatomic_read(&ctx->reader_count); + } + + /* shouldn't overflow unless there are 2^31 readers */ + assert((int32_t)rd >= 0); + return rd; +} + +void bdrv_graph_wrlock(void) +{ + GLOBAL_STATE_CODE(); + assert(!qatomic_read(&has_writer)); + + /* + * reader_count == 0: this means writer will read has_reader as 1 + * reader_count >= 1: we don't know if writer read has_writer == 0 or 1, + * but we need to wait. + * Wait by allowing other coroutine (and possible readers) to continue. + */ + do { + /* + * has_writer must be 0 while polling, otherwise we get a deadlock if + * any callback involved during AIO_WAIT_WHILE() tries to acquire the + * reader lock. + */ + qatomic_set(&has_writer, 0); + AIO_WAIT_WHILE(qemu_get_aio_context(), reader_count() >= 1); + qatomic_set(&has_writer, 1); + + /* + * We want to only check reader_count() after has_writer = 1 is visible + * to other threads. That way no more readers can sneak in after we've + * determined reader_count() == 0. + */ + smp_mb(); + } while (reader_count() >= 1); +} + +void bdrv_graph_wrunlock(void) +{ + GLOBAL_STATE_CODE(); + QEMU_LOCK_GUARD(&aio_context_list_lock); + assert(qatomic_read(&has_writer)); + + /* + * No need for memory barriers, this works in pair with + * the slow path of rdlock() and both take the lock. + */ + qatomic_store_release(&has_writer, 0); + + /* Wake up all coroutine that are waiting to read the graph */ + qemu_co_enter_all(&reader_queue, &aio_context_list_lock); +} + +void coroutine_fn bdrv_graph_co_rdlock(void) +{ + AioContext *aiocontext; + aiocontext = qemu_get_current_aio_context(); + + for (;;) { + qatomic_set(&aiocontext->reader_count, + aiocontext->reader_count + 1); + /* make sure writer sees reader_count before we check has_writer */ + smp_mb(); + + /* + * has_writer == 0: this means writer will read reader_count as >= 1 + * has_writer == 1: we don't know if writer read reader_count == 0 + * or > 0, but we need to wait anyways because + * it will write. + */ + if (!qatomic_read(&has_writer)) { + break; + } + + /* + * Synchronize access with reader_count() in bdrv_graph_wrlock(). + * Case 1: + * If this critical section gets executed first, reader_count will + * decrease and the reader will go to sleep. + * Then the writer will read reader_count that does not take into + * account this reader, and if there's no other reader it will + * enter the write section. + * Case 2: + * If reader_count() critical section gets executed first, + * then writer will read reader_count >= 1. + * It will wait in AIO_WAIT_WHILE(), but once it releases the lock + * we will enter this critical section and call aio_wait_kick(). + */ + WITH_QEMU_LOCK_GUARD(&aio_context_list_lock) { + /* + * Additional check when we use the above lock to synchronize + * with bdrv_graph_wrunlock(). + * Case 1: + * If this gets executed first, has_writer is still 1, so we reduce + * reader_count and go to sleep. + * Then the writer will set has_writer to 0 and wake up all readers, + * us included. + * Case 2: + * If bdrv_graph_wrunlock() critical section gets executed first, + * then it will set has_writer to 0 and wake up all other readers. + * Then we execute this critical section, and therefore must check + * again for has_writer, otherwise we sleep without any writer + * actually running. + */ + if (!qatomic_read(&has_writer)) { + return; + } + + /* slow path where reader sleeps */ + aiocontext->reader_count--; + aio_wait_kick(); + qemu_co_queue_wait(&reader_queue, &aio_context_list_lock); + } + } +} + +void coroutine_fn bdrv_graph_co_rdunlock(void) +{ + AioContext *aiocontext; + aiocontext = qemu_get_current_aio_context(); + + qatomic_store_release(&aiocontext->reader_count, + aiocontext->reader_count - 1); + /* make sure writer sees reader_count before we check has_writer */ + smp_mb(); + + /* + * has_writer == 0: this means reader will read reader_count decreased + * has_writer == 1: we don't know if writer read reader_count old or + * new. Therefore, kick again so on next iteration + * writer will for sure read the updated value. + */ + if (qatomic_read(&has_writer)) { + aio_wait_kick(); + } +} diff --git a/block/meson.build b/block/meson.build index c48a59571e..90011a2805 100644 --- a/block/meson.build +++ b/block/meson.build @@ -10,6 +10,7 @@ block_ss.add(files( 'blkverify.c', 'block-backend.c', 'block-copy.c', + 'graph-lock.c', 'commit.c', 'copy-on-read.c', 'preallocate.c', diff --git a/include/block/aio.h b/include/block/aio.h index d128558f1d..8e64f81d01 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -127,6 +127,15 @@ struct AioContext { /* Used by AioContext users to protect from multi-threaded access. */ QemuRecMutex lock; + /* How many readers in this AioContext are currently reading the graph. */ + uint32_t reader_count; + + /* + * List of AioContext kept in graph-lock.c + * Protected by aio_context_list_lock + */ + QTAILQ_ENTRY(AioContext) next_aio; + /* The list of registered AIO handlers. Protected by ctx->list_lock. */ AioHandlerList aio_handlers; diff --git a/include/block/block_int.h b/include/block/block_int.h index 7d50b6bbd1..b35b0138ed 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -26,6 +26,7 @@ #include "block_int-global-state.h" #include "block_int-io.h" +#include "block/graph-lock.h" /* DO NOT ADD ANYTHING IN HERE. USE ONE OF THE HEADERS INCLUDED ABOVE */ diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h new file mode 100644 index 0000000000..f975312bb6 --- /dev/null +++ b/include/block/graph-lock.h @@ -0,0 +1,129 @@ +/* + * Graph lock: rwlock to protect block layer graph manipulations (add/remove + * edges and nodes) + * + * Copyright (c) 2022 Red Hat + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see . + */ +#ifndef GRAPH_LOCK_H +#define GRAPH_LOCK_H + +#include "qemu/osdep.h" + +/** + * Graph Lock API + * This API provides a rwlock used to protect block layer + * graph modifications like edge (BdrvChild) and node (BlockDriverState) + * addition and removal. + * Currently we have 1 writer only, the Main loop, and many + * readers, mostly coroutines running in other AioContext thus other threads. + * + * We distinguish between writer (main loop, under BQL) that modifies the + * graph, and readers (all other coroutines running in various AioContext), + * that go through the graph edges, reading + * BlockDriverState ->parents and->children. + * + * The writer (main loop) has an "exclusive" access, so it first waits for + * current read to finish, and then prevents incoming ones from + * entering while it has the exclusive access. + * + * The readers (coroutines in multiple AioContext) are free to + * access the graph as long the writer is not modifying the graph. + * In case it is, they go in a CoQueue and sleep until the writer + * is done. + * + * If a coroutine changes AioContext, the counter in the original and new + * AioContext are left intact, since the writer does not care where is the + * reader, but only if there is one. + * As a result, some AioContexts might have a negative reader count, to + * balance the positive count of the AioContext that took the lock. + * This also means that when an AioContext is deleted it may have a nonzero + * reader count. In that case we transfer the count to a global shared counter + * so that the writer is always aware of all readers. + */ + +/* + * register_aiocontext: + * Add AioContext @ctx to the list of AioContext. + * This list is used to obtain the total number of readers + * currently running the graph. + */ +void register_aiocontext(AioContext *ctx); + +/* + * unregister_aiocontext: + * Removes AioContext @ctx to the list of AioContext. + */ +void unregister_aiocontext(AioContext *ctx); + +/* + * bdrv_graph_wrlock: + * Start an exclusive write operation to modify the graph. + * This means we are adding or removing an edge or a node (bs) + * from the block layer graph. + * Nobody else is allowed to access the graph. + * Set global has_writer to 1, so that the next readers will wait + * that writer is done in a coroutine queue. + * Then keep track of the running readers by counting what is the total + * amount of readers (sum of all aiocontext readers), and wait until + * they all finish with AIO_WAIT_WHILE. + * + * Must only be called from outside bdrv_graph_co_rdlock. + * The wrlock is only taken from the main loop, with BQL held, + * as only the main loop is allowed to modify the graph. + */ +void bdrv_graph_wrlock(void); + +/* + * bdrv_graph_wrunlock: + * Write finished, reset global has_writer to 0 and restart + * all readers that are waiting. + */ +void bdrv_graph_wrunlock(void); + +/* + * bdrv_graph_co_rdlock: + * Read the bs graph. This usually means traversing all nodes in + * the graph, therefore it can't happen while another thread is + * modifying it. + * Increases the reader counter of the current aiocontext, + * and if has_writer is set, it means that the writer is modifying + * the graph, therefore wait in a coroutine queue. + * The writer will then wake this coroutine once it is done. + * + * This lock should be taken from Iothreads (IO_CODE() class of functions) + * because it signals the writer that there are some + * readers currently running, or waits until the current + * write is finished before continuing. + * Calling this function from the Main Loop with BQL held + * is not necessary, since the Main Loop itself is the only + * writer, thus won't be able to read and write at the same time. + * The only exception to that is when we can't take the lock in the + * function/coroutine itself, and need to delegate the caller (usually main + * loop) to take it and wait that the coroutine ends, so that + * we always signal that a reader is running. + */ +void coroutine_fn bdrv_graph_co_rdlock(void); + +/* + * bdrv_graph_rdunlock: + * Read terminated, decrease the count of readers in the current aiocontext. + * If the writer is waiting for reads to finish (has_writer == 1), signal + * the writer that we are done via aio_wait_kick() to let it continue. + */ +void coroutine_fn bdrv_graph_co_rdunlock(void); + +#endif /* GRAPH_LOCK_H */ + From patchwork Wed Nov 16 13:48:32 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045231 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id A72EDC433FE for ; Wed, 16 Nov 2022 13:50:48 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImo-0000dP-82; Wed, 16 Nov 2022 08:49:06 -0500 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 1ovImm-0000cP-EI for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:04 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImg-0007zF-Hw for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606537; h=from:from:reply-to:subject:subject: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=r+63Rvz1O2BCywQdimBwCPmeQIwX6qzesR//g/fyLJg=; b=SPpihc3PF58AvlVmOZxUf8QswTJuZUNfXB/XJaCrjLtvFgIzQUH8dNn3dXDlZcZbhs9sJQ ZWUVQNcWerNcPKrRO8NFci8Q3hiSu/NTfVstmS70YQbXmmxfIk+4c/tgEl8xNtZ6N3I8/J RYUE/1bDosJ9ZvdjBscXXQ2mbZIIYMg= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-670-ZYHaDMSfMM2TShOgAMNDyA-1; Wed, 16 Nov 2022 08:48:53 -0500 X-MC-Unique: ZYHaDMSfMM2TShOgAMNDyA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D45B7185A78B; Wed, 16 Nov 2022 13:48:52 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 877054A9254; Wed, 16 Nov 2022 13:48:52 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 02/20] graph-lock: introduce BdrvGraphRWlock structure Date: Wed, 16 Nov 2022 08:48:32 -0500 Message-Id: <20221116134850.3051419-3-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Just a wrapper to simplify what is available to the struct AioContext. Signed-off-by: Emanuele Giuseppe Esposito --- block/graph-lock.c | 59 ++++++++++++++++++++++++++------------ include/block/aio.h | 12 ++++---- include/block/graph-lock.h | 1 + 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/block/graph-lock.c b/block/graph-lock.c index b608a89d7c..c3c6eeedad 100644 --- a/block/graph-lock.c +++ b/block/graph-lock.c @@ -44,12 +44,23 @@ static uint32_t orphaned_reader_count; /* Queue of readers waiting for the writer to finish */ static CoQueue reader_queue; +struct BdrvGraphRWlock { + /* How many readers are currently reading the graph. */ + uint32_t reader_count; + + /* + * List of BdrvGraphRWlock kept in graph-lock.c + * Protected by aio_context_list_lock + */ + QTAILQ_ENTRY(BdrvGraphRWlock) next_aio; +}; + /* - * List of AioContext. This list ensures that each AioContext + * List of BdrvGraphRWlock. This list ensures that each BdrvGraphRWlock * can safely modify only its own counter, avoid reading/writing * others and thus improving performances by avoiding cacheline bounces. */ -static QTAILQ_HEAD(, AioContext) aio_context_list = +static QTAILQ_HEAD(, BdrvGraphRWlock) aio_context_list = QTAILQ_HEAD_INITIALIZER(aio_context_list); static void __attribute__((__constructor__)) bdrv_init_graph_lock(void) @@ -60,29 +71,31 @@ static void __attribute__((__constructor__)) bdrv_init_graph_lock(void) void register_aiocontext(AioContext *ctx) { + ctx->bdrv_graph = g_new0(BdrvGraphRWlock, 1); QEMU_LOCK_GUARD(&aio_context_list_lock); - assert(ctx->reader_count == 0); - QTAILQ_INSERT_TAIL(&aio_context_list, ctx, next_aio); + assert(ctx->bdrv_graph->reader_count == 0); + QTAILQ_INSERT_TAIL(&aio_context_list, ctx->bdrv_graph, next_aio); } void unregister_aiocontext(AioContext *ctx) { QEMU_LOCK_GUARD(&aio_context_list_lock); - orphaned_reader_count += ctx->reader_count; - QTAILQ_REMOVE(&aio_context_list, ctx, next_aio); + orphaned_reader_count += ctx->bdrv_graph->reader_count; + QTAILQ_REMOVE(&aio_context_list, ctx->bdrv_graph, next_aio); + g_free(ctx->bdrv_graph); } static uint32_t reader_count(void) { - AioContext *ctx; + BdrvGraphRWlock *brdv_graph; uint32_t rd; QEMU_LOCK_GUARD(&aio_context_list_lock); /* rd can temporarly be negative, but the total will *always* be >= 0 */ rd = orphaned_reader_count; - QTAILQ_FOREACH(ctx, &aio_context_list, next_aio) { - rd += qatomic_read(&ctx->reader_count); + QTAILQ_FOREACH(brdv_graph, &aio_context_list, next_aio) { + rd += qatomic_read(&brdv_graph->reader_count); } /* shouldn't overflow unless there are 2^31 readers */ @@ -138,12 +151,17 @@ void bdrv_graph_wrunlock(void) void coroutine_fn bdrv_graph_co_rdlock(void) { - AioContext *aiocontext; - aiocontext = qemu_get_current_aio_context(); + BdrvGraphRWlock *bdrv_graph; + bdrv_graph = qemu_get_current_aio_context()->bdrv_graph; + + /* Do not lock if in main thread */ + if (qemu_in_main_thread()) { + return; + } for (;;) { - qatomic_set(&aiocontext->reader_count, - aiocontext->reader_count + 1); + qatomic_set(&bdrv_graph->reader_count, + bdrv_graph->reader_count + 1); /* make sure writer sees reader_count before we check has_writer */ smp_mb(); @@ -192,7 +210,7 @@ void coroutine_fn bdrv_graph_co_rdlock(void) } /* slow path where reader sleeps */ - aiocontext->reader_count--; + bdrv_graph->reader_count--; aio_wait_kick(); qemu_co_queue_wait(&reader_queue, &aio_context_list_lock); } @@ -201,11 +219,16 @@ void coroutine_fn bdrv_graph_co_rdlock(void) void coroutine_fn bdrv_graph_co_rdunlock(void) { - AioContext *aiocontext; - aiocontext = qemu_get_current_aio_context(); + BdrvGraphRWlock *bdrv_graph; + bdrv_graph = qemu_get_current_aio_context()->bdrv_graph; + + /* Do not lock if in main thread */ + if (qemu_in_main_thread()) { + return; + } - qatomic_store_release(&aiocontext->reader_count, - aiocontext->reader_count - 1); + qatomic_store_release(&bdrv_graph->reader_count, + bdrv_graph->reader_count - 1); /* make sure writer sees reader_count before we check has_writer */ smp_mb(); diff --git a/include/block/aio.h b/include/block/aio.h index 8e64f81d01..0f65a3cc9e 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -22,6 +22,7 @@ #include "qemu/event_notifier.h" #include "qemu/thread.h" #include "qemu/timer.h" +#include "block/graph-lock.h" typedef struct BlockAIOCB BlockAIOCB; typedef void BlockCompletionFunc(void *opaque, int ret); @@ -127,14 +128,13 @@ struct AioContext { /* Used by AioContext users to protect from multi-threaded access. */ QemuRecMutex lock; - /* How many readers in this AioContext are currently reading the graph. */ - uint32_t reader_count; - /* - * List of AioContext kept in graph-lock.c - * Protected by aio_context_list_lock + * Keep track of readers and writers of the block layer graph. + * This is essential to avoid performing additions and removal + * of nodes and edges from block graph while some + * other thread is traversing it. */ - QTAILQ_ENTRY(AioContext) next_aio; + BdrvGraphRWlock *bdrv_graph; /* The list of registered AIO handlers. Protected by ctx->list_lock. */ AioHandlerList aio_handlers; diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h index f975312bb6..fc806aefa3 100644 --- a/include/block/graph-lock.h +++ b/include/block/graph-lock.h @@ -53,6 +53,7 @@ * reader count. In that case we transfer the count to a global shared counter * so that the writer is always aware of all readers. */ +typedef struct BdrvGraphRWlock BdrvGraphRWlock; /* * register_aiocontext: From patchwork Wed Nov 16 13:48:33 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045275 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id EF3EFC433FE for ; Wed, 16 Nov 2022 14:06:14 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImx-0000lP-2S; Wed, 16 Nov 2022 08:49:15 -0500 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 1ovImv-0000k2-6J for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:13 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-00080G-6g for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:12 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606538; h=from:from:reply-to:subject:subject: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=EoqI+zJNibzmtJd5D/R+ymj0Tnap71pP+qX6lyvpoVk=; b=UUoKkGP0BZqH3tFOsw31yriiWXF9eNJ3+Ddx6LTHCBboyQRggeRnZMM92m/PA7BofnFJ/r cJZaDI8Wh9ep4vWVhsgQ3YlvHOxg+sbvS8oNdYi+J17oSKud4JKcDIJMA52dNgJ0pYwHIE VL8D/rosG2Z1CziYOeXAUglqAP3oRnY= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-672-XdazSs1qPay5Zj0ECQCwEg-1; Wed, 16 Nov 2022 08:48:53 -0500 X-MC-Unique: XdazSs1qPay5Zj0ECQCwEg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3541C1C09B6B; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id DCFA24A9254; Wed, 16 Nov 2022 13:48:52 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 03/20] async: register/unregister aiocontext in graph lock list Date: Wed, 16 Nov 2022 08:48:33 -0500 Message-Id: <20221116134850.3051419-4-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Add/remove the AioContext in aio_context_list in graph-lock.c only when it is being effectively created/destroyed. Signed-off-by: Emanuele Giuseppe Esposito --- util/async.c | 4 ++++ util/meson.build | 1 + 2 files changed, 5 insertions(+) diff --git a/util/async.c b/util/async.c index 63434ddae4..14d63b3091 100644 --- a/util/async.c +++ b/util/async.c @@ -27,6 +27,7 @@ #include "qapi/error.h" #include "block/aio.h" #include "block/thread-pool.h" +#include "block/graph-lock.h" #include "qemu/main-loop.h" #include "qemu/atomic.h" #include "qemu/rcu_queue.h" @@ -376,6 +377,7 @@ aio_ctx_finalize(GSource *source) qemu_rec_mutex_destroy(&ctx->lock); qemu_lockcnt_destroy(&ctx->list_lock); timerlistgroup_deinit(&ctx->tlg); + unregister_aiocontext(ctx); aio_context_destroy(ctx); } @@ -574,6 +576,8 @@ AioContext *aio_context_new(Error **errp) ctx->thread_pool_min = 0; ctx->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT; + register_aiocontext(ctx); + return ctx; fail: g_source_destroy(&ctx->source); diff --git a/util/meson.build b/util/meson.build index 59c1f467bb..ecee2ba899 100644 --- a/util/meson.build +++ b/util/meson.build @@ -70,6 +70,7 @@ endif if have_block util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c')) + util_ss.add(files('../block/graph-lock.c')) util_ss.add(files('base64.c')) util_ss.add(files('buffer.c')) util_ss.add(files('bufferiszero.c')) From patchwork Wed Nov 16 13:48:34 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045243 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 7A533C433FE for ; Wed, 16 Nov 2022 13:56:18 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImx-0000lj-B9; Wed, 16 Nov 2022 08:49:15 -0500 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 1ovImr-0000gR-Tc for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-0007yp-F6 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606535; h=from:from:reply-to:subject:subject: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=g2+GJnkrBcVYy0m4pv+5qDBSIXbZhzgA3Sjaa/O5BJY=; b=Kc5qxzOu65nPeoWVS05MofTF19z5nXQo+VS+PxgLXpuWctNhg9Dz1MCPec0yxj+nwaou9z wjmRNaTrS/XKfi/3ppNfREAAUumALSLcCKPombYps2VptdiCOM0TESTh26PPDHxfxi6eqC 1YUvvZfbYaol5X9kgDUAtnn9uA714FU= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-16--PatpK5XOoqoxuZxSP6Tdw-1; Wed, 16 Nov 2022 08:48:53 -0500 X-MC-Unique: -PatpK5XOoqoxuZxSP6Tdw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8B110811E67; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3DF894A9265; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 04/20] block.c: wrlock in bdrv_replace_child_noperm Date: Wed, 16 Nov 2022 08:48:34 -0500 Message-Id: <20221116134850.3051419-5-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Protect the main function where graph is modified. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 6 ++++-- include/block/block_int-common.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index d3e168408a..4ef537a9f2 100644 --- a/block.c +++ b/block.c @@ -1416,6 +1416,7 @@ static void bdrv_child_cb_attach(BdrvChild *child) assert_bdrv_graph_writable(bs); QLIST_INSERT_HEAD(&bs->children, child, next); + if (bs->drv->is_filter || (child->role & BDRV_CHILD_FILTERED)) { /* * Here we handle filters and block/raw-format.c when it behave like @@ -2829,24 +2830,25 @@ static void bdrv_replace_child_noperm(BdrvChild *child, assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs)); } + bdrv_graph_wrlock(); if (old_bs) { if (child->klass->detach) { child->klass->detach(child); } - assert_bdrv_graph_writable(old_bs); + QLIST_REMOVE(child, next_parent); } child->bs = new_bs; if (new_bs) { - assert_bdrv_graph_writable(new_bs); QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); if (child->klass->attach) { child->klass->attach(child); } } + bdrv_graph_wrunlock(); /* * If the old child node was drained but the new one is not, allow diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 791dddfd7d..fd9f40a815 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -71,6 +71,7 @@ enum BdrvTrackedRequestType { BDRV_TRACKED_TRUNCATE, }; + /* * That is not quite good that BdrvTrackedRequest structure is public, * as block/io.c is very careful about incoming offset/bytes being From patchwork Wed Nov 16 13:48:35 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045232 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 82EB6C433FE for ; Wed, 16 Nov 2022 13:50:55 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovIn9-0000vo-2d; Wed, 16 Nov 2022 08:49:27 -0500 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 1ovImw-0000kz-MZ for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:14 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-0007zT-7t for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606537; h=from:from:reply-to:subject:subject: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=Y3vxFP81L43n1Rb+FCskSOc4CHmuTpdM1qdLBdInqJM=; b=XNi3FornLvDjlicwRf5D9QFyJdKa7/MbbggoS65IL+H+HqFk8UUgJHqzDlWFeFgY25x/N0 9MbuITnCiVrLfZcCc/RILLHwOARuo3Um6b7pUFizM7DRb+vY2fqzUN6OSQ0AiXsSQWQKuM Q6Qcxkdo46tKLfV16DyAWs551m2oJ+0= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-73-Na_HCIuiMK-uEqEwWJ3Owg-1; Wed, 16 Nov 2022 08:48:54 -0500 X-MC-Unique: Na_HCIuiMK-uEqEwWJ3Owg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E00F3833A09; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9396E4A9265; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 05/20] block: remove unnecessary assert_bdrv_graph_writable() Date: Wed, 16 Nov 2022 08:48:35 -0500 Message-Id: <20221116134850.3051419-6-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org We don't protect bdrv->aio_context with the graph rwlock, so these assertions are not needed Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/block.c b/block.c index 4ef537a9f2..afab74d4da 100644 --- a/block.c +++ b/block.c @@ -7183,7 +7183,6 @@ static void bdrv_detach_aio_context(BlockDriverState *bs) if (bs->quiesce_counter) { aio_enable_external(bs->aio_context); } - assert_bdrv_graph_writable(bs); bs->aio_context = NULL; } @@ -7197,7 +7196,6 @@ static void bdrv_attach_aio_context(BlockDriverState *bs, aio_disable_external(new_context); } - assert_bdrv_graph_writable(bs); bs->aio_context = new_context; if (bs->drv && bs->drv->bdrv_attach_aio_context) { @@ -7278,7 +7276,6 @@ static void bdrv_set_aio_context_commit(void *opaque) BlockDriverState *bs = (BlockDriverState *) state->bs; AioContext *new_context = state->new_ctx; AioContext *old_context = bdrv_get_aio_context(bs); - assert_bdrv_graph_writable(bs); /* * Take the old AioContex when detaching it from bs. From patchwork Wed Nov 16 13:48:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045270 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id D45A3C4332F for ; Wed, 16 Nov 2022 14:02:25 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImv-0000jx-6f; Wed, 16 Nov 2022 08:49:13 -0500 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 1ovImo-0000e5-GP for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:06 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-0007zb-2l for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:06 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606537; h=from:from:reply-to:subject:subject: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=4YSqZKWu6ML3JWAFCUXYj1V3Azhvoz41m0uIXXFHM7E=; b=HffZGAcZL3guGBeSkZ0NM6lu/r5JJjsPzJWjG8fCCZqgjESbLRRG0IvXSe81BQXC8gWXCj 1FIFm2LLIAE8DjMRzhUVKr/AYhmlkSGyiS4mvqIgZ12WRB5rO5u6uqAs1dM4xsS1GzLI1C n9dsXPZYzDMDMtqdNdeOHyx4JnG25oE= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-311-xSR2lpHGOCa-u0h8MXEfow-1; Wed, 16 Nov 2022 08:48:54 -0500 X-MC-Unique: xSR2lpHGOCa-u0h8MXEfow-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 420D11C09B6F; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id E91F44A9254; Wed, 16 Nov 2022 13:48:53 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 06/20] block: assert that graph read and writes are performed correctly Date: Wed, 16 Nov 2022 08:48:36 -0500 Message-Id: <20221116134850.3051419-7-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Remove the old assert_bdrv_graph_writable, and replace it with the new version using graph-lock API. See the function documentation for more information. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 4 ++-- block/graph-lock.c | 11 +++++++++++ include/block/block_int-global-state.h | 17 ----------------- include/block/graph-lock.h | 15 +++++++++++++++ 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/block.c b/block.c index afab74d4da..1c870d85e6 100644 --- a/block.c +++ b/block.c @@ -1414,7 +1414,7 @@ static void bdrv_child_cb_attach(BdrvChild *child) { BlockDriverState *bs = child->opaque; - assert_bdrv_graph_writable(bs); + assert_bdrv_graph_writable(); QLIST_INSERT_HEAD(&bs->children, child, next); if (bs->drv->is_filter || (child->role & BDRV_CHILD_FILTERED)) { @@ -1461,7 +1461,7 @@ static void bdrv_child_cb_detach(BdrvChild *child) bdrv_backing_detach(child); } - assert_bdrv_graph_writable(bs); + assert_bdrv_graph_writable(); QLIST_REMOVE(child, next); if (child == bs->backing) { assert(child != bs->file); diff --git a/block/graph-lock.c b/block/graph-lock.c index c3c6eeedad..07476fd7c8 100644 --- a/block/graph-lock.c +++ b/block/graph-lock.c @@ -242,3 +242,14 @@ void coroutine_fn bdrv_graph_co_rdunlock(void) aio_wait_kick(); } } + +void assert_bdrv_graph_readable(void) +{ + assert(qemu_in_main_thread() || reader_count()); +} + +void assert_bdrv_graph_writable(void) +{ + assert(qemu_in_main_thread()); + assert(qatomic_read(&has_writer)); +} diff --git a/include/block/block_int-global-state.h b/include/block/block_int-global-state.h index b49f4eb35b..2f0993f6e9 100644 --- a/include/block/block_int-global-state.h +++ b/include/block/block_int-global-state.h @@ -310,21 +310,4 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs, */ void bdrv_drain_all_end_quiesce(BlockDriverState *bs); -/** - * Make sure that the function is running under both drain and BQL. - * The latter protects from concurrent writings - * from the GS API, while the former prevents concurrent reads - * from I/O. - */ -static inline void assert_bdrv_graph_writable(BlockDriverState *bs) -{ - /* - * TODO: this function is incomplete. Because the users of this - * assert lack the necessary drains, check only for BQL. - * Once the necessary drains are added, - * assert also for qatomic_read(&bs->quiesce_counter) > 0 - */ - assert(qemu_in_main_thread()); -} - #endif /* BLOCK_INT_GLOBAL_STATE_H */ diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h index fc806aefa3..9430707dca 100644 --- a/include/block/graph-lock.h +++ b/include/block/graph-lock.h @@ -126,5 +126,20 @@ void coroutine_fn bdrv_graph_co_rdlock(void); */ void coroutine_fn bdrv_graph_co_rdunlock(void); +/* + * assert_bdrv_graph_readable: + * Make sure that the reader is either the main loop, + * or there is at least a reader helding the rdlock. + * In this way an incoming writer is aware of the read and waits. + */ +void assert_bdrv_graph_readable(void); + +/* + * assert_bdrv_graph_writable: + * Make sure that the writer is the main loop and has set @has_writer, + * so that incoming readers will pause. + */ +void assert_bdrv_graph_writable(void); + #endif /* GRAPH_LOCK_H */ From patchwork Wed Nov 16 13:48:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045237 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 315B1C433FE for ; Wed, 16 Nov 2022 13:53:11 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImz-0000pR-UK; Wed, 16 Nov 2022 08:49:17 -0500 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 1ovImq-0000gN-Vh for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-0007zr-Fj for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606538; h=from:from:reply-to:subject:subject: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=5Bup4dqvRe9vQAe8Y48N16XATGlyaXL89PUiWG6CZqU=; b=ZnrB5tHvh0o2NVSyjtlguCIXoPVADt/j39Fh7hWSk/XOcHxKLufE/358n21pyFmFjf6bmg IeUg+1/zBxr0UDzw70c41NNZAo3larfNMRlk7CLuV73ejOV8X6hkLTm/nna82DirGrL7is NLqhBz8438FjhZgLwWkFzEOX9+sUsCA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-357-8H906ThtNxmLpbHYBdEVGg-1; Wed, 16 Nov 2022 08:48:55 -0500 X-MC-Unique: 8H906ThtNxmLpbHYBdEVGg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 97A9F855438; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4AC754A9266; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 07/20] graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD macros Date: Wed, 16 Nov 2022 08:48:37 -0500 Message-Id: <20221116134850.3051419-8-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Similar to the implementation in lockable.h, implement macros to automatically take and release the rdlock. Create the empty GraphLockable struct only to use it as a type for G_DEFINE_AUTOPTR_CLEANUP_FUNC. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/graph-lock.h | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h index 9430707dca..0d886a9ca3 100644 --- a/include/block/graph-lock.h +++ b/include/block/graph-lock.h @@ -141,5 +141,40 @@ void assert_bdrv_graph_readable(void); */ void assert_bdrv_graph_writable(void); +typedef struct GraphLockable { } GraphLockable; + +/* + * In C, compound literals have the lifetime of an automatic variable. + * In C++ it would be different, but then C++ wouldn't need QemuLockable + * either... + */ +#define GML_OBJ_() (&(GraphLockable) { }) + +static inline GraphLockable *graph_lockable_auto_lock(GraphLockable *x) +{ + bdrv_graph_co_rdlock(); + return x; +} + +static inline void graph_lockable_auto_unlock(GraphLockable *x) +{ + bdrv_graph_co_rdunlock(); +} + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(GraphLockable, graph_lockable_auto_unlock) + +#define WITH_GRAPH_RDLOCK_GUARD_(var) \ + for (g_autoptr(GraphLockable) var = graph_lockable_auto_lock(GML_OBJ_()); \ + var; \ + graph_lockable_auto_unlock(var), var = NULL) + +#define WITH_GRAPH_RDLOCK_GUARD() \ + WITH_GRAPH_RDLOCK_GUARD_(glue(graph_lockable_auto, __COUNTER__)) + +#define GRAPH_RDLOCK_GUARD(x) \ + g_autoptr(GraphLockable) \ + glue(graph_lockable_auto, __COUNTER__) G_GNUC_UNUSED = \ + graph_lockable_auto_lock(GML_OBJ_()) + #endif /* GRAPH_LOCK_H */ From patchwork Wed Nov 16 13:48:38 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045239 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id B0A1FC4332F for ; Wed, 16 Nov 2022 13:53:50 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImw-0000kW-50; Wed, 16 Nov 2022 08:49:14 -0500 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 1ovImq-0000gL-Nf for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-00080C-ON for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606538; h=from:from:reply-to:subject:subject: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=3CD4T8FZnqFnpexjp0cS8K7p/5BHP/cxjCBCVIeTGBE=; b=U47gm7MKdvxeterR9mNQ6FQPPNBFLohPhw/GW+LWzH064d9sp5Tr1yE6oYdF4R0HUOLoeu Tle3HHfAgdp1l7fq8HNj+d+CdGc5+g6qwwxGzTM9Hf637mIwQ+ZLQ+jtCTgcLeGjXmb2HX y9dZE4zB9MebWystWToxn/zz6lB1cp4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-227-D8YWGYuZMCe2nVa4BW127w-1; Wed, 16 Nov 2022 08:48:55 -0500 X-MC-Unique: D8YWGYuZMCe2nVa4BW127w-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EDF15811E67; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id A09734A9265; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 08/20] block-coroutine-wrapper.py: take the graph rdlock in bdrv_* functions Date: Wed, 16 Nov 2022 08:48:38 -0500 Message-Id: <20221116134850.3051419-9-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org All generated_co_wrapper functions create a coroutine when called from non-coroutine context. The format can be one of the two: bdrv_something() if(qemu_in_coroutine()): bdrv_co_something(); else: // create coroutine that calls bdrv_co_something(); blk_something() if(qemu_in_coroutine()): blk_co_something(); else: // create coroutine that calls blk_co_something(); // blk_co_something() then eventually calls bdrv_co_something() The bdrv_co_something functions are recursively traversing the graph, therefore they all need to be protected with the graph rdlock. Instead, blk_co_something() calls bdrv_co_something(), so given that and being always called at the root of the graph (not in recursive callbacks), they should take the graph rdlock. The contract is simple, from now on, all bdrv_co_* functions called by g_c_w callbacks assume that the graph rdlock is taken at the coroutine creation, i.e. in g_c_w or in specific coroutines (right now we just consider the g_c_w case). All the blk_co_* are responsible of taking the rdlock (at this point is still a TBD). Suggested-by: Kevin Wolf Signed-off-by: Emanuele Giuseppe Esposito --- scripts/block-coroutine-wrapper.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index 21ecb3e896..05267761f0 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -67,8 +67,11 @@ def __init__(self, return_type: str, name: str, args: str, self.return_type = return_type.strip() self.name = name.strip() self.args = [ParamDecl(arg.strip()) for arg in args.split(',')] + self.lock = True self.create_only_co = False + if variant == '_blk': + self.lock = False if variant == '_simple': self.create_only_co = True @@ -86,7 +89,6 @@ def gen_block(self, format: str) -> str: r'(?P[a-z][a-z0-9_]*)' r'\((?P[^)]*)\);$', re.MULTILINE) - def func_decl_iter(text: str) -> Iterator: for m in func_decl_re.finditer(text): yield FuncDecl(return_type=m.group('return_type'), @@ -160,6 +162,13 @@ def gen_wrapper(func: FuncDecl) -> str: func.co_name = f'{subsystem}_co_{subname}' name = func.co_name + graph_lock='' + graph_unlock='' + if func.lock: + graph_lock=' bdrv_graph_co_rdlock();' + graph_unlock=' bdrv_graph_co_rdunlock();' + + t = func.args[0].type if t == 'BlockDriverState *': bs = 'bs' @@ -192,7 +201,9 @@ def gen_wrapper(func: FuncDecl) -> str: {{ {struct_name} *s = opaque; +{graph_lock} s->ret = {name}({ func.gen_list('s->{name}') }); +{graph_unlock} s->poll_state.in_progress = false; aio_wait_kick(); From patchwork Wed Nov 16 13:48:39 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045236 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6E571C4332F for ; Wed, 16 Nov 2022 13:52:39 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImz-0000no-3b; Wed, 16 Nov 2022 08:49:17 -0500 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 1ovIms-0000gZ-Se for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-00080J-Ek for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606538; h=from:from:reply-to:subject:subject: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=xfKtFYSJu+0gJSXduWanK/SQF9KPBXZtHSX/YbC+TaQ=; b=VAoxWjpdJv5dilF3GZOUvQ1r7QeIjMu5TsKesAAH3DKCIRsXUShtX69PGapaIX81m1vtkq JewyY2KNT+xLnkBLfHdtQ2GO87Kjmr8qMMGJzuUegGJBt3vp+4VVJA24MgiyXF+pw4o6mO nrbFDq2KRAd6+n0lqJwCMlY/jzJ9tVI= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-388-TrIP72nHMEu8q_2goRrHWg-1; Wed, 16 Nov 2022 08:48:55 -0500 X-MC-Unique: TrIP72nHMEu8q_2goRrHWg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 50CAF803D49; Wed, 16 Nov 2022 13:48:55 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 032AD49BB60; Wed, 16 Nov 2022 13:48:54 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 09/20] block-backend: introduce new generated_co_wrapper_blk annotation Date: Wed, 16 Nov 2022 08:48:39 -0500 Message-Id: <20221116134850.3051419-10-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This annotation will be used to distinguish the blk_* API from the bdrv_* API in block-gen.c. The reason for this distinction is that blk_* API eventually result in always calling bdrv_*, which has implications when we introduce the read graph lock. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block-common.h | 1 + include/sysemu/block-backend-io.h | 69 ++++++++++++++++--------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/include/block/block-common.h b/include/block/block-common.h index 683e3d1c51..f70f1560c5 100644 --- a/include/block/block-common.h +++ b/include/block/block-common.h @@ -42,6 +42,7 @@ */ #define generated_co_wrapper #define generated_co_wrapper_simple +#define generated_co_wrapper_blk #include "block/dirty-bitmap.h" #include "block/blockjob.h" diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h index a47cb825e5..887a29dc59 100644 --- a/include/sysemu/block-backend-io.h +++ b/include/sysemu/block-backend-io.h @@ -110,77 +110,78 @@ int coroutine_fn blk_is_allocated_above(BlockBackend *blk, * the "I/O or GS" API. */ -int generated_co_wrapper blk_pread(BlockBackend *blk, int64_t offset, - int64_t bytes, void *buf, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_pread(BlockBackend *blk, int64_t offset, + int64_t bytes, void *buf, + BdrvRequestFlags flags); int coroutine_fn blk_co_pread(BlockBackend *blk, int64_t offset, int64_t bytes, void *buf, BdrvRequestFlags flags); -int generated_co_wrapper blk_preadv(BlockBackend *blk, int64_t offset, - int64_t bytes, QEMUIOVector *qiov, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_preadv(BlockBackend *blk, int64_t offset, + int64_t bytes, QEMUIOVector *qiov, + BdrvRequestFlags flags); int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); -int generated_co_wrapper blk_preadv_part(BlockBackend *blk, int64_t offset, - int64_t bytes, QEMUIOVector *qiov, - size_t qiov_offset, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_preadv_part(BlockBackend *blk, int64_t offset, + int64_t bytes, QEMUIOVector *qiov, + size_t qiov_offset, + BdrvRequestFlags flags); int coroutine_fn blk_co_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags); -int generated_co_wrapper blk_pwrite(BlockBackend *blk, int64_t offset, - int64_t bytes, const void *buf, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_pwrite(BlockBackend *blk, int64_t offset, + int64_t bytes, const void *buf, + BdrvRequestFlags flags); int coroutine_fn blk_co_pwrite(BlockBackend *blk, int64_t offset, int64_t bytes, const void *buf, BdrvRequestFlags flags); -int generated_co_wrapper blk_pwritev(BlockBackend *blk, int64_t offset, - int64_t bytes, QEMUIOVector *qiov, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_pwritev(BlockBackend *blk, int64_t offset, + int64_t bytes, QEMUIOVector *qiov, + BdrvRequestFlags flags); int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); -int generated_co_wrapper blk_pwritev_part(BlockBackend *blk, int64_t offset, - int64_t bytes, QEMUIOVector *qiov, - size_t qiov_offset, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_pwritev_part(BlockBackend *blk, int64_t offset, + int64_t bytes, QEMUIOVector *qiov, + size_t qiov_offset, + BdrvRequestFlags flags); int coroutine_fn blk_co_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags); -int generated_co_wrapper blk_pwrite_compressed(BlockBackend *blk, - int64_t offset, int64_t bytes, - const void *buf); +int generated_co_wrapper_blk blk_pwrite_compressed(BlockBackend *blk, + int64_t offset, int64_t bytes, + const void *buf); int coroutine_fn blk_co_pwrite_compressed(BlockBackend *blk, int64_t offset, int64_t bytes, const void *buf); -int generated_co_wrapper blk_pwrite_zeroes(BlockBackend *blk, int64_t offset, - int64_t bytes, - BdrvRequestFlags flags); +int generated_co_wrapper_blk blk_pwrite_zeroes(BlockBackend *blk, + int64_t offset, + int64_t bytes, + BdrvRequestFlags flags); int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset, int64_t bytes, BdrvRequestFlags flags); -int generated_co_wrapper blk_pdiscard(BlockBackend *blk, int64_t offset, - int64_t bytes); +int generated_co_wrapper_blk blk_pdiscard(BlockBackend *blk, int64_t offset, + int64_t bytes); int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, int64_t bytes); -int generated_co_wrapper blk_flush(BlockBackend *blk); +int generated_co_wrapper_blk blk_flush(BlockBackend *blk); int coroutine_fn blk_co_flush(BlockBackend *blk); -int generated_co_wrapper blk_ioctl(BlockBackend *blk, unsigned long int req, - void *buf); +int generated_co_wrapper_blk blk_ioctl(BlockBackend *blk, unsigned long int req, + void *buf); int coroutine_fn blk_co_ioctl(BlockBackend *blk, unsigned long int req, void *buf); -int generated_co_wrapper blk_truncate(BlockBackend *blk, int64_t offset, - bool exact, PreallocMode prealloc, - BdrvRequestFlags flags, Error **errp); +int generated_co_wrapper_blk blk_truncate(BlockBackend *blk, int64_t offset, + bool exact, PreallocMode prealloc, + BdrvRequestFlags flags, Error **errp); int coroutine_fn blk_co_truncate(BlockBackend *blk, int64_t offset, bool exact, PreallocMode prealloc, BdrvRequestFlags flags, Error **errp); From patchwork Wed Nov 16 13:48:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045238 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 96336C4332F for ; Wed, 16 Nov 2022 13:53:42 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImz-0000nu-6F; Wed, 16 Nov 2022 08:49:17 -0500 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 1ovImt-0000iT-VR for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:12 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-00080a-4l for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606539; h=from:from:reply-to:subject:subject: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=PjkJK6fNxX7PuZBAO179pYQpUeHznsG1I0kwLAp/h5g=; b=VomFCq5EJFZ2Jiqa8Rx47MbR5bQcXH6B3cnAPI5CU21FS8XDt9L1uZEZvSJP1Xvx1ctr66 F/xdRISloyYXsgvjYY+NhwK7KSSCXT4FHO7/4BZNgsTb6PS3m75OqHXrvfDel2YYCZiHmu BXiN2zl9aneTg+7Jonj1+qUwqVUrtfI= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-527-9hvFmt71McqzwyPi_hupHg-1; Wed, 16 Nov 2022 08:48:56 -0500 X-MC-Unique: 9hvFmt71McqzwyPi_hupHg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A636385A5B6; Wed, 16 Nov 2022 13:48:55 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 598604A9266; Wed, 16 Nov 2022 13:48:55 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 10/20] block-gen: assert that {bdrv/blk}_co_truncate is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:40 -0500 Message-Id: <20221116134850.3051419-11-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This function, in addition to be called by a generated_co_wrapper, is also called by the blk_* API. The strategy is to always take the lock at the function called when the coroutine is created, to avoid recursive locking. Protecting bdrv_co_truncate() implies that BlockDriver->bdrv_co_truncate() is always called with graph rdlock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 1 + block/io.c | 1 + include/block/block_int-common.h | 2 ++ 3 files changed, 4 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 333d50fb3f..0686cd6942 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -2370,6 +2370,7 @@ int coroutine_fn blk_co_truncate(BlockBackend *blk, int64_t offset, bool exact, Error **errp) { IO_OR_GS_CODE(); + GRAPH_RDLOCK_GUARD(); if (!blk_is_available(blk)) { error_setg(errp, "No medium inserted"); return -ENOMEDIUM; diff --git a/block/io.c b/block/io.c index 9bcb19e5ee..ac12725fb2 100644 --- a/block/io.c +++ b/block/io.c @@ -3295,6 +3295,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, int64_t old_size, new_bytes; int ret; IO_CODE(); + assert_bdrv_graph_readable(); /* if bs->drv == NULL, bs is closed, so there's nothing to do here */ if (!drv) { diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index fd9f40a815..d666b0c441 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -681,6 +681,8 @@ struct BlockDriver { * * If @exact is true and this function fails but would succeed * with @exact = false, it should return -ENOTSUP. + * + * Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset, bool exact, PreallocMode prealloc, From patchwork Wed Nov 16 13:48:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045233 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 0A61AC433FE for ; Wed, 16 Nov 2022 13:51:22 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImw-0000kh-E1; Wed, 16 Nov 2022 08:49:14 -0500 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 1ovImr-0000gQ-R9 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-00080g-Ei for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606539; h=from:from:reply-to:subject:subject: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=Mro4V6JNIQKqh1xbG4rtdd/lPqkptajad1lwxt70p2k=; b=iSV0RX6jPYB7n291azve8PH2yvHpCykDT8k9Cu6uuWcJlZsuYXk6/8GL2wNnK3IRtm+R/d BV7NRZGf0ZSj+SxN8WTAWxVIA7NtSv2wWuHuu+p2h3FxinuFkTg211b5KVdrsnJCGxI/oq n7z1fGhiAJxN5o7pqeXViRJtpig56TA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-529-33CxAhczMsee5qBMhDjQlg-1; Wed, 16 Nov 2022 08:48:56 -0500 X-MC-Unique: 33CxAhczMsee5qBMhDjQlg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 07F1F811E75; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id AEF7A4A9266; Wed, 16 Nov 2022 13:48:55 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 11/20] block-gen: assert that bdrv_co_{check/invalidate_cache} are always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:41 -0500 Message-Id: <20221116134850.3051419-12-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The only callers of these functions are the respective generated_co_wrapper, and they already take the lock. Protecting bdrv_co_{check/invalidate_cache}() implies that BlockDriver->bdrv_co_{check/invalidate_cache}() is always called with graph rdlock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 ++ include/block/block_int-common.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/block.c b/block.c index 1c870d85e6..c7611bed9e 100644 --- a/block.c +++ b/block.c @@ -5375,6 +5375,7 @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix) { IO_CODE(); + assert_bdrv_graph_readable(); if (bs->drv == NULL) { return -ENOMEDIUM; } @@ -6590,6 +6591,7 @@ int coroutine_fn bdrv_co_invalidate_cache(BlockDriverState *bs, Error **errp) IO_CODE(); assert(!(bs->open_flags & BDRV_O_INACTIVE)); + assert_bdrv_graph_readable(); if (bs->drv->bdrv_co_invalidate_cache) { bs->drv->bdrv_co_invalidate_cache(bs, &local_err); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index d666b0c441..f285a6b8f7 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -641,6 +641,7 @@ struct BlockDriver { /* * Invalidate any cached meta-data. + * Called with graph rdlock held. */ void coroutine_fn (*bdrv_co_invalidate_cache)(BlockDriverState *bs, Error **errp); @@ -726,6 +727,7 @@ struct BlockDriver { /* * Returns 0 for completed check, -errno for internal errors. * The check results are stored in result. + * Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_check)(BlockDriverState *bs, BdrvCheckResult *result, From patchwork Wed Nov 16 13:48:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045230 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 91880C43219 for ; Wed, 16 Nov 2022 13:50:05 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImw-0000kg-DT; Wed, 16 Nov 2022 08:49:14 -0500 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 1ovImq-0000gM-O6 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-0007zl-FE for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:08 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606538; h=from:from:reply-to:subject:subject: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=PJp3ZbKA4Z5qVER4UBk8asMbPlsuCplDYEISzsM1/Lc=; b=hzfc6+pUzC0BMLIv6VvdvJ/a5eZ0v4qwjOduvgAXFmIdWRUfwmik5tmlvVYXA0W0fEoaKA 6cssLsYzp6GDavDyHm4Vt1mZkNyYh4khzJeEGYD7jcQOaWhg+QITR3TMAJOe41DC24kW0z +ezJ9oAB48VKCicfOqcdemW8A8DCbTM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-47--fZCKPObOfiyROs1gerTEA-1; Wed, 16 Nov 2022 08:48:56 -0500 X-MC-Unique: -fZCKPObOfiyROs1gerTEA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5D823800B23; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 112C04A9266; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 12/20] block-gen: assert that bdrv_co_pwrite is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:42 -0500 Message-Id: <20221116134850.3051419-13-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This function, in addition to be called by a generated_co_wrapper, is also called elsewhere else. The strategy is to always take the lock at the function called when the coroutine is created, to avoid recursive locking. By protecting brdv_co_pwrite, we also automatically protect the following other generated_co_wrappers: blk_co_pwrite blk_co_pwritev blk_co_pwritev_part blk_co_pwrite_compressed blk_co_pwrite_zeroes Protecting bdrv_driver_pwritev_compressed() and bdrv_driver_pwritev_compressed() implies that the following BlockDriver callbacks always called with graph rdlock taken: - bdrv_aio_pwritev - bdrv_co_writev - bdrv_co_pwritev - bdrv_co_pwritev_part - bdrv_co_pwritev_compressed - bdrv_co_pwritev_compressed_part Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 1 + block/block-copy.c | 8 ++++++-- block/io.c | 2 ++ include/block/block_int-common.h | 6 ++++++ include/block/block_int-io.h | 1 + 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 0686cd6942..d48ec3a2ac 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1363,6 +1363,7 @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes, IO_CODE(); blk_wait_while_drained(blk); + GRAPH_RDLOCK_GUARD(); /* Call blk_bs() only after waiting, the graph may have changed */ bs = blk_bs(blk); diff --git a/block/block-copy.c b/block/block-copy.c index f33ab1d0b6..dabf461112 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -464,6 +464,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 graph rdlock taken. */ static int coroutine_fn block_copy_do_copy(BlockCopyState *s, int64_t offset, int64_t bytes, @@ -554,8 +556,10 @@ static coroutine_fn int block_copy_task_entry(AioTask *task) BlockCopyMethod method = t->method; int ret; - ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method, - &error_is_read); + WITH_GRAPH_RDLOCK_GUARD() { + ret = block_copy_do_copy(s, t->req.offset, t->req.bytes, &method, + &error_is_read); + } WITH_QEMU_LOCK_GUARD(&s->lock) { if (s->method == t->method) { diff --git a/block/io.c b/block/io.c index ac12725fb2..9280fb9f38 100644 --- a/block/io.c +++ b/block/io.c @@ -1012,6 +1012,7 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs, unsigned int nb_sectors; QEMUIOVector local_qiov; int ret; + assert_bdrv_graph_readable(); bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort); @@ -1090,6 +1091,7 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, int64_t offset, BlockDriver *drv = bs->drv; QEMUIOVector local_qiov; int ret; + assert_bdrv_graph_readable(); bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort); diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index f285a6b8f7..d44f825d95 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -479,6 +479,7 @@ struct BlockDriver { BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque); + /* Called with graph rdlock taken. */ BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque); @@ -515,6 +516,7 @@ struct BlockDriver { QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags); + /* Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags); @@ -532,10 +534,12 @@ struct BlockDriver { * no larger than 'max_transfer'. * * The buffer in @qiov may point directly to guest memory. + * Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); + /* Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_pwritev_part)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset, BdrvRequestFlags flags); @@ -693,8 +697,10 @@ struct BlockDriver { BlockMeasureInfo *(*bdrv_measure)(QemuOpts *opts, BlockDriverState *in_bs, Error **errp); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_pwritev_compressed)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_pwritev_compressed_part)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset); diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index 8bc061ebb8..ae88507d6a 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -69,6 +69,7 @@ static inline int coroutine_fn bdrv_co_pwrite(BdrvChild *child, { QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); IO_CODE(); + assert_bdrv_graph_readable(); return bdrv_co_pwritev(child, offset, bytes, &qiov, flags); } From patchwork Wed Nov 16 13:48:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045271 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 36466C4332F for ; Wed, 16 Nov 2022 14:02:34 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImv-0000kE-I1; Wed, 16 Nov 2022 08:49:13 -0500 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 1ovImp-0000fT-DJ for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:07 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-000810-2G for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:07 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606540; h=from:from:reply-to:subject:subject: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=Yinl0pSM3o2c/4hIiFf86CIDWq3Dli2+EqExA1nqS7M=; b=WzpcMUT9LKixGF3iPYvxaiJwCqKh8ZrB2gQJ4HmLZXe8Ga+Xfw9Da5b/M38eXmSvaDumQU Qc6ILhWGbpJYu091m/0ES5tRcl2OoUzzVME9fNCBEsOxrHpzZzR2ba/pj2eHMzGZT4MZ/d pvsBhwnXp6OnosDjc/wzCyL5VO6+bfE= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-496-bV-QpyNvM_KvbUuP-8PLAw-1; Wed, 16 Nov 2022 08:48:57 -0500 X-MC-Unique: bV-QpyNvM_KvbUuP-8PLAw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B5B94101A52A; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 669494A9268; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 13/20] block-gen: assert that bdrv_co_pwrite_{zeros/sync} is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:43 -0500 Message-Id: <20221116134850.3051419-14-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Already protected by bdrv_co_pwrite callers. Protecting bdrv_co_do_pwrite_zeroes() implies that BlockDriver->bdrv_co_pwrite_zeroes() is always called with graph rdlock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 3 +++ include/block/block_int-common.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/block/io.c b/block/io.c index 9280fb9f38..92c74648fb 100644 --- a/block/io.c +++ b/block/io.c @@ -904,6 +904,7 @@ int coroutine_fn bdrv_co_pwrite_sync(BdrvChild *child, int64_t offset, { int ret; IO_CODE(); + assert_bdrv_graph_readable(); ret = bdrv_co_pwrite(child, offset, bytes, buf, flags); if (ret < 0) { @@ -1660,6 +1661,7 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs, bs->bl.request_alignment); int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER); + assert_bdrv_graph_readable(); bdrv_check_request(offset, bytes, &error_abort); if (!drv) { @@ -2124,6 +2126,7 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, { IO_CODE(); trace_bdrv_co_pwrite_zeroes(child->bs, offset, bytes, flags); + assert_bdrv_graph_readable(); if (!(child->bs->open_flags & BDRV_O_UNMAP)) { flags &= ~BDRV_REQ_MAY_UNMAP; diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index d44f825d95..e8d2e4b6c7 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -549,6 +549,8 @@ struct BlockDriver { * would use a compact metadata representation to implement this. This * function pointer may be NULL or return -ENOSUP and .bdrv_co_writev() * will be called instead. + * + * Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_pwrite_zeroes)(BlockDriverState *bs, int64_t offset, int64_t bytes, BdrvRequestFlags flags); From patchwork Wed Nov 16 13:48:44 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045254 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 6526FC4332F for ; Wed, 16 Nov 2022 13:57:36 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovIn3-0000rJ-0t; Wed, 16 Nov 2022 08:49:21 -0500 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 1ovImw-0000kk-Dx for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:14 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-00081M-7E for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:14 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606541; h=from:from:reply-to:subject:subject: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=y1AhexmEvH/m4MQIk481Hku2zrtOuEL6VbcS01NVR/0=; b=GQtBGfQBaPmNWMrU/5Yzr2jr7jo7LO3cGtEyT22lifBLc9zjsL/skfJRIR3NbHNfy17Zv2 4DZzbsGKcMu0nMgjXlmcw46u0Hhs9/u7O7oDhj6Eob5KZE+cTafmqIeaf9X+LNaeWWmNUm Bg9XhHj23FddYCyKgwNxeEs/DkGAhj4= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-571-zRTKUX7qP56uIM-KyDHoWQ-1; Wed, 16 Nov 2022 08:48:57 -0500 X-MC-Unique: zRTKUX7qP56uIM-KyDHoWQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 15A6F101A56C; Wed, 16 Nov 2022 13:48:57 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id BCB624A9266; Wed, 16 Nov 2022 13:48:56 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 14/20] block-gen: assert that bdrv_co_pread is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:44 -0500 Message-Id: <20221116134850.3051419-15-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This function, in addition to be called by a generated_co_wrapper, is also called elsewhere else. The strategy is to always take the lock at the function called when the coroutine is created, to avoid recursive locking. By protecting brdv_co_pread, we also automatically protect the following other generated_co_wrappers: blk_co_pread blk_co_preadv blk_co_preadv_part Protecting bdrv_driver_preadv() implies that the following BlockDriver callbacks always called with graph rdlock taken: - bdrv_co_preadv_part - bdrv_co_preadv - bdrv_aio_preadv - bdrv_co_readv Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 1 + block/io.c | 1 + block/mirror.c | 6 ++++-- include/block/block_int-common.h | 5 +++++ include/block/block_int-io.h | 1 + tests/unit/test-bdrv-drain.c | 2 ++ 6 files changed, 14 insertions(+), 2 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index d48ec3a2ac..083ed6009e 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1289,6 +1289,7 @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes, IO_CODE(); blk_wait_while_drained(blk); + GRAPH_RDLOCK_GUARD(); /* Call blk_bs() only after waiting, the graph may have changed */ bs = blk_bs(blk); diff --git a/block/io.c b/block/io.c index 92c74648fb..cfc201ef91 100644 --- a/block/io.c +++ b/block/io.c @@ -942,6 +942,7 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs, unsigned int nb_sectors; QEMUIOVector local_qiov; int ret; + assert_bdrv_graph_readable(); bdrv_check_qiov_request(offset, bytes, qiov, qiov_offset, &error_abort); assert(!(flags & ~bs->supported_read_flags)); diff --git a/block/mirror.c b/block/mirror.c index 251adc5ae0..f509cc1cb1 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -389,8 +389,10 @@ static void coroutine_fn mirror_co_read(void *opaque) op->is_in_flight = true; trace_mirror_one_iteration(s, op->offset, op->bytes); - ret = bdrv_co_preadv(s->mirror_top_bs->backing, op->offset, op->bytes, - &op->qiov, 0); + WITH_GRAPH_RDLOCK_GUARD() { + ret = bdrv_co_preadv(s->mirror_top_bs->backing, op->offset, op->bytes, + &op->qiov, 0); + } mirror_read_complete(op, ret); } diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index e8d2e4b6c7..64c5bb64de 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -476,6 +476,7 @@ struct BlockDriver { Error **errp); /* aio */ + /* Called with graph rdlock held. */ BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags, BlockCompletionFunc *cb, void *opaque); @@ -489,6 +490,7 @@ struct BlockDriver { int64_t offset, int bytes, BlockCompletionFunc *cb, void *opaque); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_readv)(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov); @@ -506,11 +508,14 @@ struct BlockDriver { * no larger than 'max_transfer'. * * The buffer in @qiov may point directly to guest memory. + * + * Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_preadv)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, BdrvRequestFlags flags); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_preadv_part)(BlockDriverState *bs, int64_t offset, int64_t bytes, QEMUIOVector *qiov, size_t qiov_offset, diff --git a/include/block/block_int-io.h b/include/block/block_int-io.h index ae88507d6a..ac6ad3b3ff 100644 --- a/include/block/block_int-io.h +++ b/include/block/block_int-io.h @@ -60,6 +60,7 @@ static inline int coroutine_fn bdrv_co_pread(BdrvChild *child, { QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf, bytes); IO_CODE(); + assert_bdrv_graph_readable(); return bdrv_co_preadv(child, offset, bytes, &qiov, flags); } diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 2686a8acee..90edc2f5bf 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -967,6 +967,8 @@ static void coroutine_fn test_co_delete_by_drain(void *opaque) void *buffer = g_malloc(65536); QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buffer, 65536); + GRAPH_RDLOCK_GUARD(); + /* Pretend some internal write operation from parent to child. * Important: We have to read from the child, not from the parent! * Draining works by first propagating it all up the tree to the From patchwork Wed Nov 16 13:48:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045234 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 2F352C4332F for ; Wed, 16 Nov 2022 13:52:27 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovIn3-0000sI-Mj; Wed, 16 Nov 2022 08:49:21 -0500 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 1ovImx-0000mI-QJ for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:15 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-00082o-SG for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:15 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606544; h=from:from:reply-to:subject:subject: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=O674RGVxUBm8Idb7TTf4T44EFihJr3wsC24ssr01v7s=; b=YW07zA4bx7kM0qk1uhhYoVkXNPDD3uli1Ov8kFXnsPcLKnnipTtAtCtVdoHifUBRIKVr+N S8sWo/41SBsXIYrQaii5GM0CO+D877cr0gE67JCKcn1Z8pWr2w2+GyyLJEQRzqCsTkFwJg t3/HaOlnmRexFZvJP8ao3PBvyP1RUHs= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-573-omFFKHu1NpmXtkh_LJXddg-1; Wed, 16 Nov 2022 08:48:57 -0500 X-MC-Unique: omFFKHu1NpmXtkh_LJXddg-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6BA90811E75; Wed, 16 Nov 2022 13:48:57 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1EBB44A9266; Wed, 16 Nov 2022 13:48:57 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 15/20] block-gen: assert that {bdrv/blk}_co_flush is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:45 -0500 Message-Id: <20221116134850.3051419-16-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This function, in addition to be called by a generated_co_wrapper, is also called by the blk_* API. The strategy is to always take the lock at the function called when the coroutine is created, to avoid recursive locking. Protecting bdrv_co_flush() implies that the following BlockDriver callbacks always called with graph rdlock taken: - bdrv_co_flush - bdrv_co_flush_to_os - bdrv_co_flush_to_disk Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 3 ++- block/io.c | 1 + include/block/block_int-common.h | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/block/block-backend.c b/block/block-backend.c index 083ed6009e..d660772375 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1759,8 +1759,9 @@ int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, /* To be called between exactly one pair of blk_inc/dec_in_flight() */ static int coroutine_fn blk_co_do_flush(BlockBackend *blk) { - blk_wait_while_drained(blk); IO_CODE(); + blk_wait_while_drained(blk); + GRAPH_RDLOCK_GUARD(); if (!blk_is_available(blk)) { return -ENOMEDIUM; diff --git a/block/io.c b/block/io.c index cfc201ef91..0bf3919939 100644 --- a/block/io.c +++ b/block/io.c @@ -2757,6 +2757,7 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs) int ret = 0; IO_CODE(); + assert_bdrv_graph_readable(); bdrv_inc_in_flight(bs); if (!bdrv_is_inserted(bs) || bdrv_is_read_only(bs) || diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 64c5bb64de..bab0521943 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -661,6 +661,8 @@ struct BlockDriver { * Flushes all data for all layers by calling bdrv_co_flush for underlying * layers, if needed. This function is needed for deterministic * synchronization of the flush finishing callback. + * + * Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs); @@ -671,6 +673,8 @@ struct BlockDriver { /* * Flushes all data that was already written to the OS all the way down to * the disk (for example file-posix.c calls fsync()). + * + * Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs); @@ -678,6 +682,8 @@ struct BlockDriver { * Flushes all internal caches to the OS. The data may still sit in a * writeback cache of the host OS, but it will survive a crash of the qemu * process. + * + * Called with graph rdlock held. */ int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs); From patchwork Wed Nov 16 13:48:46 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045240 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 21F16C4332F for ; Wed, 16 Nov 2022 13:54:06 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImv-0000k8-Cb; Wed, 16 Nov 2022 08:49:13 -0500 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 1ovImr-0000gS-Vi for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-00081H-Ek for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606541; h=from:from:reply-to:subject:subject: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=wo/qRDH8s57DwZ9+HKzHDxsnu7HebCsUdmJvrjTvAMM=; b=etcIl0rhTmH1gH7MGF2XdFUBF4LLAGUHpunrDB5gtS9pDICH8LM670B9c+tJ4RpbsfzvM2 2kqUGh/rYIHRgnel4Z2VG3iW/0N1gEUiZXq0jZ+/atpDiV5qVNSmrd8QYczUQt1fVA8LdF M3NA4lzvHjq+YS6sUs85l/2gvqD6NnM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-67-UxuLkavBPuuEdQXkAfZTbA-1; Wed, 16 Nov 2022 08:48:58 -0500 X-MC-Unique: UxuLkavBPuuEdQXkAfZTbA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C1A93801755; Wed, 16 Nov 2022 13:48:57 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 744944A9265; Wed, 16 Nov 2022 13:48:57 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 16/20] block-gen: assert that bdrv_co_{read/write}v_vmstate are always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:46 -0500 Message-Id: <20221116134850.3051419-17-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The only caller of these functions is bdrv_{read/write}v_vmstate, a generated_co_wrapper function that already takes the graph read lock. Protecting bdrv_co_{read/write}v_vmstate() implies that BlockDriver->bdrv_{load/save}_vmstate() is always called with graph rdlock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 2 ++ include/block/block_int-common.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/block/io.c b/block/io.c index 0bf3919939..c9b451fecd 100644 --- a/block/io.c +++ b/block/io.c @@ -2633,6 +2633,7 @@ bdrv_co_readv_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) BlockDriverState *child_bs = bdrv_primary_bs(bs); int ret; IO_CODE(); + assert_bdrv_graph_readable(); ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL); if (ret < 0) { @@ -2665,6 +2666,7 @@ bdrv_co_writev_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos) BlockDriverState *child_bs = bdrv_primary_bs(bs); int ret; IO_CODE(); + assert_bdrv_graph_readable(); ret = bdrv_check_qiov_request(pos, qiov->size, qiov, 0, NULL); if (ret < 0) { diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index bab0521943..568c2d3092 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -724,9 +724,11 @@ struct BlockDriver { Error **errp); BlockStatsSpecific *(*bdrv_get_specific_stats)(BlockDriverState *bs); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); + /* Called with graph rdlock held. */ int coroutine_fn (*bdrv_load_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos); From patchwork Wed Nov 16 13:48:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045277 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 00FF1C433FE for ; Wed, 16 Nov 2022 14:07:40 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImx-0000lh-AE; Wed, 16 Nov 2022 08:49:15 -0500 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 1ovImr-0000gO-BZ for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-000816-El for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:09 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606540; h=from:from:reply-to:subject:subject: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=4l2gJe+3drWx6sIrzk3hxD4/frbZRdLeHipnc18CfKY=; b=FSAcIAmobfVg22H8GOhMLRn67a5M4ZlC02IcRxZJkh1ZCzGxcsxmqxw2b/ve1EiZma2z5w svMncSsKidophts/G8MRgKyPvmmvs8QGUc7LzkFF9XV6OyNSzVom3K2IFQ6RLD05GkcUTt /JOVcf96FcUlbuCZi7E8KNYAhTa/oJA= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-479-fzRwjVt-NrWR0n-supdGng-1; Wed, 16 Nov 2022 08:48:59 -0500 X-MC-Unique: fzRwjVt-NrWR0n-supdGng-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 13FBD2A59579; Wed, 16 Nov 2022 13:48:59 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id B71554A9268; Wed, 16 Nov 2022 13:48:58 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 18/20] block-gen: assert that bdrv_co_common_block_status_above is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:48 -0500 Message-Id: <20221116134850.3051419-19-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 Received-SPF: pass client-ip=170.10.129.124; envelope-from=eesposit@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org This function, in addition to be called by a generated_co_wrapper, is also called elsewhere else. The strategy is to always take the lock at the function called when the coroutine is created, to avoid recursive locking. Protecting bdrv_co_block_status() called by bdrv_co_common_block_status_above() implies that BlockDriver->bdrv_co_block_status() is always called with graph rdlock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block/backup.c | 3 +++ block/block-backend.c | 2 ++ block/block-copy.c | 2 ++ block/io.c | 2 ++ block/mirror.c | 14 +++++++++----- block/stream.c | 32 ++++++++++++++++++-------------- include/block/block_int-common.h | 2 ++ qemu-img.c | 4 +++- 8 files changed, 41 insertions(+), 20 deletions(-) diff --git a/block/backup.c b/block/backup.c index 6a9ad97a53..42b16d0136 100644 --- a/block/backup.c +++ b/block/backup.c @@ -269,7 +269,10 @@ static int coroutine_fn backup_run(Job *job, Error **errp) return -ECANCELED; } + /* rdlock protects the subsequent call to bdrv_is_allocated() */ + bdrv_graph_co_rdlock(); ret = block_copy_reset_unallocated(s->bcs, offset, &count); + bdrv_graph_co_rdunlock(); if (ret < 0) { return ret; } diff --git a/block/block-backend.c b/block/block-backend.c index 211a813523..20b772a476 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1433,6 +1433,7 @@ int coroutine_fn blk_block_status_above(BlockBackend *blk, BlockDriverState **file) { IO_CODE(); + GRAPH_RDLOCK_GUARD(); return bdrv_block_status_above(blk_bs(blk), base, offset, bytes, pnum, map, file); } @@ -1443,6 +1444,7 @@ int coroutine_fn blk_is_allocated_above(BlockBackend *blk, int64_t bytes, int64_t *pnum) { IO_CODE(); + GRAPH_RDLOCK_GUARD(); return bdrv_is_allocated_above(blk_bs(blk), base, include_base, offset, bytes, pnum); } diff --git a/block/block-copy.c b/block/block-copy.c index dabf461112..e20d2b2f78 100644 --- a/block/block-copy.c +++ b/block/block-copy.c @@ -630,6 +630,7 @@ static int coroutine_fn block_copy_is_cluster_allocated(BlockCopyState *s, assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); while (true) { + /* protected in backup_run() */ ret = bdrv_is_allocated(bs, offset, bytes, &count); if (ret < 0) { return ret; @@ -892,6 +893,7 @@ static int coroutine_fn block_copy_common(BlockCopyCallState *call_state) static void coroutine_fn block_copy_async_co_entry(void *opaque) { + GRAPH_RDLOCK_GUARD(); block_copy_common(opaque); } diff --git a/block/io.c b/block/io.c index bc9f47538c..c5b3bb0a6d 100644 --- a/block/io.c +++ b/block/io.c @@ -2215,6 +2215,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs, bool has_filtered_child; assert(pnum); + assert_bdrv_graph_readable(); *pnum = 0; total_size = bdrv_getlength(bs); if (total_size < 0) { @@ -2445,6 +2446,7 @@ bdrv_co_common_block_status_above(BlockDriverState *bs, IO_CODE(); assert(!include_base || base); /* Can't include NULL base */ + assert_bdrv_graph_readable(); if (!depth) { depth = &dummy; diff --git a/block/mirror.c b/block/mirror.c index f509cc1cb1..02ee7bba08 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -559,9 +559,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) MirrorMethod mirror_method = MIRROR_METHOD_COPY; assert(!(offset % s->granularity)); - ret = bdrv_block_status_above(source, NULL, offset, - nb_chunks * s->granularity, - &io_bytes, NULL, NULL); + WITH_GRAPH_RDLOCK_GUARD() { + ret = bdrv_block_status_above(source, NULL, offset, + nb_chunks * s->granularity, + &io_bytes, NULL, NULL); + } if (ret < 0) { io_bytes = MIN(nb_chunks * s->granularity, max_io_bytes); } else if (ret & BDRV_BLOCK_DATA) { @@ -864,8 +866,10 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) return 0; } - ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset, bytes, - &count); + WITH_GRAPH_RDLOCK_GUARD() { + ret = bdrv_is_allocated_above(bs, s->base_overlay, true, offset, + bytes, &count); + } if (ret < 0) { return ret; } diff --git a/block/stream.c b/block/stream.c index 8744ad103f..22368ce186 100644 --- a/block/stream.c +++ b/block/stream.c @@ -161,21 +161,25 @@ static int coroutine_fn stream_run(Job *job, Error **errp) copy = false; - ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n); - if (ret == 1) { - /* Allocated in the top, no need to copy. */ - } else if (ret >= 0) { - /* Copy if allocated in the intermediate images. Limit to the - * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). */ - ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs), - s->base_overlay, true, - offset, n, &n); - /* Finish early if end of backing file has been reached */ - if (ret == 0 && n == 0) { - n = len - offset; + WITH_GRAPH_RDLOCK_GUARD() { + ret = bdrv_is_allocated(unfiltered_bs, offset, STREAM_CHUNK, &n); + if (ret == 1) { + /* Allocated in the top, no need to copy. */ + } else if (ret >= 0) { + /* + * Copy if allocated in the intermediate images. Limit to the + * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). + */ + ret = bdrv_is_allocated_above(bdrv_cow_bs(unfiltered_bs), + s->base_overlay, true, + offset, n, &n); + /* Finish early if end of backing file has been reached */ + if (ret == 0 && n == 0) { + n = len - offset; + } + + copy = (ret > 0); } - - copy = (ret > 0); } trace_stream_one_iteration(s, offset, n, ret); if (copy) { diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 7c34a8e40f..9d9cd59f1e 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -623,6 +623,8 @@ struct BlockDriver { * block/io.c's bdrv_co_block_status() will utilize an unclamped * *pnum value for the block-status cache on protocol nodes, prior * to clamping *pnum for return to its caller. + * + * Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_block_status)(BlockDriverState *bs, bool want_zero, int64_t offset, int64_t bytes, int64_t *pnum, diff --git a/qemu-img.c b/qemu-img.c index 4282a34bc0..33703a6d92 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1977,7 +1977,9 @@ static void coroutine_fn convert_co_do_copy(void *opaque) qemu_co_mutex_unlock(&s->lock); break; } - n = convert_iteration_sectors(s, s->sector_num); + WITH_GRAPH_RDLOCK_GUARD() { + n = convert_iteration_sectors(s, s->sector_num); + } if (n < 0) { qemu_co_mutex_unlock(&s->lock); s->ret = n; From patchwork Wed Nov 16 13:48:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045272 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 3C837C4332F for ; Wed, 16 Nov 2022 14:03:12 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImt-0000g6-Nj; Wed, 16 Nov 2022 08:49:11 -0500 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 1ovImn-0000cs-1k for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:05 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImk-000819-2A for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:04 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606541; h=from:from:reply-to:subject:subject: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=YodyfMR91zUtXAX62Pz3ns6A49aC6vibiZQM3eutBso=; b=dbf1ilFy1v0Ri2hrz781pt3VZp5OV1NeK78mcjUPoLQJQL/hG9Y4JuOvWEPSIibmVX7w8D Qpnq0GKU2Nb+a/3dQGDZX3mngTbyC7+J0gHjAPRKPBQAP/bgVvhEFz8k43Si80HRgU1GJK 0AWQ3VM2nYrk/GlBawwqPM9H0vmfTEk= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-640-K-PADszPPMa6ediZQhQcrw-1; Wed, 16 Nov 2022 08:48:59 -0500 X-MC-Unique: K-PADszPPMa6ediZQhQcrw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6A8708582B9; Wed, 16 Nov 2022 13:48:59 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D61C4A9268; Wed, 16 Nov 2022 13:48:59 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 19/20] block-gen: assert that bdrv_co_ioctl is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:49 -0500 Message-Id: <20221116134850.3051419-20-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The only caller of this function is blk_ioctl, a generated_co_wrapper functions that needs to take the graph read lock. Protecting bdrv_co_ioctl() implies that BlockDriver->bdrv_co_ioctl() is always called with graph rdlock taken, and BlockDriver->bdrv_aio_ioctl is a coroutine_fn callback (called too with rdlock taken). Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 1 + block/io.c | 1 + include/block/block_int-common.h | 5 +++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/block/block-backend.c b/block/block-backend.c index 20b772a476..9e1c689e84 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1672,6 +1672,7 @@ blk_co_do_ioctl(BlockBackend *blk, unsigned long int req, void *buf) IO_CODE(); blk_wait_while_drained(blk); + GRAPH_RDLOCK_GUARD(); if (!blk_is_available(blk)) { return -ENOMEDIUM; diff --git a/block/io.c b/block/io.c index c5b3bb0a6d..831f277e85 100644 --- a/block/io.c +++ b/block/io.c @@ -3007,6 +3007,7 @@ int coroutine_fn bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf) }; BlockAIOCB *acb; IO_CODE(); + assert_bdrv_graph_readable(); bdrv_inc_in_flight(bs); if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) { diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 9d9cd59f1e..db97d61836 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -743,10 +743,11 @@ struct BlockDriver { void (*bdrv_eject)(BlockDriverState *bs, bool eject_flag); void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked); - /* to control generic scsi devices */ - BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs, + /* to control generic scsi devices. Called with graph rdlock taken. */ + BlockAIOCB *coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf, BlockCompletionFunc *cb, void *opaque); + /* Called with graph rdlock taken. */ int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs, unsigned long int req, void *buf); From patchwork Wed Nov 16 13:48:50 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emanuele Giuseppe Esposito X-Patchwork-Id: 13045235 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 5E4B0C433FE for ; Wed, 16 Nov 2022 13:52:28 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ovImy-0000mq-55; Wed, 16 Nov 2022 08:49:16 -0500 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 1ovIms-0000ga-W0 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:11 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1ovImm-00082Q-58 for qemu-devel@nongnu.org; Wed, 16 Nov 2022 08:49:10 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668606543; h=from:from:reply-to:subject:subject: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=zaNpmaI6ecsoj+hXOOKiKG/pJoasSNU4l5fD+NRdOuY=; b=LzyFn73H3hSc1LyRAzdK4Y1X/QJtp4B7GiobeNEd4JMS5cvp+g4dkMaNP4o6b3q2Pmtixg iXwSVCaAP1+VIqHnmSsarCMBKsPOO/U1owXPvRBcAXDv3BNRvyKzyVH6yHZ/k4VTc8yxlj qC5dqx4YapW8IIXD1U7FTWT/irr3Zc0= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-644-g1Ku6Te7N9qdeNlBxDtfQw-1; Wed, 16 Nov 2022 08:49:00 -0500 X-MC-Unique: g1Ku6Te7N9qdeNlBxDtfQw-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BEDD33850E87; Wed, 16 Nov 2022 13:48:59 +0000 (UTC) Received: from virtlab701.virt.lab.eng.bos.redhat.com (virtlab701.virt.lab.eng.bos.redhat.com [10.19.152.228]) by smtp.corp.redhat.com (Postfix) with ESMTP id 72E794A9265; Wed, 16 Nov 2022 13:48:59 +0000 (UTC) From: Emanuele Giuseppe Esposito To: qemu-block@nongnu.org Cc: Kevin Wolf , Hanna Reitz , John Snow , Paolo Bonzini , Vladimir Sementsov-Ogievskiy , Stefan Hajnoczi , Fam Zheng , Eric Blake , Cleber Rosa , qemu-devel@nongnu.org, Emanuele Giuseppe Esposito Subject: [PATCH 20/20] block-gen: assert that nbd_co_do_establish_connection is always called with graph rdlock taken Date: Wed, 16 Nov 2022 08:48:50 -0500 Message-Id: <20221116134850.3051419-21-eesposit@redhat.com> In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com> References: <20221116134850.3051419-1-eesposit@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 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: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org The only caller of this function is nbd_do_establish_connection, a generated_co_wrapper that already take the graph read lock. Signed-off-by: Emanuele Giuseppe Esposito --- block/nbd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/nbd.c b/block/nbd.c index 7d485c86d2..5cad58aaf6 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -322,6 +322,7 @@ int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs, int ret; IO_CODE(); + assert_bdrv_graph_readable(); assert(!s->ioc); s->ioc = nbd_co_establish_connection(s->conn, &s->info, blocking, errp);