netdev - Re: bpf memory model. Was: [PATCH v4 bpf-next 1/9] bpf: introduce bpf_spin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190130210536.GY4240@linux.ibm.com>
Date:   Wed, 30 Jan 2019 13:05:36 -0800
From:   "Paul E. McKenney" <paulmck@...ux.ibm.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Will Deacon <will.deacon@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        daniel@...earbox.net, jakub.kicinski@...ronome.com,
        netdev@...r.kernel.org, kernel-team@...com, mingo@...hat.com,
        jannh@...gle.com
Subject: Re: bpf memory model. Was: [PATCH v4 bpf-next 1/9] bpf: introduce
 bpf_spin_lock

On Wed, Jan 30, 2019 at 11:51:14AM -0800, Alexei Starovoitov wrote:
> On Wed, Jan 30, 2019 at 10:36:18AM -0800, Paul E. McKenney wrote:
> > On Wed, Jan 30, 2019 at 06:11:00PM +0000, Will Deacon wrote:
> > > Hi Alexei,
> > > 
> > > On Mon, Jan 28, 2019 at 01:56:24PM -0800, Alexei Starovoitov wrote:
> > > > On Mon, Jan 28, 2019 at 10:24:08AM +0100, Peter Zijlstra wrote:
> > > > > On Fri, Jan 25, 2019 at 04:17:26PM -0800, Alexei Starovoitov wrote:
> > > > > > What I want to avoid is to define the whole execution ordering model upfront.
> > > > > > We cannot say that BPF ISA is weakly ordered like alpha.
> > > > > > Most of the bpf progs are written and running on x86. We shouldn't
> > > > > > twist bpf developer's arm by artificially relaxing memory model.
> > > > > > BPF memory model is equal to memory model of underlying architecture.
> > > > > > What we can do is to make it bpf progs a bit more portable with
> > > > > > smp_rmb instructions, but we must not force weak execution on the developer.
> > > > > 
> > > > > Well, I agree with only introducing bits you actually need, and my
> > > > > smp_rmb() example might have been poorly chosen, smp_load_acquire() /
> > > > > smp_store_release() might have been a far more useful example.
> > > > > 
> > > > > But I disagree with the last part; we have to pick a model now;
> > > > > otherwise you'll pain yourself into a corner.
> > > > > 
> > > > > Also; Alpha isn't very relevant these days; however ARM64 does seem to
> > > > > be gaining a lot of attention and that is very much a weak architecture.
> > > > > Adding strongly ordered assumptions to BPF now, will penalize them in
> > > > > the long run.
> > > > 
> > > > arm64 is gaining attention just like riscV is gaining it too.
> > > > BPF jit for arm64 is very solid, while BPF jit for riscV is being worked on.
> > > > BPF is not picking sides in CPU HW and ISA battles.
> > > 
> > > It's not about picking a side, it's about providing an abstraction of the
> > > various CPU architectures out there so that the programmer doesn't need to
> > > worry about where their program may run. Hell, even if you just said "eBPF
> > > follows x86 semantics" that would be better than saying nothing (and then we
> > > could have a discussion about whether x86 semantics are really what you
> > > want).
> > 
> > To reinforce this point, the Linux-kernel memory model (tools/memory-model)
> > is that abstraction for the Linux kernel.  Why not just use that for BPF?
> 
> I already answered this earlier in the thread.
> tldr: not going to sacrifice performance.

Understood.

But can we at least say that where there are no performance consequences,
BPF should follow LKMM?  You already mentioned smp_load_acquire()
and smp_store_release(), but the void atomics (e.g., atomic_inc())
should also work because they don't provide any ordering guarantees.
The _relaxed(), _release(), and _acquire() variants of the value-returning
atomics should be just fine as well.

The other value-returning atomics have strong ordering, which is fine
on many systems, but potentially suboptimal for the weakly ordered ones.
Though you have to have pretty good locality of reference to be able to
see the difference, because otherwise cache-miss overhead dominates.

Things like cmpxchg() don't seem to fit BPF because they are normally
used in spin loops, though there are some non-spinning use cases.

You correctly pointed out that READ_ONCE() and WRITE_ONCE() are suboptimal
on systems that don't support all sizes of loads, but I bet that there
are some sizes for which they are just fine across systems, for example,
pointer size and int size.

Does that help?  Or am I missing additional cases where performance
could be degraded?

							Thanx, Paul