* [PATCH 01/10] tracing/filter: Use static allocation for filter predicates
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 02/10] tracing/filter: Separate predicate init and filter addition Jiri Olsa
` (9 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Don't dynamically allocate filter_pred struct, use static memory.
This way we can get rid of the code managing the dynamic filter_pred
struct object.
The create_pred function integrates create_logical_pred function.
This way the static predicate memory is returned only from
one place.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 57 ++++++++++--------------------------
1 files changed, 16 insertions(+), 41 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 256764e..cb295a1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -630,11 +630,7 @@ find_event_field(struct ftrace_event_call *call, char *name)
static void filter_free_pred(struct filter_pred *pred)
{
- if (!pred)
- return;
-
kfree(pred->field_name);
- kfree(pred);
}
static void filter_clear_pred(struct filter_pred *pred)
@@ -1302,39 +1298,30 @@ parse_operand:
return 0;
}
-static struct filter_pred *create_pred(int op, char *operand1, char *operand2)
+static struct filter_pred *create_pred(struct filter_parse_state *ps,
+ int op, char *operand1, char *operand2)
{
- struct filter_pred *pred;
+ static struct filter_pred pred;
- pred = kzalloc(sizeof(*pred), GFP_KERNEL);
- if (!pred)
- return NULL;
+ memset(&pred, 0, sizeof(pred));
+ pred.op = op;
- pred->field_name = kstrdup(operand1, GFP_KERNEL);
- if (!pred->field_name) {
- kfree(pred);
+ if (op == OP_AND || op == OP_OR)
+ return &pred;
+
+ if (!operand1 || !operand2) {
+ parse_error(ps, FILT_ERR_MISSING_FIELD, 0);
return NULL;
}
- strcpy(pred->regex.pattern, operand2);
- pred->regex.len = strlen(pred->regex.pattern);
-
- pred->op = op;
-
- return pred;
-}
-
-static struct filter_pred *create_logical_pred(int op)
-{
- struct filter_pred *pred;
-
- pred = kzalloc(sizeof(*pred), GFP_KERNEL);
- if (!pred)
+ pred.field_name = kstrdup(operand1, GFP_KERNEL);
+ if (!pred.field_name)
return NULL;
- pred->op = op;
+ strcpy(pred.regex.pattern, operand2);
+ pred.regex.len = strlen(pred.regex.pattern);
- return pred;
+ return &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1643,19 +1630,7 @@ static int replace_preds(struct ftrace_event_call *call,
goto fail;
}
- if (elt->op == OP_AND || elt->op == OP_OR) {
- pred = create_logical_pred(elt->op);
- goto add_pred;
- }
-
- if (!operand1 || !operand2) {
- parse_error(ps, FILT_ERR_MISSING_FIELD, 0);
- err = -EINVAL;
- goto fail;
- }
-
- pred = create_pred(elt->op, operand1, operand2);
-add_pred:
+ pred = create_pred(ps, elt->op, operand1, operand2);
if (!pred) {
err = -ENOMEM;
goto fail;
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 02/10] tracing/filter: Separate predicate init and filter addition
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
2011-08-04 10:08 ` [PATCH 01/10] tracing/filter: Use static allocation for filter predicates Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-10 20:51 ` Steven Rostedt
2011-08-04 10:08 ` [PATCH 03/10] tracing/filter: Remove field_name from filter_pred struct Jiri Olsa
` (8 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Separating the filter_pred initialization from code
that adds the filter_pred to the filter.
The create_pred returns initialized predicate, which
is added to the filter by filter_add_pred function.
Keeping dry_run variable inside replace_preds function.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 56 +++++++++++++++---------------------
1 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index cb295a1..61c8dec 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -685,8 +685,7 @@ __pop_pred_stack(struct pred_stack *stack)
static int filter_set_pred(struct event_filter *filter,
int idx,
struct pred_stack *stack,
- struct filter_pred *src,
- filter_pred_fn_t fn)
+ struct filter_pred *src)
{
struct filter_pred *dest = &filter->preds[idx];
struct filter_pred *left;
@@ -698,7 +697,6 @@ static int filter_set_pred(struct event_filter *filter,
if (!dest->field_name)
return -ENOMEM;
}
- dest->fn = fn;
dest->index = idx;
if (dest->op == OP_OR || dest->op == OP_AND) {
@@ -836,12 +834,10 @@ static void filter_free_subsystem_filters(struct event_subsystem *system)
}
}
-static int filter_add_pred_fn(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
- struct event_filter *filter,
- struct filter_pred *pred,
- struct pred_stack *stack,
- filter_pred_fn_t fn)
+static int filter_add_pred(struct filter_parse_state *ps,
+ struct event_filter *filter,
+ struct filter_pred *pred,
+ struct pred_stack *stack)
{
int idx, err;
@@ -852,7 +848,7 @@ static int filter_add_pred_fn(struct filter_parse_state *ps,
idx = filter->n_preds;
filter_clear_pred(&filter->preds[idx]);
- err = filter_set_pred(filter, idx, stack, pred, fn);
+ err = filter_set_pred(filter, idx, stack, pred);
if (err)
return err;
@@ -933,25 +929,16 @@ static filter_pred_fn_t select_comparison_fn(int op, int field_size,
return fn;
}
-static int filter_add_pred(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
- struct event_filter *filter,
- struct filter_pred *pred,
- struct pred_stack *stack,
- bool dry_run)
+static int init_pred(struct filter_parse_state *ps,
+ struct ftrace_event_call *call,
+ struct filter_pred *pred)
+
{
struct ftrace_event_field *field;
- filter_pred_fn_t fn;
+ filter_pred_fn_t fn = filter_pred_none;
unsigned long long val;
int ret;
- fn = pred->fn = filter_pred_none;
-
- if (pred->op == OP_AND)
- goto add_pred_fn;
- else if (pred->op == OP_OR)
- goto add_pred_fn;
-
field = find_event_field(call, pred->field_name);
if (!field) {
parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
@@ -997,9 +984,7 @@ static int filter_add_pred(struct filter_parse_state *ps,
if (pred->op == OP_NE)
pred->not = 1;
-add_pred_fn:
- if (!dry_run)
- return filter_add_pred_fn(ps, call, filter, pred, stack, fn);
+ pred->fn = fn;
return 0;
}
@@ -1299,6 +1284,7 @@ parse_operand:
}
static struct filter_pred *create_pred(struct filter_parse_state *ps,
+ struct ftrace_event_call *call,
int op, char *operand1, char *operand2)
{
static struct filter_pred pred;
@@ -1321,7 +1307,7 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
- return &pred;
+ return init_pred(ps, call, &pred) ? NULL : &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1630,16 +1616,20 @@ static int replace_preds(struct ftrace_event_call *call,
goto fail;
}
- pred = create_pred(ps, elt->op, operand1, operand2);
+ pred = create_pred(ps, call, elt->op, operand1, operand2);
if (!pred) {
err = -ENOMEM;
goto fail;
}
- err = filter_add_pred(ps, call, filter, pred, &stack, dry_run);
- filter_free_pred(pred);
- if (err)
- goto fail;
+ if (!dry_run) {
+ err = filter_add_pred(ps, filter, pred, &stack);
+ if (err) {
+ filter_free_pred(pred);
+ goto fail;
+ }
+ }
+ filter_free_pred(pred);
operand1 = operand2 = NULL;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 02/10] tracing/filter: Separate predicate init and filter addition
2011-08-04 10:08 ` [PATCH 02/10] tracing/filter: Separate predicate init and filter addition Jiri Olsa
@ 2011-08-10 20:51 ` Steven Rostedt
0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 20:51 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> Separating the filter_pred initialization from code
> that adds the filter_pred to the filter.
>
> The create_pred returns initialized predicate, which
> is added to the filter by filter_add_pred function.
>
> Keeping dry_run variable inside replace_preds function.
The only issue I have with this patch is the change log. It tells me
what you are doing, but does not tell us why you are doing it.
I'm pretty clueless about reviewing this patch, as I don't know what the
goal of the patch is. It's like a patch that says, increment i
+ i++;
-- Steve
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 03/10] tracing/filter: Remove field_name from filter_pred struct
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
2011-08-04 10:08 ` [PATCH 01/10] tracing/filter: Use static allocation for filter predicates Jiri Olsa
2011-08-04 10:08 ` [PATCH 02/10] tracing/filter: Separate predicate init and filter addition Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 04/10] tracing/filter: Simplify tracepoint event lookup Jiri Olsa
` (7 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
The field_name was used just for finding event's fields. This way we
don't need to care about field_name allocation/free.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace.h | 11 +-------
kernel/trace/trace_events_filter.c | 53 ++++++++---------------------------
2 files changed, 13 insertions(+), 51 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 616846b..2eb3cf6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -761,16 +761,7 @@ struct filter_pred {
filter_pred_fn_t fn;
u64 val;
struct regex regex;
- /*
- * Leaf nodes use field_name, ops is used by AND and OR
- * nodes. The field_name is always freed when freeing a pred.
- * We can overload field_name for ops and have it freed
- * as well.
- */
- union {
- char *field_name;
- unsigned short *ops;
- };
+ unsigned short *ops;
int offset;
int not;
int op;
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 61c8dec..97b93f3 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -628,18 +628,6 @@ find_event_field(struct ftrace_event_call *call, char *name)
return __find_event_field(head, name);
}
-static void filter_free_pred(struct filter_pred *pred)
-{
- kfree(pred->field_name);
-}
-
-static void filter_clear_pred(struct filter_pred *pred)
-{
- kfree(pred->field_name);
- pred->field_name = NULL;
- pred->regex.len = 0;
-}
-
static int __alloc_pred_stack(struct pred_stack *stack, int n_preds)
{
stack->preds = kzalloc(sizeof(*stack->preds)*(n_preds + 1), GFP_KERNEL);
@@ -692,11 +680,6 @@ static int filter_set_pred(struct event_filter *filter,
struct filter_pred *right;
*dest = *src;
- if (src->field_name) {
- dest->field_name = kstrdup(src->field_name, GFP_KERNEL);
- if (!dest->field_name)
- return -ENOMEM;
- }
dest->index = idx;
if (dest->op == OP_OR || dest->op == OP_AND) {
@@ -737,11 +720,7 @@ static int filter_set_pred(struct event_filter *filter,
static void __free_preds(struct event_filter *filter)
{
- int i;
-
if (filter->preds) {
- for (i = 0; i < filter->a_preds; i++)
- kfree(filter->preds[i].field_name);
kfree(filter->preds);
filter->preds = NULL;
}
@@ -839,16 +818,14 @@ static int filter_add_pred(struct filter_parse_state *ps,
struct filter_pred *pred,
struct pred_stack *stack)
{
- int idx, err;
+ int err;
if (WARN_ON(filter->n_preds == filter->a_preds)) {
parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
return -ENOSPC;
}
- idx = filter->n_preds;
- filter_clear_pred(&filter->preds[idx]);
- err = filter_set_pred(filter, idx, stack, pred);
+ err = filter_set_pred(filter, filter->n_preds, stack, pred);
if (err)
return err;
@@ -930,21 +907,14 @@ static filter_pred_fn_t select_comparison_fn(int op, int field_size,
}
static int init_pred(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
+ struct ftrace_event_field *field,
struct filter_pred *pred)
{
- struct ftrace_event_field *field;
filter_pred_fn_t fn = filter_pred_none;
unsigned long long val;
int ret;
- field = find_event_field(call, pred->field_name);
- if (!field) {
- parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
- return -EINVAL;
- }
-
pred->offset = field->offset;
if (!is_legal_op(field, pred->op)) {
@@ -1287,6 +1257,7 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
struct ftrace_event_call *call,
int op, char *operand1, char *operand2)
{
+ struct ftrace_event_field *field;
static struct filter_pred pred;
memset(&pred, 0, sizeof(pred));
@@ -1300,14 +1271,16 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
return NULL;
}
- pred.field_name = kstrdup(operand1, GFP_KERNEL);
- if (!pred.field_name)
+ field = find_event_field(call, operand1);
+ if (!field) {
+ parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
return NULL;
+ }
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
- return init_pred(ps, call, &pred) ? NULL : &pred;
+ return init_pred(ps, field, &pred) ? NULL : &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1618,18 +1591,16 @@ static int replace_preds(struct ftrace_event_call *call,
pred = create_pred(ps, call, elt->op, operand1, operand2);
if (!pred) {
- err = -ENOMEM;
+ err = -EINVAL;
goto fail;
}
+
if (!dry_run) {
err = filter_add_pred(ps, filter, pred, &stack);
- if (err) {
- filter_free_pred(pred);
+ if (err)
goto fail;
- }
}
- filter_free_pred(pred);
operand1 = operand2 = NULL;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 04/10] tracing/filter: Simplify tracepoint event lookup
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (2 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 03/10] tracing/filter: Remove field_name from filter_pred struct Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it Jiri Olsa
` (6 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
We dont need to perform lookup through the ftrace_events list,
instead we can use the 'tp_event' field.
Each perf_event contains tracepoint event field 'tp_event', which
got initialized during the tracepoint event initialization.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 97b93f3..0948905 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1894,17 +1894,14 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
int err;
struct event_filter *filter;
struct filter_parse_state *ps;
- struct ftrace_event_call *call = NULL;
+ struct ftrace_event_call *call;
mutex_lock(&event_mutex);
- list_for_each_entry(call, &ftrace_events, list) {
- if (call->event.type == event_id)
- break;
- }
+ call = event->tp_event;
err = -EINVAL;
- if (&call->list == &ftrace_events)
+ if (!call)
goto out_unlock;
err = -EEXIST;
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (3 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 04/10] tracing/filter: Simplify tracepoint event lookup Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree Jiri Olsa
` (5 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Adding walk_pred_tree function to be used for walking throught
the filter predicates.
For each predicate the callback function is called, allowing
users to add their own functionality or customize their way
through the filter predicates.
Changing check_pred_tree function to use walk_pred_tree.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 137 ++++++++++++++++++++++-------------
1 files changed, 86 insertions(+), 51 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 0948905..5b889d4 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -381,6 +381,63 @@ get_pred_parent(struct filter_pred *pred, struct filter_pred *preds,
return pred;
}
+enum walk_return {
+ WALK_PRED_ABORT,
+ WALK_PRED_PARENT,
+ WALK_PRED_DEFAULT,
+};
+
+typedef int (*filter_pred_walkcb_t) (enum move_type move,
+ struct filter_pred *pred,
+ int *err, void *data);
+
+static int walk_pred_tree(struct filter_pred *preds,
+ struct filter_pred *root,
+ filter_pred_walkcb_t cb, void *data)
+{
+ struct filter_pred *pred = root;
+ enum move_type move = MOVE_DOWN;
+ int done = 0;
+
+ if (!preds)
+ return -EINVAL;
+
+ do {
+ int err = 0, ret;
+
+ ret = cb(move, pred, &err, data);
+ if (ret == WALK_PRED_ABORT)
+ return err;
+ if (ret == WALK_PRED_PARENT)
+ goto get_parent;
+
+ switch (move) {
+ case MOVE_DOWN:
+ if (pred->left != FILTER_PRED_INVALID) {
+ pred = &preds[pred->left];
+ continue;
+ }
+ goto get_parent;
+ case MOVE_UP_FROM_LEFT:
+ pred = &preds[pred->right];
+ move = MOVE_DOWN;
+ continue;
+ case MOVE_UP_FROM_RIGHT:
+ get_parent:
+ if (pred == root)
+ break;
+ pred = get_pred_parent(pred, preds,
+ pred->parent,
+ &move);
+ continue;
+ }
+ done = 1;
+ } while (!done);
+
+ /* We are fine. */
+ return 0;
+}
+
/*
* A series of AND or ORs where found together. Instead of
* climbing up and down the tree branches, an array of the
@@ -1321,6 +1378,23 @@ static int count_preds(struct filter_parse_state *ps)
return n_preds;
}
+struct check_pred_data {
+ int count;
+ int max;
+};
+
+static int check_pred_tree_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct check_pred_data *d = data;
+
+ if (WARN_ON(d->count++ > d->max)) {
+ *err = -EINVAL;
+ return WALK_PRED_ABORT;
+ }
+ return WALK_PRED_DEFAULT;
+}
+
/*
* The tree is walked at filtering of an event. If the tree is not correctly
* built, it may cause an infinite loop. Check here that the tree does
@@ -1329,58 +1403,19 @@ static int count_preds(struct filter_parse_state *ps)
static int check_pred_tree(struct event_filter *filter,
struct filter_pred *root)
{
- struct filter_pred *preds;
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
- int done = 0;
- int max;
-
- /*
- * The max that we can hit a node is three times.
- * Once going down, once coming up from left, and
- * once coming up from right. This is more than enough
- * since leafs are only hit a single time.
- */
- max = 3 * filter->n_preds;
-
- preds = filter->preds;
- if (!preds)
- return -EINVAL;
- pred = root;
-
- do {
- if (WARN_ON(count++ > max))
- return -EINVAL;
-
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
+ struct check_pred_data data = {
+ /*
+ * The max that we can hit a node is three times.
+ * Once going down, once coming up from left, and
+ * once coming up from right. This is more than enough
+ * since leafs are only hit a single time.
+ */
+ .max = 3 * filter->n_preds,
+ .count = 0,
+ };
- /* We are fine. */
- return 0;
+ return walk_pred_tree(filter->preds, root,
+ check_pred_tree_cb, &data);
}
static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (4 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-10 21:05 ` Steven Rostedt
2011-08-04 10:08 ` [PATCH 07/10] tracing/filter: Change fold_pred_tree " Jiri Olsa
` (4 subsequent siblings)
10 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing count_leafs function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 45 +++++++++--------------------------
1 files changed, 12 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5b889d4..14a9dad 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1418,43 +1418,22 @@ static int check_pred_tree(struct event_filter *filter,
check_pred_tree_cb, &data);
}
-static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
+static int count_leafs_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
{
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
- int done = 0;
+ int *count = data;
- pred = root;
+ if ((move == MOVE_DOWN) &&
+ (pred->left == FILTER_PRED_INVALID))
+ (*count)++;
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- return 1;
- count++;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
+ return WALK_PRED_DEFAULT;
+}
+static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
+{
+ int count = 0;
+ WARN_ON(walk_pred_tree(preds, root, count_leafs_cb, &count));
return count;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree
2011-08-04 10:08 ` [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree Jiri Olsa
@ 2011-08-10 21:05 ` Steven Rostedt
0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 21:05 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
I really like these two patches (5 and 6) as I hated the duplicate code
of the two tree walks. But... see below.
On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> Changing count_leafs function to use unified predicates tree
> processing.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> kernel/trace/trace_events_filter.c | 45 +++++++++--------------------------
> 1 files changed, 12 insertions(+), 33 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 5b889d4..14a9dad 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1418,43 +1418,22 @@ static int check_pred_tree(struct event_filter *filter,
> check_pred_tree_cb, &data);
> }
>
> -static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
> +static int count_leafs_cb(enum move_type move, struct filter_pred *pred,
> + int *err, void *data)
> {
> - struct filter_pred *pred;
> - enum move_type move = MOVE_DOWN;
> - int count = 0;
> - int done = 0;
> + int *count = data;
>
> - pred = root;
> + if ((move == MOVE_DOWN) &&
> + (pred->left == FILTER_PRED_INVALID))
> + (*count)++;
>
> - do {
> - switch (move) {
> - case MOVE_DOWN:
> - if (pred->left != FILTER_PRED_INVALID) {
> - pred = &preds[pred->left];
> - continue;
> - }
> - /* A leaf at the root is just a leaf in the tree */
> - if (pred == root)
> - return 1;
> - count++;
> - pred = get_pred_parent(pred, preds,
> - pred->parent, &move);
> - continue;
> - case MOVE_UP_FROM_LEFT:
> - pred = &preds[pred->right];
> - move = MOVE_DOWN;
> - continue;
> - case MOVE_UP_FROM_RIGHT:
> - if (pred == root)
> - break;
> - pred = get_pred_parent(pred, preds,
> - pred->parent, &move);
> - continue;
> - }
> - done = 1;
> - } while (!done);
> + return WALK_PRED_DEFAULT;
> +}
>
> +static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
> +{
> + int count = 0;
> + WARN_ON(walk_pred_tree(preds, root, count_leafs_cb, &count));
Please do not put functionality in a WARN_ON(). Some people have
WARN_ON() turn into nops.
Make this a:
ret = walk_pred_tree(preds, root, count_leafs_cb, &count);
WARN_ON(ret);
-- Steve
> return count;
> }
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 07/10] tracing/filter: Change fold_pred_tree function to use walk_pred_tree
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (5 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 08/10] tracing/filter: Change fold_pred " Jiri Olsa
` (3 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing fold_pred_tree function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 65 +++++++++++-------------------------
1 files changed, 20 insertions(+), 45 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 14a9dad..ebe13cc 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1494,6 +1494,24 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
return 0;
}
+static int fold_pred_tree_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct filter_pred *preds = data;
+
+ if (move != MOVE_DOWN)
+ return WALK_PRED_DEFAULT;
+ if (!(pred->index & FILTER_PRED_FOLD))
+ return WALK_PRED_DEFAULT;
+
+ *err = fold_pred(preds, pred);
+ if (*err)
+ return WALK_PRED_ABORT;
+
+ /* eveyrhing below is folded, continue with parent */
+ return WALK_PRED_PARENT;
+}
+
/*
* To optimize the processing of the ops, if we have several "ors" or
* "ands" together, we can put them in an array and process them all
@@ -1502,51 +1520,8 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
static int fold_pred_tree(struct event_filter *filter,
struct filter_pred *root)
{
- struct filter_pred *preds;
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int done = 0;
- int err;
-
- preds = filter->preds;
- if (!preds)
- return -EINVAL;
- pred = root;
-
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->index & FILTER_PRED_FOLD) {
- err = fold_pred(preds, pred);
- if (err)
- return err;
- /* Folded nodes are like leafs */
- } else if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
-
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return 0;
+ return walk_pred_tree(filter->preds, root, fold_pred_tree_cb,
+ filter->preds);
}
static int replace_preds(struct ftrace_event_call *call,
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 08/10] tracing/filter: Change fold_pred function to use walk_pred_tree
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (6 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 07/10] tracing/filter: Change fold_pred_tree " Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 10:08 ` [PATCH 09/10] tracing/filter: Change filter_match_preds " Jiri Olsa
` (2 subsequent siblings)
10 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing fold_pred_tree function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 68 +++++++++++++++++------------------
1 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ebe13cc..d4e0c09 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1437,13 +1437,40 @@ static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
return count;
}
+struct fold_pred_data {
+ struct filter_pred *root;
+ int count;
+ int children;
+};
+
+static int fold_pred_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct fold_pred_data *d = data;
+ struct filter_pred *root = d->root;
+
+ if (move != MOVE_DOWN)
+ return WALK_PRED_DEFAULT;
+ if (pred->left != FILTER_PRED_INVALID)
+ return WALK_PRED_DEFAULT;
+
+ if (WARN_ON(d->count == d->children)) {
+ *err = -EINVAL;
+ return WALK_PRED_ABORT;
+ }
+
+ pred->index &= ~FILTER_PRED_FOLD;
+ root->ops[d->count++] = pred->index;
+ return WALK_PRED_DEFAULT;
+}
+
static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
{
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
+ struct fold_pred_data data = {
+ .root = root,
+ .count = 0,
+ };
int children;
- int done = 0;
/* No need to keep the fold flag */
root->index &= ~FILTER_PRED_FOLD;
@@ -1461,37 +1488,8 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
return -ENOMEM;
root->val = children;
-
- pred = root;
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- if (WARN_ON(count == children))
- return -EINVAL;
- pred->index &= ~FILTER_PRED_FOLD;
- root->ops[count++] = pred->index;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return 0;
+ data.children = children;
+ return walk_pred_tree(preds, root, fold_pred_cb, &data);
}
static int fold_pred_tree_cb(enum move_type move, struct filter_pred *pred,
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCH 09/10] tracing/filter: Change filter_match_preds function to use walk_pred_tree
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (7 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 08/10] tracing/filter: Change fold_pred " Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-10 21:14 ` Steven Rostedt
2011-08-04 10:08 ` [PATCH 10/10] tracing/filter: Add startup tests for events filter Jiri Olsa
2011-08-10 21:25 ` [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Steven Rostedt
10 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing filter_match_preds function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 119 +++++++++++++++++-------------------
1 files changed, 55 insertions(+), 64 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index d4e0c09..55b13b9 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -467,6 +467,7 @@ static int process_ops(struct filter_pred *preds,
for (i = 0; i < op->val; i++) {
pred = &preds[op->ops[i]];
+ BUG_ON(!pred->fn);
match = pred->fn(pred, rec);
if (!!match == type)
return match;
@@ -474,92 +475,82 @@ static int process_ops(struct filter_pred *preds,
return match;
}
+struct filter_match_preds_data {
+ struct filter_pred *preds;
+ int match;
+ void *rec;
+};
+
+static int filter_match_preds_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct filter_match_preds_data *d = data;
+
+ *err = 0;
+ switch (move) {
+ case MOVE_DOWN:
+ /* only AND and OR have children */
+ if (pred->left != FILTER_PRED_INVALID) {
+ /* If ops is set, then it was folded. */
+ if (!pred->ops)
+ return WALK_PRED_DEFAULT;
+ /* We can treat folded ops as a leaf node */
+ d->match = process_ops(d->preds, pred, d->rec);
+ } else {
+ BUG_ON(!pred->fn);
+ d->match = pred->fn(pred, d->rec);
+ }
+
+ return WALK_PRED_PARENT;
+ case MOVE_UP_FROM_LEFT:
+ /*
+ * Check for short circuits.
+ *
+ * Optimization: !!match == (pred->op == OP_OR)
+ * is the same as:
+ * if ((match && pred->op == OP_OR) ||
+ * (!match && pred->op == OP_AND))
+ */
+ if (!!d->match == (pred->op == OP_OR))
+ return WALK_PRED_PARENT;
+ break;
+ case MOVE_UP_FROM_RIGHT:
+ break;
+ }
+
+ return WALK_PRED_DEFAULT;
+}
+
/* return 1 if event matches, 0 otherwise (discard) */
int filter_match_preds(struct event_filter *filter, void *rec)
{
- int match = -1;
- enum move_type move = MOVE_DOWN;
struct filter_pred *preds;
- struct filter_pred *pred;
struct filter_pred *root;
+ struct filter_match_preds_data data = {
+ /* match is currently meaningless */
+ .match = -1,
+ .rec = rec,
+ };
int n_preds;
- int done = 0;
/* no filter is considered a match */
if (!filter)
return 1;
n_preds = filter->n_preds;
-
if (!n_preds)
return 1;
/*
* n_preds, root and filter->preds are protect with preemption disabled.
*/
- preds = rcu_dereference_sched(filter->preds);
root = rcu_dereference_sched(filter->root);
if (!root)
return 1;
- pred = root;
-
- /* match is currently meaningless */
- match = -1;
-
- do {
- switch (move) {
- case MOVE_DOWN:
- /* only AND and OR have children */
- if (pred->left != FILTER_PRED_INVALID) {
- /* If ops is set, then it was folded. */
- if (!pred->ops) {
- /* keep going to down the left side */
- pred = &preds[pred->left];
- continue;
- }
- /* We can treat folded ops as a leaf node */
- match = process_ops(preds, pred, rec);
- } else
- match = pred->fn(pred, rec);
- /* If this pred is the only pred */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- /*
- * Check for short circuits.
- *
- * Optimization: !!match == (pred->op == OP_OR)
- * is the same as:
- * if ((match && pred->op == OP_OR) ||
- * (!match && pred->op == OP_AND))
- */
- if (!!match == (pred->op == OP_OR)) {
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- /* now go down the right side of the tree. */
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- /* We finished this equation. */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return match;
+ data.preds = preds = rcu_dereference_sched(filter->preds);
+ WARN_ON(walk_pred_tree(preds, root, filter_match_preds_cb, &data));
+ return data.match;
}
EXPORT_SYMBOL_GPL(filter_match_preds);
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 09/10] tracing/filter: Change filter_match_preds function to use walk_pred_tree
2011-08-04 10:08 ` [PATCH 09/10] tracing/filter: Change filter_match_preds " Jiri Olsa
@ 2011-08-10 21:14 ` Steven Rostedt
2011-08-10 21:16 ` Steven Rostedt
0 siblings, 1 reply; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 21:14 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> Changing filter_match_preds function to use unified predicates tree
> processing.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> kernel/trace/trace_events_filter.c | 119 +++++++++++++++++-------------------
> 1 files changed, 55 insertions(+), 64 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index d4e0c09..55b13b9 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -467,6 +467,7 @@ static int process_ops(struct filter_pred *preds,
>
> for (i = 0; i < op->val; i++) {
> pred = &preds[op->ops[i]];
> + BUG_ON(!pred->fn);
Why the added BUG_ON()s? Do we really need to crash the kernel on error
here?
-- Steve
> match = pred->fn(pred, rec);
> if (!!match == type)
> return match;
> @@ -474,92 +475,82 @@ static int process_ops(struct filter_pred *preds,
> return match;
> }
>
> +struct filter_match_preds_data {
> + struct filter_pred *preds;
> + int match;
> + void *rec;
> +};
> +
> +static int filter_match_preds_cb(enum move_type move, struct filter_pred *pred,
> + int *err, void *data)
> +{
> + struct filter_match_preds_data *d = data;
> +
> + *err = 0;
> + switch (move) {
> + case MOVE_DOWN:
> + /* only AND and OR have children */
> + if (pred->left != FILTER_PRED_INVALID) {
> + /* If ops is set, then it was folded. */
> + if (!pred->ops)
> + return WALK_PRED_DEFAULT;
> + /* We can treat folded ops as a leaf node */
> + d->match = process_ops(d->preds, pred, d->rec);
> + } else {
> + BUG_ON(!pred->fn);
> + d->match = pred->fn(pred, d->rec);
> + }
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 09/10] tracing/filter: Change filter_match_preds function to use walk_pred_tree
2011-08-10 21:14 ` Steven Rostedt
@ 2011-08-10 21:16 ` Steven Rostedt
0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 21:16 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
On Wed, 2011-08-10 at 17:14 -0400, Steven Rostedt wrote:
> On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> > Changing filter_match_preds function to use unified predicates tree
> > processing.
> >
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > ---
> > kernel/trace/trace_events_filter.c | 119 +++++++++++++++++-------------------
> > 1 files changed, 55 insertions(+), 64 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index d4e0c09..55b13b9 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -467,6 +467,7 @@ static int process_ops(struct filter_pred *preds,
> >
> > for (i = 0; i < op->val; i++) {
> > pred = &preds[op->ops[i]];
> > + BUG_ON(!pred->fn);
>
> Why the added BUG_ON()s? Do we really need to crash the kernel on error
> here?
I mean, could be instead have:
if (!WARN_ON_ONCE(!pred->fn))
> > match = pred->fn(pred, rec);
> > if (!!match == type)
> > return match;
> > @@ -474,92 +475,82 @@ static int process_ops(struct filter_pred *preds,
> > return match;
> > }
> >
> > +struct filter_match_preds_data {
> > + struct filter_pred *preds;
> > + int match;
> > + void *rec;
> > +};
> > +
> > +static int filter_match_preds_cb(enum move_type move, struct filter_pred *pred,
> > + int *err, void *data)
> > +{
> > + struct filter_match_preds_data *d = data;
> > +
> > + *err = 0;
> > + switch (move) {
> > + case MOVE_DOWN:
> > + /* only AND and OR have children */
> > + if (pred->left != FILTER_PRED_INVALID) {
> > + /* If ops is set, then it was folded. */
> > + if (!pred->ops)
> > + return WALK_PRED_DEFAULT;
> > + /* We can treat folded ops as a leaf node */
> > + d->match = process_ops(d->preds, pred, d->rec);
> > + } else {
> > + BUG_ON(!pred->fn);
if (!WARN_ON_ONCE(!pred->fn))
> > + d->match = pred->fn(pred, d->rec);
-- Steve
> > + }
>
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH 10/10] tracing/filter: Add startup tests for events filter
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (8 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 09/10] tracing/filter: Change filter_match_preds " Jiri Olsa
@ 2011-08-04 10:08 ` Jiri Olsa
2011-08-04 11:22 ` [PATCHv2 " Jiri Olsa
2011-08-10 21:25 ` [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Steven Rostedt
10 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 10:08 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Adding automated tests running as late_initcall. Tests are
compiled in with CONFIG_FTRACE_STARTUP_TEST option.
Adding test event "ftrace_test_filter" used to simulate
filter processing during event occurance.
String filters are compiled and tested against several
test events with different values.
Also testing that evaluation of explicit predicates is ommited
due to the lazy filter evaluation.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
include/trace/events/test.h | 45 ++++++++
kernel/trace/trace.h | 3 +
kernel/trace/trace_events_filter.c | 207 ++++++++++++++++++++++++++++++++++++
3 files changed, 255 insertions(+), 0 deletions(-)
create mode 100644 include/trace/events/test.h
diff --git a/include/trace/events/test.h b/include/trace/events/test.h
new file mode 100644
index 0000000..81c5484
--- /dev/null
+++ b/include/trace/events/test.h
@@ -0,0 +1,45 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM test
+
+#if !defined(_TRACE_TEST_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEST_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(ftrace_test_filter,
+
+ TP_PROTO(int a, int b, int c, int d, int e, int f, int g, int h),
+
+ TP_ARGS(a, b, c, d, e, f, g, h),
+
+ TP_STRUCT__entry(
+ __field(int, a)
+ __field(int, b)
+ __field(int, c)
+ __field(int, d)
+ __field(int, e)
+ __field(int, f)
+ __field(int, g)
+ __field(int, h)
+ ),
+
+ TP_fast_assign(
+ __entry->a = a;
+ __entry->b = b;
+ __entry->c = c;
+ __entry->d = d;
+ __entry->e = e;
+ __entry->f = f;
+ __entry->g = g;
+ __entry->h = h;
+ ),
+
+ TP_printk("a %d, b %d, c %d, d %d, e %d, f %d, g %d, h %d",
+ __entry->a, __entry->b, __entry->c, __entry->d,
+ __entry->e, __entry->f, __entry->g, __entry->h)
+);
+
+#endif /* _TRACE_TEST_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2eb3cf6..4c7540a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -762,6 +762,9 @@ struct filter_pred {
u64 val;
struct regex regex;
unsigned short *ops;
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ struct ftrace_event_field *field;
+#endif
int offset;
int not;
int op;
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 55b13b9..e4c8e6b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1328,6 +1328,9 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ pred.field = field;
+#endif
return init_pred(ps, field, &pred) ? NULL : &pred;
}
@@ -1923,3 +1926,207 @@ out_unlock:
#endif /* CONFIG_PERF_EVENTS */
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/test.h>
+
+static int test_get_filter(char *filter_str, struct ftrace_event_call *call,
+ struct event_filter **pfilter)
+{
+ struct event_filter *filter;
+ struct filter_parse_state *ps;
+ int err = -ENOMEM;
+
+ filter = __alloc_filter();
+ if (!filter)
+ goto out;
+
+ ps = kzalloc(sizeof(*ps), GFP_KERNEL);
+ if (!ps)
+ goto free_filter;
+
+ parse_init(ps, filter_ops, filter_str);
+ err = filter_parse(ps);
+ if (err)
+ goto free_ps;
+
+ err = replace_preds(call, filter, ps, filter_str, false);
+ if (!err)
+ *pfilter = filter;
+
+ free_ps:
+ filter_opstack_clear(ps);
+ postfix_clear(ps);
+ kfree(ps);
+
+ free_filter:
+ if (err)
+ __free_filter(filter);
+
+ out:
+ return err;
+}
+
+#define DATA_REC(m, va, vb, vc, vd, ve, vf, vg, vh, nvisit) \
+{ \
+ .filter = FILTER, \
+ .rec = { .a = va, .b = vb, .c = vc, .d = vd, \
+ .e = ve, .f = vf, .g = vg, .h = vh }, \
+ .match = m, \
+ .not_visited = nvisit, \
+}
+#define YES 1
+#define NO 0
+
+static struct test_filter_data_t {
+ char *filter;
+ struct ftrace_raw_ftrace_test_filter rec;
+ int match;
+ char *not_visited;
+} test_filter_data[] = {
+#define FILTER "a == 1 && b == 1 && c == 1 && d == 1 && " \
+ "e == 1 && f == 1 && g == 1 && h == 1"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, ""),
+ DATA_REC(NO, 0, 1, 1, 1, 1, 1, 1, 1, "bcdefgh"),
+ DATA_REC(NO, 1, 1, 1, 1, 1, 1, 1, 0, ""),
+#undef FILTER
+#define FILTER "a == 1 || b == 1 || c == 1 || d == 1 || " \
+ "e == 1 || f == 1 || g == 1 || h == 1"
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 0, 0, 0, 0, 0, 0, "bcdefgh"),
+#undef FILTER
+#define FILTER "(a == 1 || b == 1) && (c == 1 || d == 1) && " \
+ "(e == 1 || f == 1) && (g == 1 || h == 1)"
+ DATA_REC(NO, 0, 0, 1, 1, 1, 1, 1, 1, "dfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 0, 1, 0, 1, "bd"),
+ DATA_REC(NO, 1, 0, 1, 0, 0, 1, 0, 0, "bd"),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) || (c == 1 && d == 1) || " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 0, 1, 1, 1, 1, 1, 1, "efgh"),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 1, 1, ""),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) && (c == 1 && d == 1) && " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 0, "gh"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, ""),
+#undef FILTER
+#define FILTER "((a == 1 || b == 1) || (c == 1 || d == 1) || " \
+ "(e == 1 || f == 1)) && (g == 1 || h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 1, "bcdef"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, "h"),
+#undef FILTER
+#define FILTER "((((((((a == 1) && (b == 1)) || (c == 1)) && (d == 1)) || " \
+ "(e == 1)) && (f == 1)) || (g == 1)) && (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "ceg"),
+ DATA_REC(NO, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(NO, 1, 0, 1, 0, 1, 0, 1, 0, ""),
+#undef FILTER
+#define FILTER "((((((((a == 1) || (b == 1)) && (c == 1)) || (d == 1)) && " \
+ "(e == 1)) || (f == 1)) && (g == 1)) || (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "bdfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 1, 0, 1, 0, "bdfh"),
+};
+
+#undef DATA_REC
+#undef FILTER
+#undef YES
+#undef NO
+
+#define DATA_CNT (sizeof(test_filter_data)/sizeof(struct test_filter_data_t))
+
+static int test_pred_visited;
+
+static int test_pred_visited_fn(struct filter_pred *pred, void *event)
+{
+ struct ftrace_event_field *field = pred->field;
+
+ test_pred_visited = 1;
+ printk(KERN_INFO "\npred visited %s\n", field->name);
+ return 1;
+}
+
+static int test_walk_pred_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ char *fields = data;
+
+ if ((move == MOVE_DOWN) &&
+ (pred->left == FILTER_PRED_INVALID)) {
+ struct ftrace_event_field *field = pred->field;
+
+ if (!field) {
+ WARN(1, "all leafs should have field defined");
+ return WALK_PRED_DEFAULT;
+ }
+ if (!strchr(fields, *field->name))
+ return WALK_PRED_DEFAULT;
+
+ WARN_ON(!pred->fn);
+ pred->fn = test_pred_visited_fn;
+ }
+ return WALK_PRED_DEFAULT;
+}
+
+static __init int ftrace_test_event_filter(void)
+{
+ int i;
+
+ printk(KERN_INFO "Testing ftrace filter: ");
+
+ for (i = 0; i < DATA_CNT; i++) {
+ struct event_filter *filter = NULL;
+ struct test_filter_data_t *d = &test_filter_data[i];
+ int err;
+
+ err = test_get_filter(d->filter, &event_ftrace_test_filter,
+ &filter);
+ if (err) {
+ printk(KERN_INFO
+ "Failed to get filter for '%s', err %d\n",
+ d->filter, err);
+ break;
+ }
+
+ if (*d->not_visited)
+ walk_pred_tree(filter->preds, filter->root,
+ test_walk_pred_cb,
+ d->not_visited);
+
+ test_pred_visited = 0;
+ err = filter_match_preds(filter, &d->rec);
+
+ if (test_pred_visited) {
+ printk(KERN_INFO
+ "Failed, unwanted pred visited for filter %s\n",
+ d->filter);
+ break;
+ }
+
+ if (err != d->match) {
+ printk(KERN_INFO
+ "Failed to match filter '%s', expected %d\n",
+ d->filter, d->match);
+ break;
+ }
+ }
+
+ if (i == DATA_CNT)
+ printk(KERN_CONT "OK\n");
+
+ return 0;
+}
+
+late_initcall(ftrace_test_event_filter);
+
+#endif /* CONFIG_FTRACE_STARTUP_TEST */
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCHv2 10/10] tracing/filter: Add startup tests for events filter
2011-08-04 10:08 ` [PATCH 10/10] tracing/filter: Add startup tests for events filter Jiri Olsa
@ 2011-08-04 11:22 ` Jiri Olsa
2011-08-10 21:24 ` Steven Rostedt
0 siblings, 1 reply; 30+ messages in thread
From: Jiri Olsa @ 2011-08-04 11:22 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel
oops,
I sent wrong version with small leak, proper one attached..
thanks,
jirka
---
Adding automated tests running as late_initcall. Tests are
compiled in with CONFIG_FTRACE_STARTUP_TEST option.
Adding test event "ftrace_test_filter" used to simulate
filter processing during event occurance.
String filters are compiled and tested against several
test events with different values.
Also testing that evaluation of explicit predicates is ommited
due to the lazy filter evaluation.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
include/trace/events/test.h | 45 ++++++++
kernel/trace/trace.h | 3 +
kernel/trace/trace_events_filter.c | 209 ++++++++++++++++++++++++++++++++++++
3 files changed, 257 insertions(+), 0 deletions(-)
create mode 100644 include/trace/events/test.h
diff --git a/include/trace/events/test.h b/include/trace/events/test.h
new file mode 100644
index 0000000..81c5484
--- /dev/null
+++ b/include/trace/events/test.h
@@ -0,0 +1,45 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM test
+
+#if !defined(_TRACE_TEST_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEST_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(ftrace_test_filter,
+
+ TP_PROTO(int a, int b, int c, int d, int e, int f, int g, int h),
+
+ TP_ARGS(a, b, c, d, e, f, g, h),
+
+ TP_STRUCT__entry(
+ __field(int, a)
+ __field(int, b)
+ __field(int, c)
+ __field(int, d)
+ __field(int, e)
+ __field(int, f)
+ __field(int, g)
+ __field(int, h)
+ ),
+
+ TP_fast_assign(
+ __entry->a = a;
+ __entry->b = b;
+ __entry->c = c;
+ __entry->d = d;
+ __entry->e = e;
+ __entry->f = f;
+ __entry->g = g;
+ __entry->h = h;
+ ),
+
+ TP_printk("a %d, b %d, c %d, d %d, e %d, f %d, g %d, h %d",
+ __entry->a, __entry->b, __entry->c, __entry->d,
+ __entry->e, __entry->f, __entry->g, __entry->h)
+);
+
+#endif /* _TRACE_TEST_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2eb3cf6..4c7540a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -762,6 +762,9 @@ struct filter_pred {
u64 val;
struct regex regex;
unsigned short *ops;
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ struct ftrace_event_field *field;
+#endif
int offset;
int not;
int op;
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 55b13b9..66ccdec 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1328,6 +1328,9 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ pred.field = field;
+#endif
return init_pred(ps, field, &pred) ? NULL : &pred;
}
@@ -1923,3 +1926,209 @@ out_unlock:
#endif /* CONFIG_PERF_EVENTS */
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/test.h>
+
+static int test_get_filter(char *filter_str, struct ftrace_event_call *call,
+ struct event_filter **pfilter)
+{
+ struct event_filter *filter;
+ struct filter_parse_state *ps;
+ int err = -ENOMEM;
+
+ filter = __alloc_filter();
+ if (!filter)
+ goto out;
+
+ ps = kzalloc(sizeof(*ps), GFP_KERNEL);
+ if (!ps)
+ goto free_filter;
+
+ parse_init(ps, filter_ops, filter_str);
+ err = filter_parse(ps);
+ if (err)
+ goto free_ps;
+
+ err = replace_preds(call, filter, ps, filter_str, false);
+ if (!err)
+ *pfilter = filter;
+
+ free_ps:
+ filter_opstack_clear(ps);
+ postfix_clear(ps);
+ kfree(ps);
+
+ free_filter:
+ if (err)
+ __free_filter(filter);
+
+ out:
+ return err;
+}
+
+#define DATA_REC(m, va, vb, vc, vd, ve, vf, vg, vh, nvisit) \
+{ \
+ .filter = FILTER, \
+ .rec = { .a = va, .b = vb, .c = vc, .d = vd, \
+ .e = ve, .f = vf, .g = vg, .h = vh }, \
+ .match = m, \
+ .not_visited = nvisit, \
+}
+#define YES 1
+#define NO 0
+
+static struct test_filter_data_t {
+ char *filter;
+ struct ftrace_raw_ftrace_test_filter rec;
+ int match;
+ char *not_visited;
+} test_filter_data[] = {
+#define FILTER "a == 1 && b == 1 && c == 1 && d == 1 && " \
+ "e == 1 && f == 1 && g == 1 && h == 1"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, ""),
+ DATA_REC(NO, 0, 1, 1, 1, 1, 1, 1, 1, "bcdefgh"),
+ DATA_REC(NO, 1, 1, 1, 1, 1, 1, 1, 0, ""),
+#undef FILTER
+#define FILTER "a == 1 || b == 1 || c == 1 || d == 1 || " \
+ "e == 1 || f == 1 || g == 1 || h == 1"
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 0, 0, 0, 0, 0, 0, "bcdefgh"),
+#undef FILTER
+#define FILTER "(a == 1 || b == 1) && (c == 1 || d == 1) && " \
+ "(e == 1 || f == 1) && (g == 1 || h == 1)"
+ DATA_REC(NO, 0, 0, 1, 1, 1, 1, 1, 1, "dfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 0, 1, 0, 1, "bd"),
+ DATA_REC(NO, 1, 0, 1, 0, 0, 1, 0, 0, "bd"),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) || (c == 1 && d == 1) || " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 0, 1, 1, 1, 1, 1, 1, "efgh"),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 1, 1, ""),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) && (c == 1 && d == 1) && " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 0, "gh"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, ""),
+#undef FILTER
+#define FILTER "((a == 1 || b == 1) || (c == 1 || d == 1) || " \
+ "(e == 1 || f == 1)) && (g == 1 || h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 1, "bcdef"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, "h"),
+#undef FILTER
+#define FILTER "((((((((a == 1) && (b == 1)) || (c == 1)) && (d == 1)) || " \
+ "(e == 1)) && (f == 1)) || (g == 1)) && (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "ceg"),
+ DATA_REC(NO, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(NO, 1, 0, 1, 0, 1, 0, 1, 0, ""),
+#undef FILTER
+#define FILTER "((((((((a == 1) || (b == 1)) && (c == 1)) || (d == 1)) && " \
+ "(e == 1)) || (f == 1)) && (g == 1)) || (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "bdfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 1, 0, 1, 0, "bdfh"),
+};
+
+#undef DATA_REC
+#undef FILTER
+#undef YES
+#undef NO
+
+#define DATA_CNT (sizeof(test_filter_data)/sizeof(struct test_filter_data_t))
+
+static int test_pred_visited;
+
+static int test_pred_visited_fn(struct filter_pred *pred, void *event)
+{
+ struct ftrace_event_field *field = pred->field;
+
+ test_pred_visited = 1;
+ printk(KERN_INFO "\npred visited %s\n", field->name);
+ return 1;
+}
+
+static int test_walk_pred_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ char *fields = data;
+
+ if ((move == MOVE_DOWN) &&
+ (pred->left == FILTER_PRED_INVALID)) {
+ struct ftrace_event_field *field = pred->field;
+
+ if (!field) {
+ WARN(1, "all leafs should have field defined");
+ return WALK_PRED_DEFAULT;
+ }
+ if (!strchr(fields, *field->name))
+ return WALK_PRED_DEFAULT;
+
+ WARN_ON(!pred->fn);
+ pred->fn = test_pred_visited_fn;
+ }
+ return WALK_PRED_DEFAULT;
+}
+
+static __init int ftrace_test_event_filter(void)
+{
+ int i;
+
+ printk(KERN_INFO "Testing ftrace filter: ");
+
+ for (i = 0; i < DATA_CNT; i++) {
+ struct event_filter *filter = NULL;
+ struct test_filter_data_t *d = &test_filter_data[i];
+ int err;
+
+ err = test_get_filter(d->filter, &event_ftrace_test_filter,
+ &filter);
+ if (err) {
+ printk(KERN_INFO
+ "Failed to get filter for '%s', err %d\n",
+ d->filter, err);
+ break;
+ }
+
+ if (*d->not_visited)
+ walk_pred_tree(filter->preds, filter->root,
+ test_walk_pred_cb,
+ d->not_visited);
+
+ test_pred_visited = 0;
+ err = filter_match_preds(filter, &d->rec);
+
+ __free_filter(filter);
+
+ if (test_pred_visited) {
+ printk(KERN_INFO
+ "Failed, unwanted pred visited for filter %s\n",
+ d->filter);
+ break;
+ }
+
+ if (err != d->match) {
+ printk(KERN_INFO
+ "Failed to match filter '%s', expected %d\n",
+ d->filter, d->match);
+ break;
+ }
+ }
+
+ if (i == DATA_CNT)
+ printk(KERN_CONT "OK\n");
+
+ return 0;
+}
+
+late_initcall(ftrace_test_event_filter);
+
+#endif /* CONFIG_FTRACE_STARTUP_TEST */
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCHv2 10/10] tracing/filter: Add startup tests for events filter
2011-08-04 11:22 ` [PATCHv2 " Jiri Olsa
@ 2011-08-10 21:24 ` Steven Rostedt
0 siblings, 0 replies; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 21:24 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
On Thu, 2011-08-04 at 13:22 +0200, Jiri Olsa wrote:
> oops,
> I sent wrong version with small leak, proper one attached..
>
> thanks,
> jirka
>
>
> ---
> Adding automated tests running as late_initcall. Tests are
> compiled in with CONFIG_FTRACE_STARTUP_TEST option.
>
> Adding test event "ftrace_test_filter" used to simulate
> filter processing during event occurance.
>
> String filters are compiled and tested against several
> test events with different values.
>
> Also testing that evaluation of explicit predicates is ommited
> due to the lazy filter evaluation.
>
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> ---
> include/trace/events/test.h | 45 ++++++++
> kernel/trace/trace.h | 3 +
> kernel/trace/trace_events_filter.c | 209 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 257 insertions(+), 0 deletions(-)
> create mode 100644 include/trace/events/test.h
>
> diff --git a/include/trace/events/test.h b/include/trace/events/test.h
> new file mode 100644
> index 0000000..81c5484
> --- /dev/null
> +++ b/include/trace/events/test.h
Hmm, I'd like to keep this out of the include/trace/events directory.
Could you put this directly into the kernel/trace/ directory, using the
Makefile tricks in samples/trace_events/Makefile and the
TRACE_INCLUDE_PATH as seen in samples/trace_events/trace-events-sample.h
-- Steve
> @@ -0,0 +1,45 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM test
> +
> +#if !defined(_TRACE_TEST_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_TEST_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(ftrace_test_filter,
> +
> + TP_PROTO(int a, int b, int c, int d, int e, int f, int g, int h),
> +
> + TP_ARGS(a, b, c, d, e, f, g, h),
> +
> + TP_STRUCT__entry(
> + __field(int, a)
> + __field(int, b)
> + __field(int, c)
> + __field(int, d)
> + __field(int, e)
> + __field(int, f)
> + __field(int, g)
> + __field(int, h)
> + ),
> +
> + TP_fast_assign(
> + __entry->a = a;
> + __entry->b = b;
> + __entry->c = c;
> + __entry->d = d;
> + __entry->e = e;
> + __entry->f = f;
> + __entry->g = g;
> + __entry->h = h;
> + ),
> +
> + TP_printk("a %d, b %d, c %d, d %d, e %d, f %d, g %d, h %d",
> + __entry->a, __entry->b, __entry->c, __entry->d,
> + __entry->e, __entry->f, __entry->g, __entry->h)
> +);
> +
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH 0/10] ftrace/filter: Trace events filtering related fixies
2011-08-04 10:08 [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Jiri Olsa
` (9 preceding siblings ...)
2011-08-04 10:08 ` [PATCH 10/10] tracing/filter: Add startup tests for events filter Jiri Olsa
@ 2011-08-10 21:25 ` Steven Rostedt
2011-08-11 14:21 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
10 siblings, 2 replies; 30+ messages in thread
From: Steven Rostedt @ 2011-08-10 21:25 UTC (permalink / raw)
To: Jiri Olsa; +Cc: fweisbec, mingo, linux-kernel
On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> hi,
>
> on the way to perf/ftrace filtering changes, I made some changes
> to the current events filtering code. It could be divided into 3
> parts roughly:
>
> - remove unnecessary dynamic allocations (patches 1 - 4)
> - unify the predicate tree walking (patches 5 - 9)
> - added startup test for event filtering (patch 10)
>
> attached patches:
> - 01/10 tracing/filter: Use static allocation for filter predicates
> - 02/10 tracing/filter: Separate predicate init and filter addition
> - 03/10 tracing/filter: Remove field_name from filter_pred struct
> - 04/10 tracing/filter: Simplify tracepoint event lookup
> - 05/10 tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it
> - 06/10 tracing/filter: Change count_leafs function to use walk_pred_tree
> - 07/10 tracing/filter: Change fold_pred_tree function to use walk_pred_tree
> - 08/10 tracing/filter: Change fold_pred function to use walk_pred_tree
> - 09/10 tracing/filter: Change filter_match_preds function to use walk_pred_tree
> - 10/10 tracing/filter: Add startup tests for events filter
Other than the comments I made, I really like this patch set. Thanks a
lot for the hard work put into this!
If you do an update, I'll pull it into my queue for 3.2.
-- Steve
^ permalink raw reply [flat|nested] 30+ messages in thread* Re: [PATCH 0/10] ftrace/filter: Trace events filtering related fixies
2011-08-10 21:25 ` [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Steven Rostedt
@ 2011-08-11 14:21 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
1 sibling, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:21 UTC (permalink / raw)
To: Steven Rostedt; +Cc: fweisbec, mingo, linux-kernel
On Wed, Aug 10, 2011 at 05:25:35PM -0400, Steven Rostedt wrote:
> On Thu, 2011-08-04 at 12:08 +0200, Jiri Olsa wrote:
> > hi,
> >
> > on the way to perf/ftrace filtering changes, I made some changes
> > to the current events filtering code. It could be divided into 3
> > parts roughly:
> >
> > - remove unnecessary dynamic allocations (patches 1 - 4)
> > - unify the predicate tree walking (patches 5 - 9)
> > - added startup test for event filtering (patch 10)
> >
> > attached patches:
> > - 01/10 tracing/filter: Use static allocation for filter predicates
> > - 02/10 tracing/filter: Separate predicate init and filter addition
> > - 03/10 tracing/filter: Remove field_name from filter_pred struct
> > - 04/10 tracing/filter: Simplify tracepoint event lookup
> > - 05/10 tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it
> > - 06/10 tracing/filter: Change count_leafs function to use walk_pred_tree
> > - 07/10 tracing/filter: Change fold_pred_tree function to use walk_pred_tree
> > - 08/10 tracing/filter: Change fold_pred function to use walk_pred_tree
> > - 09/10 tracing/filter: Change filter_match_preds function to use walk_pred_tree
> > - 10/10 tracing/filter: Add startup tests for events filter
>
> Other than the comments I made, I really like this patch set. Thanks a
> lot for the hard work put into this!
>
> If you do an update, I'll pull it into my queue for 3.2.
thanks for comments, I'll send out v2 shortly
jirka
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCHv2 0/10] ftrace/filter: Trace events filtering related fixies
2011-08-10 21:25 ` [PATCH 0/10] ftrace/filter: Trace events filtering related fixies Steven Rostedt
2011-08-11 14:21 ` Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 01/10] tracing/filter: Use static allocation for filter predicates Jiri Olsa
` (9 more replies)
1 sibling, 10 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel
hi,
on the way to perf/ftrace filtering changes, I made some changes
to the current events filtering code. It could be divided into 3
parts roughly:
- remove unnecessary dynamic allocations (patches 1 - 4)
- unify the predicate tree walking (patches 5 - 9)
- added startup test for event filtering (patch 10)
attached patches:
- 01/10 tracing/filter: Use static allocation for filter predicates
- 02/10 tracing/filter: Separate predicate init and filter addition
- 03/10 tracing/filter: Remove field_name from filter_pred struct
- 04/10 tracing/filter: Simplify tracepoint event lookup
- 05/10 tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it
- 06/10 tracing/filter: Change count_leafs function to use walk_pred_tree
- 07/10 tracing/filter: Change fold_pred_tree function to use walk_pred_tree
- 08/10 tracing/filter: Change fold_pred function to use walk_pred_tree
- 09/10 tracing/filter: Change filter_match_preds function to use walk_pred_tree
- 10/10 tracing/filter: Add startup tests for events filter
v2 changes:
- 02/10 - changed patch change log
- 06/10 - putting functionality out of WARN_ON
- 09/10 - using WARN_ON_ONCE instead of BUG_ON
- putting functionality out of WARN_ON
- 10/10 - moved/renamed test.h into kernel/trace/trace_events_filter_test.h
wbr,
jirka
---
kernel/trace/Makefile | 2 +
kernel/trace/trace.h | 14 +-
kernel/trace/trace_events_filter.c | 786 ++++++++++++++++++-------------
kernel/trace/trace_events_filter_test.h | 50 ++
4 files changed, 510 insertions(+), 342 deletions(-)
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 01/10] tracing/filter: Use static allocation for filter predicates
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 02/10] tracing/filter: Separate predicate init and filter addition Jiri Olsa
` (8 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Don't dynamically allocate filter_pred struct, use static memory.
This way we can get rid of the code managing the dynamic filter_pred
struct object.
The create_pred function integrates create_logical_pred function.
This way the static predicate memory is returned only from
one place.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 57 ++++++++++--------------------------
1 files changed, 16 insertions(+), 41 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 256764e..cb295a1 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -630,11 +630,7 @@ find_event_field(struct ftrace_event_call *call, char *name)
static void filter_free_pred(struct filter_pred *pred)
{
- if (!pred)
- return;
-
kfree(pred->field_name);
- kfree(pred);
}
static void filter_clear_pred(struct filter_pred *pred)
@@ -1302,39 +1298,30 @@ parse_operand:
return 0;
}
-static struct filter_pred *create_pred(int op, char *operand1, char *operand2)
+static struct filter_pred *create_pred(struct filter_parse_state *ps,
+ int op, char *operand1, char *operand2)
{
- struct filter_pred *pred;
+ static struct filter_pred pred;
- pred = kzalloc(sizeof(*pred), GFP_KERNEL);
- if (!pred)
- return NULL;
+ memset(&pred, 0, sizeof(pred));
+ pred.op = op;
- pred->field_name = kstrdup(operand1, GFP_KERNEL);
- if (!pred->field_name) {
- kfree(pred);
+ if (op == OP_AND || op == OP_OR)
+ return &pred;
+
+ if (!operand1 || !operand2) {
+ parse_error(ps, FILT_ERR_MISSING_FIELD, 0);
return NULL;
}
- strcpy(pred->regex.pattern, operand2);
- pred->regex.len = strlen(pred->regex.pattern);
-
- pred->op = op;
-
- return pred;
-}
-
-static struct filter_pred *create_logical_pred(int op)
-{
- struct filter_pred *pred;
-
- pred = kzalloc(sizeof(*pred), GFP_KERNEL);
- if (!pred)
+ pred.field_name = kstrdup(operand1, GFP_KERNEL);
+ if (!pred.field_name)
return NULL;
- pred->op = op;
+ strcpy(pred.regex.pattern, operand2);
+ pred.regex.len = strlen(pred.regex.pattern);
- return pred;
+ return &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1643,19 +1630,7 @@ static int replace_preds(struct ftrace_event_call *call,
goto fail;
}
- if (elt->op == OP_AND || elt->op == OP_OR) {
- pred = create_logical_pred(elt->op);
- goto add_pred;
- }
-
- if (!operand1 || !operand2) {
- parse_error(ps, FILT_ERR_MISSING_FIELD, 0);
- err = -EINVAL;
- goto fail;
- }
-
- pred = create_pred(elt->op, operand1, operand2);
-add_pred:
+ pred = create_pred(ps, elt->op, operand1, operand2);
if (!pred) {
err = -ENOMEM;
goto fail;
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 02/10] tracing/filter: Separate predicate init and filter addition
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 01/10] tracing/filter: Use static allocation for filter predicates Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 03/10] tracing/filter: Remove field_name from filter_pred struct Jiri Olsa
` (7 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Making the code cleaner by having one function to fully prepare
the predicate (create_pred), and another to add the predicate to
the filter (filter_add_pred).
As a benefit, this way the dry_run flag stays only inside the
replace_preds function and is not passed deeper.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 56 +++++++++++++++---------------------
1 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index cb295a1..61c8dec 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -685,8 +685,7 @@ __pop_pred_stack(struct pred_stack *stack)
static int filter_set_pred(struct event_filter *filter,
int idx,
struct pred_stack *stack,
- struct filter_pred *src,
- filter_pred_fn_t fn)
+ struct filter_pred *src)
{
struct filter_pred *dest = &filter->preds[idx];
struct filter_pred *left;
@@ -698,7 +697,6 @@ static int filter_set_pred(struct event_filter *filter,
if (!dest->field_name)
return -ENOMEM;
}
- dest->fn = fn;
dest->index = idx;
if (dest->op == OP_OR || dest->op == OP_AND) {
@@ -836,12 +834,10 @@ static void filter_free_subsystem_filters(struct event_subsystem *system)
}
}
-static int filter_add_pred_fn(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
- struct event_filter *filter,
- struct filter_pred *pred,
- struct pred_stack *stack,
- filter_pred_fn_t fn)
+static int filter_add_pred(struct filter_parse_state *ps,
+ struct event_filter *filter,
+ struct filter_pred *pred,
+ struct pred_stack *stack)
{
int idx, err;
@@ -852,7 +848,7 @@ static int filter_add_pred_fn(struct filter_parse_state *ps,
idx = filter->n_preds;
filter_clear_pred(&filter->preds[idx]);
- err = filter_set_pred(filter, idx, stack, pred, fn);
+ err = filter_set_pred(filter, idx, stack, pred);
if (err)
return err;
@@ -933,25 +929,16 @@ static filter_pred_fn_t select_comparison_fn(int op, int field_size,
return fn;
}
-static int filter_add_pred(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
- struct event_filter *filter,
- struct filter_pred *pred,
- struct pred_stack *stack,
- bool dry_run)
+static int init_pred(struct filter_parse_state *ps,
+ struct ftrace_event_call *call,
+ struct filter_pred *pred)
+
{
struct ftrace_event_field *field;
- filter_pred_fn_t fn;
+ filter_pred_fn_t fn = filter_pred_none;
unsigned long long val;
int ret;
- fn = pred->fn = filter_pred_none;
-
- if (pred->op == OP_AND)
- goto add_pred_fn;
- else if (pred->op == OP_OR)
- goto add_pred_fn;
-
field = find_event_field(call, pred->field_name);
if (!field) {
parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
@@ -997,9 +984,7 @@ static int filter_add_pred(struct filter_parse_state *ps,
if (pred->op == OP_NE)
pred->not = 1;
-add_pred_fn:
- if (!dry_run)
- return filter_add_pred_fn(ps, call, filter, pred, stack, fn);
+ pred->fn = fn;
return 0;
}
@@ -1299,6 +1284,7 @@ parse_operand:
}
static struct filter_pred *create_pred(struct filter_parse_state *ps,
+ struct ftrace_event_call *call,
int op, char *operand1, char *operand2)
{
static struct filter_pred pred;
@@ -1321,7 +1307,7 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
- return &pred;
+ return init_pred(ps, call, &pred) ? NULL : &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1630,16 +1616,20 @@ static int replace_preds(struct ftrace_event_call *call,
goto fail;
}
- pred = create_pred(ps, elt->op, operand1, operand2);
+ pred = create_pred(ps, call, elt->op, operand1, operand2);
if (!pred) {
err = -ENOMEM;
goto fail;
}
- err = filter_add_pred(ps, call, filter, pred, &stack, dry_run);
- filter_free_pred(pred);
- if (err)
- goto fail;
+ if (!dry_run) {
+ err = filter_add_pred(ps, filter, pred, &stack);
+ if (err) {
+ filter_free_pred(pred);
+ goto fail;
+ }
+ }
+ filter_free_pred(pred);
operand1 = operand2 = NULL;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 03/10] tracing/filter: Remove field_name from filter_pred struct
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 01/10] tracing/filter: Use static allocation for filter predicates Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 02/10] tracing/filter: Separate predicate init and filter addition Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 04/10] tracing/filter: Simplify tracepoint event lookup Jiri Olsa
` (6 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
The field_name was used just for finding event's fields. This way we
don't need to care about field_name allocation/free.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace.h | 11 +-------
kernel/trace/trace_events_filter.c | 53 ++++++++---------------------------
2 files changed, 13 insertions(+), 51 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 616846b..2eb3cf6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -761,16 +761,7 @@ struct filter_pred {
filter_pred_fn_t fn;
u64 val;
struct regex regex;
- /*
- * Leaf nodes use field_name, ops is used by AND and OR
- * nodes. The field_name is always freed when freeing a pred.
- * We can overload field_name for ops and have it freed
- * as well.
- */
- union {
- char *field_name;
- unsigned short *ops;
- };
+ unsigned short *ops;
int offset;
int not;
int op;
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 61c8dec..97b93f3 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -628,18 +628,6 @@ find_event_field(struct ftrace_event_call *call, char *name)
return __find_event_field(head, name);
}
-static void filter_free_pred(struct filter_pred *pred)
-{
- kfree(pred->field_name);
-}
-
-static void filter_clear_pred(struct filter_pred *pred)
-{
- kfree(pred->field_name);
- pred->field_name = NULL;
- pred->regex.len = 0;
-}
-
static int __alloc_pred_stack(struct pred_stack *stack, int n_preds)
{
stack->preds = kzalloc(sizeof(*stack->preds)*(n_preds + 1), GFP_KERNEL);
@@ -692,11 +680,6 @@ static int filter_set_pred(struct event_filter *filter,
struct filter_pred *right;
*dest = *src;
- if (src->field_name) {
- dest->field_name = kstrdup(src->field_name, GFP_KERNEL);
- if (!dest->field_name)
- return -ENOMEM;
- }
dest->index = idx;
if (dest->op == OP_OR || dest->op == OP_AND) {
@@ -737,11 +720,7 @@ static int filter_set_pred(struct event_filter *filter,
static void __free_preds(struct event_filter *filter)
{
- int i;
-
if (filter->preds) {
- for (i = 0; i < filter->a_preds; i++)
- kfree(filter->preds[i].field_name);
kfree(filter->preds);
filter->preds = NULL;
}
@@ -839,16 +818,14 @@ static int filter_add_pred(struct filter_parse_state *ps,
struct filter_pred *pred,
struct pred_stack *stack)
{
- int idx, err;
+ int err;
if (WARN_ON(filter->n_preds == filter->a_preds)) {
parse_error(ps, FILT_ERR_TOO_MANY_PREDS, 0);
return -ENOSPC;
}
- idx = filter->n_preds;
- filter_clear_pred(&filter->preds[idx]);
- err = filter_set_pred(filter, idx, stack, pred);
+ err = filter_set_pred(filter, filter->n_preds, stack, pred);
if (err)
return err;
@@ -930,21 +907,14 @@ static filter_pred_fn_t select_comparison_fn(int op, int field_size,
}
static int init_pred(struct filter_parse_state *ps,
- struct ftrace_event_call *call,
+ struct ftrace_event_field *field,
struct filter_pred *pred)
{
- struct ftrace_event_field *field;
filter_pred_fn_t fn = filter_pred_none;
unsigned long long val;
int ret;
- field = find_event_field(call, pred->field_name);
- if (!field) {
- parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
- return -EINVAL;
- }
-
pred->offset = field->offset;
if (!is_legal_op(field, pred->op)) {
@@ -1287,6 +1257,7 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
struct ftrace_event_call *call,
int op, char *operand1, char *operand2)
{
+ struct ftrace_event_field *field;
static struct filter_pred pred;
memset(&pred, 0, sizeof(pred));
@@ -1300,14 +1271,16 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
return NULL;
}
- pred.field_name = kstrdup(operand1, GFP_KERNEL);
- if (!pred.field_name)
+ field = find_event_field(call, operand1);
+ if (!field) {
+ parse_error(ps, FILT_ERR_FIELD_NOT_FOUND, 0);
return NULL;
+ }
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
- return init_pred(ps, call, &pred) ? NULL : &pred;
+ return init_pred(ps, field, &pred) ? NULL : &pred;
}
static int check_preds(struct filter_parse_state *ps)
@@ -1618,18 +1591,16 @@ static int replace_preds(struct ftrace_event_call *call,
pred = create_pred(ps, call, elt->op, operand1, operand2);
if (!pred) {
- err = -ENOMEM;
+ err = -EINVAL;
goto fail;
}
+
if (!dry_run) {
err = filter_add_pred(ps, filter, pred, &stack);
- if (err) {
- filter_free_pred(pred);
+ if (err)
goto fail;
- }
}
- filter_free_pred(pred);
operand1 = operand2 = NULL;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 04/10] tracing/filter: Simplify tracepoint event lookup
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (2 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 03/10] tracing/filter: Remove field_name from filter_pred struct Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it Jiri Olsa
` (5 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
We dont need to perform lookup through the ftrace_events list,
instead we can use the 'tp_event' field.
Each perf_event contains tracepoint event field 'tp_event', which
got initialized during the tracepoint event initialization.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 97b93f3..0948905 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1894,17 +1894,14 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
int err;
struct event_filter *filter;
struct filter_parse_state *ps;
- struct ftrace_event_call *call = NULL;
+ struct ftrace_event_call *call;
mutex_lock(&event_mutex);
- list_for_each_entry(call, &ftrace_events, list) {
- if (call->event.type == event_id)
- break;
- }
+ call = event->tp_event;
err = -EINVAL;
- if (&call->list == &ftrace_events)
+ if (!call)
goto out_unlock;
err = -EEXIST;
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (3 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 04/10] tracing/filter: Simplify tracepoint event lookup Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree Jiri Olsa
` (4 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Adding walk_pred_tree function to be used for walking throught
the filter predicates.
For each predicate the callback function is called, allowing
users to add their own functionality or customize their way
through the filter predicates.
Changing check_pred_tree function to use walk_pred_tree.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 137 ++++++++++++++++++++++-------------
1 files changed, 86 insertions(+), 51 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 0948905..5b889d4 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -381,6 +381,63 @@ get_pred_parent(struct filter_pred *pred, struct filter_pred *preds,
return pred;
}
+enum walk_return {
+ WALK_PRED_ABORT,
+ WALK_PRED_PARENT,
+ WALK_PRED_DEFAULT,
+};
+
+typedef int (*filter_pred_walkcb_t) (enum move_type move,
+ struct filter_pred *pred,
+ int *err, void *data);
+
+static int walk_pred_tree(struct filter_pred *preds,
+ struct filter_pred *root,
+ filter_pred_walkcb_t cb, void *data)
+{
+ struct filter_pred *pred = root;
+ enum move_type move = MOVE_DOWN;
+ int done = 0;
+
+ if (!preds)
+ return -EINVAL;
+
+ do {
+ int err = 0, ret;
+
+ ret = cb(move, pred, &err, data);
+ if (ret == WALK_PRED_ABORT)
+ return err;
+ if (ret == WALK_PRED_PARENT)
+ goto get_parent;
+
+ switch (move) {
+ case MOVE_DOWN:
+ if (pred->left != FILTER_PRED_INVALID) {
+ pred = &preds[pred->left];
+ continue;
+ }
+ goto get_parent;
+ case MOVE_UP_FROM_LEFT:
+ pred = &preds[pred->right];
+ move = MOVE_DOWN;
+ continue;
+ case MOVE_UP_FROM_RIGHT:
+ get_parent:
+ if (pred == root)
+ break;
+ pred = get_pred_parent(pred, preds,
+ pred->parent,
+ &move);
+ continue;
+ }
+ done = 1;
+ } while (!done);
+
+ /* We are fine. */
+ return 0;
+}
+
/*
* A series of AND or ORs where found together. Instead of
* climbing up and down the tree branches, an array of the
@@ -1321,6 +1378,23 @@ static int count_preds(struct filter_parse_state *ps)
return n_preds;
}
+struct check_pred_data {
+ int count;
+ int max;
+};
+
+static int check_pred_tree_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct check_pred_data *d = data;
+
+ if (WARN_ON(d->count++ > d->max)) {
+ *err = -EINVAL;
+ return WALK_PRED_ABORT;
+ }
+ return WALK_PRED_DEFAULT;
+}
+
/*
* The tree is walked at filtering of an event. If the tree is not correctly
* built, it may cause an infinite loop. Check here that the tree does
@@ -1329,58 +1403,19 @@ static int count_preds(struct filter_parse_state *ps)
static int check_pred_tree(struct event_filter *filter,
struct filter_pred *root)
{
- struct filter_pred *preds;
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
- int done = 0;
- int max;
-
- /*
- * The max that we can hit a node is three times.
- * Once going down, once coming up from left, and
- * once coming up from right. This is more than enough
- * since leafs are only hit a single time.
- */
- max = 3 * filter->n_preds;
-
- preds = filter->preds;
- if (!preds)
- return -EINVAL;
- pred = root;
-
- do {
- if (WARN_ON(count++ > max))
- return -EINVAL;
-
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
+ struct check_pred_data data = {
+ /*
+ * The max that we can hit a node is three times.
+ * Once going down, once coming up from left, and
+ * once coming up from right. This is more than enough
+ * since leafs are only hit a single time.
+ */
+ .max = 3 * filter->n_preds,
+ .count = 0,
+ };
- /* We are fine. */
- return 0;
+ return walk_pred_tree(filter->preds, root,
+ check_pred_tree_cb, &data);
}
static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (4 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 05/10] tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 07/10] tracing/filter: Change fold_pred_tree " Jiri Olsa
` (3 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing count_leafs function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 47 ++++++++++-------------------------
1 files changed, 14 insertions(+), 33 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5b889d4..ebbb261 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1418,43 +1418,24 @@ static int check_pred_tree(struct event_filter *filter,
check_pred_tree_cb, &data);
}
-static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
+static int count_leafs_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
{
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
- int done = 0;
+ int *count = data;
- pred = root;
+ if ((move == MOVE_DOWN) &&
+ (pred->left == FILTER_PRED_INVALID))
+ (*count)++;
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- return 1;
- count++;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
+ return WALK_PRED_DEFAULT;
+}
+
+static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
+{
+ int count = 0, ret;
+ ret = walk_pred_tree(preds, root, count_leafs_cb, &count);
+ WARN_ON(ret);
return count;
}
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 07/10] tracing/filter: Change fold_pred_tree function to use walk_pred_tree
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (5 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 08/10] tracing/filter: Change fold_pred " Jiri Olsa
` (2 subsequent siblings)
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing fold_pred_tree function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 65 +++++++++++-------------------------
1 files changed, 20 insertions(+), 45 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index ebbb261..d8aa100 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1496,6 +1496,24 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
return 0;
}
+static int fold_pred_tree_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct filter_pred *preds = data;
+
+ if (move != MOVE_DOWN)
+ return WALK_PRED_DEFAULT;
+ if (!(pred->index & FILTER_PRED_FOLD))
+ return WALK_PRED_DEFAULT;
+
+ *err = fold_pred(preds, pred);
+ if (*err)
+ return WALK_PRED_ABORT;
+
+ /* eveyrhing below is folded, continue with parent */
+ return WALK_PRED_PARENT;
+}
+
/*
* To optimize the processing of the ops, if we have several "ors" or
* "ands" together, we can put them in an array and process them all
@@ -1504,51 +1522,8 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
static int fold_pred_tree(struct event_filter *filter,
struct filter_pred *root)
{
- struct filter_pred *preds;
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int done = 0;
- int err;
-
- preds = filter->preds;
- if (!preds)
- return -EINVAL;
- pred = root;
-
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->index & FILTER_PRED_FOLD) {
- err = fold_pred(preds, pred);
- if (err)
- return err;
- /* Folded nodes are like leafs */
- } else if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
-
- /* A leaf at the root is just a leaf in the tree */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return 0;
+ return walk_pred_tree(filter->preds, root, fold_pred_tree_cb,
+ filter->preds);
}
static int replace_preds(struct ftrace_event_call *call,
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 08/10] tracing/filter: Change fold_pred function to use walk_pred_tree
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (6 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 07/10] tracing/filter: Change fold_pred_tree " Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 09/10] tracing/filter: Change filter_match_preds " Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 10/10] tracing/filter: Add startup tests for events filter Jiri Olsa
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing fold_pred_tree function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 68 +++++++++++++++++------------------
1 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index d8aa100..f44e68b 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1439,13 +1439,40 @@ static int count_leafs(struct filter_pred *preds, struct filter_pred *root)
return count;
}
+struct fold_pred_data {
+ struct filter_pred *root;
+ int count;
+ int children;
+};
+
+static int fold_pred_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct fold_pred_data *d = data;
+ struct filter_pred *root = d->root;
+
+ if (move != MOVE_DOWN)
+ return WALK_PRED_DEFAULT;
+ if (pred->left != FILTER_PRED_INVALID)
+ return WALK_PRED_DEFAULT;
+
+ if (WARN_ON(d->count == d->children)) {
+ *err = -EINVAL;
+ return WALK_PRED_ABORT;
+ }
+
+ pred->index &= ~FILTER_PRED_FOLD;
+ root->ops[d->count++] = pred->index;
+ return WALK_PRED_DEFAULT;
+}
+
static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
{
- struct filter_pred *pred;
- enum move_type move = MOVE_DOWN;
- int count = 0;
+ struct fold_pred_data data = {
+ .root = root,
+ .count = 0,
+ };
int children;
- int done = 0;
/* No need to keep the fold flag */
root->index &= ~FILTER_PRED_FOLD;
@@ -1463,37 +1490,8 @@ static int fold_pred(struct filter_pred *preds, struct filter_pred *root)
return -ENOMEM;
root->val = children;
-
- pred = root;
- do {
- switch (move) {
- case MOVE_DOWN:
- if (pred->left != FILTER_PRED_INVALID) {
- pred = &preds[pred->left];
- continue;
- }
- if (WARN_ON(count == children))
- return -EINVAL;
- pred->index &= ~FILTER_PRED_FOLD;
- root->ops[count++] = pred->index;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return 0;
+ data.children = children;
+ return walk_pred_tree(preds, root, fold_pred_cb, &data);
}
static int fold_pred_tree_cb(enum move_type move, struct filter_pred *pred,
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 09/10] tracing/filter: Change filter_match_preds function to use walk_pred_tree
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (7 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 08/10] tracing/filter: Change fold_pred " Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
2011-08-11 14:25 ` [PATCHv2 10/10] tracing/filter: Add startup tests for events filter Jiri Olsa
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Changing filter_match_preds function to use unified predicates tree
processing.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/trace_events_filter.c | 124 +++++++++++++++++-------------------
1 files changed, 58 insertions(+), 66 deletions(-)
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index f44e68b..319c3ca 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -467,99 +467,91 @@ static int process_ops(struct filter_pred *preds,
for (i = 0; i < op->val; i++) {
pred = &preds[op->ops[i]];
- match = pred->fn(pred, rec);
+ if (!WARN_ON_ONCE(!pred->fn))
+ match = pred->fn(pred, rec);
if (!!match == type)
return match;
}
return match;
}
+struct filter_match_preds_data {
+ struct filter_pred *preds;
+ int match;
+ void *rec;
+};
+
+static int filter_match_preds_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ struct filter_match_preds_data *d = data;
+
+ *err = 0;
+ switch (move) {
+ case MOVE_DOWN:
+ /* only AND and OR have children */
+ if (pred->left != FILTER_PRED_INVALID) {
+ /* If ops is set, then it was folded. */
+ if (!pred->ops)
+ return WALK_PRED_DEFAULT;
+ /* We can treat folded ops as a leaf node */
+ d->match = process_ops(d->preds, pred, d->rec);
+ } else {
+ if (!WARN_ON_ONCE(!pred->fn))
+ d->match = pred->fn(pred, d->rec);
+ }
+
+ return WALK_PRED_PARENT;
+ case MOVE_UP_FROM_LEFT:
+ /*
+ * Check for short circuits.
+ *
+ * Optimization: !!match == (pred->op == OP_OR)
+ * is the same as:
+ * if ((match && pred->op == OP_OR) ||
+ * (!match && pred->op == OP_AND))
+ */
+ if (!!d->match == (pred->op == OP_OR))
+ return WALK_PRED_PARENT;
+ break;
+ case MOVE_UP_FROM_RIGHT:
+ break;
+ }
+
+ return WALK_PRED_DEFAULT;
+}
+
/* return 1 if event matches, 0 otherwise (discard) */
int filter_match_preds(struct event_filter *filter, void *rec)
{
- int match = -1;
- enum move_type move = MOVE_DOWN;
struct filter_pred *preds;
- struct filter_pred *pred;
struct filter_pred *root;
- int n_preds;
- int done = 0;
+ struct filter_match_preds_data data = {
+ /* match is currently meaningless */
+ .match = -1,
+ .rec = rec,
+ };
+ int n_preds, ret;
/* no filter is considered a match */
if (!filter)
return 1;
n_preds = filter->n_preds;
-
if (!n_preds)
return 1;
/*
* n_preds, root and filter->preds are protect with preemption disabled.
*/
- preds = rcu_dereference_sched(filter->preds);
root = rcu_dereference_sched(filter->root);
if (!root)
return 1;
- pred = root;
-
- /* match is currently meaningless */
- match = -1;
-
- do {
- switch (move) {
- case MOVE_DOWN:
- /* only AND and OR have children */
- if (pred->left != FILTER_PRED_INVALID) {
- /* If ops is set, then it was folded. */
- if (!pred->ops) {
- /* keep going to down the left side */
- pred = &preds[pred->left];
- continue;
- }
- /* We can treat folded ops as a leaf node */
- match = process_ops(preds, pred, rec);
- } else
- match = pred->fn(pred, rec);
- /* If this pred is the only pred */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- case MOVE_UP_FROM_LEFT:
- /*
- * Check for short circuits.
- *
- * Optimization: !!match == (pred->op == OP_OR)
- * is the same as:
- * if ((match && pred->op == OP_OR) ||
- * (!match && pred->op == OP_AND))
- */
- if (!!match == (pred->op == OP_OR)) {
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- /* now go down the right side of the tree. */
- pred = &preds[pred->right];
- move = MOVE_DOWN;
- continue;
- case MOVE_UP_FROM_RIGHT:
- /* We finished this equation. */
- if (pred == root)
- break;
- pred = get_pred_parent(pred, preds,
- pred->parent, &move);
- continue;
- }
- done = 1;
- } while (!done);
-
- return match;
+ data.preds = preds = rcu_dereference_sched(filter->preds);
+ ret = walk_pred_tree(preds, root, filter_match_preds_cb, &data);
+ WARN_ON(ret);
+ return data.match;
}
EXPORT_SYMBOL_GPL(filter_match_preds);
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [PATCHv2 10/10] tracing/filter: Add startup tests for events filter
2011-08-11 14:25 ` [PATCHv2 " Jiri Olsa
` (8 preceding siblings ...)
2011-08-11 14:25 ` [PATCHv2 09/10] tracing/filter: Change filter_match_preds " Jiri Olsa
@ 2011-08-11 14:25 ` Jiri Olsa
9 siblings, 0 replies; 30+ messages in thread
From: Jiri Olsa @ 2011-08-11 14:25 UTC (permalink / raw)
To: rostedt, fweisbec, mingo; +Cc: linux-kernel, Jiri Olsa
Adding automated tests running as late_initcall. Tests are
compiled in with CONFIG_FTRACE_STARTUP_TEST option.
Adding test event "ftrace_test_filter" used to simulate
filter processing during event occurance.
String filters are compiled and tested against several
test events with different values.
Also testing that evaluation of explicit predicates is ommited
due to the lazy filter evaluation.
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
kernel/trace/Makefile | 2 +
kernel/trace/trace.h | 3 +
kernel/trace/trace_events_filter.c | 209 +++++++++++++++++++++++++++++++
kernel/trace/trace_events_filter_test.h | 50 ++++++++
4 files changed, 264 insertions(+), 0 deletions(-)
create mode 100644 kernel/trace/trace_events_filter_test.h
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 761c510..b384ed5 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -15,6 +15,8 @@ ifdef CONFIG_TRACING_BRANCHES
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
endif
+CFLAGS_trace_events_filter.o := -I$(src)
+
#
# Make the trace clocks available generally: it's infrastructure
# relied on by ptrace for example:
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2eb3cf6..4c7540a 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -762,6 +762,9 @@ struct filter_pred {
u64 val;
struct regex regex;
unsigned short *ops;
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ struct ftrace_event_field *field;
+#endif
int offset;
int not;
int op;
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 319c3ca..6a642e2 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1329,6 +1329,9 @@ static struct filter_pred *create_pred(struct filter_parse_state *ps,
strcpy(pred.regex.pattern, operand2);
pred.regex.len = strlen(pred.regex.pattern);
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+ pred.field = field;
+#endif
return init_pred(ps, field, &pred) ? NULL : &pred;
}
@@ -1926,3 +1929,209 @@ out_unlock:
#endif /* CONFIG_PERF_EVENTS */
+#ifdef CONFIG_FTRACE_STARTUP_TEST
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+#define CREATE_TRACE_POINTS
+#include "trace_events_filter_test.h"
+
+static int test_get_filter(char *filter_str, struct ftrace_event_call *call,
+ struct event_filter **pfilter)
+{
+ struct event_filter *filter;
+ struct filter_parse_state *ps;
+ int err = -ENOMEM;
+
+ filter = __alloc_filter();
+ if (!filter)
+ goto out;
+
+ ps = kzalloc(sizeof(*ps), GFP_KERNEL);
+ if (!ps)
+ goto free_filter;
+
+ parse_init(ps, filter_ops, filter_str);
+ err = filter_parse(ps);
+ if (err)
+ goto free_ps;
+
+ err = replace_preds(call, filter, ps, filter_str, false);
+ if (!err)
+ *pfilter = filter;
+
+ free_ps:
+ filter_opstack_clear(ps);
+ postfix_clear(ps);
+ kfree(ps);
+
+ free_filter:
+ if (err)
+ __free_filter(filter);
+
+ out:
+ return err;
+}
+
+#define DATA_REC(m, va, vb, vc, vd, ve, vf, vg, vh, nvisit) \
+{ \
+ .filter = FILTER, \
+ .rec = { .a = va, .b = vb, .c = vc, .d = vd, \
+ .e = ve, .f = vf, .g = vg, .h = vh }, \
+ .match = m, \
+ .not_visited = nvisit, \
+}
+#define YES 1
+#define NO 0
+
+static struct test_filter_data_t {
+ char *filter;
+ struct ftrace_raw_ftrace_test_filter rec;
+ int match;
+ char *not_visited;
+} test_filter_data[] = {
+#define FILTER "a == 1 && b == 1 && c == 1 && d == 1 && " \
+ "e == 1 && f == 1 && g == 1 && h == 1"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, ""),
+ DATA_REC(NO, 0, 1, 1, 1, 1, 1, 1, 1, "bcdefgh"),
+ DATA_REC(NO, 1, 1, 1, 1, 1, 1, 1, 0, ""),
+#undef FILTER
+#define FILTER "a == 1 || b == 1 || c == 1 || d == 1 || " \
+ "e == 1 || f == 1 || g == 1 || h == 1"
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 0, 0, 0, 0, 0, 0, "bcdefgh"),
+#undef FILTER
+#define FILTER "(a == 1 || b == 1) && (c == 1 || d == 1) && " \
+ "(e == 1 || f == 1) && (g == 1 || h == 1)"
+ DATA_REC(NO, 0, 0, 1, 1, 1, 1, 1, 1, "dfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 0, 1, 0, 1, "bd"),
+ DATA_REC(NO, 1, 0, 1, 0, 0, 1, 0, 0, "bd"),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) || (c == 1 && d == 1) || " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 0, 1, 1, 1, 1, 1, 1, "efgh"),
+ DATA_REC(YES, 0, 0, 0, 0, 0, 0, 1, 1, ""),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+#undef FILTER
+#define FILTER "(a == 1 && b == 1) && (c == 1 && d == 1) && " \
+ "(e == 1 && f == 1) || (g == 1 && h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 0, "gh"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 1, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, ""),
+#undef FILTER
+#define FILTER "((a == 1 || b == 1) || (c == 1 || d == 1) || " \
+ "(e == 1 || f == 1)) && (g == 1 || h == 1)"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 0, 1, "bcdef"),
+ DATA_REC(NO, 0, 0, 0, 0, 0, 0, 0, 0, ""),
+ DATA_REC(YES, 1, 1, 1, 1, 1, 0, 1, 1, "h"),
+#undef FILTER
+#define FILTER "((((((((a == 1) && (b == 1)) || (c == 1)) && (d == 1)) || " \
+ "(e == 1)) && (f == 1)) || (g == 1)) && (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "ceg"),
+ DATA_REC(NO, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(NO, 1, 0, 1, 0, 1, 0, 1, 0, ""),
+#undef FILTER
+#define FILTER "((((((((a == 1) || (b == 1)) && (c == 1)) || (d == 1)) && " \
+ "(e == 1)) || (f == 1)) && (g == 1)) || (h == 1))"
+ DATA_REC(YES, 1, 1, 1, 1, 1, 1, 1, 1, "bdfh"),
+ DATA_REC(YES, 0, 1, 0, 1, 0, 1, 0, 1, ""),
+ DATA_REC(YES, 1, 0, 1, 0, 1, 0, 1, 0, "bdfh"),
+};
+
+#undef DATA_REC
+#undef FILTER
+#undef YES
+#undef NO
+
+#define DATA_CNT (sizeof(test_filter_data)/sizeof(struct test_filter_data_t))
+
+static int test_pred_visited;
+
+static int test_pred_visited_fn(struct filter_pred *pred, void *event)
+{
+ struct ftrace_event_field *field = pred->field;
+
+ test_pred_visited = 1;
+ printk(KERN_INFO "\npred visited %s\n", field->name);
+ return 1;
+}
+
+static int test_walk_pred_cb(enum move_type move, struct filter_pred *pred,
+ int *err, void *data)
+{
+ char *fields = data;
+
+ if ((move == MOVE_DOWN) &&
+ (pred->left == FILTER_PRED_INVALID)) {
+ struct ftrace_event_field *field = pred->field;
+
+ if (!field) {
+ WARN(1, "all leafs should have field defined");
+ return WALK_PRED_DEFAULT;
+ }
+ if (!strchr(fields, *field->name))
+ return WALK_PRED_DEFAULT;
+
+ WARN_ON(!pred->fn);
+ pred->fn = test_pred_visited_fn;
+ }
+ return WALK_PRED_DEFAULT;
+}
+
+static __init int ftrace_test_event_filter(void)
+{
+ int i;
+
+ printk(KERN_INFO "Testing ftrace filter: ");
+
+ for (i = 0; i < DATA_CNT; i++) {
+ struct event_filter *filter = NULL;
+ struct test_filter_data_t *d = &test_filter_data[i];
+ int err;
+
+ err = test_get_filter(d->filter, &event_ftrace_test_filter,
+ &filter);
+ if (err) {
+ printk(KERN_INFO
+ "Failed to get filter for '%s', err %d\n",
+ d->filter, err);
+ break;
+ }
+
+ if (*d->not_visited)
+ walk_pred_tree(filter->preds, filter->root,
+ test_walk_pred_cb,
+ d->not_visited);
+
+ test_pred_visited = 0;
+ err = filter_match_preds(filter, &d->rec);
+
+ __free_filter(filter);
+
+ if (test_pred_visited) {
+ printk(KERN_INFO
+ "Failed, unwanted pred visited for filter %s\n",
+ d->filter);
+ break;
+ }
+
+ if (err != d->match) {
+ printk(KERN_INFO
+ "Failed to match filter '%s', expected %d\n",
+ d->filter, d->match);
+ break;
+ }
+ }
+
+ if (i == DATA_CNT)
+ printk(KERN_CONT "OK\n");
+
+ return 0;
+}
+
+late_initcall(ftrace_test_event_filter);
+
+#endif /* CONFIG_FTRACE_STARTUP_TEST */
diff --git a/kernel/trace/trace_events_filter_test.h b/kernel/trace/trace_events_filter_test.h
new file mode 100644
index 0000000..bfd4dba
--- /dev/null
+++ b/kernel/trace/trace_events_filter_test.h
@@ -0,0 +1,50 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM test
+
+#if !defined(_TRACE_TEST_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEST_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(ftrace_test_filter,
+
+ TP_PROTO(int a, int b, int c, int d, int e, int f, int g, int h),
+
+ TP_ARGS(a, b, c, d, e, f, g, h),
+
+ TP_STRUCT__entry(
+ __field(int, a)
+ __field(int, b)
+ __field(int, c)
+ __field(int, d)
+ __field(int, e)
+ __field(int, f)
+ __field(int, g)
+ __field(int, h)
+ ),
+
+ TP_fast_assign(
+ __entry->a = a;
+ __entry->b = b;
+ __entry->c = c;
+ __entry->d = d;
+ __entry->e = e;
+ __entry->f = f;
+ __entry->g = g;
+ __entry->h = h;
+ ),
+
+ TP_printk("a %d, b %d, c %d, d %d, e %d, f %d, g %d, h %d",
+ __entry->a, __entry->b, __entry->c, __entry->d,
+ __entry->e, __entry->f, __entry->g, __entry->h)
+);
+
+#endif /* _TRACE_TEST_H || TRACE_HEADER_MULTI_READ */
+
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_PATH .
+#define TRACE_INCLUDE_FILE trace_events_filter_test
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
1.7.1
^ permalink raw reply [flat|nested] 30+ messages in thread