On Sep 10, 2021, at 11:50 AM, Peter Zijlstra [off-list ref] wrote:

On Fri, Sep 10, 2021 at 08:40:27PM +0200, Peter Zijlstra wrote:

quoted

On Fri, Sep 10, 2021 at 06:27:36PM +0000, Song Liu wrote:

quoted

This works great and saves 3 entries! We have the following now:

Yay!

quoted

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.

quoted

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.

quoted

ID: 2 from __brk_limit+477192263 to __brk_limit+477143880  # trampoline 
ID: 3 from __bpf_prog_enter+34 to __brk_limit+477192251

-ENOCLUE

quoted

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