[RFC,PATCH 0/3] trace,perf: enabling ftrace/function tracepoint

[RFC,PATCH 0/3] trace,perf: enabling ftrace/function tracepoint

[Jiri Olsa]

hi,

this is just RFC patch with patche series showing the
direction I'm taking.. _RFC_ ;)

I was discussing with Frederic the possibility to have function trace
available for perf processing, and the possibility to have it used
as starting/stopping events, and probably more.

The 1st 2 patches adding the registration function for the
ftrace/function tracepoint to be usable by both trace and perf
via tracepoint interface.

The 3rd patch is the biggest hack and is trying to add filtering
support.  I'm currently looking on the filtering code to come up
with some better idea of hooking this type of filtering in.

attached patches:
1/3 - add support for enabling ftrace/function tracepoint event
2/3 - add support for registering ftrace/function tracepoint event via perf
3/3 - add filter support for ftrace/function tracepoint event

I could read perf counts from ftrace/function tracepoint using
attached program.

Any thoughts/ideas about this direction or projecting some other
would be just great :)


thanks a lot,
jirka
---
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <linux/perf_event.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <sys/wait.h>

static int trace_event__id(const char *evname)
{
	char *filename;
	int err = -1, fd;

	if (asprintf(&filename,
		     "/debug/tracing/events/ftrace/%s/id",
		     evname) < 0)
		return -1;

	fd = open(filename, O_RDONLY);
	if (fd >= 0) {
		char id[16];
		if (read(fd, id, sizeof(id)) > 0)
			err = atoi(id);
		close(fd);
	}

	free(filename);
	return err;
}

static inline int
sys_perf_event_open(struct perf_event_attr *attr,
		pid_t pid, int cpu, int group_fd, 
		unsigned long flags)
{       
	attr->size = sizeof(*attr);
	return syscall(__NR_perf_event_open, attr, pid, cpu,
		group_fd, flags);
}

int main(int argc, char **argv)
{
	struct perf_event_attr attr;
	int fd;
	pid_t pid;
	int status;
	long count;
	int ret;
	int id = trace_event__id("function");
	int child_ready_pipe[2];

	if (id < 0) {
		printf("is debugfs mounted on /sys/kernel/debug?\n");
		return -1;
	}

        if (pipe(child_ready_pipe) < 0) {
		perror("pipe failed");
		return -1;
	}

	memset(&attr, 0, sizeof(attr));
	attr.type = PERF_TYPE_TRACEPOINT;
	attr.config = id;
	attr.enable_on_exec = 1;

	pid = fork();
	if (!pid) {
		char buf;
		char *cmd = "/bin/sleep";
		char *newargv[] = { cmd, "2", NULL };
		char *newenviron[] = { NULL };

		/* wait for parent to setup the counter */
		close(child_ready_pipe[1]);
		if (read(child_ready_pipe[0], &buf, 1) == -1) {
			perror("unable to read pipe");
			return -1;
		}
		close(child_ready_pipe[0]);

		execve(cmd, newargv, newenviron);
		perror("execve failed");
		return -1;
	}


	fd = sys_perf_event_open(&attr, pid, -1, -1, 0);
	if (fd < 0) {
		perror("sys_perf_event_open failed");
		return -1;
	}

	if (ioctl(fd, PERF_EVENT_IOC_SET_FILTER, "ip == sys_close", 0)) {
		perror("ioctl failed");
		return -1;
	}

	/* tell the child it can go nuts */
	close(child_ready_pipe[1]);
	close(child_ready_pipe[0]);

	waitpid(pid, &status, 0);

	ret = read(fd, &count, sizeof(count));
	if (ret <= 0) {
		perror("read failed");
		return -1;
	}

	printf("count = %ld\n", count);
	return 0;
}

[RFC,PATCH 1/3] trace: add support for enabling ftrace/function tracepoint event

[Jiri Olsa]

Refactored function trace code to be able to resuse the functionality
for tracing via global_trace.

