Messages in this thread |  | | Subject | Re: [PATCH 06/10] tracing/filter: Change count_leafs function to use walk_pred_tree | | From | Steven Rostedt <> | | Date | Wed, 10 Aug 2011 17:05:58 -0400 |
| |
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; > } >
|  |