From nobody Wed Nov 5 10:15:13 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1534178488706422.1681636904384; Mon, 13 Aug 2018 09:41:28 -0700 (PDT) Received: from localhost ([::1]:40435 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fpFuC-0006yK-Vh for importer@patchew.org; Mon, 13 Aug 2018 12:41:21 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43323) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fpFs7-0004kQ-Vf for qemu-devel@nongnu.org; Mon, 13 Aug 2018 12:39:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fpFs2-00011f-W6 for qemu-devel@nongnu.org; Mon, 13 Aug 2018 12:39:11 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:46859) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fpFs2-000111-QC; Mon, 13 Aug 2018 12:39:06 -0400 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id D95F621E07; Mon, 13 Aug 2018 12:39:04 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute4.internal (MEProxy); Mon, 13 Aug 2018 12:39:04 -0400 Received: from localhost (flamenco.cs.columbia.edu [128.59.20.216]) by mail.messagingengine.com (Postfix) with ESMTPA id 490C1E4695; Mon, 13 Aug 2018 12:39:03 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=braap.org; h=cc :date:from:in-reply-to:message-id:references:subject:to :x-me-sender:x-me-sender:x-sasl-enc; s=mesmtp; bh=fT5SV38HN3bzav KifHZKt8TAy/X7pQ0qw0AHK2aL1M0=; b=Ix3JYDoBosuHL3PLiCECXZHuQz3z20 IzOSiN4w3w0VEdKLpbMccYQt0bQ+e1xTvundrb7oQ9wrC3d/efRbg6RQoL+fM17L EfMXACZjSBSBHCoR1elEWlym649Lyfmysdf7iMcEpRzc7hvcmZnMCcuXrdvN3bt6 hnMpOEGNfw9DI= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:in-reply-to:message-id :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; bh=fT5SV38HN3bzavKifHZKt8TAy/X7pQ0qw0AHK2aL1M0=; b=eA1xt8jD AW4W1f0ErdqCwOcpreH6a7742NvdT5wj47hWOVp7R2vJVLOsnWh+aTOnuOOvmloN +d79m0U5QrZsJ9eP2KfGSCDkOf0DceAL0Cc53rEDsHBxnUFCE9njHKMauMbV7Z7e WJuhR0sbklV7FbsyEOba6ps7+/1Oyp+vcR9hdDMDU2AKrghFD8sAQJxAyp6MqY2G n0rBMkIuCOAgb4cWjT011nU38r0OZTetj4We60syUN+0bs67d7+/q6kNwr6mMISF +aifBCYXCXunGfZEOtwWya8ASctdPx/GTKuchH8sue7GX+RhMSoquuQ0S+hwlU2x rZs4rdP9an0I4w== X-ME-Proxy: X-ME-Sender: From: "Emilio G. Cota" To: qemu-devel@nongnu.org Date: Mon, 13 Aug 2018 12:38:59 -0400 Message-Id: <20180813163859.17964-4-cota@braap.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180813163859.17964-1-cota@braap.org> References: <20180813163859.17964-1-cota@braap.org> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 66.111.4.25 Subject: [Qemu-devel] [PATCH 3/3] qom: implement CPU list with an RCU QLIST X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Crosthwaite , Riku Voipio , Alexander Graf , Laurent Vivier , qemu-ppc@nongnu.org, Paolo Bonzini , David Gibson , Richard Henderson Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Iterating over the list without using atomics is undefined behaviour, since the list can be modified concurrently by other threads (e.g. every time a new thread is created in user-mode). Fix it by implementing the CPU list as an RCU QLIST. This requires a little bit of extra work to insert CPUs at the tail of the list and to iterate over the list in reverse order (see previous patch). One might be tempted to just insert new CPUs at the head of the list. However, I think this might lead to hard-to-debug issues, since it is possible that callers are assuming that CPUs are inserted at the tail (just like spapr code did in the previous patch). So instead of auditing all callers, this patch simply keeps the old behaviour. Signed-off-by: Emilio G. Cota --- cpus-common.c | 19 +++++++++++++++---- cpus.c | 2 +- exec.c | 2 +- include/qom/cpu.h | 15 +++++++-------- linux-user/main.c | 2 +- linux-user/syscall.c | 2 +- 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/cpus-common.c b/cpus-common.c index 6eaedae60b..d8e8db48dd 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -77,6 +77,8 @@ static void finish_safe_work(CPUState *cpu) =20 void cpu_list_add(CPUState *cpu) { + CPUState *cs, *cs_next; + qemu_mutex_lock(&qemu_cpu_list_lock); if (cpu->cpu_index =3D=3D UNASSIGNED_CPU_INDEX) { cpu->cpu_index =3D cpu_get_free_index(); @@ -84,7 +86,18 @@ void cpu_list_add(CPUState *cpu) } else { assert(!cpu_index_auto_assigned); } - QTAILQ_INSERT_TAIL(&cpus, cpu, node); + /* poor man's tail insert */ + CPU_FOREACH_SAFE(cs, cs_next) { + if (cs_next =3D=3D NULL) { + break; + } + } + if (cs =3D=3D NULL) { + QLIST_INSERT_HEAD_RCU(&cpus, cpu, node); + } else { + g_assert(cs_next =3D=3D NULL); + QLIST_INSERT_AFTER_RCU(cs, cpu, node); + } cpu->in_cpu_list =3D true; qemu_mutex_unlock(&qemu_cpu_list_lock); =20 @@ -100,9 +113,7 @@ void cpu_list_remove(CPUState *cpu) return; } =20 - assert(!(cpu_index_auto_assigned && cpu !=3D QTAILQ_LAST(&cpus, CPUTai= lQ))); - - QTAILQ_REMOVE(&cpus, cpu, node); + QLIST_REMOVE_RCU(cpu, node); cpu->cpu_index =3D UNASSIGNED_CPU_INDEX; qemu_mutex_unlock(&qemu_cpu_list_lock); } diff --git a/cpus.c b/cpus.c index b5844b7103..82e298d825 100644 --- a/cpus.c +++ b/cpus.c @@ -1491,7 +1491,7 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg) atomic_mb_set(&cpu->exit_request, 0); } =20 - qemu_tcg_rr_wait_io_event(cpu ? cpu : QTAILQ_FIRST(&cpus)); + qemu_tcg_rr_wait_io_event(cpu ? cpu : QLIST_FIRST_RCU(&cpus)); deal_with_unplugged_cpus(); } =20 diff --git a/exec.c b/exec.c index 4f5df07b6a..cab0da6919 100644 --- a/exec.c +++ b/exec.c @@ -114,7 +114,7 @@ int target_page_bits; bool target_page_bits_decided; #endif =20 -struct CPUTailQ cpus =3D QTAILQ_HEAD_INITIALIZER(cpus); +struct CPUTailQ cpus =3D QLIST_HEAD_INITIALIZER(cpus); /* current CPU in the current thread. It is only valid inside cpu_exec() */ __thread CPUState *current_cpu; diff --git a/include/qom/cpu.h b/include/qom/cpu.h index aa555e27a7..407c57254d 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -26,6 +26,7 @@ #include "exec/memattrs.h" #include "qapi/qapi-types-run-state.h" #include "qemu/bitmap.h" +#include "qemu/rcu_queue.h" #include "qemu/queue.h" #include "qemu/thread.h" =20 @@ -371,7 +372,7 @@ struct CPUState { struct GDBRegisterState *gdb_regs; int gdb_num_regs; int gdb_num_g_regs; - QTAILQ_ENTRY(CPUState) node; + QLIST_ENTRY(CPUState) node; =20 /* ice debug support */ QTAILQ_HEAD(breakpoints_head, CPUBreakpoint) breakpoints; @@ -436,15 +437,13 @@ struct CPUState { GArray *iommu_notifiers; }; =20 -QTAILQ_HEAD(CPUTailQ, CPUState); +QLIST_HEAD(CPUTailQ, CPUState); extern struct CPUTailQ cpus; -#define CPU_NEXT(cpu) QTAILQ_NEXT(cpu, node) -#define CPU_FOREACH(cpu) QTAILQ_FOREACH(cpu, &cpus, node) +#define CPU_NEXT(cpu) QLIST_NEXT_RCU(cpu, node) +#define CPU_FOREACH(cpu) QLIST_FOREACH_RCU(cpu, &cpus, node) #define CPU_FOREACH_SAFE(cpu, next_cpu) \ - QTAILQ_FOREACH_SAFE(cpu, &cpus, node, next_cpu) -#define CPU_FOREACH_REVERSE(cpu) \ - QTAILQ_FOREACH_REVERSE(cpu, &cpus, CPUTailQ, node) -#define first_cpu QTAILQ_FIRST(&cpus) + QLIST_FOREACH_SAFE_RCU(cpu, &cpus, node, next_cpu) +#define first_cpu QLIST_FIRST_RCU(&cpus) =20 extern __thread CPUState *current_cpu; =20 diff --git a/linux-user/main.c b/linux-user/main.c index ea00dd9057..99afb14ae5 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -126,7 +126,7 @@ void fork_end(int child) Discard information about the parent threads. */ CPU_FOREACH_SAFE(cpu, next_cpu) { if (cpu !=3D thread_cpu) { - QTAILQ_REMOVE(&cpus, cpu, node); + QLIST_REMOVE_RCU(cpu, node); } } qemu_init_cpu_list(); diff --git a/linux-user/syscall.c b/linux-user/syscall.c index dfc851cc35..41b592dee3 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8040,7 +8040,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long = arg1, TaskState *ts; =20 /* Remove the CPU from the list. */ - QTAILQ_REMOVE(&cpus, cpu, node); + QLIST_REMOVE_RCU(cpu, node); =20 cpu_list_unlock(); =20 --=20 2.17.1