[PATCH] Fix writing to trace/trace_options

[PATCH] Fix writing to trace/trace_options

[Aneesh Kumar K.V]

writing to trace/trace_options use the index of the array
to find the value of the flag. With branch tracer flag
defined conditionally, this breaks writing to trace_options
with branch tracer disabled.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 004aafd..ef7fe15 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -259,9 +259,7 @@ static const char *trace_options[] = {
 	"sched-tree",
 	"ftrace_printk",
 	"ftrace_preempt",
-#ifdef CONFIG_BRANCH_TRACER
 	"branch",
-#endif
 	"annotate",
 	"dump",
 	NULL
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index b24d350..d048b63 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -481,9 +481,7 @@ enum trace_iterator_flags {
 	TRACE_ITER_SCHED_TREE		= 0x200,
 	TRACE_ITER_PRINTK		= 0x400,
 	TRACE_ITER_PREEMPTONLY		= 0x800,
-#ifdef CONFIG_BRANCH_TRACER
 	TRACE_ITER_BRANCH		= 0x1000,
-#endif
 	TRACE_ITER_ANNOTATE		= 0x2000,
 	TRACE_ITER_DUMP			= 0x4000,
 };
-- 
tg: (e186e8e..) an/branch-tracer-fix.patch (depends on: an/ext4-ftrace-test.patch)

[PATCH] ftrace: Add new entry type TRACE_BIN_DUMP

[Aneesh Kumar K.V]

The ftrace dump_entry can be used to dump binary data from
kernel to userspace.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

---
 kernel/trace/trace.c |    8 ++++++++
 kernel/trace/trace.h |   11 +++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index c0d1170..004aafd 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2106,6 +2106,14 @@ static enum print_line_t print_dump_fmt(struct trace_iterator *iter)
 			trace_seq_print_cont(s, iter);
 		break;
 	}
+	case TRACE_BIN_DUMP: {
+		struct dump_entry *field;
+
+		trace_assign_type(field, entry);
+
+		trace_seq_putmem(s, field->buf, field->len);
+		break;
+	}
 	}
 	return TRACE_TYPE_HANDLED;
 }
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 358d2a6..b24d350 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -26,6 +26,7 @@ enum trace_type {
 	TRACE_BOOT_CALL,
 	TRACE_BOOT_RET,
 	TRACE_FN_RET,
+	TRACE_BIN_DUMP,
 
 	__TRACE_LAST_TYPE
 };
@@ -107,6 +108,15 @@ struct print_entry {
 	char			buf[];
 };
 
+/*
+ * bin dump entry:
+ */
+struct dump_entry {
+	struct trace_entry ent;
+	int     len;
+	char    buf[];
+};
+
 #define TRACE_OLD_SIZE		88
 
 struct trace_field_cont {
@@ -249,6 +259,7 @@ extern void __ftrace_bad_type(void);
 		IF_ASSIGN(var, ent, struct trace_boot_ret, TRACE_BOOT_RET);\
 		IF_ASSIGN(var, ent, struct trace_branch, TRACE_BRANCH); \
 		IF_ASSIGN(var, ent, struct ftrace_ret_entry, TRACE_FN_RET);\
+		IF_ASSIGN(var, ent, struct dump_entry, TRACE_BIN_DUMP);	\
 		__ftrace_bad_type();					\
 	} while (0)
 
-- 
tg: (b382bc2..) an/ftrace-bin-dump.patch (depends on: an/ftrace-dp-printk.patch)

[PATCH] ftrace: add proper bin iterator support

[Aneesh Kumar K.V]

In s_show after print_trace_line we call trace_print_seq which
does a strlen on the buffer containing binary data. I guess that would
give wrong results for bin data even though we are null terminating it.
We also don't want header in the output file if we have  TRACE_ITER_BIN.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

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

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 4a90462..f6e21d8 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2080,10 +2080,51 @@ static enum print_line_t print_trace_line(struct trace_iterator *iter)
 	return print_trace_fmt(iter);
 }
 
