From nobody Thu Nov 28 08:03:20 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass(p=none dis=none) header.from=linaro.org ARC-Seal: i=1; a=rsa-sha256; t=1694103334; cv=none; d=zohomail.com; s=zohoarc; b=fhcdjNtH2rLvBLmq6c3cR/Ovigc7Ycr/0uOoDyw2cd3g0JgBlXvSJT/QL09oSyOVXoZxtOsQo0NF91dNMtkGDQuspfbkoWKn97548x+Vlf3eXNRTfzhhwRR/hqGYN33YRAUuI36NmrUP7J9WNaovZuYgRvijLQUDOtLj5Lzl2eo= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1694103334; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=nZtQiYFBoet8GeTNKrldfa5lNv35E4p88I3RORT4DCI=; b=AMe9eEwJeIaa/fguzxl+tbYgK+Z51q2xstReppnU4sXeCH4MfwpqA8CMoPzk2JJHHD67xE47uT0q0n8MKfH1S+tH4mILrn8XHIHljsYGXkvqgAp16lb8fB8sFroBUTdgvJ06+ZLzcZ8+Md4/65HiSPHmFqRDjcJWth0d8OY8zxs= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1694103334370762.7813650794567; Thu, 7 Sep 2023 09:15:34 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qeHeF-0007Ld-88; Thu, 07 Sep 2023 12:14: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 1qeHeB-0007LE-LU for qemu-devel@nongnu.org; Thu, 07 Sep 2023 12:14:23 -0400 Received: from mail-lj1-x230.google.com ([2a00:1450:4864:20::230]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qeHe8-0006V9-On for qemu-devel@nongnu.org; Thu, 07 Sep 2023 12:14:23 -0400 Received: by mail-lj1-x230.google.com with SMTP id 38308e7fff4ca-2bcc4347d2dso19432621fa.0 for ; Thu, 07 Sep 2023 09:14:20 -0700 (PDT) Received: from m1x-phil.lan (176-131-222-226.abo.bbox.fr. [176.131.222.226]) by smtp.gmail.com with ESMTPSA id le18-20020a170906ae1200b0099bcbaa242asm10574220ejb.9.2023.09.07.09.14.16 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Thu, 07 Sep 2023 09:14:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1694103259; x=1694708059; darn=nongnu.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=nZtQiYFBoet8GeTNKrldfa5lNv35E4p88I3RORT4DCI=; b=gYmtNK37s0oCQFuDamOwGQRm7JmRGSGyVqwe0mN5zuQqpQFsTAww5NVMKMNJsHK8Fb 9E2T8M8yyVTky0q1Zqtxf6kXYaejh24y0Z+kqMevI2yRGDlyYlnb0vu/UpHkY16YrtbX pkSPQpKDxlLDsCnB73TUC6/XWZf5BM4hZ67tz79+UPlwfkeSEUYcQKC62ogJPagDPQAe fUHcq3mLI4sI/t/gK9RfAICOyM89NCdJrUzHx99pSLyYRCzycfY6tgpEmY9Z01nlLJ8a 0ivv7pFhagboRAn5Rn6RS56/uLWZQwG0l7mdrPIyZ9ga7EPEpwfvbbTxobtDvyU5u922 0Lyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694103259; x=1694708059; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=nZtQiYFBoet8GeTNKrldfa5lNv35E4p88I3RORT4DCI=; b=DCHAN4IcBKv2KIrDuZUbbYdapfudIHZF4IOg42BX/R8pWw++a8NWEZtMW46T9+Ohp3 vRXCA7UhncbPpD3+yuI5T2ua8zwzYO6y+RjBMGslnWG/ikHYfRqpP268DqBUoG4Nwd+g +bgGBvWQH0imGLD+VI3mx+cUIp1EtDNiwaHv8BWqImDMg9yyh3JTPEhD9P9D02mcUFa3 vsUL5lXKbWibD4LJlhHAsOa1yvg84TOrLZIasXTOPoVHTfvkh9sEcvm+bLJpG2T6Kyjl yo0ohB/u7mxbfVkHkTg+Vs4IUabwfkk3jazZnkAM6Jftevja+qERUbDDIq0Iqb1R/p26 NCbg== X-Gm-Message-State: AOJu0YyveRfZZhkTpnfLCjzPXR2fxcT34dwOgaAKu38F2jk9K02iBwSx yfgcYyLx1lBCC44bQDJFFqGI75SkwzNnVe+GcqU= X-Google-Smtp-Source: AGHT+IHMkvaYmiJJ78akdF4LMGDmXoHIt6lx7YTOCy89LL8zfh/GU8tJlVltDFPp+tJfU4Xs1Yc+hQ== X-Received: by 2002:a2e:9f04:0:b0:2b6:fa71:5bae with SMTP id u4-20020a2e9f04000000b002b6fa715baemr5137908ljk.12.1694103258707; Thu, 07 Sep 2023 09:14:18 -0700 (PDT) From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= To: qemu-devel@nongnu.org, Richard Henderson , Paolo Bonzini Cc: WANG Xuerui , David Hildenbrand , Sergey Fedorov , Peter Xu , Stefan Hajnoczi , =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= , "Emilio G . Cota" , Richard Purdie , =?UTF-8?q?Alex=20Benn=C3=A9e?= , Anton Johansson , Peter Maydell , qemu-stable@nongnu.org Subject: [RFC PATCH] softmmu: Fix async_run_on_cpu() use in tcg_commit_cpu() Date: Thu, 7 Sep 2023 18:14:14 +0200 Message-ID: <20230907161415.6102-1-philmd@linaro.org> X-Mailer: git-send-email 2.41.0 MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=2a00:1450:4864:20::230; envelope-from=philmd@linaro.org; helo=mail-lj1-x230.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 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+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZohoMail-DKIM: pass (identity @linaro.org) X-ZM-MESSAGEID: 1694103335660100001 CPUState::halt_cond is an accelerator specific pointer, used in particular by TCG (which tcg_commit() is about). The pointer is set by the AccelOpsClass::create_vcpu_thread() handler. AccelOpsClass::create_vcpu_thread() is called by the generic qemu_init_vcpu(), which expect the accelerator handler to eventually call cpu_thread_signal_created() which is protected with a QemuCond. It is safer to check the vCPU is created with this field rather than the 'halt_cond' pointer set in create_vcpu_thread() before the vCPU thread is initialized. This avoids calling tcg_commit() until all CPUs are realized. Here we can see for a machine with N CPUs, tcg_commit() is called N times before the 'machine_creation_done' event: (lldb) settings set -- target.run-args "-M" "virt" "-smp" "512" "-displa= y" "none" (lldb) breakpoint set --name qemu_machine_creation_done --one-shot true (lldb) breakpoint set --name tcg_commit_cpu --auto-continue true (lldb) run Process 84089 launched: 'qemu-system-aarch64' (arm64) Process 84089 stopped * thread #1, queue =3D 'com.apple.main-thread', stop reason =3D one-shot = breakpoint 2 (lldb) breakpoint list --brief Current breakpoints: 2: name =3D 'tcg_commit_cpu', locations =3D 2, resolved =3D 2, hit count = =3D 512 Options: enabled auto-continue ^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^ Having the following backtrace: (lldb) bt * thread #1, queue =3D 'com.apple.main-thread', stop reason =3D breakpoin= t 2.1 * frame #0: 0x1005d0fe0 qemu-system-aarch64`tcg_commit [inlined] tcg_co= mmit_cpu(cpu=3D0x108460000, data=3D(host_ptr =3D 0x600003b05c00, target_ptr= =3D 105553178156032)) at physmem.c:2493:63 frame #1: 0x1005d0fe0 qemu-system-aarch64`tcg_commit(listener=3D) at physmem.c:2527:9 frame #2: 0x1005cd220 qemu-system-aarch64`memory_listener_register [i= nlined] listener_add_address_space(listener=3D0x600003b05c18, as=3D) at memory.c:3014:9 frame #3: 0x1005cd148 qemu-system-aarch64`memory_listener_register(li= stener=3D0x600003b05c18, as=3D0x16fdfe720) at memory.c:3077:5 frame #4: 0x1005d0f40 qemu-system-aarch64`cpu_address_space_init(cpu= =3D, asidx=3D, prefix=3D, mr=3D) at physmem.c:773:9 [artificial] frame #5: 0x100389a64 qemu-system-aarch64`arm_cpu_realizefn(dev=3D0x1= 08460000, errp=3D0x16fdfe720) at cpu.c:2244:5 frame #6: 0x10062af28 qemu-system-aarch64`device_set_realized(obj=3D<= unavailable>, value=3D, errp=3D0x16fdfe7d8) at qdev.c:510:13 frame #7: 0x100632518 qemu-system-aarch64`property_set_bool(obj=3D0x1= 08460000, v=3D, name=3D, opaque=3D0x600000013e50,= errp=3D0x16fdfe7d8) at object.c:2285:5 frame #8: 0x100630808 qemu-system-aarch64`object_property_set(obj=3D0= x108460000, name=3D"realized", v=3D0x600003e02100, errp=3D0x16fdfe7d8) at o= bject.c:1420:5 frame #9: 0x1006345ac qemu-system-aarch64`object_property_set_qobject= (obj=3D, name=3D, value=3D, errp=3D<= unavailable>) at qom-qobject.c:28:10 frame #10: 0x100630c80 qemu-system-aarch64`object_property_set_bool(o= bj=3D, name=3D, value=3D, errp=3D) at object.c:1489:15 frame #11: 0x10062a188 qemu-system-aarch64`qdev_realize(dev=3D, bus=3D, errp=3D) at qdev.c:292:12 [artifi= cial] frame #12: 0x100319c30 qemu-system-aarch64`machvirt_init(machine=3D0x= 103562480) at virt.c:2248:9 frame #13: 0x100090edc qemu-system-aarch64`machine_run_board_init(mac= hine=3D0x103562480, mem_path=3D, errp=3D) at mach= ine.c:1469:5 frame #14: 0x1002a2684 qemu-system-aarch64`qmp_x_exit_preconfig [inli= ned] qemu_init_board at vl.c:2543:5 frame #15: 0x1002a2650 qemu-system-aarch64`qmp_x_exit_preconfig(errp= =3D) at vl.c:2634:5 frame #16: 0x1002a5dd4 qemu-system-aarch64`qemu_init(argc=3D, argv=3D) at vl.c:3642:9 frame #17: 0x100627d64 qemu-system-aarch64`main(argc=3D,= argv=3D) at main.c:47:5 When can then invert the if ladders for clarity: in the unlikely case of the caller being executed on the vCPU thread, directly dispatch, otherwise defer until quiescence. Cc: qemu-stable@nongnu.org Fixes: 0d58c66068 ("softmmu: Use async_run_on_cpu in tcg_commit") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1866 Signed-off-by: Philippe Mathieu-Daud=C3=A9 --- RFC: I tried my best to understand and explain, but this is still black magic to me... --- softmmu/physmem.c | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 18277ddd67..12ef9d7d27 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2505,22 +2505,27 @@ static void tcg_commit(MemoryListener *listener) cpuas =3D container_of(listener, CPUAddressSpace, tcg_as_listener); cpu =3D cpuas->cpu; =20 - /* - * Defer changes to as->memory_dispatch until the cpu is quiescent. - * Otherwise we race between (1) other cpu threads and (2) ongoing - * i/o for the current cpu thread, with data cached by mmu_lookup(). - * - * In addition, queueing the work function will kick the cpu back to - * the main loop, which will end the RCU critical section and reclaim - * the memory data structures. - * - * That said, the listener is also called during realize, before - * all of the tcg machinery for run-on is initialized: thus halt_cond. - */ - if (cpu->halt_cond) { - async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_HOST_PTR(cpuas)); - } else { + if (!cpu->created) { + /* + * The listener is also called during realize, before + * all of the tcg machinery for run-on is initialized. + */ + return; + } + + if (unlikely(qemu_cpu_is_self(cpu))) { tcg_commit_cpu(cpu, RUN_ON_CPU_HOST_PTR(cpuas)); + } else { + /* + * Defer changes to as->memory_dispatch until the cpu is quiescent. + * Otherwise we race between (1) other cpu threads and (2) ongoing + * i/o for the current cpu thread, with data cached by mmu_lookup(= ). + * + * In addition, queueing the work function will kick the cpu back = to + * the main loop, which will end the RCU critical section and recl= aim + * the memory data structures. + */ + async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_HOST_PTR(cpuas)); } } =20 --=20 2.41.0