Re: [PATCH] tracing: Add disable-filter-buf option

[Google]

On Fri, 15 Dec 2023 14:08:40 -0500
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:

> On 2023-12-15 13:43, Steven Rostedt wrote:
> > On Fri, 15 Dec 2023 13:25:07 -0500
> > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote:
> >>
> >> I am not against exposing an ABI that allows userspace to alter the
> >> filter behavior. I disagree on the way you plan to expose the ABI.
> > 
> > These are no different than the knobs for sched_debug
> 
> These are very much different. The sched features are enabled at
> build-time by modifying kernel/sched/features.h. There is no userspace
> ABI involved.
> 
> > 
> >>
> >> Exposing this option as an ABI in this way exposes too much internal
> >> ring buffer implementation details to userspace.
> > 
> > There's already lots of exposure using options. The options are just
> > knobs, nothing more.
> > 
> >>
> >> It exposes the following details which IMHO should be hidden or
> >> configurable in a way that allows moving to a whole new mechanism
> >> which will have significantly different characteristics in the
> >> future:
> >>
> >> It exposes that:
> >>
> >> - filtering uses a copy to a temporary buffer, and
> >> - that this copy is enabled by default.
> >>
> >> Once exposed, those design constraints become immutable due to ABI.
> > 
> > No it is not. There is no such thing as "immutable ABI". The rule is
> > "don't break user space" If this functionality in the kernel goes away,
> > the knob could become a nop, and I doubt any user space will break
> > because of it.
> > 
> > That is, the only burden is keeping this option exposed. But it could
> > be just like that light switch that has nothing connected to it. It's
> > still there, but does nothing if you switch it. This knob can act the
> > same way. This does not in anyway prevent future innovation.
> 
> I am not comfortable with exposing internal ring buffer implementation
> details to userspace which may or may not be deprecated as no-ops
> in the future. This will lead to useless clutter.

Hmm, but this may change the ring buffer consumption rate if the filter
is enabled. The ring buffer may be filled quickly than the user expected.
Thus if user specifies the rare condition, most of the events on the ring
buffer is filled with garbage. And user will know the buffer size *seems*
smaller than the setting.
I think copying overhead will be a secondary effect, the biggest noticable
difference is how many events are recorded in the ring buffer. Thus, what
about naming the option as "filter-on-buffer"?

If we introduce filtering on input directly, at that point we will use
it if "filter-on-buffer = no", because this is also not noticable from
users.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>