tracing: queued updates

GIT PULL New Steven Rostedt 4 participants 30 messages Oct 10, 2011 – Oct 12, 2011

Subsystems (7)

x86 Tracing ftrace Module support +3 more subsystems

lore

Share & tools

lore

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #0 lore raw

Ingo,

With kernel.org down, I've queued up quite a lot of patches for 3.2. As the list is slowly growing, I would like to get these into tip so we can focus on the new patches.

I removed the trace_printk() patches as Peter and I are still discussing how they should work. But the rest of the patches are good to go.

-- Steve

Please pull the latest tip/perf/core tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/core

Head SHA1: 87912fdfc1378a7e7e54a169ef9eebeafa1af04f

Jiri Olsa (10):
tracing/filter: Use static allocation for filter predicates tracing/filter: Separate predicate init and filter addition tracing/filter: Remove field_name from filter_pred struct tracing/filter: Simplify tracepoint event lookup
tracing/filter: Unify predicate tree walking, change check_pred_tree function to use it tracing/filter: Change count_leafs function to use walk_pred_tree tracing/filter: Change fold_pred_tree function to use walk_pred_tree tracing/filter: Change fold_pred function to use walk_pred_tree tracing/filter: Change filter_match_preds function to use walk_pred_tree tracing/filter: Add startup tests for events filter

Kevin Winchester (1):
x86: jump_label: arch_jump_label_text_poke_early: add missing __init

Mathieu Desnoyers (1):
Tracepoint: Dissociate from module mutex

Steven Rostedt (6):
tracing: Clean up tb_fmt to not give faulty compile warning
tracing: Add preempt disable for filter self test
tracing: Add a counter clock for those that do not trust clocks
tracing: Fix preemptirqsoff tracer to not stop at preempt off
tracing: Warn on output if the function tracer was found corrupted
tracing: Do not allocate buffer for trace_marker

Vaibhav Nagarnaik (2):
trace: Add a new readonly entry to report total buffer size
trace: Add ring buffer stats to measure rate of events

----

 arch/x86/kernel/jump_label.c            |    2 +-
 include/linux/module.h                  |   12 -
 include/linux/ring_buffer.h             |    2 +
 include/linux/trace_clock.h             |    1 +
 include/linux/tracepoint.h              |   25 +-
 kernel/module.c                         |   47 --
 kernel/trace/Makefile                   |    2 +
 kernel/trace/ftrace.c                   |    8 +
 kernel/trace/ring_buffer.c              |   70 +++-
 kernel/trace/trace.c                    |  173 ++++++--
 kernel/trace/trace.h                    |   16 +-
 kernel/trace/trace_clock.c              |   12 +
 kernel/trace/trace_events_filter.c      |  795 ++++++++++++++++++-------------
 kernel/trace/trace_events_filter_test.h |   50 ++
 kernel/trace/trace_irqsoff.c            |    4 +-
 kernel/trace/trace_printk.c             |   19 +-
 kernel/tracepoint.c                     |  169 ++++++-
 17 files changed, 928 insertions(+), 479 deletions(-)

Steven Rostedt

author maintainer

2011-10-10 13:44 UTC #1 lore raw

PATCH 1/20 tracing: Clean up tb_fmt to not give faulty compile warning

From: Steven Rostedt <[email protected]>

gcc incorrectly states that the variable "fmt" is uninitialized when CC_OPITMIZE_FOR_SIZE is set.

Instead of just blindly setting fmt to NULL, the code is cleaned up a little to be a bit easier for humans to follow, as well as gcc to know the variables are initialized.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_printk.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

kernel/trace/trace_printk.c +10 -9

index 1f06468..6fd4ffd 100644
@@ -59,18 +59,19 @@ void hold_module_trace_bprintk_format(const char **start, const char **end)
 			continue;
 		}
 
+		fmt = NULL;
 		tb_fmt = kmalloc(sizeof(*tb_fmt), GFP_KERNEL);
-		if (tb_fmt)
+		if (tb_fmt) {
 			fmt = kmalloc(strlen(*iter) + 1, GFP_KERNEL);
-		if (tb_fmt && fmt) {
-			list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
-			strcpy(fmt, *iter);
-			tb_fmt->fmt = fmt;
-			*iter = tb_fmt->fmt;
-		} else {
-			kfree(tb_fmt);
-			*iter = NULL;
+			if (fmt) {
+				list_add_tail(&tb_fmt->list, &trace_bprintk_fmt_list);
+				strcpy(fmt, *iter);
+				tb_fmt->fmt = fmt;
+			} else
+				kfree(tb_fmt);
 		}
+		*iter = fmt;
+
 	}
 	mutex_unlock(&btrace_mutex);
 }

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:46 UTC #2 lore raw

PATCH 2/20 Tracepoint: Dissociate from module mutex

In reply to Steven Rostedt

From: Mathieu Desnoyers <[email protected]>

Copy the information needed from struct module into a local module list held within tracepoint.c from within the module coming/going notifier.

