lkml.org 
[lkml]   [2009]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] tracing/filters: allow on-the-fly filter switching

    * Tom Zanussi <tzanussi@gmail.com> wrote:

    > This patch allows event filters to be safely removed or switched
    > on-the-fly while avoiding the use of rcu or the suspension of
    > tracing of previous versions.
    >
    > It does it by adding a new filter_pred_none() predicate function
    > which does nothing and by never deallocating either the predicates
    > or any of the filter_pred members used in matching; the predicate
    > lists are allocated and initialized during ftrace_event_calls
    > initialization.
    >
    > Whenever a filter is removed or replaced, the filter_pred_*
    > functions currently in use by the affected ftrace_event_call are
    > immediately switched over to to the filter_pred_none() function,
    > while the rest of the filter_pred members are left intact,
    > allowing any currently executing filter_pred_* functions to finish
    > up, using the values they're currently using.
    >
    > In the case of filter replacement, the new predicate values are
    > copied into the old predicates after the above step, and the
    > filter_pred_none() functions are replaced by the filter_pred_*
    > functions for the new filter. In this case, it is possible though
    > very unlikely that a previous filter_pred_* is still running even
    > after the filter_pred_none() switch and the switch to the new
    > filter_pred_*. In that case, however, because nothing has been
    > deallocated in the filter_pred, the worst that can happen is that
    > the old filter_pred_* function sees the new values and as a result
    > produces either a false positive or a false negative, depending on
    > the values it finds.
    >
    > So one downside to this method is that rarely, it can produce a
    > bad match during the filter switch, but it should be possible to
    > live with that, IMHO.

    Yeah.

    It is really a strong thing to avoid RCU here. Instrumentation
    should be self-sufficient to a large degree, and it does not get any
    more lowlevel than filter expression evaluation engine. Forcing the
    use of rcu_read_lock() there would limit its utility.

    > The other downside is that at least in this patch the predicate
    > lists are always pre-allocated, taking up memory from the start.
    > They could probably be allocated on first-use, and de-allocated
    > when tracing is completely stopped - if this patch makes sense, I
    > could create another one to do that later on.

    That's not a big issue IMO.

    > Oh, and it also places a restriction on the size of __arrays in
    > events, currently set to 128, since they can't be larger than the
    > now embedded str_val arrays in the filter_pred struct.

    that's OK too - we really want pre-calculated filter expressions and
    as atomic evaluations as possible. So having the maximum width
    specified is no big deal.

    The only exception would be if we ever do PATH_MAX type of field
    value comparisons - and i dont see any reason why not, once tracing
    is extended to the VFS or once the syscall tracer . That would
    increase it to 4096 bytes, making the max kzalloc larger than page
    size - still not outrageous so not a big problem. Just lets keep it
    in mind that 128 is a bit on the low side.

    also:

    > + if (!val_str || !strlen(val_str)
    > + || strlen(val_str) >= MAX_FILTER_STR_VAL) {
    > pred->field_name = NULL;
    > return -EINVAL;
    > }

    it might be quite cryptic to the user why a complex expression was
    not installed. I think a single-line KERN_INFO syslog entry would be
    most helpful.

    Ingo


    \
     
     \ /
      Last update: 2009-04-13 23:55    [W:2.329 / U:0.988 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site