From owner-svn-src-projects@FreeBSD.ORG Tue Sep 18 15:39:58 2012 Return-Path: Delivered-To: svn-src-projects@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B302E1065675; Tue, 18 Sep 2012 15:39:58 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx1.freebsd.org (Postfix) with ESMTP id EE9508FC1B; Tue, 18 Sep 2012 15:39:56 +0000 (UTC) Received: by lbbgg13 with SMTP id gg13so127823lbb.13 for ; Tue, 18 Sep 2012 08:39:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:reply-to:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=PTEdgrneNafq+yU3tzH3SLeSbBbGZayTGHvPU4SIZro=; b=ij/xjf1Az6yg6RpRolpLnBW7hFRU6CjZP5CGCtDZrGL9a0xfGnKk4KdPQYyOlpgUe3 mO2SWypJ6f6J+eqBl1QjpY8dbEYRCz3G+pBBa5OV8EpwWW6mGe78PmIHwfFTp2pCdfUX izGxnL9D3f43JCaYe8+chy2X/TI3sCMHwvgKj8ok31uX7Xd0If26SlesMv8JZK7TvrgA K+BYioTpSWGmLhB0buiJ6dWy6Lq2w4GcfLKHJzAh4LrxkaWNeh6PEqF1bc0UD3kAK8sW EpvrevTLFsRHw49FMS/KoA4hf76O9StkogKplZbFwQ3iwS8yNr0MJkIbJeWk5ye2ep7I bMGA== MIME-Version: 1.0 Received: by 10.112.103.71 with SMTP id fu7mr86296lbb.21.1347982795671; Tue, 18 Sep 2012 08:39:55 -0700 (PDT) Sender: asmrookie@gmail.com Received: by 10.112.102.39 with HTTP; Tue, 18 Sep 2012 08:39:55 -0700 (PDT) In-Reply-To: <201209180832.50694.jhb@freebsd.org> References: <201207301350.q6UDobCI099069@svn.freebsd.org> <505849DB.3090704@FreeBSD.org> <201209180832.50694.jhb@freebsd.org> Date: Tue, 18 Sep 2012 16:39:55 +0100 X-Google-Sender-Auth: vu_nnb2WId7s-RTe-XGJyRpyUzI Message-ID: From: Attilio Rao To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: Davide Italiano , src-committers@freebsd.org, David Chisnall , Jeff Roberson , Dimitry Andric , svn-src-projects@freebsd.org, Konstantin Belousov Subject: Re: svn commit: r238907 - projects/calloutng/sys/kern X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: attilio@FreeBSD.org List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Sep 2012 15:39:58 -0000 On 9/18/12, John Baldwin wrote: > On Tuesday, September 18, 2012 6:29:31 am David Chisnall wrote: >> On 18 Sep 2012, at 11:15, Dimitry Andric wrote: >> >> > Please use gcc's __sync_synchronize() builtin[1] instead, which is >> > specifically for this purpose. Clang also supports it. >> > >> > The builtin will emit actual memory barrier instructions, if the target >> > architecture supports it, otherwise it will emit the same asm statement >> > you show above. See contrib/gcc/builtins.c, around line 5584, function >> > expand_builtin_synchronize(). >> >> From Attilio's description of the problem in IRC, I believe that > atomic_signal_fence() is the correct thing to use here. He stated that he > cares about reordering of memory access with regard to the current CPU, but > > not with regard to other CPUs / threads. He also said that he only cares > about the compiler performing the reordering, not about the CPU, but I > suspect > that is incorrect as there are numerous subtle bugs that can creep in on > weakly-ordered architectures (e.g. Alpha, ARMv8) if you only have a compiler > > barrier. > > Not true. Barriers only affect the order that writes are posted to > external > viewers (e.g. other processors and devices that perform DMA). On a single > CPU barriers are completely meaningless. The types of barriers Attilio is > worried about are things like RAW and WAR hazards. CPUs generally handle > these things internally (except for ia64 and it's stop bits on bundles, but > the compiler is required to insert those to ensure that things still > complete > in "program order"). > > Specifically, CPUs will only reorder instructions in a way that does not > violate a RAW or WAR hazard. Compilers have similar constraints (they can > move constants out of a loop because it is not a WAR). Given that I think > your last comment is largely bollocks. :) > >> That said, this is likely to be incorrect, because it's very unusual for > that to actually be the requirement, especially in multithreaded code (where > > the atomic.h stuff is actually important). In most of the cases where > __compiler_membar() is being used, you actually want at least a partial > barrier. > > The specific cases where Attilio wants to use a pure compiler barrier > without a full atomic op (that will include appropriate barriers already) > are attempting to force the compiler to safely order actions that are > sensitive to preemption (e.g. ensuring that td_pinned and td_critnest are > properly set before a critical section is entered so that any interrupt > that occurs is guaranteed to "see" the result of sched_pin() or > critical_enter() before any protected accesses are performed, and similarly > to ensure that any protected accesses are completed before the weak "lock" > is released via sched_unpin() or critical_exit()). You can think of these > as WAR or RAW hazards that the compiler simply has no way of knowing about > (and can't). However, assuming the compiler is correct, there are no > WAR or RAW hazards that are not visible to the CPU. > > This is actually very similar to signal handling in userland (signals are > basically interrupts for userland), so it may be that atomic_signal_fence() > is > in fact be correct. Per standard description, atomic_signal_fence() seems being very similar to hw memory barrier semantic rather than compiler memory barrier. Also, the standard specifically mention signal handler, besides our interpretation of it being applicable to interrupts in kernel context too. In general one may argue we use stdatomic.h rather than atomic.h but I completely agree with kib@ on his later e-mail on the topic. In definitive, I think that the patch I proposed is still fully valid, modulo the fallback. Attilio -- Peace can only be achieved by understanding - A. Einstein