Re: [PATCH v3 bpf-next 1/4] tracing/probe: Add PERF_EVENT_IOC_QUERY_PROBE ioctl

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Aug 20, 2019 at 10:58:47AM -0700, Daniel Xu wrote:
> Hi Peter,
> 
> On Tue Aug 20, 2019 at 4:45 PM Peter Zijlstra wrote:
> > On Fri, Aug 16, 2019 at 03:31:46PM -0700, Daniel Xu wrote:
> > > It's useful to know [uk]probe's nmissed and nhit stats. For example with
> > > tracing tools, it's important to know when events may have been lost.
> > > debugfs currently exposes a control file to get this information, but
> > > it is not compatible with probes registered with the perf API.
> > 
> > What is this nmissed and nhit stuff?
> 
> nmissed is the number of times the probe's handler should have been run
> but didn't. nhit is the number of times the probes handler has run. I've
> documented this information in the uapi header. If you'd like, I can put
> it in the commit message too.

That comment just says: 'number of times this probe was temporarily
disabled', which says exactly nothing.

But reading the kprobe code seems to suggest this happens on recursive
kprobes, which I'm thinking is a dodgy situation in the first place.

ftrace and perf in general don't keep counts of events lost due to
recursion, so why should we do this for kprobes? Also, while you write
to support uprobes, it doesn't actually suffer from this (it cannot,
uprobes cannot recurse), so supporting it makes no sense.

And with that, the name QUERY_PROBE also makes no sense, because it is
not specific to [uk]probes, all software events suffer this.

And I'm not sure an additional ioctl() is the right way, supposing we
want to expose this at all. You've mentioned no alternative approached,
I'm thinking PERF_FORMAT_LOST might be possible, or maybe a
PERF_RECORD_LOST extention.

Of course, then you get to implement it for tracepoints and software
events too.