Re: [PATCH bpf-next 05/12] bpf: Add missed value to kprobe perf link info

From: Jiri Olsa <olsajiri@gmail.com>
To: Hou Tao <houtao@huaweicloud.com>
Cc: bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@chromium.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Daniel Xu <dxu@dxuuu.xyz>, Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>
Subject: Re: [PATCH bpf-next 05/12] bpf: Add missed value to kprobe perf link info
Date: Tue, 5 Sep 2023 09:19:41 +0200	[thread overview]
Message-ID: <ZPbWjW8/do1YCstj@krava> (raw)
In-Reply-To: <4540a09e-43a4-c258-710c-af895138ba6b@huaweicloud.com>

On Tue, Sep 05, 2023 at 10:23:27AM +0800, Hou Tao wrote:
> Hi,
> 
> On 8/28/2023 3:55 PM, Jiri Olsa wrote:
> > Add missed value to kprobe attached through perf link info to
> > hold the stats of missed kprobe handler execution.
> >
> > The kprobe's missed counter gets incremented when kprobe handler
> > is not executed due to another kprobe running on the same cpu.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/linux/trace_events.h   |  6 ++++--
> >  include/uapi/linux/bpf.h       |  1 +
> >  kernel/bpf/syscall.c           | 14 ++++++++------
> >  kernel/trace/bpf_trace.c       |  5 +++--
> >  kernel/trace/trace_kprobe.c    |  5 ++++-
> >  tools/include/uapi/linux/bpf.h |  1 +
> >  6 files changed, 21 insertions(+), 11 deletions(-)
> >
> 
> SNIP
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 17c21c0b2dd1..998c88874507 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1546,7 +1546,8 @@ NOKPROBE_SYMBOL(kretprobe_perf_func);
> >  
> >  int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
> >  			const char **symbol, u64 *probe_offset,
> > -			u64 *probe_addr, bool perf_type_tracepoint)
> > +			u64 *probe_addr, unsigned long *missed,
> > +			bool perf_type_tracepoint)
> >  {
> >  	const char *pevent = trace_event_name(event->tp_event);
> >  	const char *group = event->tp_event->class->system;
> > @@ -1565,6 +1566,8 @@ int bpf_get_kprobe_info(const struct perf_event *event, u32 *fd_type,
> >  	*probe_addr = kallsyms_show_value(current_cred()) ?
> >  		      (unsigned long)tk->rp.kp.addr : 0;
> >  	*symbol = tk->symbol;
> > +	if (missed)
> > +		*missed = tk->rp.kp.nmissed;
> 
> According to the implement of probes_profile_seq_show(), the missed
> counter for kretprobe should be tk->rp.kp.nmissed + tk->rp.nmissed. I
> think it would be a good idea to factor out a common helper to get the
> missed counter for kprobe or kretprobe.

ok, makes sense.. will check, I was also thinking to move
bpf_get_kprobe_info args into struct

jirka

> >  	return 0;
> >  }
> >  #endif	/* CONFIG_PERF_EVENTS */
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index b754edfb0cd7..5a39c7a13499 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -6539,6 +6539,7 @@ struct bpf_link_info {
> >  					__u32 name_len;
> >  					__u32 offset; /* offset from func_name */
> >  					__u64 addr;
> > +					__u64 missed;
> >  				} kprobe; /* BPF_PERF_EVENT_KPROBE, BPF_PERF_EVENT_KRETPROBE */
> >  				struct {
> >  					__aligned_u64 tp_name;   /* in/out */
> 