[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231215123458.63f57238@rorschach.local.home>
Date: Fri, 15 Dec 2023 12:34:58 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux trace kernel
<linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu
<mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH] tracing: Add disable-filter-buf option
On Fri, 15 Dec 2023 12:24:14 -0500
Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> On 2023-12-15 12:04, Steven Rostedt wrote:
> > On Fri, 15 Dec 2023 10:53:39 -0500
> > Mathieu Desnoyers <mathieu.desnoyers@...icios.com> wrote:
> [...]
> >>
> >> So rather than stacking tons of "on/off" switches for filter
> >> features, how about you let users express the mechanism they
> >> want to use for filtering with a string instead ? e.g.
> >>
> >> filter-method="temp-buffer"
> >> filter-method="ring-buffer"
> >> filter-method="input-arguments"
> >
> > If I add other ways to filter, it will be a separate file to control
> > that, but all options are on/off switches. Even if I add other
> > functionality to the way buffers are created, this will still have the
> > same functionality to turn the entire thing on or off.
>
> I'll be clearer then: I think this is a bad ABI. In your reply, you justify
> this bad ABI by implementation motivations.
What's wrong with a way to stop the copying ?
The option is just a way to say "I don't want to do the copy into the
buffer, I want to go directly into it"
>
> I don't care about the implementation, I care about the ABI, and
> I feel that your reply is not addressing my concern at all.
Maybe I don't understand your concern.
It's an on/off switch (like all options are). This is just a way to say
"I want to indirect copying of the event before filtering or not".
The "input-argument" part above may never happen. What's different
between tracefs and LTTng, is that all events are created by the
subsystem not by me. You don't use the TRACE_EVENT() macro, but you
need to manually create each event you care about yourself. It's more
of a burden but you also then have the luxury of hooking to the input
portion. That is not exposed, and that is by design. As that could
possibly make all tracepoints an ABI, and you'll have people like
peterz nacking any new tracepoint in the scheduler. He doesn't allow
trace events anymore because of that exposure.
>
> Moreover, double-negative boolean options (disable-X=false) are confusing.
I agree with this, and will rename it to "filter-buffer" instead. I
blame getting up at 3am for my 5:15am flight for not thinking clearly
on that ;-)
Thanks!
-- Steve
Powered by blists - more mailing lists