This vastly simplifies locking of tracepoint registration / unregistration, because we don't have to take the module mutex to register and unregister tracepoints anymore. Steven Rostedt ran into dependency problems related to modules mutex vs kprobes mutex vs ftrace mutex vs tracepoint mutex that seems to be hard to fix without removing this dependency between tracepoint and module mutex. (note: it should be investigated whether kprobes could benefit of being dissociated from the modules mutex too.)

This also fixes module handling of tracepoint list iterators, because it was expecting the list to be sorted by pointer address. Given we have control on our own list now, it's OK to sort this list which has tracepoints as its only purpose. The reason why this sorting is required is to handle the fact that seq files (and any read() operation from user-space) cannot hold the tracepoint mutex across multiple calls, so list entries may vanish between calls. With sorting, the tracepoint iterator becomes usable even if the list don't contain the exact item pointed to by the iterator anymore.

Signed-off-by Mathieu Desnoyers <[email protected]>

Acked-by Jason Baron <[email protected]>

CC Ingo Molnar <[email protected]>

CC Lai Jiangshan <[email protected]>

CC Peter Zijlstra <[email protected]>

CC Thomas Gleixner <[email protected]>

CC Masami Hiramatsu <[email protected]>

Link http://lkml.kernel.org/r/20110810191839.GC8525@Krystal open here

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/module.h     |   12 ---
 include/linux/tracepoint.h |   25 +++----
 kernel/module.c            |   47 ------------
 kernel/tracepoint.c        |  169 ++++++++++++++++++++++++++++++++++++++------
 4 files changed, 158 insertions(+), 95 deletions(-)

include/linux/module.h +0 ~~-12~~

index 1c30087..8639216 100644
@@ -580,9 +580,6 @@ int unregister_module_notifier(struct notifier_block * nb);
 
 extern void print_modules(void);
 
-extern void module_update_tracepoints(void);
-extern int module_get_iter_tracepoints(struct tracepoint_iter *iter);
-
 #else /* !CONFIG_MODULES... */
 #define EXPORT_SYMBOL(sym)
 #define EXPORT_SYMBOL_GPL(sym)

@@ -698,15 +695,6 @@ static inline int unregister_module_notifier(struct notifier_block * nb)
 static inline void print_modules(void)
 {
 }
-
-static inline void module_update_tracepoints(void)
-{
-}
-
-static inline int module_get_iter_tracepoints(struct tracepoint_iter *iter)
-{
-	return 0;
-}
 #endif /* CONFIG_MODULES */
 
 #ifdef CONFIG_SYSFS

include/linux/tracepoint.h +11 ~~-14~~

index d530a44..df0a779 100644
@@ -54,8 +54,18 @@ extern int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
 						void *data);
 extern void tracepoint_probe_update_all(void);
 
+#ifdef CONFIG_MODULES
+struct tp_module {
+	struct list_head list;
+	unsigned int num_tracepoints;
+	struct tracepoint * const *tracepoints_ptrs;
+};
+#endif /* CONFIG_MODULES */
+
 struct tracepoint_iter {
-	struct module *module;
+#ifdef CONFIG_MODULES
+	struct tp_module *module;
+#endif /* CONFIG_MODULES */
 	struct tracepoint * const *tracepoint;
 };

@@ -63,8 +73,6 @@ extern void tracepoint_iter_start(struct tracepoint_iter *iter);
 extern void tracepoint_iter_next(struct tracepoint_iter *iter);
 extern void tracepoint_iter_stop(struct tracepoint_iter *iter);
 extern void tracepoint_iter_reset(struct tracepoint_iter *iter);
