On Fri, Jan 25, 2019 at 04:17:26PM -0800, Alexei Starovoitov wrote: > On Fri, Jan 25, 2019 at 11:23:12AM +0100, Peter Zijlstra wrote: > > On Thu, Jan 24, 2019 at 03:58:59PM -0800, Alexei Starovoitov wrote: > > > On Thu, Jan 24, 2019 at 07:01:09PM +0100, Peter Zijlstra wrote: > > > > > > And this would again be the moment where I go pester you about the BPF > > > > memory model :-) > > > > > > hehe :) > > > How do you propose to define it in a way that it applies to all archs > > > and yet doesn't penalize x86 ? > > > "Assume minimum execution ordering model" the way kernel does > > > unfortunately is not usable, since bpf doesn't have a luxury > > > of using nice #defines that convert into nops on x86. > > > > Why not? Surely the JIT can fix it up? That is, suppose you were to have > > smp_rmb() as a eBPF instruction then the JIT (which knows what > > architecture it is targeting) can simply avoid emitting instructions for > > it. > > I'm all for adding new instructions that solve real use cases. > imo bpf_spin_lock() is the first step in helping with concurrency. > At plumbers conference we agreed to add new sync_fetch_and_add() > and xchg() instructions. That's a second step. > smp_rmb/wmb/mb should be added as well. > JITs will patch them depending on architecture. > > 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. > > Similarly; could something like this not also help with the spinlock > > thing? Use that generic test-and-set thing for the interpreter, but > > provide a better implementation in the JIT? > > May be eventually. We can add cmpxchg insn, but the verifier still > doesn't support loops. We made a lot of progress in bounded loop research > over the last 2 years, but loops in bpf are still a year away. > We considered adding 'bpf_spin_lock' as a new instruction instead of helper > call, > but that approach has a lot of negatives, so we went with the helper. Ah, but the loop won't be in the BPF program itself. The BPF program would only have had the BPF_SPIN_LOCK instruction, the JIT them emits code similar to queued_spin_lock()/queued_spin_unlock() (or calls to out-of-line versions of them). There isn't anything that mandates the JIT uses the exact same locking routines the interpreter does, is there?
