From mboxrd@z Thu Jan 1 00:00:00 1970 From: Will Deacon Subject: Re: [PATCH bpf-next 2/3] tools, perf: use smp_{rmb,mb} barriers instead of {rmb,mb} Date: Fri, 19 Oct 2018 12:02:43 +0100 Message-ID: <20181019110243.GC14246@arm.com> 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> <1e3eea86-f708-f48b-4db4-911a84b94e06@iogearbox.net> <20181019035340.ahjocmdj2o2zam4m@ast-mbp.dhcp.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Daniel Borkmann , Peter Zijlstra , paulmck@linux.vnet.ibm.com, acme@redhat.com, yhs@fb.com, john.fastabend@gmail.com, netdev@vger.kernel.org To: Alexei Starovoitov Return-path: Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:50540 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726903AbeJSTIQ (ORCPT ); Fri, 19 Oct 2018 15:08:16 -0400 Content-Disposition: inline In-Reply-To: <20181019035340.ahjocmdj2o2zam4m@ast-mbp.dhcp.thefacebook.com> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Oct 18, 2018 at 08:53:42PM -0700, Alexei Starovoitov wrote: > On Thu, Oct 18, 2018 at 09:00:46PM +0200, Daniel Borkmann wrote: > > On 10/18/2018 05:33 PM, 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. > > > > But are you sure the C11 memory model matches exact same model as kernel? > > Seems like last time Will looked into it [0] it wasn't the case ... > > I'm only suggesting equivalence of __atomic_load_n(ptr, __ATOMIC_ACQUIRE) > with kernel's smp_load_acquire(). > I've seen a bunch of user space ring buffer implementations implemented > with __atomic_load_n() primitives. > But let's ask experts who live in both worlds. One thing to be wary of is if there is an implementation choice between how to implement load-acquire and store-release for a given architecture. In these situations, it's often important that concurrent software agrees on the "mapping", so we'd need to be sure that (a) All userspace compilers that we care about have compatible mappings and (b) These mappings are compatible with the kernel code. Will