-extern int tracepoint_get_iter_range(struct tracepoint * const **tracepoint,
-	struct tracepoint * const *begin, struct tracepoint * const *end);
 
 /*
  * tracepoint_synchronize_unregister must be called between the last tracepoint

@@ -78,17 +86,6 @@ static inline void tracepoint_synchronize_unregister(void)
 
 #define PARAMS(args...) args
 
-#ifdef CONFIG_TRACEPOINTS
-extern
-void tracepoint_update_probe_range(struct tracepoint * const *begin,
-	struct tracepoint * const *end);
-#else
-static inline
-void tracepoint_update_probe_range(struct tracepoint * const *begin,
-	struct tracepoint * const *end)
-{ }
-#endif /* CONFIG_TRACEPOINTS */
-
 #endif /* _LINUX_TRACEPOINT_H */
 
 /*

kernel/module.c +0 ~~-47~~

50 lines not on this page · raw message

kernel/tracepoint.c +147 ~~-22~~

279 lines not on this page · raw message

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:46 UTC #3 lore raw

PATCH 3/20 x86: jump_label: arch_jump_label_text_poke_early: add missing __init

In reply to Steven Rostedt

From: Kevin Winchester <[email protected]>

arch_jump_label_text_poke_early calls text_poke_early, which is an __init function. Thus arch_jump_label_text_poke_early should be the same.

Signed-off-by Kevin Winchester <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

[ Use __init_or_module instead of __init ]

Signed-off-by Steven Rostedt <[email protected]>

---
 arch/x86/kernel/jump_label.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kernel/jump_label.c +1 -1

index 3fee346..cacdd46 100644
@@ -42,7 +42,7 @@ void arch_jump_label_transform(struct jump_entry *entry,
 	put_online_cpus();
 }
 
-void arch_jump_label_text_poke_early(jump_label_t addr)
+void __init_or_module arch_jump_label_text_poke_early(jump_label_t addr)
 {
 	text_poke_early((void *)addr, ideal_nops[NOP_ATOMIC5],
 			JUMP_LABEL_NOP_SIZE);

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:45 UTC #4 lore raw

PATCH 4/20 tracing/filter: Use static allocation for filter predicates

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |   57 ++++++++++--------------------------
 1 files changed, 16 insertions(+), 41 deletions(-)

kernel/trace/trace_events_filter.c +16 ~~-41~~

index 256764e..cb295a1 100644
@@ -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;

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #5 lore raw

PATCH 5/20 tracing/filter: Separate predicate init and filter addition

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |   56 +++++++++++++++---------------------
 1 files changed, 23 insertions(+), 33 deletions(-)

kernel/trace/trace_events_filter.c +23 ~~-33~~

index cb295a1..61c8dec 100644
@@ -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;
 	}

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #6 lore raw

PATCH 6/20 tracing/filter: Remove field_name from filter_pred struct

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace.h               |   11 +-------
 kernel/trace/trace_events_filter.c |   53 ++++++++---------------------------
 2 files changed, 13 insertions(+), 51 deletions(-)

kernel/trace/trace.h +1 ~~-10~~

index 616846b..2eb3cf6 100644
@@ -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;

kernel/trace/trace_events_filter.c +12 ~~-41~~

127 lines not on this page · raw message

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #7 lore raw

PATCH 7/20 tracing/filter: Simplify tracepoint event lookup

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

kernel/trace/trace_events_filter.c +3 -6

index 97b93f3..0948905 100644
@@ -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;

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:45 UTC #8 lore raw

PATCH 8/20 tracing/filter: Unify predicate tree walking, change check_pred_tree

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |  137 ++++++++++++++++++++++-------------
 1 files changed, 86 insertions(+), 51 deletions(-)

kernel/trace/trace_events_filter.c +86 ~~-51~~

index 0948905..5b889d4 100644
@@ -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)

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:44 UTC #9 lore raw

PATCH 9/20 tracing/filter: Change count_leafs function to use walk_pred_tree

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

Changing count_leafs function to use unified predicates tree processing.

Signed-off-by Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |   47 ++++++++++-------------------------
 1 files changed, 14 insertions(+), 33 deletions(-)

kernel/trace/trace_events_filter.c +14 ~~-33~~

index 5b889d4..ebbb261 100644
@@ -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;
 }

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:44 UTC #10 lore raw

PATCH 10/20 tracing/filter: Change fold_pred_tree function to use walk_pred_tree

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

Changing fold_pred_tree function to use unified predicates tree processing.

Signed-off-by Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |   65 +++++++++++-------------------------
 1 files changed, 20 insertions(+), 45 deletions(-)

kernel/trace/trace_events_filter.c +20 ~~-45~~

index ebbb261..d8aa100 100644
@@ -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,

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:43 UTC #11 lore raw

PATCH 11/20 tracing/filter: Change fold_pred function to use walk_pred_tree

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

Changing fold_pred_tree function to use unified predicates tree processing.

Signed-off-by Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |   68 +++++++++++++++++------------------
 1 files changed, 33 insertions(+), 35 deletions(-)

kernel/trace/trace_events_filter.c +33 ~~-35~~

index d8aa100..f44e68b 100644
@@ -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,

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:44 UTC #12 lore raw

PATCH 12/20 tracing/filter: Change filter_match_preds function to use

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

Changing filter_match_preds function to use unified predicates tree processing.

Signed-off-by Jiri Olsa <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |  124 +++++++++++++++++-------------------
 1 files changed, 58 insertions(+), 66 deletions(-)

kernel/trace/trace_events_filter.c +58 ~~-66~~

index f44e68b..319c3ca 100644
@@ -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);

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #13 lore raw

PATCH 13/20 tracing/filter: Add startup tests for events filter

In reply to Steven Rostedt

From: Jiri Olsa <[email protected]>

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 <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 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

kernel/trace/Makefile +2 -0

index 761c510..b384ed5 100644
@@ -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:

kernel/trace/trace.h +3 -0

index 2eb3cf6..4c7540a 100644
@@ -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;

kernel/trace/trace_events_filter.c +209 -0

218 lines not on this page · raw message

kernel/trace/trace_events_filter_test.h new file +50 -0

50 lines not on this page · raw message

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:41 UTC #14 lore raw

PATCH 14/20 tracing: Add preempt disable for filter self test

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The self testing for event filters does not really need preemption disabled as there are no races at the time of testing, but the functions it calls uses rcu_dereference_sched() which will complain if preemption is not disabled.

Cc Jiri Olsa <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_events_filter.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

kernel/trace/trace_events_filter.c +6 -0

index 6a642e2..816d3d0 100644
@@ -2101,6 +2101,11 @@ static __init int ftrace_test_event_filter(void)
 			break;
 		}
 
+		/*
+		 * The preemption disabling is not really needed for self
+		 * tests, but the rcu dereference will complain without it.
+		 */
+		preempt_disable();
 		if (*d->not_visited)
 			walk_pred_tree(filter->preds, filter->root,
 				       test_walk_pred_cb,

@@ -2108,6 +2113,7 @@ static __init int ftrace_test_event_filter(void)
 
 		test_pred_visited = 0;
 		err = filter_match_preds(filter, &d->rec);
+		preempt_enable();
 
 		__free_filter(filter);

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:43 UTC #15 lore raw

PATCH 15/20 trace: Add a new readonly entry to report total buffer size

In reply to Steven Rostedt

From: Vaibhav Nagarnaik <[email protected]>

The current file "buffer_size_kb" reports the size of per-cpu buffer and not the overall memory allocated which could be misleading. A new file "buffer_total_size_kb" adds up all the enabled CPU buffer sizes and reports it. This is only a readonly entry.

Signed-off-by Vaibhav Nagarnaik <[email protected]>

Cc Michael Rubin <[email protected]>

Cc David Sharp <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

kernel/trace/trace.c +33 -0

index e5df02c..0117678 100644
@@ -3569,6 +3569,30 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
 }
 
 static ssize_t