Added reg function for ftrace tracepoint events and added support
for enabling ftrace/function tracepoint.

wbr,
jirka
---
 kernel/trace/ftrace.c          |   35 +++++++++++++++++++++++++++++++++++
 kernel/trace/trace.c           |   25 +++++++++++++++++++++++++
 kernel/trace/trace.h           |    8 ++++++++
 kernel/trace/trace_export.c    |    1 +
 kernel/trace/trace_functions.c |   32 +++++---------------------------
 5 files changed, 74 insertions(+), 27 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ee24fa1..00fe267 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3218,6 +3218,41 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
 	return ret;
 }
 
+static void
+ftrace_function_call_trace(unsigned long ip, unsigned long parent_ip)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	trace_function_call_global(flags, preempt_count(), ip, parent_ip);
+	local_irq_restore(flags);
+}
+
+static struct ftrace_ops ftrace_trace_ops __read_mostly = {
+	.func = ftrace_function_call_trace,
+};
+
+int ftrace_event_class_register(struct ftrace_event_call *call,
+				enum trace_reg type)
+{
+	int etype = call->event.type;
+
+	if (etype != TRACE_FN)
+		return -EINVAL;
+
+	switch (type) {
+	case TRACE_REG_REGISTER:
+		return register_ftrace_function(&ftrace_trace_ops);
+	case TRACE_REG_UNREGISTER:
+		return unregister_ftrace_function(&ftrace_trace_ops);
+	case TRACE_REG_PERF_REGISTER:
+	case TRACE_REG_PERF_UNREGISTER:
+		break;
+	}
+
+	return -EINVAL;
+}
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 
 static int ftrace_graph_active;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d38c16a..6283e0c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1224,6 +1224,31 @@ trace_function(struct trace_array *tr,
 }
 
 void
+trace_function_call(struct trace_array *tr, unsigned long flags, int pc,
+		    unsigned long ip, unsigned long parent_ip)
+{
+	struct trace_array_cpu *data;
+	long disabled;
+	int cpu;
+
+	cpu = raw_smp_processor_id();
+	data = tr->data[cpu];
+	disabled = atomic_inc_return(&data->disabled);
+
+	if (likely(disabled == 1))
+		trace_function(tr, ip, parent_ip, flags, pc);
+
+	atomic_dec(&data->disabled);
+}
+
+void
+trace_function_call_global(unsigned long flags, int pc,
+			   unsigned long ip, unsigned long parent_ip)
+{
+	trace_function_call(&global_trace, flags, pc, ip, parent_ip);
+}
+
+void
 ftrace(struct trace_array *tr, struct trace_array_cpu *data,
        unsigned long ip, unsigned long parent_ip, unsigned long flags,
        int pc)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 5e9dfc6..3298333 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -343,6 +343,11 @@ void trace_function(struct trace_array *tr,
 		    unsigned long ip,
 		    unsigned long parent_ip,
 		    unsigned long flags, int pc);
+void trace_function_call(struct trace_array *tr,
+			 unsigned long flags, int pc,
+			 unsigned long ip, unsigned long parent_ip);
+void trace_function_call_global(unsigned long flags, int pc,
+				unsigned long ip, unsigned long parent_ip);
 void trace_graph_function(struct trace_array *tr,
 		    unsigned long ip,
 		    unsigned long parent_ip,
@@ -773,6 +778,9 @@ extern struct list_head ftrace_events;
 extern const char *__start___trace_bprintk_fmt[];
 extern const char *__stop___trace_bprintk_fmt[];
 
+extern int ftrace_event_class_register(struct ftrace_event_call *call,
+				       enum trace_reg type);
+
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(call, struct_name, id, tstruct, print)		\
 	extern struct ftrace_event_call					\
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index bbeec31..0b0906a 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -159,6 +159,7 @@ struct ftrace_event_class event_class_ftrace_##call = {			\
 	.system			= __stringify(TRACE_SYSTEM),		\
 	.define_fields		= ftrace_define_fields_##call,		\
 	.fields			= LIST_HEAD_INIT(event_class_ftrace_##call.fields),\
+	.reg			= ftrace_event_class_register,		\
 };									\
 									\
 struct ftrace_event_call __used event_##call = {			\
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 16aee4d..175a8e2 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -50,11 +50,7 @@ static void function_trace_start(struct trace_array *tr)
 static void
 function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip)
 {
-	struct trace_array *tr = func_trace;
-	struct trace_array_cpu *data;
 	unsigned long flags;
-	long disabled;
-	int cpu;
 	int pc;
 
 	if (unlikely(!ftrace_function_enabled))
@@ -63,26 +59,16 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip)
 	pc = preempt_count();
 	preempt_disable_notrace();
 	local_save_flags(flags);
