On 11/11/2015 07:31 PM, Peter Zijlstra wrote:
On Wed, Nov 11, 2015 at 10:11:33AM -0800, Alexei Starovoitov wrote:
On Wed, Nov 11, 2015 at 06:57:41PM +0100, Peter Zijlstra wrote:
On Wed, Nov 11, 2015 at 12:35:48PM -0500, David Miller wrote:
From: Alexei Starovoitov <alexei.starovoi...@gmail.com>
Date: Wed, 11 Nov 2015 09:27:00 -0800
BPF_XADD == atomic_add() in kernel. period.
we are not going to deprecate it or introduce something else.
Agreed, it makes no sense to try and tie C99 or whatever atomic
semantics to something that is already clearly defined to have
exactly kernel atomic_add() semantics.
Dave, this really doesn't make any sense to me. __sync primitives have
well defined semantics and (e)BPF is violating this.
bpf_xadd was never meant to be __sync_fetch_and_add equivalent.
From the day one it meant to be atomic_add() as kernel does it.
I did piggy back on __sync in the llvm backend because it was the quick
and dirty way to move forward.
In retrospect I should have introduced a clean intrinstic for that instead,
but it's not too late to do it now. user space we can change at any time
unlike kernel.
I would argue that breaking userspace (language in this case) is equally
bad. Programs that used to work will now no longer work.
Well, on that note, it's not like you just change the target to bpf in your
Makefile and can compile (& load into the kernel) anything you want with it.
You do have to write small, restricted programs from scratch for a specific
use-case with the limited set of helper functions and intrinsics that are
available from the kernel. So I don't think that "Programs that used to work
will now no longer work." holds if you regard it as such.
Furthermore, the fetch_and_add (or XADD) name has well defined
semantics, which (e)BPF also violates.
bpf_xadd also didn't meant to be 'fetch'. It was void return from the beginning.
Then why the 'X'? The XADD name, does and always has meant: eXchange-ADD,
this means it must have a return value.
You using the XADD name for something that is not in fact XADD is just
wrong.
Atomicy is hard enough as it is, backends giving random interpretations
to them isn't helping anybody.
no randomness.
You mean every other backend translating __sync_fetch_and_add()
differently than you isn't random on your part?
bpf_xadd == atomic_add() in kernel.
imo that is the simplest and cleanest intepretantion one can have, no?
Wrong though, if you'd named it BPF_ADD, sure, XADD, not so much. That
is 'randomly' co-opting something that has well defined meaning and
semantics with something else.
It also baffles me that Alexei is seemingly unwilling to change/rev the
(e)BPF instructions, which would be invisible to the regular user, he
does want to change the language itself, which will impact all
'scripts'.
well, we cannot change it in kernel because it's ABI.
You can always rev it. Introduce a new set, and wait for users of the
old set to die, then remove it. We do that all the time with Linux ABI.
I'm not against adding new insns. We definitely can, but let's figure out why?
Is anything broken? No.
Yes, __sync_fetch_and_add() is broken when pulled through the eBPF
backend.
So what new insns make sense?
Depends a bit on how fancy you want to go. If you want to support weakly
ordered architectures at full speed you'll need more (and more
complexity) than if you decide to not go that way.
The simplest option would be a fully ordered compare-and-swap operation.
That is enough to implement everything else (at a cost). The other
extreme is a weak ll/sc with an optimizer pass recognising various forms
to translate into 'better' native instructions.
Add new one that does 'fetch_and_add' ? What is the real use case it
will be used for?
Look at all the atomic_{add,dec}_return*() users in the kernel. A typical
example would be a reader-writer lock implementations. See
include/asm-generic/rwsem.h for examples.
Adding new intrinsic to llvm is not a big deal. I'll add it as soon
as I have time to work on it or if somebody beats me to it I would be
glad to test it and apply it.
This isn't a speed coding contest. You want to think about this
properly.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html