Re: WARNING in bpf_bprintf_prepare

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Nov 10, 2022 at 12:53:16AM +0100, Jiri Olsa wrote:

SNIP

> > > > > ---
> > > > > diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
> > > > > index 6a13220d2d27..5a354ae096e5 100644
> > > > > --- a/include/trace/bpf_probe.h
> > > > > +++ b/include/trace/bpf_probe.h
> > > > > @@ -78,11 +78,15 @@
> > > > >  #define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
> > > > >
> > > > >  #define __BPF_DECLARE_TRACE(call, proto, args)                         \
> > > > > +static DEFINE_PER_CPU(int, __bpf_trace_tp_active_##call);              \
> > > > >  static notrace void                                                    \
> > > > >  __bpf_trace_##call(void *__data, proto)                                        \
> > > > >  {                                                                      \
> > > > >         struct bpf_prog *prog = __data;                                 \
> > > > > -       CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args));  \
> > > > > +                                                                       \
> > > > > +       if (likely(this_cpu_inc_return(__bpf_trace_tp_active_##call) == 1))             \
> > > > > +               CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args));  \
> > > > > +       this_cpu_dec(__bpf_trace_tp_active_##call);                                     \
> > > > >  }
> > > >
> > > > This approach will hurt real use cases where
> > > > multiple and different raw_tp progs run on the same cpu.
> > >
> > > would the 2 levels of nesting help in here?
> > >
> > > I can imagine the change above would break use case where we want to
> > > trigger tracepoints in irq context that interrupted task that's already
> > > in the same tracepoint
> > >
> > > with 2 levels of nesting we would trigger that tracepoint from irq and
> > > would still be safe with bpf_bprintf_prepare buffer
> > 
> > How would these 2 levels work?
> 
> just using the active counter like below, but I haven't tested it yet
> 
> jirka

seems to be working
Hao Sun, could you please test this patch?

thanks,
jirka
> 
> 
> ---
> diff --git a/include/trace/bpf_probe.h b/include/trace/bpf_probe.h
> index 6a13220d2d27..ca5dd34478b7 100644
> --- a/include/trace/bpf_probe.h
> +++ b/include/trace/bpf_probe.h
> @@ -78,11 +78,15 @@
>  #define CAST_TO_U64(...) CONCATENATE(__CAST, COUNT_ARGS(__VA_ARGS__))(__VA_ARGS__)
>  
>  #define __BPF_DECLARE_TRACE(call, proto, args)				\
> +static DEFINE_PER_CPU(int, __bpf_trace_tp_active_##call);		\
>  static notrace void							\
>  __bpf_trace_##call(void *__data, proto)					\
>  {									\
>  	struct bpf_prog *prog = __data;					\
> -	CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args));	\
> +									\
> +	if (likely(this_cpu_inc_return(__bpf_trace_tp_active_##call) < 3))		\
> +		CONCATENATE(bpf_trace_run, COUNT_ARGS(args))(prog, CAST_TO_U64(args));	\
> +	this_cpu_dec(__bpf_trace_tp_active_##call);					\
>  }
>  
>  #undef DECLARE_EVENT_CLASS