-	cpu = raw_smp_processor_id();
-	data = tr->data[cpu];
-	disabled = atomic_inc_return(&data->disabled);
 
-	if (likely(disabled == 1))
-		trace_function(tr, ip, parent_ip, flags, pc);
+	trace_function_call(func_trace, flags, pc, ip, parent_ip);
 
-	atomic_dec(&data->disabled);
 	preempt_enable_notrace();
 }
 
 static void
-function_trace_call(unsigned long ip, unsigned long parent_ip)
+function_trace_call_irq(unsigned long ip, unsigned long parent_ip)
 {
-	struct trace_array *tr = func_trace;
-	struct trace_array_cpu *data;
 	unsigned long flags;
-	long disabled;
-	int cpu;
-	int pc;
 
 	if (unlikely(!ftrace_function_enabled))
 		return;
@@ -92,16 +78,9 @@ function_trace_call(unsigned long ip, unsigned long parent_ip)
 	 * recursive protection is performed.
 	 */
 	local_irq_save(flags);
-	cpu = raw_smp_processor_id();
-	data = tr->data[cpu];
-	disabled = atomic_inc_return(&data->disabled);
 
-	if (likely(disabled == 1)) {
-		pc = preempt_count();
-		trace_function(tr, ip, parent_ip, flags, pc);
-	}
+	trace_function_call(func_trace, flags, preempt_count(), ip, parent_ip);
 
-	atomic_dec(&data->disabled);
 	local_irq_restore(flags);
 }
 
@@ -148,7 +127,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip)
 
 static struct ftrace_ops trace_ops __read_mostly =
 {
-	.func = function_trace_call,
+	.func = function_trace_call_irq,
 };
 
 static struct ftrace_ops trace_stack_ops __read_mostly =
@@ -180,7 +159,7 @@ static void tracing_start_function_trace(void)
 	if (trace_flags & TRACE_ITER_PREEMPTONLY)
 		trace_ops.func = function_trace_call_preempt_only;
 	else
-		trace_ops.func = function_trace_call;
+		trace_ops.func = function_trace_call_irq;
 
 	if (func_flags.val & TRACE_FUNC_OPT_STACK)
 		register_ftrace_function(&trace_stack_ops);
@@ -400,4 +379,3 @@ static __init int init_function_trace(void)
 	return register_tracer(&function_trace);
 }
 device_initcall(init_function_trace);
-
-- 
1.7.1

[RFC,PATCH 2/3] trace,perf: add support for registering ftrace/function tracepoint event via perf

[Jiri Olsa]

Added support to enable ftrace/function tracepoint via perf interface.

wbr,
jirka
---
 kernel/trace/ftrace.c |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 00fe267..49fbc8c 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3228,10 +3228,41 @@ ftrace_function_call_trace(unsigned long ip, unsigned long parent_ip)
 	local_irq_restore(flags);
 }
 
