Re: [PATCH RFC v4 net-next 00/26] BPF syscall, maps, verifier, samples, llvm

[Andy Lutomirski]

On Wed, Aug 13, 2014 at 10:30 AM, Alexei Starovoitov <ast@xxxxxxxxxxxx> wrote:
> On Wed, Aug 13, 2014 at 1:52 AM, David Laight <David.Laight@xxxxxxxxxx> wrote:
>> From: Of Alexei Starovoitov
>>> one more RFC...
>>>
>>> Major difference vs previous set is a new 'load 64-bit immediate' eBPF insn.
>>> Which is first 16-byte instruction. It shows how eBPF ISA can be extended
>>> while maintaining backward compatibility, but mainly it cleans up eBPF
>>> program access to maps and improves run-time performance.
>>
>> Wouldn't it be more sensible to follow the scheme used by a lot of cpus
>> and add a 'load high' instruction (follow with 'add' or 'or').
>
> that was what I used before in pred_tree_walker->ebpf patch
> (4 existing instructions (2 movs, shift, or) to load 'pred' pointer)
> It's slower in interpreter than single instruction.
>
>> It still takes 16 bytes to load a 64bit immediate value, but the instruction
>> size remains constant.
>
> size of instruction is not important. 99% of instructions are 8 byte long
> and one is 16 byte. Big deal. It doesn't affect interpreter performance,
> easy for verifier and was straightforward to do in LLVM as well.
>
>> There is nothing to stop any JIT software detecting the instruction pair.
>
> well, it's actually very complicated to detect a sequence of
> instructions that compute single 64-bit value.
> Patch #11 detects and patches pseudo BPF_LD_IMM64 in
> a single 'for' loop (see replace_map_fd_with_map_ptr), because
> it's _single_ instruction. Any sequence of insns would require
> building control and data flow graphs for verifier and JIT.
> If you remember I resisted initially when Chema proposed
> 'load 64-bit immediate' equivalent, since back then the use cases
> didn't require it. With maps done via FDs, the need has arisen.

But don't you need some kind of detection anyway to handle the case
where something jumps to the middle of the "load 64-bit immediate"?  I
think it would be fine to require a particular sequence to load 64-bit
immediates if you want the JIT to optimize it well, though.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/