+tracing_total_entries_read(struct file *filp, char __user *ubuf,
+				size_t cnt, loff_t *ppos)
+{
+	struct trace_array *tr = filp->private_data;
+	char buf[64];
+	int r, cpu;
+	unsigned long size = 0, expanded_size = 0;
+
+	mutex_lock(&trace_types_lock);
+	for_each_tracing_cpu(cpu) {
+		size += tr->entries >> 10;
+		if (!ring_buffer_expanded)
+			expanded_size += trace_buf_size >> 10;
+	}
+	if (ring_buffer_expanded)
+		r = sprintf(buf, "%lu\n", size);
+	else
+		r = sprintf(buf, "%lu (expanded: %lu)\n", size, expanded_size);
+	mutex_unlock(&trace_types_lock);
+
+	return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
+}
+
+static ssize_t
 tracing_free_buffer_write(struct file *filp, const char __user *ubuf,
 			  size_t cnt, loff_t *ppos)
 {

@@ -3739,6 +3763,12 @@ static const struct file_operations tracing_entries_fops = {
 	.llseek		= generic_file_llseek,
 };
 
+static const struct file_operations tracing_total_entries_fops = {
+	.open		= tracing_open_generic,
+	.read		= tracing_total_entries_read,
+	.llseek		= generic_file_llseek,
+};
+
 static const struct file_operations tracing_free_buffer_fops = {
 	.write		= tracing_free_buffer_write,
 	.release	= tracing_free_buffer_release,

@@ -4450,6 +4480,9 @@ static __init int tracer_init_debugfs(void)
 	trace_create_file("buffer_size_kb", 0644, d_tracer,
 			&global_trace, &tracing_entries_fops);
 
+	trace_create_file("buffer_total_size_kb", 0444, d_tracer,
+			&global_trace, &tracing_total_entries_fops);
+
 	trace_create_file("free_buffer", 0644, d_tracer,
 			&global_trace, &tracing_free_buffer_fops);

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:43 UTC #16 lore raw

PATCH 16/20 trace: Add ring buffer stats to measure rate of events

In reply to Steven Rostedt

From: Vaibhav Nagarnaik <[email protected]>

The stats file under per_cpu folder provides the number of entries, overruns and other statistics about the CPU ring buffer. However, the numbers do not provide any indication of how full the ring buffer is in bytes compared to the overall size in bytes. Also, it is helpful to know the rate at which the cpu buffer is filling up.

This patch adds an entry "bytes: " in printed stats for per_cpu ring buffer which provides the actual bytes consumed in the ring buffer. This field includes the number of bytes used by recorded events and the padding bytes added when moving the tail pointer to next page.

It also adds the following time stamps:
"oldest event ts:" - the oldest timestamp in the ring buffer "now ts:" - the timestamp at the time of reading

The field "now ts" provides a consistent time snapshot to the userspace when being read. This is read from the same trace clock used by tracing event timestamps.

Together, these values provide the rate at which the buffer is filling up, from the formula:
bytes / (now_ts - oldest_event_ts)

Signed-off-by Vaibhav Nagarnaik <[email protected]>

Cc Michael Rubin <[email protected]>

Cc David Sharp <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/ring_buffer.h |    2 +
 kernel/trace/ring_buffer.c  |   70 ++++++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace.c        |   13 ++++++++
 3 files changed, 84 insertions(+), 1 deletions(-)

include/linux/ring_buffer.h +2 -0

index b891de9..67be037 100644
@@ -154,6 +154,8 @@ void ring_buffer_record_enable(struct ring_buffer *buffer);
 void ring_buffer_record_disable_cpu(struct ring_buffer *buffer, int cpu);
 void ring_buffer_record_enable_cpu(struct ring_buffer *buffer, int cpu);
 
+unsigned long ring_buffer_oldest_event_ts(struct ring_buffer *buffer, int cpu);
+unsigned long ring_buffer_bytes_cpu(struct ring_buffer *buffer, int cpu);
 unsigned long ring_buffer_entries(struct ring_buffer *buffer);
 unsigned long ring_buffer_overruns(struct ring_buffer *buffer);
 unsigned long ring_buffer_entries_cpu(struct ring_buffer *buffer, int cpu);

kernel/trace/ring_buffer.c +69 -1

136 lines not on this page · raw message

kernel/trace/trace.c +13 -0

index 0117678..b419070 100644
@@ -4056,6 +4056,8 @@ tracing_stats_read(struct file *filp, char __user *ubuf,
 	struct trace_array *tr = &global_trace;
 	struct trace_seq *s;
 	unsigned long cnt;
+	unsigned long long t;
+	unsigned long usec_rem;
 
 	s = kmalloc(sizeof(*s), GFP_KERNEL);
 	if (!s)

@@ -4072,6 +4074,17 @@ tracing_stats_read(struct file *filp, char __user *ubuf,
 	cnt = ring_buffer_commit_overrun_cpu(tr->buffer, cpu);
 	trace_seq_printf(s, "commit overrun: %ld\n", cnt);
 
+	cnt = ring_buffer_bytes_cpu(tr->buffer, cpu);
+	trace_seq_printf(s, "bytes: %ld\n", cnt);
+
+	t = ns2usecs(ring_buffer_oldest_event_ts(tr->buffer, cpu));
+	usec_rem = do_div(t, USEC_PER_SEC);
+	trace_seq_printf(s, "oldest event ts: %5llu.%06lu\n", t, usec_rem);
+
+	t = ns2usecs(ring_buffer_time_stamp(tr->buffer, cpu));
+	usec_rem = do_div(t, USEC_PER_SEC);
+	trace_seq_printf(s, "now ts: %5llu.%06lu\n", t, usec_rem);
+
 	count = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->len);
 
 	kfree(s);

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:42 UTC #17 lore raw

PATCH 17/20 tracing: Add a counter clock for those that do not trust clocks

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When debugging tight race conditions, it can be helpful to have a synchronized tracing method. Although in most cases the global clock provides this functionality, if timings is not the issue, it is more comforting to know that the order of events really happened in a precise order.

Instead of using a clock, add a "counter" that is simply an incrementing atomic 64bit counter that orders the events as they are perceived to happen.

The trace_clock_counter() is added from the attempt by Peter Zijlstra trying to convert the trace_clock_global() to it. I took Peter's counter code and made trace_clock_counter() instead, and added it to the choice of clocks. Just echo counter > /debug/tracing/trace_clock to activate it.

Requested-by Thomas Gleixner <[email protected]>

Requested-by Peter Zijlstra <[email protected]>

Reviewed-by Valdis Kletnieks <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 include/linux/trace_clock.h |    1 +
 kernel/trace/trace.c        |    1 +
 kernel/trace/trace_clock.c  |   12 ++++++++++++
 3 files changed, 14 insertions(+), 0 deletions(-)

include/linux/trace_clock.h +1 -0

index 7a81303..4eb4902 100644
@@ -15,5 +15,6 @@
 extern u64 notrace trace_clock_local(void);
 extern u64 notrace trace_clock(void);
 extern u64 notrace trace_clock_global(void);
+extern u64 notrace trace_clock_counter(void);
 
 #endif /* _LINUX_TRACE_CLOCK_H */

kernel/trace/trace.c +1 -0

index b419070..4b8df0d 100644
@@ -435,6 +435,7 @@ static struct {
 } trace_clocks[] = {
 	{ trace_clock_local,	"local" },
 	{ trace_clock_global,	"global" },
+	{ trace_clock_counter,	"counter" },
 };
 
 int trace_clock_id;

kernel/trace/trace_clock.c +12 -0

index 6302747..3947835 100644
@@ -113,3 +113,15 @@ u64 notrace trace_clock_global(void)
 
 	return now;
 }
+
+static atomic64_t trace_counter;
+
+/*
+ * trace_clock_counter(): simply an atomic counter.
+ * Use the trace_counter "counter" for cases where you do not care
+ * about timings, but are interested in strict ordering.
+ */
+u64 notrace trace_clock_counter(void)
+{
+	return atomic64_add_return(1, &trace_counter);
+}

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:42 UTC #18 lore raw

PATCH 18/20 tracing: Fix preemptirqsoff tracer to not stop at preempt off

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

If irqs are disabled when preemption count reaches zero, the preemptirqsoff tracer should not flag that as the end.

When interrupts are enabled and preemption count is not zero the preemptirqsoff correctly continues its tracing.

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace_irqsoff.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

kernel/trace/trace_irqsoff.c +2 -2

index 667aa8c..a1a3359 100644
@@ -505,13 +505,13 @@ EXPORT_SYMBOL(trace_hardirqs_off_caller);
 #ifdef CONFIG_PREEMPT_TRACER
 void trace_preempt_on(unsigned long a0, unsigned long a1)
 {
-	if (preempt_trace())
+	if (preempt_trace() && !irq_trace())
 		stop_critical_timing(a0, a1);
 }
 
 void trace_preempt_off(unsigned long a0, unsigned long a1)
 {
-	if (preempt_trace())
+	if (preempt_trace() && !irq_trace())
 		start_critical_timing(a0, a1);
 }
 #endif /* CONFIG_PREEMPT_TRACER */

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:42 UTC #19 lore raw

PATCH 19/20 tracing: Warn on output if the function tracer was found corrupted

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

As the function tracer is very intrusive, lots of self checks are performed on the tracer and if something is found to be strange it will shut itself down keeping it from corrupting the rest of the kernel. This shutdown may still allow functions to be traced, as the tracing only stops new modifications from happening. Trying to stop the function tracer itself can cause more harm as it requires code modification.

Although a WARN_ON() is executed, a user may not notice it. To help the user see that something isn't right with the tracing of the system a big warning is added to the output of the tracer that lets the user know that their data may be incomplete.

Reported-by Thomas Gleixner <[email protected]>

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/ftrace.c |    8 ++++++++
 kernel/trace/trace.c  |   15 +++++++++++++++
 kernel/trace/trace.h  |    2 ++
 3 files changed, 25 insertions(+), 0 deletions(-)

kernel/trace/ftrace.c +8 -0

index c3e4575..077d853 100644
@@ -3863,6 +3863,14 @@ void ftrace_kill(void)
 }
 
 /**
+ * Test if ftrace is dead or not.
+ */
+int ftrace_is_dead(void)
+{
+	return ftrace_disabled;
+}
+
+/**
  * register_ftrace_function - register a function for profiling
  * @ops - ops structure that holds the function for profiling.
  *

kernel/trace/trace.c +15 -0

index 4b8df0d..13f2b84 100644
@@ -2160,6 +2160,14 @@ void trace_default_header(struct seq_file *m)
 	}
 }
 
+static void test_ftrace_alive(struct seq_file *m)
+{
+	if (!ftrace_is_dead())
+		return;
+	seq_printf(m, "# WARNING: FUNCTION TRACING IS CORRUPTED\n");
+	seq_printf(m, "#          MAY BE MISSING FUNCTION EVENTS\n");
+}
+
 static int s_show(struct seq_file *m, void *v)
 {
 	struct trace_iterator *iter = v;

@@ -2169,6 +2177,7 @@ static int s_show(struct seq_file *m, void *v)
 		if (iter->tr) {
 			seq_printf(m, "# tracer: %s\n", iter->trace->name);
 			seq_puts(m, "#\n");
+			test_ftrace_alive(m);
 		}
 		if (iter->trace && iter->trace->print_header)
 			iter->trace->print_header(m);

@@ -4613,6 +4622,12 @@ __ftrace_dump(bool disable_tracing, enum ftrace_dump_mode oops_dump_mode)
 
 	tracing_off();
 
+	/* Did function tracer already get disabled? */
+	if (ftrace_is_dead()) {
+		printk("# WARNING: FUNCTION TRACING IS CORRUPTED\n");
+		printk("#          MAY BE MISSING FUNCTION EVENTS\n");
+	}
+
 	if (disable_tracing)
 		ftrace_kill();

kernel/trace/trace.h +2 -0

index 4c7540a..aa5d94a 100644
@@ -579,11 +579,13 @@ static inline int ftrace_trace_task(struct task_struct *task)
 
 	return test_tsk_trace_trace(task);
 }