+static void
+ftrace_function_call_perf(unsigned long ip, unsigned long parent_ip)
+{
+	struct ftrace_entry *entry;
+	struct hlist_head *head;
+	unsigned long flags;
+	int rctx;
+
+	local_irq_save(flags);
+
+	BUILD_BUG_ON(sizeof(*entry) > PERF_MAX_TRACE_SIZE);
+
+	entry = (struct ftrace_entry *) perf_trace_buf_prepare(sizeof(*entry),
+						TRACE_FN, NULL, &rctx);
+	if (!entry)
+		return;
+
+	entry->ip = ip;
+	entry->parent_ip = parent_ip;
+
+	head = this_cpu_ptr(event_function.perf_events);
+	perf_trace_buf_submit(entry, sizeof(*entry), rctx, 0,
+			      1, NULL, head);
+
+	local_irq_restore(flags);
+}
+
 static struct ftrace_ops ftrace_trace_ops __read_mostly = {
 	.func = ftrace_function_call_trace,
 };
 
+static struct ftrace_ops ftrace_perf_ops __read_mostly = {
+	.func = ftrace_function_call_perf,
+};
+
 int ftrace_event_class_register(struct ftrace_event_call *call,
 				enum trace_reg type)
 {
@@ -3246,8 +3277,9 @@ int ftrace_event_class_register(struct ftrace_event_call *call,
 	case TRACE_REG_UNREGISTER:
 		return unregister_ftrace_function(&ftrace_trace_ops);
 	case TRACE_REG_PERF_REGISTER:
+		return register_ftrace_function(&ftrace_perf_ops);
 	case TRACE_REG_PERF_UNREGISTER:
-		break;
+		return unregister_ftrace_function(&ftrace_perf_ops);
 	}
 
 	return -EINVAL;
-- 
1.7.1

[RFC,PATCH 3/3] trace,perf: add filter support for ftrace/function tracepoint event

[Jiri Olsa]

Added support to be able to pass "ip == glob" filter for the ftrace/function
tracepoint.

Respective functions are hot-patched/updated when the filter is set,
and the filter predicate is set to be allways true. Probably missing
several cases.. not sure this is the way.

wbr,
jirka

---
 include/linux/ftrace_event.h       |    3 ++
 kernel/trace/ftrace.c              |   35 ++++++++++++++++++++++++++++
 kernel/trace/trace.h               |    2 +
 kernel/trace/trace_events_filter.c |   45 ++++++++++++++++++++++++++++++++---
 kernel/trace/trace_export.c        |   26 ++++++++++++++++----
 5 files changed, 102 insertions(+), 9 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 22b32af..bbf7cc6 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -144,6 +144,8 @@ struct ftrace_event_class {
 	struct list_head	*(*get_fields)(struct ftrace_event_call *);
 	struct list_head	fields;
 	int			(*raw_init)(struct ftrace_event_call *);
+	void			(*filter)(struct ftrace_event_call *call,
+					  int enable);
 };
 
 extern int ftrace_event_reg(struct ftrace_event_call *event,
@@ -221,6 +223,7 @@ enum {
 	FILTER_STATIC_STRING,
 	FILTER_DYN_STRING,
 	FILTER_PTR_STRING,
+	FILTER_TRACE_FN,
 };
 
 #define EVENT_STORAGE_SIZE 128
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 49fbc8c..b52db6e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -3285,6 +3285,41 @@ int ftrace_event_class_register(struct ftrace_event_call *call,
 	return -EINVAL;
 }
 
+void ftrace_event_tracefn_filter(int enable)
+{
+	char *filter = event_function.data;
+
+	if (enable) {
+		WARN_ON(!filter);
+		if (filter)
+			ftrace_match_records(filter, strlen(filter), 1);
+	} else {
+		ftrace_filter_reset(1);
+		if (filter) {
+			kfree(filter);
+			event_function.data = NULL;
+		}
+	}
+
+	mutex_lock(&ftrace_lock);
+	if (ftrace_enabled)
+		ftrace_run_update_code(FTRACE_ENABLE_CALLS);
+	mutex_unlock(&ftrace_lock);
+}
+
+void ftrace_event_class_filter(struct ftrace_event_call *call, int enable)
+{
+	int etype = call->event.type;
+
+	switch (etype) {
+	case TRACE_FN:
+		ftrace_event_tracefn_filter(enable);
+		break;
+	default:
+		break;
+	}
+}
+
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 
 static int ftrace_graph_active;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 3298333..bc2537e 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -780,6 +780,8 @@ extern const char *__stop___trace_bprintk_fmt[];
 
 extern int ftrace_event_class_register(struct ftrace_event_call *call,
 				       enum trace_reg type);
+extern void ftrace_event_class_filter(struct ftrace_event_call *call,
+				      int enable);
 
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(call, struct_name, id, tstruct, print)		\
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 8008ddc..5a847a5 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -238,6 +238,11 @@ static int filter_pred_none(struct filter_pred *pred, void *event)
 	return 0;
 }
 
+static int filter_pred_all(struct filter_pred *pred, void *event)
+{
+	return 1;
+}
+
 /*
  * regex_match_foo - Basic regex callbacks
  *
@@ -755,8 +760,23 @@ static void __free_preds(struct event_filter *filter)
 	filter->n_preds = 0;
 }
 
+static void filter_enable(struct ftrace_event_call *call)
+{
+	struct ftrace_event_class *class = call->class;
+
+	call->flags |= TRACE_EVENT_FL_FILTERED;
+
+	if (class && class->filter)
+		class->filter(call, 1);
+}
+
 static void filter_disable(struct ftrace_event_call *call)
 {
+	struct ftrace_event_class *class = call->class;
+
+	if (class && class->filter)
+		class->filter(call, 0);
+
 	call->flags &= ~TRACE_EVENT_FL_FILTERED;
 }
 
@@ -883,6 +903,11 @@ static bool is_string_field(struct ftrace_event_field *field)
 	       field->filter_type == FILTER_PTR_STRING;
 }
 
+static bool is_tracefn_field(struct ftrace_event_field *field)
+{
+	return field->filter_type == FILTER_TRACE_FN;
+}
+
 static int is_legal_op(struct ftrace_event_field *field, int op)
 {
 	if (is_string_field(field) &&
@@ -969,7 +994,15 @@ static int filter_add_pred(struct filter_parse_state *ps,
 		return -EINVAL;
 	}
 
-	if (is_string_field(field)) {
+	if (is_tracefn_field(field)) {
+		/* allow only one function trace predicate */
+		if (call->data) {
+			parse_error(ps, FILT_ERR_TOO_MANY_OPERANDS, 0);
+			return -EINVAL;
+		}
+		fn = filter_pred_all;
+		call->data = kstrdup(pred->regex.pattern, GFP_KERNEL);
+	} else if (is_string_field(field)) {
 		filter_build_regex(pred);
 
 		if (field->filter_type == FILTER_STATIC_STRING) {
@@ -1760,7 +1793,7 @@ static int replace_system_preds(struct event_subsystem *system,
 			parse_error(ps, FILT_ERR_BAD_SUBSYS_FILTER, 0);
 			append_filter_err(ps, filter);
 		} else
-			call->flags |= TRACE_EVENT_FL_FILTERED;
+			filter_enable(call);
 		/*
 		 * Regardless of if this returned an error, we still
 		 * replace the filter for the call.
@@ -1853,7 +1886,7 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string)
 		filter_disable(call);
 		append_filter_err(ps, filter);
 	} else
-		call->flags |= TRACE_EVENT_FL_FILTERED;
+		filter_enable(call);
 out:
 	/*
 	 * Always swap the call filter with the new filter
@@ -1965,6 +1998,8 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
 	if (&call->list == &ftrace_events)
 		goto out_unlock;
 
+	filter_disable(call);
+
 	err = -EEXIST;
 	if (event->filter)
 		goto out_unlock;
@@ -1986,8 +2021,10 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
 		goto free_ps;
 
 	err = replace_preds(call, filter, ps, filter_str, false);
-	if (!err)
+	if (!err) {
+		filter_enable(call);
 		event->filter = filter;
+	}
 
 free_ps:
 	filter_opstack_clear(ps);
diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c
index 0b0906a..b1d5301 100644
--- a/kernel/trace/trace_export.c
+++ b/kernel/trace/trace_export.c
@@ -67,7 +67,7 @@ static void __always_unused ____ftrace_check_##name(void)	\
 	ret = trace_define_field(event_call, #type, #item,		\
 				 offsetof(typeof(field), item),		\
 				 sizeof(field.item),			\
-				 is_signed_type(type), FILTER_OTHER);	\
+				 is_signed_type(type), filter_type);	\
 	if (ret)							\
 		return ret;
 
@@ -77,7 +77,7 @@ static void __always_unused ____ftrace_check_##name(void)	\
 				 offsetof(typeof(field),		\
 					  container.item),		\
 				 sizeof(field.container.item),		\
-				 is_signed_type(type), FILTER_OTHER);	\
+				 is_signed_type(type), filter_type);	\
 	if (ret)							\
 		return ret;
 
@@ -91,7 +91,7 @@ static void __always_unused ____ftrace_check_##name(void)	\
 		ret = trace_define_field(event_call, event_storage, #item, \
 				 offsetof(typeof(field), item),		\
 				 sizeof(field.item),			\
-				 is_signed_type(type), FILTER_OTHER);	\
+				 is_signed_type(type), filter_type);	\
 		mutex_unlock(&event_storage_mutex);			\
 		if (ret)						\
 			return ret;					\
@@ -104,7 +104,7 @@ static void __always_unused ____ftrace_check_##name(void)	\
 				 offsetof(typeof(field),		\
 					  container.item),		\
 				 sizeof(field.container.item),		\
-				 is_signed_type(type), FILTER_OTHER);	\
+				 is_signed_type(type), filter_type);	\
 	if (ret)							\
 		return ret;
 
@@ -112,10 +112,24 @@ static void __always_unused ____ftrace_check_##name(void)	\
 #define __dynamic_array(type, item)					\
 	ret = trace_define_field(event_call, #type, #item,		\
 				 offsetof(typeof(field), item),		\
-				 0, is_signed_type(type), FILTER_OTHER);\
+				 0, is_signed_type(type), filter_type);	\
 	if (ret)							\
 		return ret;
 
+#define FILTER_TYPE_TRACE_FN		FILTER_TRACE_FN
+#define FILTER_TYPE_TRACE_GRAPH_ENT	FILTER_OTHER
+#define FILTER_TYPE_TRACE_GRAPH_RET	FILTER_OTHER
+#define FILTER_TYPE_TRACE_CTX		FILTER_OTHER
+#define FILTER_TYPE_TRACE_WAKE		FILTER_OTHER
+#define FILTER_TYPE_TRACE_STACK		FILTER_OTHER
+#define FILTER_TYPE_TRACE_USER_STACK	FILTER_OTHER
+#define FILTER_TYPE_TRACE_BPRINT	FILTER_OTHER
+#define FILTER_TYPE_TRACE_PRINT		FILTER_OTHER
+#define FILTER_TYPE_TRACE_MMIO_RW	FILTER_OTHER
+#define FILTER_TYPE_TRACE_MMIO_MAP	FILTER_OTHER
+#define FILTER_TYPE_TRACE_BRANCH	FILTER_OTHER
+#define FILTER_TYPE(arg)		FILTER_TYPE_##arg
+
 #undef FTRACE_ENTRY
 #define FTRACE_ENTRY(name, struct_name, id, tstruct, print)		\
 int									\
@@ -123,6 +137,7 @@ ftrace_define_fields_##name(struct ftrace_event_call *event_call)	\
 {									\
 	struct struct_name field;					\
 	int ret;							\
+	int filter_type = FILTER_TYPE(id);				\
 									\
 	tstruct;							\
 									\
@@ -160,6 +175,7 @@ struct ftrace_event_class event_class_ftrace_##call = {			\
 	.define_fields		= ftrace_define_fields_##call,		\
 	.fields			= LIST_HEAD_INIT(event_class_ftrace_##call.fields),\
 	.reg			= ftrace_event_class_register,		\
+	.filter			= ftrace_event_class_filter,		\
 };									\
 									\
 struct ftrace_event_call __used event_##call = {			\
-- 
1.7.1

Re: [RFC,PATCH 3/3] trace,perf: add filter support for ftrace/function tracepoint event

[Frederic Weisbecker]

On Thu, Apr 21, 2011 at 12:40:58PM +0200, Jiri Olsa wrote:
> Added support to be able to pass "ip == glob" filter for the ftrace/function
> tracepoint.
> 
> Respective functions are hot-patched/updated when the filter is set,
> and the filter predicate is set to be allways true.

Ideally, if the filter is set to "ip", we want to apply your hook
and not make the filter appearing in the tracing path. ie, Instead of
setting a filter with filter_pred_all callback, we should have no filter
to process from ftrace_function_call_*() things.

We probably need to make an exception when we filter parent_ip,
which can't be processed using hotpatching, so we need a real
filter for it.


> Probably missing
> several cases.. not sure this is the way.
> 
> wbr,
> jirka
> 
> ---
>  include/linux/ftrace_event.h       |    3 ++
>  kernel/trace/ftrace.c              |   35 ++++++++++++++++++++++++++++
>  kernel/trace/trace.h               |    2 +
>  kernel/trace/trace_events_filter.c |   45 ++++++++++++++++++++++++++++++++---
>  kernel/trace/trace_export.c        |   26 ++++++++++++++++----
>  5 files changed, 102 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 22b32af..bbf7cc6 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -144,6 +144,8 @@ struct ftrace_event_class {
>  	struct list_head	*(*get_fields)(struct ftrace_event_call *);
>  	struct list_head	fields;
>  	int			(*raw_init)(struct ftrace_event_call *);
> +	void			(*filter)(struct ftrace_event_call *call,
> +					  int enable);

Good, but this should take the parsed filter as a parameter.
But that's also a problem of un-globalizing the function tracer. Several
perf events, and ftrace, should be able to set different filters. Thus
you need to compute a kind of union of all these filters and keep
track of who needs which function.

perf event A may want to trace func1()
perf event B may want to trace func2()

So you need to enable func1() and func2(), although they both
have set different filters for that, but trigger an event on A
only when func1() is called, and trigger an event on B only when func2()
is called.

So we probably need a kind of function hlist to keep track of that.

I started something a while ago:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
	tracing/hlist

The thing is very outdated now I think, and it needed some rework.

>  };
>  
>  extern int ftrace_event_reg(struct ftrace_event_call *event,
> @@ -221,6 +223,7 @@ enum {
>  	FILTER_STATIC_STRING,
>  	FILTER_DYN_STRING,
>  	FILTER_PTR_STRING,
> +	FILTER_TRACE_FN,

But yeah that kind of interface is cool. Although I would rather
call that FILTER_CUSTOM or FILTER_OVERRIDEN, to avoid confusion
with TRACE_FN, as this is deemed to express the fact we override
the interpretation of the filter. ftrace should be only a specific
user of that. More are perhaps coming.

Also, we may need to override the effects of the filtering endpoint
to express every capabilities of set_ftrace_filter, like
trace_on/trace_off commands. So that we can get rid of that global file in
the end.

One idea was to have a "triggers" directory in each event directories,
and have one intepretation of filters per file:

$ls triggers
filter
trace_on
trace_off

Then, when the user tries:

$ echo "ip == lambda" > trace_off

This simply disables tracing when we hit the lambda function.
Echoing the same in filter would filter when we hit that function.

The filter file in that triggers directory would refer to the same
inode of the filter file in the event directory. We would just keep
the old one for compatibility.

>  };
>  
>  #define EVENT_STORAGE_SIZE 128
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 49fbc8c..b52db6e 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -3285,6 +3285,41 @@ int ftrace_event_class_register(struct ftrace_event_call *call,
>  	return -EINVAL;
>  }
>  
> +void ftrace_event_tracefn_filter(int enable)
> +{
> +	char *filter = event_function.data;
> +
> +	if (enable) {
> +		WARN_ON(!filter);
> +		if (filter)
> +			ftrace_match_records(filter, strlen(filter), 1);
> +	} else {
> +		ftrace_filter_reset(1);
> +		if (filter) {
> +			kfree(filter);
> +			event_function.data = NULL;
> +		}
> +	}
> +
> +	mutex_lock(&ftrace_lock);
> +	if (ftrace_enabled)
> +		ftrace_run_update_code(FTRACE_ENABLE_CALLS);
> +	mutex_unlock(&ftrace_lock);
> +}

Note we may have complicated expressions linked with either && or ||.
Thus you need to be able to compute unions and intersections or
functions sets. The ftrace_match_record() won't be sufficient enough
for that. Best would be to evaluate the whole tree of the parsed expression.

This can be done gradually though, we can start with simple expressions
and reject complicated ones to begin with.

Thanks.

Re: [RFC,PATCH 0/3] trace,perf: enabling ftrace/function tracepoint

[Frederic Weisbecker]

On Thu, Apr 21, 2011 at 12:40:55PM +0200, Jiri Olsa wrote:
> hi,
> 
> this is just RFC patch with patche series showing the
> direction I'm taking.. _RFC_ ;)
> 
> I was discussing with Frederic the possibility to have function trace
> available for perf processing, and the possibility to have it used
> as starting/stopping events, and probably more.
> 
> The 1st 2 patches adding the registration function for the
> ftrace/function tracepoint to be usable by both trace and perf
> via tracepoint interface.
> 
> The 3rd patch is the biggest hack and is trying to add filtering
> support.  I'm currently looking on the filtering code to come up
> with some better idea of hooking this type of filtering in.
> 
> attached patches:
> 1/3 - add support for enabling ftrace/function tracepoint event
> 2/3 - add support for registering ftrace/function tracepoint event via perf
> 3/3 - add filter support for ftrace/function tracepoint event
> 
> I could read perf counts from ftrace/function tracepoint using
> attached program.
> 
> Any thoughts/ideas about this direction or projecting some other
> would be just great :)
> 
> 
> thanks a lot,
> jirka

So the direction is kinda good but the thing needs to be unglobalized
(if that word ever exists). Filters need to apply individually to
each users of the function tracer, that's the trickiest part.

We also need to catch up with set_ftrace_filter and set_ftrace_notrace
using filters and triggers, so that we can get rid of the global files
in the end and have only local effects (per perf_event/ftrace).

But that's a good start.

Thanks.

Re: [RFC,PATCH 0/3] trace,perf: enabling ftrace/function tracepoint

[Steven Rostedt]

On Thu, 2011-04-21 at 17:27 +0200, Frederic Weisbecker wrote:

> So the direction is kinda good but the thing needs to be unglobalized
> (if that word ever exists). Filters need to apply individually to
> each users of the function tracer, that's the trickiest part.
> 
> We also need to catch up with set_ftrace_filter and set_ftrace_notrace
> using filters and triggers, so that we can get rid of the global files
> in the end and have only local effects (per perf_event/ftrace).
> 
> But that's a good start.

Note, As I just finished the last of my "tinkering" jobs, I'm now
starting on my bigger projects. The first being the rewrite of the
ftrace function tracing internals.

I'll start it today, but as I have off tomorrow, and my wife has already
filled all my time up with Honeydew tasks (and this extends into the
weekend), I'll have to continue it next week. But hopefully, if all goes
well, I should have an RFC ready by the end of next week.

-- Steve