Messages in this thread |  | | Date | Thu, 21 Oct 2021 10:47:00 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v2 14/14] bpf,x86: Respect X86_FEATURE_RETPOLINE* |
| |
On Wed, Oct 20, 2021 at 05:05:02PM -0700, Alexei Starovoitov wrote: > On Wed, Oct 20, 2021 at 01:09:51PM +0200, Peter Zijlstra wrote:
> > @@ -446,25 +440,8 @@ static void emit_bpf_tail_call_indirect( > > { > > int tcc_off = -4 - round_up(stack_depth, 8); > > u8 *prog = *pprog, *start = *pprog; > > - int pop_bytes = 0; > > - int off1 = 42; > > - int off2 = 31; > > - int off3 = 9; > > - > > - /* count the additional bytes used for popping callee regs from stack > > - * that need to be taken into account for each of the offsets that > > - * are used for bailing out of the tail call > > - */ > > - pop_bytes = get_pop_bytes(callee_regs_used); > > - off1 += pop_bytes; > > - off2 += pop_bytes; > > - off3 += pop_bytes; > > - > > - if (stack_depth) { > > - off1 += 7; > > - off2 += 7; > > - off3 += 7; > > - } > > + static int out_label = -1; > > Interesting idea!
I nicked it from emit_bpf_tail_call() in the 32bit jit :-) It seemed a lot more robust than the 64bit one and I couldn't figure out why the difference.
> All insn emits trying to do the right thing from the start. > Here the logic assumes that there will be at least two passes over image. > I think that is correct, but we never had such assumption.
That's not exactly true; I think image is NULL on every first run, so all insn that depend on it will be wrong to start with. Equally there's a number of insn that seem to depend on addrs[i], that also requires at least two passes.
> A comment is certainly must have.
I can certainly add one, although I think we'll disagree on the comment style :-)
> The race is possible too. Not sure whether READ_ONCE/WRITE_ONCE > are really warranted though. Might be overkill.
Is there concurrency on the jit?
> Once you have a git branch with all the changes I can give it a go.
Ok, I'll go polish this thing and stick it in the tree mentioned in the cover letter.
> Also you can rely on our BPF CI. > Just cc your patchset to bpf@vger and add [PATCH bpf-next] to a subject. > In patchwork there will be "bpf/vmtest-bpf-next" link that > builds kernel, selftests and runs everything.
What's a patchwork and where do I find it?
> It's pretty much the same as selftests/bpf/vmtest.sh, but with the latest > clang nightly and other deps like pahole.
nice.
|  |