Messages in this thread | | | Date | Fri, 15 Dec 2023 12:04:17 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing: Add disable-filter-buf option |
| |
On Fri, 15 Dec 2023 10:53:39 -0500 Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > > > I'm not convinced that a boolean state is what you need here.
I will admit the biggest motivation for this was to allow for debugging ;-)
> > Yes, today you are in a position where you have two options: > > a) use the filter buffer, which falls back on copy to ring buffer > if nested, > > b) disable the filter buffer, and thus always copy to ring buffer > before filtering, > > But I think it would not be far-fetched to improve the implementation > of the filter-buffer to have one filter-buffer per nesting level > (per execution context), or just implement the filter buffer as a > per-cpu stack, which would remove the need to fall back on copy > to ring buffer when nested. Or you could even do like LTTng and > filter on the input arguments directly.
The filtering on the input arguments is much more difficult as they are not exposed to user space. I plan on adding that feature, but it will be more tied to the probe infrastructure, and be separate from this type of filtering.
When looking at removing the discard, I found that the histogram logic currently depends on writing to the ring buffer directly. That's because it needs to know the timestamp, and may or may not discard it. I'll have to look at this code more and see if I can get rid of that. In fact, this patch taps into that logic (it's what added the tracing_set_filter_buffering() function.
> > But each of these changes would add yet another boolean tunable, > which can quickly make the mix hard to understand from a user > perspective.
Honestly, if I do the stack filtering (it's already per_cpu), it will replace this altogether, and this option will still be available. That is, it will switch off buffering an event regardless of the implementation.
> > 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.
Thanks for the review.
-- Steve
| |