+extern int ftrace_is_dead(void);
 #else
 static inline int ftrace_trace_task(struct task_struct *task)
 {
 	return 1;
 }
+extern static inline int ftrace_is_dead(void) { return 0; }
 #endif
 
 /*

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:42 UTC #20 lore raw

PATCH 20/20 tracing: Do not allocate buffer for trace_marker

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

When doing intense tracing, the kmalloc inside trace_marker can introduce side effects to what is being traced.

As trace_marker() is used by userspace to inject data into the kernel ring buffer, it needs to do so with the least amount of intrusion to the operations of the kernel or the user space application.

As the ring buffer is designed to write directly into the buffer without the need to make a temporary buffer, and userspace already went through the hassle of knowing how big the write will be, we can simply pin the userspace pages and write the data directly into the buffer. This improves the impact of tracing via trace_marker tremendously!

Thanks to Peter Zijlstra and Thomas Gleixner for pointing out the use of get_user_pages_fast() and kmap_atomic().

Signed-off-by Steven Rostedt <[email protected]>

---
 kernel/trace/trace.c |  111 +++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 83 insertions(+), 28 deletions(-)

kernel/trace/trace.c +83 ~~-28~~

index 13f2b84..f86efe9 100644
@@ -3628,22 +3628,24 @@ tracing_free_buffer_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-static int mark_printk(const char *fmt, ...)
-{
-	int ret;
-	va_list args;
-	va_start(args, fmt);
-	ret = trace_vprintk(0, fmt, args);
-	va_end(args);
-	return ret;
-}
-
 static ssize_t
 tracing_mark_write(struct file *filp, const char __user *ubuf,
 					size_t cnt, loff_t *fpos)
 {
-	char *buf;
-	size_t written;
+	unsigned long addr = (unsigned long)ubuf;
+	struct ring_buffer_event *event;
+	struct ring_buffer *buffer;
+	struct print_entry *entry;
+	unsigned long irq_flags;
+	struct page *pages[2];
+	int nr_pages = 1;
+	ssize_t written;
+	void *page1;
+	void *page2;
+	int offset;
+	int size;
+	int len;
+	int ret;
 
 	if (tracing_disabled)
 		return -EINVAL;

@@ -3651,28 +3653,81 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
 	if (cnt > TRACE_BUF_SIZE)
 		cnt = TRACE_BUF_SIZE;
 
-	buf = kmalloc(cnt + 2, GFP_KERNEL);
-	if (buf == NULL)
-		return -ENOMEM;
+	/*
+	 * Userspace is injecting traces into the kernel trace buffer.
+	 * We want to be as non intrusive as possible.
+	 * To do so, we do not want to allocate any special buffers
+	 * or take any locks, but instead write the userspace data
+	 * straight into the ring buffer.
+	 *
+	 * First we need to pin the userspace buffer into memory,
+	 * which, most likely it is, because it just referenced it.
+	 * But there's no guarantee that it is. By using get_user_pages_fast()
+	 * and kmap_atomic/kunmap_atomic() we can get access to the
+	 * pages directly. We then write the data directly into the
+	 * ring buffer.
+	 */
+	BUILD_BUG_ON(TRACE_BUF_SIZE >= PAGE_SIZE);
 
