On Tue, Dec 6, 2022 at 9:23 PM Hao Sun <sunhao.th@xxxxxxxxx> wrote:
>
>
>
> > On 7 Dec 2022, at 10:14 AM, Namhyung Kim <namhyung@xxxxxxxxx> wrote:
> >
> > On Tue, Dec 06, 2022 at 12:09:51PM -0800, Alexei Starovoitov wrote:
> >> On Mon, Dec 5, 2022 at 4:28 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> >>> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> >>> index 3bbd3f0c810c..d27b7dc77894 100644
> >>> --- a/kernel/trace/bpf_trace.c
> >>> +++ b/kernel/trace/bpf_trace.c
> >>> @@ -2252,9 +2252,8 @@ void bpf_put_raw_tracepoint(struct bpf_raw_event_map *btp)
> >>> }
> >>>
> >>> static __always_inline
> >>> -void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
> >>> +void __bpf_trace_prog_run(struct bpf_prog *prog, u64 *args)
> >>> {
> >>> - cant_sleep();
> >>> if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
> >>> bpf_prog_inc_misses_counter(prog);
> >>> goto out;
> >>> @@ -2266,6 +2265,22 @@ void __bpf_trace_run(struct bpf_prog *prog, u64 *args)
> >>> this_cpu_dec(*(prog->active));
> >>> }
> >>>
> >>> +static __always_inline
> >>> +void __bpf_trace_run(struct bpf_raw_event_data *data, u64 *args)
> >>> +{
> >>> + struct bpf_prog *prog = data->prog;
> >>> +
> >>> + cant_sleep();
> >>> + if (unlikely(!data->recursion))
> >>> + return __bpf_trace_prog_run(prog, args);
> >>> +
> >>> + if (unlikely(this_cpu_inc_return(*(data->recursion))))
> >>> + goto out;
> >>> + __bpf_trace_prog_run(prog, args);
> >>> +out:
> >>> + this_cpu_dec(*(data->recursion));
> >>> +}
> >>
> >> This is way too much run-time and memory overhead to address
> >> this corner case. Pls come up with some other approach.
> >> Sorry I don't have decent suggestions at the moment.
> >> For now we can simply disallow attaching to contention_begin.
> >>
> >
> > How about this? It seems to work for me.
>
> How about progs that are attached with kprobe?
> See this one:
> https://lore.kernel.org/bpf/CACkBjsb3GRw5aiTT=RCUs3H5aum_QN+B0ZqZA=MvjspUP6NFMg@xxxxxxxxxxxxxx/T/#u
Oh sorry, I'm just talking about the lock contention tracepoints.
For kprobe + printk, I don't have a good solution and I think it needs
some rework to use trylock as Andrii mentioned.
Thanks,
Namhyung
