> On Sep 10, 2021, at 11:50 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > On Fri, Sep 10, 2021 at 08:40:27PM +0200, Peter Zijlstra wrote: >> On Fri, Sep 10, 2021 at 06:27:36PM +0000, Song Liu wrote: >> >>> This works great and saves 3 entries! We have the following now: >> >> Yay! >> >>> ID: 0 from bpf_get_branch_snapshot+18 to intel_pmu_snapshot_branch_stack+0 >> >> is unavoidable, we need to end up in intel_pmu_snapshot_branch_stack() >> eventually. >> >>> ID: 1 from __brk_limit+477143934 to bpf_get_branch_snapshot+0 >> >> could be elided by having the JIT emit the call to >> intel_pmu_snapshot_branch_stack directly, instead of laundering it >> through that helper I suppose. >> >>> ID: 2 from __brk_limit+477192263 to __brk_limit+477143880 # trampoline >>> ID: 3 from __bpf_prog_enter+34 to __brk_limit+477192251 >> >> -ENOCLUE >> >>> ID: 4 from migrate_disable+60 to __bpf_prog_enter+9 >>> ID: 5 from __bpf_prog_enter+4 to migrate_disable+0 >> >> I suppose we can reduce that to a single branch if we inline >> migrate_disable() here, that thing unfortunately needs one branch >> itself. > > Oooh, since we put local_irq_save/restore() in > intel_pmu_snapshot_branch_stack(), we no longer need to be after > migrate_disable(). You could go back to placing it earlier! Hmm.. not really. We call migrate_disable() before entering the BPF program. And the helper calls snapshot_branch_stack() inside the BPF program. To move it to before migrate_disable(), we will have to add a "whether to snapshot branch stack" check before entering the BPF program. This check, while is cheap, is added to all BPF programs on this hook, even when the program does not use snapshot at all. So we would rather keep all logic inside the helper, and not touch the common path. Thanks, Song
