From patchwork Tue Aug 13 20:23:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 818850 Delivered-To: patch@linaro.org Received: by 2002:adf:cd01:0:b0:367:895a:4699 with SMTP id w1csp430546wrm; Tue, 13 Aug 2024 13:25:46 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCV3uulo+hOE7cd+Sq5Zynh2KgG7yFWFvxfjlYFkhcH1kXJzB7LuopH3DvcWjVmEhh/S0iWfAwp0igomw1F3Ifej X-Google-Smtp-Source: AGHT+IE1ZcxGZZkZBVReUoSDeRWm71GJ888TUl9Qfnr8GGc2ng3gZkPL6rx0+eswqsENkismDkY8 X-Received: by 2002:a05:690c:2892:b0:64a:3d7c:2782 with SMTP id 00721157ae682-6ac99fa8e59mr6013557b3.41.1723580746121; Tue, 13 Aug 2024 13:25:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1723580746; cv=none; d=google.com; s=arc-20160816; b=f0LdLxdE0MSC90a6n6caKmfderJbGVys3JhpWJBI/MaCucNlTn2/4KpyFbTLLCsg1t M9QsgYOi0QBmOwZb5Ec+rklObdn1MmmBNdkQ7fXsocGvgxZxvX+fqq8heDpuZGACOqXI uIUWTc6O49XklfcRdaYFo5B7P6NbvL9oHRLGIKdHLT/eIuivnhx1ZrGCEnt7/QM8d/FV sNtyfB2LLK2tj2hijeIFZaI43WpNo5sxepsiTfwaycmqYhDBC1mpf/bD5MskG8wVtdly FyHv5r1CAwoIh8mw+cC1cdjbCUaAySu6NYEWmk5iqcBSVLFyyU2cq2pibS127fZpRMHW NzlA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=/5zKsXaXwrEo4pGBpcSfU84L/65BuquefdOBBeKcqhg=; fh=Ldnso273692IMU0X0frFRUfKP3dGY55NqM4JNuJFPqQ=; b=NFCxOKHCHjRGixqC03F380fl1Zt1ChV33n+BwcPXIDeK2swwPmFySbYUG3C9tLAO9C C6FDyf+HIa+f2Eqsei/Ak4A5cdbi6GREEE4VInBPjR2D2zdgRsZ97yuvevUHNaYY+BC9 DDPADU9PZTzanTamdkjGpXDC081yMn0LYlllFiUbkcgegQpobrtn84YSwidDbHiVP8Pe MAV4eYvalGKSXeYlOa6vdDOXapBgxlryS3TOGujven64Ike2cUrfIXDM1G1q2tyqRR/k VAtp1XoxCuEmCDEt/RwCvNeFfk8+k+rR6Ae9GzWfsZk4pGl30B4rXgPvRZ354OOzxKUB fdtQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cPVvYHJa; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org; dara=neutral header.i=@linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id 6a1803df08f44-6bd82e59c66si92457616d6.347.2024.08.13.13.25.45 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 13 Aug 2024 13:25:46 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=cPVvYHJa; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org; dara=neutral header.i=@linaro.org Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sdy4B-0007Yw-Ga; Tue, 13 Aug 2024 16:24:27 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sdy41-0006hd-V4 for qemu-devel@nongnu.org; Tue, 13 Aug 2024 16:24:18 -0400 Received: from mail-wr1-x436.google.com ([2a00:1450:4864:20::436]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1sdy3Y-0006Je-Fs for qemu-devel@nongnu.org; Tue, 13 Aug 2024 16:24:17 -0400 Received: by mail-wr1-x436.google.com with SMTP id ffacd0b85a97d-3685a5e7d3cso3639570f8f.1 for ; Tue, 13 Aug 2024 13:23:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1723580625; x=1724185425; darn=nongnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=/5zKsXaXwrEo4pGBpcSfU84L/65BuquefdOBBeKcqhg=; b=cPVvYHJafNG4aZYnutbCAah1kyFesnq+5VNbyTNqI9+4zBDHyDb+t93UUtiE6KltBI 9c7GUQUTaUzcsu5i/XsltPEMgMhVCFbI8GCPK/2x9sk425z49NQ6OB2EOGNFCPL+d7q6 JnioTUeERV6+sL1WCfj+Iqv1c2QO3QIyaLWGXddvrrJOuSkmOzt4zLspMmOH46/lYHVj rzWbqpuax1My7/8zPZo6CKeAYv/ZKEjQRTrasjhzbEMu0et8UOpNE0D0lBhBcHTXpjwr k8zjuJZN8v6BtaEAGMC+mT36Do/X4SP7mOeMOoO1us/hbiIOgBzjfh9z4FKs+RKS9WEN cXhw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723580625; x=1724185425; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/5zKsXaXwrEo4pGBpcSfU84L/65BuquefdOBBeKcqhg=; b=PPLNY8dAgRQsiCXHeJKP9QPq7WLzsykqP2BGopuFputj1AtNOzfL7BMC8oJ38FGxyQ rhgJid5b4i1K21EDd+CFSS/7gFK0pqm9JPuyQLOwLP2vIM3wA/SLuxxsGP9O4wNQX1+J TN1rZgLXvkKsKQifDS69RWKDbpOzxYgzp7rUNYVGPqfBNxL+OXkGVDLMHwMmHuLMAyBX tEW1TAKPoLcP0lh9Upna7ulo/4iYANMDqFy+eZgQDE3wDUPSuZxaN/bi2uwjc/XvDN6r dzHcp11Ui2siCRMoo4Yhn33YPCSzDUHxVRsesSW8HFijNlAyLBBICRI6xHXSy3ls/goR OfKg== X-Gm-Message-State: AOJu0Ywpf7dlEsRagrz5AriZ9bcHBNNSOrTM/d3dQrWJtOvz4HTUTNDd ZuP1L1TIxpHHYJcs+Zn2I6lPTyoEZWlTb0ZiOuE2yeom3tVI4r2TSlC7AoKUYTc= X-Received: by 2002:a5d:4985:0:b0:367:9769:35b0 with SMTP id ffacd0b85a97d-37177756b87mr471286f8f.4.1723580625112; Tue, 13 Aug 2024 13:23:45 -0700 (PDT) Received: from draig.lan ([85.9.250.243]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36e4c36ba5csm11299186f8f.2.2024.08.13.13.23.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Aug 2024 13:23:40 -0700 (PDT) Received: from draig.lan (localhost [IPv6:::1]) by draig.lan (Postfix) with ESMTP id 7859860773; Tue, 13 Aug 2024 21:23:31 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: Alistair Francis , Michael Roth , Palmer Dabbelt , Mahmoud Mandour , Pavel Dovgalyuk , Yoshinori Sato , Weiwei Li , Eduardo Habkost , =?utf-8?q?Marc-Andr=C3=A9_Lureau?= , Markus Armbruster , David Hildenbrand , Beraldo Leal , Liu Zhiwei , Eric Auger , Song Gao , =?utf-8?q?Alex_Benn=C3=A9e?= , qemu-arm@nongnu.org, Peter Xu , Jiri Pirko , Eric Blake , Fabiano Rosas , qemu-s390x@nongnu.org, Peter Maydell , "Michael S. Tsirkin" , Daniel Henrique Barboza , John Snow , Alexandre Iooss , Konstantin Kostiuk , Pierrick Bouvier , Cleber Rosa , Ilya Leoshkevich , qemu-riscv@nongnu.org, Thomas Huth , Paolo Bonzini , Richard Henderson , Jason Wang , Bin Meng , =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , Wainer dos Santos Moschetta Subject: [PATCH v2 21/21] plugins: fix race condition with scoreboards Date: Tue, 13 Aug 2024 21:23:29 +0100 Message-Id: <20240813202329.1237572-22-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240813202329.1237572-1-alex.bennee@linaro.org> References: <20240813202329.1237572-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::436; envelope-from=alex.bennee@linaro.org; helo=mail-wr1-x436.google.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, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 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+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org From: Pierrick Bouvier A deadlock can be created if a new vcpu (a) triggers a scoreboard reallocation, and another vcpu (b) wants to create a new scoreboard at the same time. In this case, (a) holds the plugin lock, and starts an exclusive section, waiting for (b). But at the same time, (b) is waiting for plugin lock. The solution is to drop the lock before entering the exclusive section. This bug can be easily reproduced by creating a callback for any tb exec, that allocates a new scoreboard. In this case, as soon as we reach more than 16 vcpus, the deadlock occurs. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2344 Signed-off-by: Pierrick Bouvier Message-Id: <20240812220748.95167-2-pierrick.bouvier@linaro.org> [AJB: tweak var position to meet coding style] Signed-off-by: Alex Bennée --- plugins/core.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/plugins/core.c b/plugins/core.c index 12c67b4b4e..2897453cac 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -214,30 +214,49 @@ CPUPluginState *qemu_plugin_create_vcpu_state(void) static void plugin_grow_scoreboards__locked(CPUState *cpu) { - if (cpu->cpu_index < plugin.scoreboard_alloc_size) { + size_t scoreboard_size = plugin.scoreboard_alloc_size; + bool need_realloc = false; + + if (cpu->cpu_index < scoreboard_size) { return; } - bool need_realloc = FALSE; - while (cpu->cpu_index >= plugin.scoreboard_alloc_size) { - plugin.scoreboard_alloc_size *= 2; - need_realloc = TRUE; + while (cpu->cpu_index >= scoreboard_size) { + scoreboard_size *= 2; + need_realloc = true; } + if (!need_realloc) { + return; + } - if (!need_realloc || QLIST_EMPTY(&plugin.scoreboards)) { - /* nothing to do, we just updated sizes for future scoreboards */ + if (QLIST_EMPTY(&plugin.scoreboards)) { + /* just update size for future scoreboards */ + plugin.scoreboard_alloc_size = scoreboard_size; return; } + /* + * A scoreboard creation/deletion might be in progress. If a new vcpu is + * initialized at the same time, we are safe, as the new + * plugin.scoreboard_alloc_size was not yet written. + */ + qemu_rec_mutex_unlock(&plugin.lock); + /* cpus must be stopped, as tb might still use an existing scoreboard. */ start_exclusive(); - struct qemu_plugin_scoreboard *score; - QLIST_FOREACH(score, &plugin.scoreboards, entry) { - g_array_set_size(score->data, plugin.scoreboard_alloc_size); + /* re-acquire lock */ + qemu_rec_mutex_lock(&plugin.lock); + /* in case another vcpu is created between unlock and exclusive section. */ + if (scoreboard_size > plugin.scoreboard_alloc_size) { + struct qemu_plugin_scoreboard *score; + QLIST_FOREACH(score, &plugin.scoreboards, entry) { + g_array_set_size(score->data, scoreboard_size); + } + plugin.scoreboard_alloc_size = scoreboard_size; + /* force all tb to be flushed, as scoreboard pointers were changed. */ + tb_flush(cpu); } - /* force all tb to be flushed, as scoreboard pointers were changed. */ - tb_flush(cpu); end_exclusive(); }