Hi Peter, > On Aug 19, 2021, at 11:27 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > On Thu, Aug 19, 2021 at 06:22:07PM +0000, Song Liu wrote: > >>> And if we're going to be adding new pmu::methods then I figure one that >>> does the whole sample state might be more useful. >> >> What do you mean by "whole sample state"? To integrate with exiting >> perf_sample_data, like perf_output_sample()? > > Yeah, the PMI can/does set more than data->br_stack, but I'm now > thinking that without an actual PMI, much of that will not be possible. > br_stack is special here. > > Oh well, carry on I suppose. Here is another design choice that I would like to know your opinion on. Say we don't use BPF here. Instead, we use perf_kprobe. On a kretprobe, we can use branch_stack to figure out what branches we took before the return, say what happened when sys_perf_event_open returns -EINVAL? To achieve this, we will need two events from the kernel's perspective, one INTEL_FIXED_VLBR_EVENT attached to hardware pmu, and a kretprobe event. Let's call them VLBR-event and software-event respectively. When the software-event triggers, it need to read branch_stack snapshot from the VLBR-event's pmu, which is kind of weird. We will need some connection between these two events. Also, to keep more useful data in LBR registers, we want minimal number of branches between software-event triggers and perf_pmu_disable(hardware_pmu). I guess the best way is to add a pointer, branch_stack_pmu, to perf_event, and use it directly when the software- event triggers. Note that, the pmu will not go away. So event the VLBR- event get freed by accident, we will not crash the kernel (we may read garbage data though). Does this make sense to you? BPF case will be similar to this. We will replace the software-event with a BPF program, and still need the VLBR-event on each CPU. Another question is, how do we crate the VLBR-event. On way is to create it in user space, and somehow pass the information to the software-event or the BPF program. Another approach is to create it in the kernel with perf_event_create_kernel_counter(). Which of the two do you like better? Thanks, Song