From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Zijlstra Subject: Re: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb} Date: Fri, 19 Oct 2018 10:04:18 +0200 Message-ID: <20181019080418.GZ3121@hirez.programming.kicks-ass.net> References: <20181017144156.16639-1-daniel@iogearbox.net> <20181017144156.16639-3-daniel@iogearbox.net> <20181017155050.GM3121@hirez.programming.kicks-ass.net> <55f86215-44a8-2bb8-b1d0-a77a142dc697@iogearbox.net> <20181018081434.GT3121@hirez.programming.kicks-ass.net> <20181018153307.ayvmq6du3gnsyvro@ast-mbp.dhcp.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Daniel Borkmann , paulmck@linux.vnet.ibm.com, will.deacon@arm.com, acme@redhat.com, yhs@fb.com, john.fastabend@gmail.com, netdev@vger.kernel.org To: Alexei Starovoitov Return-path: Received: from merlin.infradead.org ([205.233.59.134]:60918 "EHLO merlin.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726758AbeJSQJa (ORCPT ); Fri, 19 Oct 2018 12:09:30 -0400 Content-Disposition: inline In-Reply-To: <20181018153307.ayvmq6du3gnsyvro@ast-mbp.dhcp.thefacebook.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Oct 18, 2018 at 08:33:09AM -0700, Alexei Starovoitov wrote: > On Thu, Oct 18, 2018 at 05:04:34PM +0200, Daniel Borkmann wrote: > > #endif /* _TOOLS_LINUX_ASM_IA64_BARRIER_H */ > > diff --git a/tools/arch/powerpc/include/asm/barrier.h b/tools/arch/powerpc/include/asm/barrier.h > > index a634da0..905a2c6 100644 > > --- a/tools/arch/powerpc/include/asm/barrier.h > > +++ b/tools/arch/powerpc/include/asm/barrier.h > > @@ -27,4 +27,20 @@ > > #define rmb() __asm__ __volatile__ ("sync" : : : "memory") > > #define wmb() __asm__ __volatile__ ("sync" : : : "memory") > > > > +#if defined(__powerpc64__) > > +#define smp_lwsync() __asm__ __volatile__ ("lwsync" : : : "memory") > > + > > +#define smp_store_release(p, v) \ > > +do { \ > > + smp_lwsync(); \ > > + WRITE_ONCE(*p, v); \ > > +} while (0) > > + > > +#define smp_load_acquire(p) \ > > +({ \ > > + typeof(*p) ___p1 = READ_ONCE(*p); \ > > + smp_lwsync(); \ > > + ___p1; \ > > I don't like this proliferation of asm. > Why do we think that we can do better job than compiler? > can we please use gcc builtins instead? > https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html > __atomic_load_n(ptr, __ATOMIC_ACQUIRE); > __atomic_store_n(ptr, val, __ATOMIC_RELEASE); > are done specifically for this use case if I'm not mistaken. > I think it pays to learn what compiler provides. My problem with using the C11 stuff for this is that we're then limited to compilers that actually support that. The kernel has a minimum of gcc-4.6 (and thus perf does too I think) and gcc-4.6 does not have C11. What Daniel writes is also true; the kernel and C11 memory models don't align; but you're right in that for this purpose the C11 load-acquire and store-release would indeed suffice.