Re: [PATCH 2/6] tracing/profile: Add filter support

Frederic Weisbecker
2009-09-08T02:01:26Z

On Mon, Sep 07, 2009 at 04:12:53PM +0800, Li Zefan wrote:
> - add ftrace_profile_set_filter(), to set filter for a profile event
>
> - filter is enabled when profile probe is registered
>
> - filter is disabled when profile probe is unregistered
>
> - in ftrace_profile_##call(), record events only when
> filter_match_preds() returns 1
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Well, I feel a bit uncomfortable with this approach.

The events approach taken by perf is different from ftrace.

ftrace events activation/deactivation, ring buffer captures,
filters are all globals. And this is nice to perform kernel
tracing from debugfs files.

But perf has a per counter instance approach. This means
that when a tracepoint counter registers a filter, this should
be private to this tracepoint counter and not propagated to the
others.

So this should rely on a kind of per tracepoint counter
attribute, something that we should probably be stored in
the struct hw_perf_counter like:

--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -467,6 +467,7 @@ struct hw_perf_counter {
union { /* software */
atomic64_t count;
struct hrtimer hrtimer;
+ struct event_filter filter;
};
};
atomic64_t prev_count;

You may need to get the current perf context that can
be found in current->perf_counter_ctxp and then iterate
through the counter_list of this ctx to find the current counter
attached to this tracepoint (using the event id).

What is not nice is that we need to iterate in O(n), n beeing the
number of tracepoint counters attached to the current counter
context.

So to avoid the following costly sequence in the tracing fastpath:

- deref ctx->current->perf_counter_ctxp
- list every ctx->counter_list
- find the counter that matches
- deref counter->filter and test...

You could keep the profile_filter field (and profile_filter_active)
in struct ftrace_event_call but allocate them per cpu and
write these fields for a given event each time we enter/exit a
counter context that has a counter that uses this given event.

That's something we could do by using a struct pmu specific for
tracepoints. More precisely with enable/disable callbacks that would do
specific things and then relay on the perf_ops_generic pmu
callbacks.

the struct pmu::enable()/disable() callbacks are functions that are called
each time we schedule in/out a task group that has a counter that
uses the given pmu.
Ie: they are called each time we schedule in/out a counter.

So you have a struct ftrace_event_call. This event can be used in
several different counters instance at the same time. But in a given cpu,
only one of these counters can be currently in use.

Now if we build a simple struct pmu tp_pmu that mostly relays
on the perf_ops_generic pmu but also have a specific enable/disable
pair that calls perf_swcounter_enable/disable and also does:

/*
* We are scheduling this counter on the smp_processor_id() cpu
* (we are in atomic context, ctx->lock acquired) then we
* can safely write a local (per cpu) shortcut from the filter_profile
* field in the event to the counter filter.
*/
static int perf_tpcounter_enable(struct perf_counter *counter)
{
struct event_filter *filter, **shortcut;
int *enable;
struct ftrace_event_call *call;
int cpu = smp_processor_if();

call = find_event_by_id(counter->attr.config);
filter = &counter->hw.filter;

shortcut = &per_cpu_ptr(call->filter_profile, cpu)
enable = &per_cpu_var(call->filter_profile_enabled, cpu)

if (filter is present) {
*enable = 1;
*shortcut = filter;
}

return perf_swcounter_enable(counter);
}

/* We schedule out this counter from this cpu, erase the shortcut */
static void perf_tpcounter_disable(struct perf_counter *counter)
{
/* ... */
enable = 0;
shortcut = NULL;

perf_swcounter_disable(counter);
}

static const struct pmu perf_ops_tracepoint = {
.enable = perf_tpcounter_enable,
.disable = perf_tpcounter_disable,
.read = perf_swcounter_read,
.unthrottle = perf_swcounter_unthrottle,
};

See? Then you can have a O(1) retrieval of the current per
counter filter to apply from ftrace_profile_##call()

I hope I haven't been too much confusing...