From patchwork Tue Dec 6 20:27:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Stabellini X-Patchwork-Id: 9463219 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id AD5FE60236 for ; Tue, 6 Dec 2016 20:30:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9E9C8284C4 for ; Tue, 6 Dec 2016 20:30:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9099E284DA; Tue, 6 Dec 2016 20:30:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 17B58284C4 for ; Tue, 6 Dec 2016 20:30:28 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cEMKy-0005yw-Iy; Tue, 06 Dec 2016 20:27:40 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cEMKx-0005yq-N0 for xen-devel@lists.xen.org; Tue, 06 Dec 2016 20:27:39 +0000 Received: from [193.109.254.147] by server-4.bemta-6.messagelabs.com id F4/92-28568-A3F17485; Tue, 06 Dec 2016 20:27:38 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrCIsWRWlGSWpSXmKPExsVybKJsh66VvHu EwZ3t7BZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa8auJxNYCi6KVFz6fZW1gfEaXxcjF4eQwFRG ifvvX7FDOLOZJHZ938LcxcjJwSKgJfH1QDsjiM0mYCjx98kmti5GDg4JIHvJZw6QsIiArsSzB c/YQHqZBZYwShyfsZEdJCEsYCnx6M8bNhCbU8BO4v3hl2BzeAW8JL7dPs0KsewHo8TlNfeZQB KiQJMO/fvDBlEkKHFy5hMWEJsZ6Ijl07eB2RICGRLzeuawQtheEotuXIKy1SSuntvEPIFRcBa S9llI2hcwMq1i1ChOLSpLLdI1MtRLKspMzyjJTczM0TU0MNPLTS0uTkxPzUlMKtZLzs/dxAgM UQYg2MH4Z1nAIUZJDiYlUd72P24RQnxJ+SmVGYnFGfFFpTmpxYcYZTg4lCR4N8u6RwgJFqWmp 1akZeYAowUmLcHBoyTCexEkzVtckJhbnJkOkTrFqMuxaffap0xCLHn5ealS4ryvQIoEQIoySv PgRsAi9xKjrJQwLyPQUUI8BalFuZklqPKvGMU5GJWEea3lgKbwZOaVwG16BXQEE9ARJ447gxx RkoiQkmpgXDmLVXJS1l7vKxU7jZhV0s5WbHFa8aJ5fd3Dg9/rmnY/Wumi9f5Is5ZoQ5xiqYxe 5Rfb88Y5Ggn9fKdyS3UXla77nDBbQXNdlHL9xN33V75Mb7XZdPFd/Sdl1muOM9Lvq+kUXbCVv FK+rfPJ0vRmi9kWB9Sve8Y2bxGLuhOsqjmJVfNO6iZ1JZbijERDLeai4kQANrXC0dcCAAA= X-Env-Sender: sstabellini@kernel.org X-Msg-Ref: server-8.tower-27.messagelabs.com!1481056056!65717297!1 X-Originating-IP: [198.145.29.136] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.0.16; banners=-,-,- X-VirusChecked: Checked Received: (qmail 17898 invoked from network); 6 Dec 2016 20:27:37 -0000 Received: from mail.kernel.org (HELO mail.kernel.org) (198.145.29.136) by server-8.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 6 Dec 2016 20:27:37 -0000 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 4474520165; Tue, 6 Dec 2016 20:27:35 +0000 (UTC) Received: from [10.1.10.56] (96-82-76-110-static.hfc.comcastbusiness.net [96.82.76.110]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id DEB302014A; Tue, 6 Dec 2016 20:27:32 +0000 (UTC) Date: Tue, 6 Dec 2016 12:27:31 -0800 (PST) From: Stefano Stabellini X-X-Sender: sstabellini@sstabellini-ThinkPad-X260 To: Andrew Cooper In-Reply-To: <81e6d999-ee32-ef51-5a11-3e5a0690a191@citrix.com> Message-ID: References: <1480932317-22962-1-git-send-email-andrew.cooper3@citrix.com> <1480932317-22962-3-git-send-email-andrew.cooper3@citrix.com> <81e6d999-ee32-ef51-5a11-3e5a0690a191@citrix.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-Virus-Scanned: ClamAV using ClamSMTP Cc: Julien Grall , Stefano Stabellini , Jan Beulich , Xen-devel Subject: Re: [Xen-devel] [PATCH 2/4] xen/x86: Drop erronious barriers X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Tue, 6 Dec 2016, Andrew Cooper wrote: > On 05/12/2016 19:17, Stefano Stabellini wrote: > > On Mon, 5 Dec 2016, Andrew Cooper wrote: > >> None of these barriers serve any purpose, as they are not synchronising with > >> any anything on remote CPUs. > >> > >> Signed-off-by: Andrew Cooper > >> --- > >> CC: Jan Beulich > >> CC: Stefano Stabellini > >> CC: Julien Grall > >> > >> Restricting to just the $ARCH maintainers, as this is a project-wide sweep. > >> > >> Julien/Stefano: I think the ARM smpboot inhereted the erronious barrier usage > >> from x86, but I don't know whether further development has gained a dependence > >> on them. > > We turned them into smp_wmb already (kudos to IanC). > > Right, but the entire point I am trying to argue is that they are not > needed in the first place. This is the current code: CPU 1 CPU 0 ----- ----- init stuff read cpu_online_map write barrier write cpu_online_map do more initialization write barrier init more stuff I agree that it's wrong, because the second write barrier in start_secondary is useless and in addition we are missing a read barrier in __cpu_up, corresponding to the first write barrier in start_secondary. I think it should look like: CPU 1 CPU 0 ----- ----- init stuff read cpu_online_map write barrier read barrier write cpu_online_map do more initialization init more stuff The patch is as follow. Julien, what do you think? Also, do we need to change the remaming smp_wmb() in start_secondary to wmb() to ensure execution ordering as well as memory access ordering? Signed-off-by: Stefano Stabellini diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 90ad1d0..c841a15 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -311,7 +311,6 @@ void start_secondary(unsigned long boot_phys_offset, /* Now report this CPU is up */ cpumask_set_cpu(cpuid, &cpu_online_map); - smp_wmb(); local_irq_enable(); local_abort_enable(); @@ -408,6 +407,7 @@ int __cpu_up(unsigned int cpu) cpu_relax(); process_pending_softirqs(); } + smp_rmb(); /* * Nuke start of day info before checking one last time if the CPU