+static int s_bin_show(struct seq_file *m, void *v)
+{
+	int len;
+	enum print_line_t ret;
+	struct trace_iterator *iter = v;
+
+	if (iter->ent == NULL)
+		return 0;
+
+	if (iter->trace && iter->trace->print_line) {
+		ret = iter->trace->print_line(iter);
+		if (ret != TRACE_TYPE_UNHANDLED)
+			goto trace_handled;
+	}
+	/*
+	 * If  trace type is unhandled or if we don't have
+	 * a print_line call the standard bin_fmt callback
+	 */
+	print_bin_fmt(iter);
+
+trace_handled:
+	/* copy the trace buffer to seq file buffer */
+	if (iter->seq.len >= PAGE_SIZE)
+		len = PAGE_SIZE - 1;
+	else
+		len = iter->seq.len;
+
+	if (m->count + len < m->size) {
+		memcpy(m->buf + m->count, iter->seq.buffer, len);
+		m->count += len;
+	} else
+		m->count = m->size;
+	trace_seq_reset(&iter->seq);
+
+	return 0;
+}
+
+
 static int s_show(struct seq_file *m, void *v)
 {
 	struct trace_iterator *iter = v;
 
+	if (trace_flags & TRACE_ITER_BIN)
+		return s_bin_show(m, v);
+
 	if (iter->ent == NULL) {
 		if (iter->tr) {
 			seq_printf(m, "# tracer: %s\n", iter->trace->name);
-- 
tg: (7195b67..) an/ftrace-bin-iterator.patch (depends on: master)

[PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace

[Aneesh Kumar K.V]

The trace add a new interface debug_dump() which can be used
to dump binary data using ftrace framework. Use the dump mode
of iterator by 'echo dump > iter_ctrl' to read the value in
userspace

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

---
 include/linux/debug_dump.h     |   10 ++++
 kernel/trace/Kconfig           |    6 ++
 kernel/trace/Makefile          |    1 +
 kernel/trace/trace_debugdump.c |  103 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 0 deletions(-)

diff --git a/include/linux/debug_dump.h b/include/linux/debug_dump.h
new file mode 100644
index 0000000..2c1836d
--- /dev/null
+++ b/include/linux/debug_dump.h
@@ -0,0 +1,10 @@
+#ifndef _LINUX_DEBUGDUMP_H
+#define _LINUX_DEBUGDUMP_H
+#include <linux/marker.h>
+
+#ifdef CONFIG_DEBUG_DUMP
+extern int debug_dump(void *p, int len);
+#else
+#define debug_dump(p, len)
+#endif
+#endif /*  _LINUX_DEBUGDUMP_H */
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 30eccc8..247cf3f 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -261,4 +261,10 @@ config DEBUG_PRINT
 	help
 	  This tracer can be used for printk style debugging.
 
+config DEBUG_DUMP
+	bool "Trace based dump support"
+	depends on DEBUG_KERNEL
+	select TRACING
+	help
+	  This tracer can be used to dump binary data from kernel
 endmenu
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 86e5869..b1c6dec 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -32,5 +32,6 @@ obj-$(CONFIG_BOOT_TRACER) += trace_boot.o
 obj-$(CONFIG_FUNCTION_RET_TRACER) += trace_functions_return.o
 obj-$(CONFIG_TRACE_BRANCH_PROFILING) += trace_branch.o
 obj-$(CONFIG_DEBUG_PRINT) += trace_debugprint.o
+obj-$(CONFIG_DEBUG_DUMP) += trace_debugdump.o
 
 libftrace-y := ftrace.o
diff --git a/kernel/trace/trace_debugdump.c b/kernel/trace/trace_debugdump.c
new file mode 100644
index 0000000..3ebae2a
--- /dev/null
+++ b/kernel/trace/trace_debugdump.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright IBM Corporation, 2008
+ * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/ftrace.h>
+
+#include "trace.h"
+
+static struct trace_array *ctx_trace;
+static int __read_mostly dd_tracer_enabled;
+
+int debug_dump(void *bin_data, int len)
+{
+	int size;
+	unsigned long irq_flags;
+	struct dump_entry *entry;
+	struct ring_buffer_event *event;
+
+	if (!dd_tracer_enabled)
+		return 0;
+
+	size = sizeof(*entry) + len;
+	event = ring_buffer_lock_reserve(ctx_trace->buffer, size, &irq_flags);
+	if (!event)
+		return 0;
+	entry = ring_buffer_event_data(event);
+	tracing_generic_entry_update(&entry->ent, 0, preempt_count());
+	entry->ent.type	= TRACE_BIN_DUMP;
+	entry->len	= len;
+
+	memcpy(&entry->buf, bin_data, len);
+	ring_buffer_unlock_commit(ctx_trace->buffer, event, irq_flags);
+
+	return len;
+}
+EXPORT_SYMBOL_GPL(debug_dump);
+
+static void dd_reset(struct trace_array *tr)
+{
+	int cpu;
+
+	tr->time_start = ftrace_now(tr->cpu);
+
+	for_each_online_cpu(cpu)
+		tracing_reset(tr, cpu);
+}
+
+static void dd_trace_start(struct trace_array *tr)
+{
+	dd_reset(tr);
+	dd_tracer_enabled = 1;
+}
+
+static void dd_trace_stop(struct trace_array *tr)
+{
+	dd_tracer_enabled = 0;
+	/* Nothing to do! */
+}
+
+static void dd_trace_init(struct trace_array *tr)
+{
+	ctx_trace = tr;
+	/*
+	 * enable the trace during init if
+	 * we have enabled flag already set
+	 */
+	if (tracing_is_enabled())
+		dd_trace_start(tr);
+}
+
+static void dd_trace_reset(struct trace_array *tr)
+{
+	if (dd_tracer_enabled)
+		dd_trace_stop(tr);
+}
+
+struct tracer dd_trace __read_mostly =
+{
+	.name		= "debug_dump",
+	.init		= dd_trace_init,
+	.reset		= dd_trace_reset,
+	.start		= dd_trace_start,
+	.stop		= dd_trace_stop,
+};
+
+__init static int init_dd_trace(void)
+{
+	return register_tracer(&dd_trace);
+}
+device_initcall(init_dd_trace);
-- 
tg: (140170e..) an/ftrace-debug-dump.patch (depends on: an/ftrace-bin-dump.patch)

[PATCH] ftrace: Add debug_print trace to print data from kernel to userspace

[Aneesh Kumar K.V]

The trace add a new interface dp_printk() which can be used
to dump data from kernel to user space using ftrace framework.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

---
 include/linux/debug_print.h     |   12 +++++
 kernel/trace/Kconfig            |    7 +++
 kernel/trace/Makefile           |    1 +
 kernel/trace/trace_debugprint.c |   90 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 0 deletions(-)

diff --git a/include/linux/debug_print.h b/include/linux/debug_print.h
new file mode 100644
index 0000000..db4397b
--- /dev/null
+++ b/include/linux/debug_print.h
@@ -0,0 +1,12 @@
+#ifndef _LINUX_DEBUGPRINT_H
+#define _LINUX_DEBUGPRINT_H
+#include <linux/marker.h>
+
+#ifdef CONFIG_DEBUG_PRINT
+extern int do_dp_printk(const char *fmt, ...);
+#define dp_printk(fmt, arg...) \
+	do_dp_printk(fmt, ##arg)
+#else
+#define dp_printk(fmt, arg...)
+#endif
+#endif /*  _LINUX_DEBUGPRINT_H */
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 9c89526..30eccc8 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -254,4 +254,11 @@ config FTRACE_STARTUP_TEST
 	  functioning properly. It will do tests on all the configured
 	  tracers of ftrace.
 
+config DEBUG_PRINT
+	bool "Trace based printk support"
+	depends on DEBUG_KERNEL
+	select TRACING
+	help
+	  This tracer can be used for printk style debugging.
+
 endmenu
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index 1a8c925..86e5869 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -31,5 +31,6 @@ obj-$(CONFIG_MMIOTRACE) += trace_mmiotrace.o
 obj-$(CONFIG_BOOT_TRACER) += trace_boot.o
 obj-$(CONFIG_FUNCTION_RET_TRACER) += trace_functions_return.o
 obj-$(CONFIG_TRACE_BRANCH_PROFILING) += trace_branch.o
+obj-$(CONFIG_DEBUG_PRINT) += trace_debugprint.o
 
 libftrace-y := ftrace.o
diff --git a/kernel/trace/trace_debugprint.c b/kernel/trace/trace_debugprint.c
new file mode 100644
index 0000000..2c3ac3d
--- /dev/null
+++ b/kernel/trace/trace_debugprint.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright IBM Corporation, 2008
+ * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/debugfs.h>
+#include <linux/ftrace.h>
+
+#include "trace.h"
+
+static int __read_mostly dp_tracer_enabled;
+
+int do_dp_printk(const char *fmt, ...)
+{
+	int ret;
+	va_list args;
+
+	if (!dp_tracer_enabled)
+		return 0;
+
+	va_start(args, fmt);
+	ret = trace_vprintk(0, fmt, args);
+	va_end(args);
+	return ret;
+}
+EXPORT_SYMBOL(do_dp_printk);
+
+static void dp_reset(struct trace_array *tr)
+{
+	int cpu;
+
+	tr->time_start = ftrace_now(tr->cpu);
+
+	for_each_online_cpu(cpu)
+		tracing_reset(tr, cpu);
+}
+
+static void dp_trace_start(struct trace_array *tr)
+{
+	dp_reset(tr);
+	dp_tracer_enabled = 1;
+}
+
+static void dp_trace_stop(struct trace_array *tr)
+{
+	dp_tracer_enabled = 0;
+	/* Nothing to do! */
+}
+
+static void dp_trace_init(struct trace_array *tr)
+{
+	/*
+	 * enable the trace during init if
+	 * we have enabled flag already set
+	 */
+	if (tracing_is_enabled())
+		dp_trace_start(tr);
+}
+
+static void dp_trace_reset(struct trace_array *tr)
+{
+	if (dp_tracer_enabled)
+		dp_trace_stop(tr);
+}
+
+struct tracer dp_trace __read_mostly =
+{
+	.name		= "debug_print",
+	.init		= dp_trace_init,
+	.reset		= dp_trace_reset,
+	.start		= dp_trace_start,
+	.stop		= dp_trace_stop,
+};
+
+__init static int init_dp_trace(void)
+{
+	return register_tracer(&dp_trace);
+}
+device_initcall(init_dp_trace);
-- 
tg: (bfb3409..) an/ftrace-dp-printk.patch (depends on: an/ftrace-dump-iterator.patch)

[PATCH] ftrace: Add dump iteator

[Aneesh Kumar K.V]

This iteator doesn't print headers and tracer information in
the ouput trace file. This can be obtained to get a dmesg style
trace output file. Also can be used to dump binary data to via
trace

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

---
 kernel/trace/trace.c |   92 ++++++++++++++++++++++++++++++++++++++++++--------
 kernel/trace/trace.h |    1 +
 2 files changed, 79 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f6e21d8..c0d1170 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -263,6 +263,7 @@ static const char *trace_options[] = {
 	"branch",
 #endif
 	"annotate",
+	"dump",
 	NULL
 };
 
@@ -2038,6 +2039,77 @@ static enum print_line_t print_bin_fmt(struct trace_iterator *iter)
 	return 1;
 }
 
+static enum print_line_t print_dump_fmt(struct trace_iterator *iter)
+{
+	struct trace_seq *s = &iter->seq;
+	struct trace_entry *entry;
+
+	entry = iter->ent;
+
+	if (entry->type == TRACE_CONT)
+		return TRACE_TYPE_HANDLED;
+
+
+	switch (entry->type) {
+	case TRACE_FN: {
+		struct ftrace_entry *field;
+
+		trace_assign_type(field, entry);
+
+		SEQ_PUT_FIELD_RET(s, entry->pid);
+		SEQ_PUT_FIELD_RET(s, iter->cpu);
+		SEQ_PUT_FIELD_RET(s, iter->ts);
+
+		SEQ_PUT_FIELD_RET(s, field->ip);
+		SEQ_PUT_FIELD_RET(s, field->parent_ip);
+		break;
+	}
+	case TRACE_CTX: {
+		struct ctx_switch_entry *field;
+
+		trace_assign_type(field, entry);
+
+		SEQ_PUT_FIELD_RET(s, entry->pid);
+		SEQ_PUT_FIELD_RET(s, iter->cpu);
+		SEQ_PUT_FIELD_RET(s, iter->ts);
+
+		SEQ_PUT_FIELD_RET(s, field->prev_pid);
+		SEQ_PUT_FIELD_RET(s, field->prev_prio);
+		SEQ_PUT_FIELD_RET(s, field->prev_state);
+		SEQ_PUT_FIELD_RET(s, field->next_pid);
+		SEQ_PUT_FIELD_RET(s, field->next_prio);
+		SEQ_PUT_FIELD_RET(s, field->next_state);
+		break;
+	}
+	case TRACE_SPECIAL:
+	case TRACE_STACK: {
+		struct special_entry *field;
+
+		trace_assign_type(field, entry);
+
+		SEQ_PUT_FIELD_RET(s, entry->pid);
+		SEQ_PUT_FIELD_RET(s, iter->cpu);
+		SEQ_PUT_FIELD_RET(s, iter->ts);
+
+		SEQ_PUT_FIELD_RET(s, field->arg1);
+		SEQ_PUT_FIELD_RET(s, field->arg2);
+		SEQ_PUT_FIELD_RET(s, field->arg3);
+		break;
+	}
+	case TRACE_PRINT: {
+		struct print_entry *field;
+
+		trace_assign_type(field, entry);
+
+		trace_seq_printf(s, "%s", field->buf);
+		if (entry->flags & TRACE_FLAG_CONT)
+			trace_seq_print_cont(s, iter);
+		break;
+	}
+	}
+	return TRACE_TYPE_HANDLED;
+}
+
 static int trace_empty(struct trace_iterator *iter)
 {
 	int cpu;
@@ -2074,6 +2146,9 @@ static enum print_line_t print_trace_line(struct trace_iterator *iter)
 	if (trace_flags & TRACE_ITER_RAW)
 		return print_raw_fmt(iter);
 
+	if (trace_flags & TRACE_ITER_DUMP)
+		return print_dump_fmt(iter);
+
 	if (iter->iter_flags & TRACE_FILE_LAT_FMT)
 		return print_lat_fmt(iter, iter->idx, iter->cpu);
 
@@ -2083,24 +2158,12 @@ static enum print_line_t print_trace_line(struct trace_iterator *iter)
 static int s_bin_show(struct seq_file *m, void *v)
 {
 	int len;
-	enum print_line_t ret;
 	struct trace_iterator *iter = v;
 
 	if (iter->ent == NULL)
 		return 0;
 
-	if (iter->trace && iter->trace->print_line) {
-		ret = iter->trace->print_line(iter);
-		if (ret != TRACE_TYPE_UNHANDLED)
-			goto trace_handled;
-	}
-	/*
-	 * If  trace type is unhandled or if we don't have
-	 * a print_line call the standard bin_fmt callback
-	 */
-	print_bin_fmt(iter);
-
-trace_handled:
+	print_trace_line(iter);
 	/* copy the trace buffer to seq file buffer */
 	if (iter->seq.len >= PAGE_SIZE)
 		len = PAGE_SIZE - 1;
@@ -2122,7 +2185,8 @@ static int s_show(struct seq_file *m, void *v)
 {
 	struct trace_iterator *iter = v;
 
-	if (trace_flags & TRACE_ITER_BIN)
+	if ((trace_flags & TRACE_ITER_BIN) ||
+			(trace_flags & TRACE_ITER_DUMP))
 		return s_bin_show(m, v);
 
 	if (iter->ent == NULL) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 790ea8c..358d2a6 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -474,6 +474,7 @@ enum trace_iterator_flags {
 	TRACE_ITER_BRANCH		= 0x1000,
 #endif
 	TRACE_ITER_ANNOTATE		= 0x2000,
+	TRACE_ITER_DUMP			= 0x4000,
 };
 
 /*
-- 
tg: (47b0062..) an/ftrace-dump-iterator.patch (depends on: an/ftrace-bin-iterator.patch)

Re: [PATCH] Fix writing to trace/trace_options

[Aneesh Kumar K.V]

On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> writing to trace/trace_options use the index of the array
> to find the value of the flag. With branch tracer flag
> defined conditionally, this breaks writing to trace_options
> with branch tracer disabled.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

tg export didn't add numbering to the patch based on dependency.
So the series is in the below order

[PATCH] ftrace: add proper bin iterator support
[PATCH] ftrace: Add dump iteator
[PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
[PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
[PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
[PATCH] Fix writing to trace/trace_options

The patches are against -tip with HEAD
7195b6707adcd00f413ce07e6b9954b4c597495c

-aneesh

Re: [PATCH] Fix writing to trace/trace_options

[Steven Rostedt]

On Sun, 16 Nov 2008, Aneesh Kumar K.V wrote:

> On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> > writing to trace/trace_options use the index of the array
> > to find the value of the flag. With branch tracer flag
> > defined conditionally, this breaks writing to trace_options
> > with branch tracer disabled.
> > 
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> tg export didn't add numbering to the patch based on dependency.
> So the series is in the below order
> 
> [PATCH] ftrace: add proper bin iterator support
> [PATCH] ftrace: Add dump iteator
> [PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
> [PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
> [PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
> [PATCH] Fix writing to trace/trace_options
> 
> The patches are against -tip with HEAD
> 7195b6707adcd00f413ce07e6b9954b4c597495c

Thanks Aneesh,

I'll look closer at these patches on Monday.

-- Steve

Re: [PATCH] Fix writing to trace/trace_options

[Ingo Molnar]

* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:

> On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> > writing to trace/trace_options use the index of the array
> > to find the value of the flag. With branch tracer flag
> > defined conditionally, this breaks writing to trace_options
> > with branch tracer disabled.
> > 
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> tg export didn't add numbering to the patch based on dependency.
> So the series is in the below order
> 
> [PATCH] ftrace: add proper bin iterator support
> [PATCH] ftrace: Add dump iteator
> [PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
> [PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
> [PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
> [PATCH] Fix writing to trace/trace_options
> 
> The patches are against -tip with HEAD
> 7195b6707adcd00f413ce07e6b9954b4c597495c

hm, i'm not sure about this. We already do binary dumping, but only 
for the cases where we actually know the structure of the data (i.e. 
binary dumping is just an output format, not a tracing type). And that 
is good so.

In your patchset right now nothing uses debug_dump(ptr, len) so it's 
hard to see exactly how we should shape it. What specific usages do 
you have in mind?

	Ingo

Re: [PATCH] Fix writing to trace/trace_options

[Aneesh Kumar K.V]

On Mon, Nov 17, 2008 at 12:17:47PM +0100, Ingo Molnar wrote:
> 
> * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> 
> > On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> > > writing to trace/trace_options use the index of the array
> > > to find the value of the flag. With branch tracer flag
> > > defined conditionally, this breaks writing to trace_options
> > > with branch tracer disabled.
> > > 
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > 
> > tg export didn't add numbering to the patch based on dependency.
> > So the series is in the below order
> > 
> > [PATCH] ftrace: add proper bin iterator support
> > [PATCH] ftrace: Add dump iteator
> > [PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
> > [PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
> > [PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
> > [PATCH] Fix writing to trace/trace_options
> > 
> > The patches are against -tip with HEAD
> > 7195b6707adcd00f413ce07e6b9954b4c597495c
> 
> hm, i'm not sure about this. We already do binary dumping, but only 
> for the cases where we actually know the structure of the data (i.e. 
> binary dumping is just an output format, not a tracing type). And that 
> is good so.

Why do we need to limit to know structures. debug_dump can be looked at
as a debugging helper which allows the user to send more data in binary
format. Later user space can decide to look at the values. I had the
below test case done to check the patches.

 	int err;
+	struct data {
+		char i;
+		int k;
+	};
+	struct data mydata = {.i = 'c', .k = 10};
 
 	/*
 	 * If we have encountered a bitmap-format file, the size limit
 	 * is smaller than s_maxbytes, which is for extent-mapped files.
 	 */
+	dp_printk("%s with value %d\n", __func__, pos);
+	debug_dump(&mydata, sizeof(mydata));


> 
> In your patchset right now nothing uses debug_dump(ptr, len) so it's 
> hard to see exactly how we should shape it. What specific usages do 
> you have in mind?

If you are not convinced about debug_dump you may want to pick the first
three patches that include a bug fix and support for dp_printk.

-aneesh

Re: [PATCH] Fix writing to trace/trace_options

[Ingo Molnar]

* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:

> On Mon, Nov 17, 2008 at 12:17:47PM +0100, Ingo Molnar wrote:
> > 
> > * Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
> > 
> > > On Sun, Nov 16, 2008 at 04:07:58PM +0530, Aneesh Kumar K.V wrote:
> > > > writing to trace/trace_options use the index of the array
> > > > to find the value of the flag. With branch tracer flag
> > > > defined conditionally, this breaks writing to trace_options
> > > > with branch tracer disabled.
> > > > 
> > > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> > > 
> > > tg export didn't add numbering to the patch based on dependency.
> > > So the series is in the below order
> > > 
> > > [PATCH] ftrace: add proper bin iterator support
> > > [PATCH] ftrace: Add dump iteator
> > > [PATCH] ftrace: Add debug_print trace to print data from kernel to userspace
> > > [PATCH] ftrace: Add new entry type TRACE_BIN_DUMP
> > > [PATCH] ftrace: Add debug_dump trace to dump binary data from kernel to userspace
> > > [PATCH] Fix writing to trace/trace_options
> > > 
> > > The patches are against -tip with HEAD
> > > 7195b6707adcd00f413ce07e6b9954b4c597495c
> > 
> > hm, i'm not sure about this. We already do binary dumping, but only 
> > for the cases where we actually know the structure of the data (i.e. 
> > binary dumping is just an output format, not a tracing type). And that 
> > is good so.
> 
> Why do we need to limit to know structures. debug_dump can be looked at
> as a debugging helper which allows the user to send more data in binary
> format. Later user space can decide to look at the values. I had the
> below test case done to check the patches.
> 
>  	int err;
> +	struct data {
> +		char i;
> +		int k;
> +	};
> +	struct data mydata = {.i = 'c', .k = 10};
>  
>  	/*
>  	 * If we have encountered a bitmap-format file, the size limit
>  	 * is smaller than s_maxbytes, which is for extent-mapped files.
>  	 */
> +	dp_printk("%s with value %d\n", __func__, pos);
> +	debug_dump(&mydata, sizeof(mydata));
> 
> 
> > 
> > In your patchset right now nothing uses debug_dump(ptr, len) so 
> > it's hard to see exactly how we should shape it. What specific 
> > usages do you have in mind?
> 
> If you are not convinced about debug_dump you may want to pick the 
> first three patches that include a bug fix and support for 
> dp_printk.

i'm still wondering about the specific usecase you have in mind. Where 
do we want to dump unstructured data to user-space? If you look at all 
the existing ftrace plugins we know the structure of the logged data 
very well, and most of the plugins have semantic dependencies on that 
data content as well.

and having binary log data that _only_ user-space can process 
meaningfully, while it's opaque to the tracer plugin, sounds like a 
poor concept to me.

	Ingo

Re: [PATCH] Fix writing to trace/trace_options

[Steven Rostedt]

On Sun, 16 Nov 2008, Aneesh Kumar K.V wrote:

> writing to trace/trace_options use the index of the array
> to find the value of the flag. With branch tracer flag
> defined conditionally, this breaks writing to trace_options
> with branch tracer disabled.

Ug, you're right!

> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> ---
>  kernel/trace/trace.c |    2 --
>  kernel/trace/trace.h |    2 --
>  2 files changed, 0 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 004aafd..ef7fe15 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -259,9 +259,7 @@ static const char *trace_options[] = {
>  	"sched-tree",
>  	"ftrace_printk",
>  	"ftrace_preempt",
> -#ifdef CONFIG_BRANCH_TRACER
>  	"branch",

Perhaps what we need is to have this NULL if CONFIG_BRANCH_TRACER is
not set, and change the other code to skip NULL pointers.

-- Steve

> -#endif
>  	"annotate",
>  	"dump",
>  	NULL
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index b24d350..d048b63 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -481,9 +481,7 @@ enum trace_iterator_flags {
>  	TRACE_ITER_SCHED_TREE		= 0x200,
>  	TRACE_ITER_PRINTK		= 0x400,
>  	TRACE_ITER_PREEMPTONLY		= 0x800,
> -#ifdef CONFIG_BRANCH_TRACER
>  	TRACE_ITER_BRANCH		= 0x1000,
> -#endif
>  	TRACE_ITER_ANNOTATE		= 0x2000,
>  	TRACE_ITER_DUMP			= 0x4000,
>  };
> -- 
> tg: (e186e8e..) an/branch-tracer-fix.patch (depends on: an/ext4-ftrace-test.patch)
> 
> 

Re: [PATCH] Fix writing to trace/trace_options

[Ingo Molnar]

* Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:

> writing to trace/trace_options use the index of the array
> to find the value of the flag. With branch tracer flag
> defined conditionally, this breaks writing to trace_options
> with branch tracer disabled.
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> 
> ---
>  kernel/trace/trace.c |    2 --
>  kernel/trace/trace.h |    2 --
>  2 files changed, 0 insertions(+), 4 deletions(-)

applied to tip/tracing/branch-tracer, thanks Aneesh!

	Ingo