Re: [PATCH 4/4] perf record: Handle argument change in sched_switch

Andrii Nakryiko
2022-04-26T23:55:36Z

On Fri, Apr 22, 2022 at 3:49 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Recently sched_switch tracepoint added a new argument for prev_state,
> but it's hard to handle the change in a BPF program. Instead, we can
> check the function prototype in BTF before loading the program.
>
> Thus I make two copies of the tracepoint handler and select one based
> on the BTF info.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/bpf_off_cpu.c | 32 +++++++++++++++
> tools/perf/util/bpf_skel/off_cpu.bpf.c | 55 ++++++++++++++++++++------
> 2 files changed, 76 insertions(+), 11 deletions(-)
>

[...]

>
> +SEC("tp_btf/sched_switch")
> +int on_switch3(u64 *ctx)
> +{
> + struct task_struct *prev, *next;
> + int state;
> +
> + if (!enabled)
> + return 0;
> +
> + /*
> + * TP_PROTO(bool preempt, struct task_struct *prev,
> + * struct task_struct *next)
> + */
> + prev = (struct task_struct *)ctx[1];
> + next = (struct task_struct *)ctx[2];

you don't have to have two BPF programs for this, you can use
read-only variable to make this choice.

On BPF side

const volatile bool has_prev_state = false;

...

if (has_prev_state) {
prev = (struct task_struct *)ctx[2];
next = (struct task_struct *)ctx[3];
} else {
prev = (struct task_struct *)ctx[1];
next = (struct task_struct *)ctx[2];
}

And from user-space side you do your detection and before skeleton is loaded:

skel->rodata->has_prev_state = <whatever you detected>

But I'm still hoping that this prev_state argument can be moved to the
end ([0]) to make all this unnecessary.

[0] https://lore.kernel.org/lkml/93a20759600c05b6d9e4359a1517c88e06b44834.camel@fb.com/

> +
> + state = get_task_state(prev);
> +
> + return on_switch(ctx, prev, next, state);
> +}
> + …