-	if (copy_from_user(buf, ubuf, cnt)) {
-		kfree(buf);
-		return -EFAULT;
+	/* check if we cross pages */
+	if ((addr & PAGE_MASK) != ((addr + cnt) & PAGE_MASK))
+		nr_pages = 2;
+
+	offset = addr & (PAGE_SIZE - 1);
+	addr &= PAGE_MASK;
+
+	ret = get_user_pages_fast(addr, nr_pages, 0, pages);
+	if (ret < nr_pages) {
+		while (--ret >= 0)
+			put_page(pages[ret]);
+		written = -EFAULT;
+		goto out;
 	}
-	if (buf[cnt-1] != '\n') {
-		buf[cnt] = '\n';
-		buf[cnt+1] = '\0';
+
+	page1 = kmap_atomic(pages[0]);
+	if (nr_pages == 2)
+		page2 = kmap_atomic(pages[1]);
+
+	local_save_flags(irq_flags);
+	size = sizeof(*entry) + cnt + 2; /* possible \n added */
+	buffer = global_trace.buffer;
+	event = trace_buffer_lock_reserve(buffer, TRACE_PRINT, size,
+					  irq_flags, preempt_count());
+	if (!event) {
+		/* Ring buffer disabled, return as if not open for write */
+		written = -EBADF;
+		goto out_unlock;
+	}
+
+	entry = ring_buffer_event_data(event);
+	entry->ip = _THIS_IP_;
+
+	if (nr_pages == 2) {
+		len = PAGE_SIZE - offset;
+		memcpy(&entry->buf, page1 + offset, len);
+		memcpy(&entry->buf[len], page2, cnt - len);
 	} else
-		buf[cnt] = '\0';
+		memcpy(&entry->buf, page1 + offset, cnt);
 
-	written = mark_printk("%s", buf);
-	kfree(buf);
-	*fpos += written;
+	if (entry->buf[cnt - 1] != '\n') {
+		entry->buf[cnt] = '\n';
+		entry->buf[cnt + 1] = '\0';
+	} else
+		entry->buf[cnt] = '\0';
+
+	ring_buffer_unlock_commit(buffer, event);
 
-	/* don't tell userspace we wrote more - it might confuse them */
-	if (written > cnt)
-		written = cnt;
+	written = cnt;
 
+	*fpos += written;
+
+ out_unlock:
+	if (nr_pages == 2)
+		kunmap_atomic(page2);
+	kunmap_atomic(page1);
+	while (nr_pages > 0)
+		put_page(pages[--nr_pages]);
+ out:
 	return written;
 }

Signature · 1 line

1.7.6.3

Steven Rostedt

author maintainer

2011-10-10 13:52 UTC #21 lore raw

In reply to Steven Rostedt

Quoted 3 lines · Steven Rostedt

Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

Oops, I need to update my scripts. The new path is:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git

-- Steve

Quoted 2 lines

tip/perf/core

Ingo Molnar

2011-10-11 05:52 UTC #22 lore raw

Quoted 9 lines · Steven Rostedt

Quoted 3 lines · Steven Rostedt
Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
Oops, I need to update my scripts. The new path is:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git

Ok - this one worked.

Thanks,

Ingo

Ingo Molnar

2011-10-11 08:16 UTC #23 lore raw

Quoted 7 lines · Ingo Molnar

Quoted 4 lines · Steven Rostedt
Quoted 2 lines · Steven Rostedt
Please pull the latest tip/perf/core tree, which can be found at:

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
Oops, I need to update my scripts. The new path is:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
Ok - this one worked.

but it has not been build tested with the function tracer disabled:

kernel/trace/trace.h:588:1: error: multiple storage classes in declaration specifiers

Thanks,

Ingo

Steven Rostedt

author maintainer

2011-10-11 11:14 UTC #24 lore raw

Quoted 3 lines · Ingo Molnar

but it has not been build tested with the function tracer disabled:
kernel/trace/trace.h:588:1: error: multiple storage classes in declaration specifiers

Hmm, I did run randconfig on it a couple of times. But as it is a randconfig, it must have enabled function tracer each time :-/

I'll need to update my tests with some default configs to try. Including allyes, mod and allno, and with and without function tracing enabled.

Will fix.

Thanks,

-- Steve

Ingo Molnar

2011-10-11 05:52 UTC #25 lore raw

In reply to Steven Rostedt

Quoted 10 lines · Steven Rostedt

Ingo,
With kernel.org down, I've queued up quite a lot of patches for 3.2. As the list is slowly growing, I would like to get these into tip so we can focus on the new patches.
I removed the trace_printk() patches as Peter and I are still discussing how they should work. But the rest of the patches are good to go.

Quoted 7 lines

-- Steve
Please pull the latest tip/perf/core tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/core

Hm, this URI does not seem to work for me - yesterday and today i was getting:

 $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
 fatal: The remote end hung up unexpectedly

I am able to pull Linus's tree successfully:

 $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
 From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux
 * branch            master     -> FETCH_HEAD
 Already up-to-date.

So git.kernel.org connectivity is OK.

Thanks,

Ingo

[email protected]

2011-10-11 21:09 UTC #26 lore raw

Quoted 2 lines · Ingo Molnar

 $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
 fatal: The remote end hung up unexpectedly

Is it possible to get 'git' to say something more informative than "hung up unexpectedly"? "Tree not found, check URL" or similar would be nice...

Ingo Molnar

2011-10-12 08:08 UTC #27 lore raw

Quoted 8 lines · * [email protected] <[email protected]> wrote:

Quoted 2 lines · Ingo Molnar
 $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
 fatal: The remote end hung up unexpectedly
Is it possible to get 'git' to say something more informative than "hung up unexpectedly"? "Tree not found, check URL" or similar would be nice...

I've Cc:-ed the Git development list which has lots of very responsive gents on it eager to improve Git.

I see this message dozens of times per year and i am always confused about the message.

Firstly, arguably, typoing something is not 'fatal' really - it's just a resource that was not found on the server.

Secondly, and more importantly, the reason for the failed pull is indeed important to know, if you want to resolve the problem with a minimum fuss:

- Was it the tree that didnt exist?
- Or the branch?
- Or was there some other problem [such as a truly unexpectedly closed transport socket]?

It's really useful for a painless UI flow to disambiguate failure messages into clearly actionable variants.

Thanks,

Ingo

Jeff King

2011-10-12 14:26 UTC #28 lore raw

Quoted 8 lines · Ingo Molnar

Quoted 8 lines · Ingo Molnar
Quoted 3 lines
 $ git pull git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/perf/core
 fatal: The remote end hung up unexpectedly
Is it possible to get 'git' to say something more informative than "hung up unexpectedly"? "Tree not found, check URL" or similar would be nice...

It's not possible for the client to say anything more. The server sees that the request isn't valid and hangs up without saying anything. So the server needs to be changed to output better responses.

Quoted 7 lines

Firstly, arguably, typoing something is not 'fatal' really - it's just a resource that was not found on the server.
Secondly, and more importantly, the reason for the failed pull is indeed important to know, if you want to resolve the problem with a minimum fuss:

Quoted 7 lines

- Was it the tree that didnt exist?
- Or the branch?
- Or was there some other problem [such as a truly unexpectedly closed transport socket]?
It's really useful for a painless UI flow to disambiguate failure messages into clearly actionable variants.

I agree. I think some people are concerned with leaking information about which repos exist and how they are configured. That is probably not a big problem for a public site like kernel.org, though.

You might find this thread interesting:

http://thread.gmane.org/gmane.comp.version-control.git/182529/focus=182642

It seems to have resulted in a patch that will at least say "access denied" for every error. Which is a step up from "the remote end hung up unexpectedly", but I do think most users would appreciate it being more specific.

Perhaps we just need a config option to turn on more verbose messages, if the site decides that there's no security implications to doing so.

-Peff

Ingo Molnar

2011-10-12 16:31 UTC #29 lore raw

Quoted 7 lines · Jeff King

Quoted 2 lines
It's really useful for a painless UI flow to disambiguate failure messages into clearly actionable variants.
I agree. I think some people are concerned with leaking information about which repos exist and how they are configured. That is probably not a big problem for a public site like kernel.org, though.

Well, a gitconfig option could be provided to not leak that small amount of info - but it would otherwise be weird for an OSS SCM to default to a behavior that only makes sense with closed source, right? :-)

Thanks,

Ingo