[for-next][PATCH 0/5] tracing: few updates

[for-next][PATCH 0/5] tracing: few updates

[Steven Rostedt]

I final have time to start looking at my INBOX. There's several patches
that need to be pushed for 3.12. They are mostly clean ups and not anything
urgent.

I'm still in the process of going through my INBOX (backwards in time)
so I may find new stuff as well.

-- Steve

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

Head SHA1: fc30f13b7c1b87b44ee364462c3408c913f01439


Li Zefan (1)
      tracing/syscalls: Annotate raw_init function with __init

Oleg Nesterov (3)
      tracing: Kill trace_create_file_ops() and friends
      tracing: Don't pass file_operations array to event_create_dir()
      tracing: Kill the !CONFIG_MODULES code in trace_events.c

Steven Rostedt (Red Hat) (1)
      tracing: Add __tracepoint_string() to export string pointers

----
 b/include/asm-generic/vmlinux.lds.h |    7 +
 b/include/linux/ftrace_event.h      |   34 ++++++++
 b/kernel/trace/trace.h              |    3 
 b/kernel/trace/trace_events.c       |  153 ++----------------------------------
 b/kernel/trace/trace_printk.c       |   19 ++++
 b/kernel/trace/trace_syscalls.c     |   10 +-
 kernel/trace/trace_events.c         |   64 ++++-----------
 7 files changed, 94 insertions(+), 196 deletions(-)

[for-next][PATCH 1/5] tracing: Add __tracepoint_string() to export string pointers

[Steven Rostedt]

[-- Attachment #1: 0000-tracing-Add-__tracepoint_string-to-export-string-poi.patch --]
[-- Type: text/plain, Size: 7124 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

There are several tracepoints (mostly in RCU), that reference a string
pointer and uses the print format of "%s" to display the string that
exists in the kernel, instead of copying the actual string to the
ring buffer (saves time and ring buffer space).

But this has an issue with userspace tools that read the binary buffers
that has the address of the string but has no access to what the string
itself is. The end result is just output that looks like:

 rcu_dyntick:          ffffffff818adeaa 1 0
 rcu_dyntick:          ffffffff818adeb5 0 140000000000000
 rcu_dyntick:          ffffffff818adeb5 0 140000000000000
 rcu_utilization:      ffffffff8184333b
 rcu_utilization:      ffffffff8184333b

The above is pretty useless when read by the userspace tools. Ideally
we would want something that looks like this:

 rcu_dyntick:          Start 1 0
 rcu_dyntick:          End 0 140000000000000
 rcu_dyntick:          Start 140000000000000 0
 rcu_callback:         rcu_preempt rhp=0xffff880037aff710 func=put_cred_rcu 0/4
 rcu_callback:         rcu_preempt rhp=0xffff880078961980 func=file_free_rcu 0/5
 rcu_dyntick:          End 0 1

The trace_printk() which also only stores the address of the string
format instead of recording the string into the buffer itself, exports
the mapping of kernel addresses to format strings via the printk_format
file in the debugfs tracing directory.

The tracepoint strings can use this same method and output the format
to the same file and the userspace tools will be able to decipher
the address without any modification.

The tracepoint strings need its own section to save the strings because
the trace_printk section will cause the trace_printk() buffers to be
allocated if anything exists within the section. trace_printk() is only
used for debugging and should never exist in the kernel, we can not use
the trace_printk sections.

Add a new tracepoint_str section that will also be examined by the output
of the printk_format file.

Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/asm-generic/vmlinux.lds.h |    7 ++++++-
 include/linux/ftrace_event.h      |   34 ++++++++++++++++++++++++++++++++++
 kernel/trace/trace.h              |    3 +++
 kernel/trace/trace_printk.c       |   19 +++++++++++++++++++
 4 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 69732d2..83e2c31 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -122,8 +122,12 @@
 #define TRACE_PRINTKS() VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .;      \
 			 *(__trace_printk_fmt) /* Trace_printk fmt' pointer */ \
 			 VMLINUX_SYMBOL(__stop___trace_bprintk_fmt) = .;
+#define TRACEPOINT_STR() VMLINUX_SYMBOL(__start___tracepoint_str) = .;	\
+			 *(__tracepoint_str) /* Trace_printk fmt' pointer */ \
+			 VMLINUX_SYMBOL(__stop___tracepoint_str) = .;
 #else
 #define TRACE_PRINTKS()
+#define TRACEPOINT_STR()
 #endif
 
 #ifdef CONFIG_FTRACE_SYSCALLS
@@ -190,7 +194,8 @@
 	VMLINUX_SYMBOL(__stop___verbose) = .;				\
 	LIKELY_PROFILE()		       				\
 	BRANCH_PROFILE()						\
-	TRACE_PRINTKS()
+	TRACE_PRINTKS()							\
+	TRACEPOINT_STR()
 
 /*
  * Data section helpers
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index 4372658..81af18a 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -357,6 +357,40 @@ do {									\
 		__trace_printk(ip, fmt, ##args);			\
 } while (0)
 
+/**
+ * tracepoint_string - register constant persistent string to trace system
+ * @str - a constant persistent string that will be referenced in tracepoints
+ *
+ * If constant strings are being used in tracepoints, it is faster and
+ * more efficient to just save the pointer to the string and reference
+ * that with a printf "%s" instead of saving the string in the ring buffer
+ * and wasting space and time.
+ *
+ * The problem with the above approach is that userspace tools that read
+ * the binary output of the trace buffers do not have access to the string.
+ * Instead they just show the address of the string which is not very
+ * useful to users.
+ *
+ * With tracepoint_string(), the string will be registered to the tracing
+ * system and exported to userspace via the debugfs/tracing/printk_formats
+ * file that maps the string address to the string text. This way userspace
+ * tools that read the binary buffers have a way to map the pointers to
+ * the ASCII strings they represent.
+ *
+ * The @str used must be a constant string and persistent as it would not
+ * make sense to show a string that no longer exists. But it is still fine
+ * to be used with modules, because when modules are unloaded, if they
+ * had tracepoints, the ring buffers are cleared too. As long as the string
+ * does not change during the life of the module, it is fine to use
+ * tracepoint_string() within a module.
+ */
+#define tracepoint_string(str)						\
+	({								\
+		static const char *___tp_str __tracepoint_string = str; \
+		___tp_str;						\
+	})
+#define __tracepoint_string	__attribute__((section("__tracepoint_str")))
+
 #ifdef CONFIG_PERF_EVENTS
 struct perf_event;
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 4a4f6e1..ba321f1 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1022,6 +1022,9 @@ extern struct list_head ftrace_events;
 extern const char *__start___trace_bprintk_fmt[];
 extern const char *__stop___trace_bprintk_fmt[];
 
+extern const char *__start___tracepoint_str[];
+extern const char *__stop___tracepoint_str[];
+
 void trace_printk_init_buffers(void);
 void trace_printk_start_comm(void);
 int trace_keep_overwrite(struct tracer *tracer, u32 mask, int set);
diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
index a9077c1..2900817 100644
--- a/kernel/trace/trace_printk.c
+++ b/kernel/trace/trace_printk.c
@@ -244,12 +244,31 @@ static const char **find_next(void *v, loff_t *pos)
 {
 	const char **fmt = v;
 	int start_index;
+	int last_index;
 
 	start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
 
 	if (*pos < start_index)
 		return __start___trace_bprintk_fmt + *pos;
 
+	/*
+	 * The __tracepoint_str section is treated the same as the
+	 * __trace_printk_fmt section. The difference is that the
+	 * __trace_printk_fmt section should only be used by trace_printk()
+	 * in a debugging environment, as if anything exists in that section
+	 * the trace_prink() helper buffers are allocated, which would just
+	 * waste space in a production environment.
+	 *
+	 * The __tracepoint_str sections on the other hand are used by
+	 * tracepoints which need to map pointers to their strings to
+	 * the ASCII text for userspace.
+	 */
+	last_index = start_index;
+	start_index = __stop___tracepoint_str - __start___tracepoint_str;
+
+	if (*pos < last_index + start_index)
+		return __start___tracepoint_str + (*pos - last_index);
+
 	return find_next_mod_format(start_index, v, fmt, pos);
 }
 
-- 
1.7.10.4

[for-next][PATCH 2/5] tracing/syscalls: Annotate raw_init function with __init

[Steven Rostedt]

[-- Attachment #1: 0001-tracing-syscalls-Annotate-raw_init-function-with-__i.patch --]
[-- Type: text/plain, Size: 1850 bytes --]

From: Li Zefan <lizefan@huawei.com>

init_syscall_trace() can only be called during kernel bootup only, so we can
mark it and the functions it calls as __init.

Link: http://lkml.kernel.org/r/51528E89.6080508@huawei.com

Signed-off-by: Li Zefan <lizefan@huawei.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_syscalls.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 8fd0365..559329d 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -200,8 +200,8 @@ extern char *__bad_type_size(void);
 		#type, #name, offsetof(typeof(trace), name),		\
 		sizeof(trace.name), is_signed_type(type)
 
-static
-int  __set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
+static int __init
+__set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
 {
 	int i;
 	int pos = 0;
@@ -228,7 +228,7 @@ int  __set_enter_print_fmt(struct syscall_metadata *entry, char *buf, int len)
 	return pos;
 }
 
-static int set_syscall_print_fmt(struct ftrace_event_call *call)
+static int __init set_syscall_print_fmt(struct ftrace_event_call *call)
 {
 	char *print_fmt;
 	int len;
@@ -253,7 +253,7 @@ static int set_syscall_print_fmt(struct ftrace_event_call *call)
 	return 0;
 }
 
-static void free_syscall_print_fmt(struct ftrace_event_call *call)
+static void __init free_syscall_print_fmt(struct ftrace_event_call *call)
 {
 	struct syscall_metadata *entry = call->data;
 
@@ -459,7 +459,7 @@ static void unreg_event_syscall_exit(struct ftrace_event_file *file,
 	mutex_unlock(&syscall_trace_lock);
 }
 
-static int init_syscall_trace(struct ftrace_event_call *call)
+static int __init init_syscall_trace(struct ftrace_event_call *call)
 {
 	int id;
 	int num;
-- 
1.7.10.4

[for-next][PATCH 3/5] tracing: Kill trace_create_file_ops() and friends

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-Kill-trace_create_file_ops-and-friends.patch --]
[-- Type: text/plain, Size: 7336 bytes --]

From: Oleg Nesterov <oleg@redhat.com>

trace_create_file_ops() allocates the copy of id/filter/format/enable
file_operations to set "f_op->owner = mod" for fops_get().

However after the recent changes there is no reason to prevent rmmod
even if one of these files is opened. A file operation can do nothing
but fail after remove_event_file_dir() clears ->i_private for every
file removed by trace_module_remove_events().

Kill "struct ftrace_module_file_ops" and fix the compilation errors.

Link: http://lkml.kernel.org/r/20130731173132.GA31033@redhat.com

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events.c |  153 +++----------------------------------------
 1 file changed, 9 insertions(+), 144 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 29a7ebc..2ec8273 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1683,8 +1683,7 @@ __trace_early_add_new_event(struct ftrace_event_call *call,
 }
 
 struct ftrace_module_file_ops;
-static void __add_event_to_tracers(struct ftrace_event_call *call,
-				   struct ftrace_module_file_ops *file_ops);
+static void __add_event_to_tracers(struct ftrace_event_call *call);
 
 /* Add an additional event_call dynamically */
 int trace_add_event_call(struct ftrace_event_call *call)
@@ -1695,7 +1694,7 @@ int trace_add_event_call(struct ftrace_event_call *call)
 
 	ret = __register_event(call, NULL);
 	if (ret >= 0)
-		__add_event_to_tracers(call, NULL);
+		__add_event_to_tracers(call);
 
 	mutex_unlock(&event_mutex);
 	mutex_unlock(&trace_types_lock);
@@ -1769,100 +1768,21 @@ int trace_remove_event_call(struct ftrace_event_call *call)
 
 #ifdef CONFIG_MODULES
 
-static LIST_HEAD(ftrace_module_file_list);
-
-/*
- * Modules must own their file_operations to keep up with
- * reference counting.
- */
-struct ftrace_module_file_ops {
-	struct list_head		list;
-	struct module			*mod;
-	struct file_operations		id;
-	struct file_operations		enable;
-	struct file_operations		format;
-	struct file_operations		filter;
-};
-
-static struct ftrace_module_file_ops *
-find_ftrace_file_ops(struct ftrace_module_file_ops *file_ops, struct module *mod)
-{
-	/*
-	 * As event_calls are added in groups by module,
-	 * when we find one file_ops, we don't need to search for
-	 * each call in that module, as the rest should be the
-	 * same. Only search for a new one if the last one did
-	 * not match.
-	 */
-	if (file_ops && mod == file_ops->mod)
-		return file_ops;
-
-	list_for_each_entry(file_ops, &ftrace_module_file_list, list) {
-		if (file_ops->mod == mod)
-			return file_ops;
-	}
-	return NULL;
-}
-
-static struct ftrace_module_file_ops *
-trace_create_file_ops(struct module *mod)
-{
-	struct ftrace_module_file_ops *file_ops;
-
-	/*
-	 * This is a bit of a PITA. To allow for correct reference
-	 * counting, modules must "own" their file_operations.
-	 * To do this, we allocate the file operations that will be
-	 * used in the event directory.
-	 */
-
-	file_ops = kmalloc(sizeof(*file_ops), GFP_KERNEL);
-	if (!file_ops)
-		return NULL;
-
-	file_ops->mod = mod;
-
-	file_ops->id = ftrace_event_id_fops;
-	file_ops->id.owner = mod;
-
-	file_ops->enable = ftrace_enable_fops;
-	file_ops->enable.owner = mod;
-
-	file_ops->filter = ftrace_event_filter_fops;
-	file_ops->filter.owner = mod;
-
-	file_ops->format = ftrace_event_format_fops;
-	file_ops->format.owner = mod;
-
-	list_add(&file_ops->list, &ftrace_module_file_list);
-
-	return file_ops;
-}
-
 static void trace_module_add_events(struct module *mod)
 {
-	struct ftrace_module_file_ops *file_ops = NULL;
 	struct ftrace_event_call **call, **start, **end;
 
 	start = mod->trace_events;
 	end = mod->trace_events + mod->num_trace_events;
 
-	if (start == end)
-		return;
-
-	file_ops = trace_create_file_ops(mod);
-	if (!file_ops)
-		return;
-
 	for_each_event(call, start, end) {
 		__register_event(*call, mod);
-		__add_event_to_tracers(*call, file_ops);
+		__add_event_to_tracers(*call);
 	}
 }
 
 static void trace_module_remove_events(struct module *mod)
 {
-	struct ftrace_module_file_ops *file_ops;
 	struct ftrace_event_call *call, *p;
 	bool clear_trace = false;
 
@@ -1874,16 +1794,6 @@ static void trace_module_remove_events(struct module *mod)
 			__trace_remove_event_call(call);
 		}
 	}
-
-	/* Now free the file_operations */
-	list_for_each_entry(file_ops, &ftrace_module_file_list, list) {
-		if (file_ops->mod == mod)
-			break;
-	}
-	if (&file_ops->list != &ftrace_module_file_list) {
-		list_del(&file_ops->list);
-		kfree(file_ops);
-	}
 	up_write(&trace_event_sem);
 
 	/*
@@ -1919,62 +1829,22 @@ static int trace_module_notify(struct notifier_block *self,
 	return 0;
 }
 
-static int
-__trace_add_new_mod_event(struct ftrace_event_call *call,
-			  struct trace_array *tr,
-			  struct ftrace_module_file_ops *file_ops)
-{
-	return __trace_add_new_event(call, tr,
-				     &file_ops->id, &file_ops->enable,
-				     &file_ops->filter, &file_ops->format);
-}
-
 #else
-static inline struct ftrace_module_file_ops *
-find_ftrace_file_ops(struct ftrace_module_file_ops *file_ops, struct module *mod)
-{
-	return NULL;
-}
 static inline int trace_module_notify(struct notifier_block *self,
 				      unsigned long val, void *data)
 {
 	return 0;
 }
-static inline int
-__trace_add_new_mod_event(struct ftrace_event_call *call,
-			  struct trace_array *tr,
-			  struct ftrace_module_file_ops *file_ops)
-{
-	return -ENODEV;
-}
 #endif /* CONFIG_MODULES */
 
 /* Create a new event directory structure for a trace directory. */
 static void
 __trace_add_event_dirs(struct trace_array *tr)
 {
-	struct ftrace_module_file_ops *file_ops = NULL;
 	struct ftrace_event_call *call;
 	int ret;
 
 	list_for_each_entry(call, &ftrace_events, list) {
-		if (call->mod) {
-			/*
-			 * Directories for events by modules need to
-			 * keep module ref counts when opened (as we don't
-			 * want the module to disappear when reading one
-			 * of these files). The file_ops keep account of
-			 * the module ref count.
-			 */
-			file_ops = find_ftrace_file_ops(file_ops, call->mod);
-			if (!file_ops)
-				continue; /* Warn? */
-			ret = __trace_add_new_mod_event(call, tr, file_ops);
-			if (ret < 0)
-				pr_warning("Could not create directory for event %s\n",
-					   call->name);
-			continue;
-		}
 		ret = __trace_add_new_event(call, tr,
 					    &ftrace_event_id_fops,
 					    &ftrace_enable_fops,
@@ -2332,21 +2202,16 @@ __trace_remove_event_dirs(struct trace_array *tr)
 		remove_event_file_dir(file);
 }
 
-static void
-__add_event_to_tracers(struct ftrace_event_call *call,
-		       struct ftrace_module_file_ops *file_ops)
+static void __add_event_to_tracers(struct ftrace_event_call *call)
 {
 	struct trace_array *tr;
 
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		if (file_ops)
-			__trace_add_new_mod_event(call, tr, file_ops);
-		else
-			__trace_add_new_event(call, tr,
-					      &ftrace_event_id_fops,
-					      &ftrace_enable_fops,
-					      &ftrace_event_filter_fops,
-					      &ftrace_event_format_fops);
+		__trace_add_new_event(call, tr,
+				      &ftrace_event_id_fops,
+				      &ftrace_enable_fops,
+				      &ftrace_event_filter_fops,
+				      &ftrace_event_format_fops);
 	}
 }
 
-- 
1.7.10.4

[for-next][PATCH 4/5] tracing: Dont pass file_operations array to event_create_dir()

[Steven Rostedt]

[-- Attachment #1: 0003-tracing-Don-t-pass-file_operations-array-to-event_cr.patch --]
[-- Type: text/plain, Size: 4161 bytes --]

From: Oleg Nesterov <oleg@redhat.com>

Now that event_create_dir() and __trace_add_new_event() always
use the same file_operations we can kill these arguments and
simplify the code.

Link: http://lkml.kernel.org/r/20130731173135.GA31040@redhat.com

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events.c |   46 +++++++++++--------------------------------
 1 file changed, 12 insertions(+), 34 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 2ec8273..4e706a0 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1489,12 +1489,7 @@ event_subsystem_dir(struct trace_array *tr, const char *name,
 }
 
 static int
-event_create_dir(struct dentry *parent,
-		 struct ftrace_event_file *file,
-		 const struct file_operations *id,
-		 const struct file_operations *enable,
-		 const struct file_operations *filter,
-		 const struct file_operations *format)
+event_create_dir(struct dentry *parent, struct ftrace_event_file *file)
 {
 	struct ftrace_event_call *call = file->event_call;
 	struct trace_array *tr = file->tr;
@@ -1522,12 +1517,13 @@ event_create_dir(struct dentry *parent,
 
 	if (call->class->reg && !(call->flags & TRACE_EVENT_FL_IGNORE_ENABLE))
 		trace_create_file("enable", 0644, file->dir, file,
-				  enable);
+				  &ftrace_enable_fops);
 
 #ifdef CONFIG_PERF_EVENTS
 	if (call->event.type && call->class->reg)
 		trace_create_file("id", 0444, file->dir,
-				  (void *)(long)call->event.type, id);
+				  (void *)(long)call->event.type,
+				  &ftrace_event_id_fops);
 #endif
 
 	/*
@@ -1544,10 +1540,10 @@ event_create_dir(struct dentry *parent,
 		}
 	}
 	trace_create_file("filter", 0644, file->dir, call,
-			  filter);
+			  &ftrace_event_filter_fops);
 
 	trace_create_file("format", 0444, file->dir, call,
-			  format);
+			  &ftrace_event_format_fops);
 
 	return 0;
 }
@@ -1648,12 +1644,7 @@ trace_create_new_event(struct ftrace_event_call *call,
 
 /* Add an event to a trace directory */
 static int
-__trace_add_new_event(struct ftrace_event_call *call,
-		      struct trace_array *tr,
-		      const struct file_operations *id,
-		      const struct file_operations *enable,
-		      const struct file_operations *filter,
-		      const struct file_operations *format)
+__trace_add_new_event(struct ftrace_event_call *call, struct trace_array *tr)
 {
 	struct ftrace_event_file *file;
 
@@ -1661,7 +1652,7 @@ __trace_add_new_event(struct ftrace_event_call *call,
 	if (!file)
 		return -ENOMEM;
 
-	return event_create_dir(tr->event_dir, file, id, enable, filter, format);
+	return event_create_dir(tr->event_dir, file);
 }
 
 /*
@@ -1845,11 +1836,7 @@ __trace_add_event_dirs(struct trace_array *tr)
 	int ret;
 
 	list_for_each_entry(call, &ftrace_events, list) {
-		ret = __trace_add_new_event(call, tr,
-					    &ftrace_event_id_fops,
-					    &ftrace_enable_fops,
-					    &ftrace_event_filter_fops,
-					    &ftrace_event_format_fops);
+		ret = __trace_add_new_event(call, tr);
 		if (ret < 0)
 			pr_warning("Could not create directory for event %s\n",
 				   call->name);
@@ -2157,11 +2144,7 @@ __trace_early_add_event_dirs(struct trace_array *tr)
 
 
 	list_for_each_entry(file, &tr->events, list) {
-		ret = event_create_dir(tr->event_dir, file,
-				       &ftrace_event_id_fops,
-				       &ftrace_enable_fops,
-				       &ftrace_event_filter_fops,
-				       &ftrace_event_format_fops);
+		ret = event_create_dir(tr->event_dir, file);
 		if (ret < 0)
 			pr_warning("Could not create directory for event %s\n",
 				   file->event_call->name);
@@ -2206,13 +2189,8 @@ static void __add_event_to_tracers(struct ftrace_event_call *call)
 {
 	struct trace_array *tr;
 
-	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
-		__trace_add_new_event(call, tr,
-				      &ftrace_event_id_fops,
-				      &ftrace_enable_fops,
-				      &ftrace_event_filter_fops,
-				      &ftrace_event_format_fops);
-	}
+	list_for_each_entry(tr, &ftrace_trace_arrays, list)
+		__trace_add_new_event(call, tr);
 }
 
 static struct notifier_block trace_module_nb = {
-- 
1.7.10.4

[for-next][PATCH 5/5] tracing: Kill the !CONFIG_MODULES code in trace_events.c

[Steven Rostedt]

[-- Attachment #1: 0004-tracing-Kill-the-CONFIG_MODULES-code-in-trace_events.patch --]
[-- Type: text/plain, Size: 1891 bytes --]

From: Oleg Nesterov <oleg@redhat.com>

Move trace_module_nb under CONFIG_MODULES and kill the dummy
trace_module_notify(). Imho it doesn't make sense to define
"struct notifier_block" and its .notifier_call just to avoid
"ifdef" in event_trace_init(), and all other !CONFIG_MODULES
code has already gone away.

Link: http://lkml.kernel.org/r/20130731173137.GA31043@redhat.com

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 4e706a0..368a4d5 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1820,12 +1820,10 @@ static int trace_module_notify(struct notifier_block *self,
 	return 0;
 }
 
-#else
-static inline int trace_module_notify(struct notifier_block *self,
-				      unsigned long val, void *data)
-{
-	return 0;
-}
+static struct notifier_block trace_module_nb = {
+	.notifier_call = trace_module_notify,
+	.priority = 0,
+};
 #endif /* CONFIG_MODULES */
 
 /* Create a new event directory structure for a trace directory. */
@@ -2193,11 +2191,6 @@ static void __add_event_to_tracers(struct ftrace_event_call *call)
 		__trace_add_new_event(call, tr);
 }
 
-static struct notifier_block trace_module_nb = {
-	.notifier_call = trace_module_notify,
-	.priority = 0,
-};
-
 extern struct ftrace_event_call *__start_ftrace_events[];
 extern struct ftrace_event_call *__stop_ftrace_events[];
 
@@ -2402,10 +2395,11 @@ static __init int event_trace_init(void)
 	if (ret)
 		return ret;
 
+#ifdef CONFIG_MODULES
 	ret = register_module_notifier(&trace_module_nb);
 	if (ret)
 		pr_warning("Failed to register trace events module notifier\n");
-
+#endif
 	return 0;
 }
 early_initcall(event_trace_memsetup);
-- 
1.7.10.4

Re: [for-next][PATCH 1/5] tracing: Add __tracepoint_string() to export string pointers

[Paul E. McKenney]

On Thu, Aug 22, 2013 at 12:32:26PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> 
> There are several tracepoints (mostly in RCU), that reference a string
> pointer and uses the print format of "%s" to display the string that
> exists in the kernel, instead of copying the actual string to the
> ring buffer (saves time and ring buffer space).
> 
> But this has an issue with userspace tools that read the binary buffers
> that has the address of the string but has no access to what the string
> itself is. The end result is just output that looks like:
> 
>  rcu_dyntick:          ffffffff818adeaa 1 0
>  rcu_dyntick:          ffffffff818adeb5 0 140000000000000
>  rcu_dyntick:          ffffffff818adeb5 0 140000000000000
>  rcu_utilization:      ffffffff8184333b
>  rcu_utilization:      ffffffff8184333b
> 
> The above is pretty useless when read by the userspace tools. Ideally
> we would want something that looks like this:
> 
>  rcu_dyntick:          Start 1 0
>  rcu_dyntick:          End 0 140000000000000
>  rcu_dyntick:          Start 140000000000000 0
>  rcu_callback:         rcu_preempt rhp=0xffff880037aff710 func=put_cred_rcu 0/4
>  rcu_callback:         rcu_preempt rhp=0xffff880078961980 func=file_free_rcu 0/5
>  rcu_dyntick:          End 0 1
> 
> The trace_printk() which also only stores the address of the string
> format instead of recording the string into the buffer itself, exports
> the mapping of kernel addresses to format strings via the printk_format
> file in the debugfs tracing directory.
> 
> The tracepoint strings can use this same method and output the format
> to the same file and the userspace tools will be able to decipher
> the address without any modification.
> 
> The tracepoint strings need its own section to save the strings because
> the trace_printk section will cause the trace_printk() buffers to be
> allocated if anything exists within the section. trace_printk() is only
> used for debugging and should never exist in the kernel, we can not use
> the trace_printk sections.
> 
> Add a new tracepoint_str section that will also be examined by the output
> of the printk_format file.
> 
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  include/asm-generic/vmlinux.lds.h |    7 ++++++-
>  include/linux/ftrace_event.h      |   34 ++++++++++++++++++++++++++++++++++
>  kernel/trace/trace.h              |    3 +++
>  kernel/trace/trace_printk.c       |   19 +++++++++++++++++++
>  4 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 69732d2..83e2c31 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -122,8 +122,12 @@
>  #define TRACE_PRINTKS() VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .;      \
>  			 *(__trace_printk_fmt) /* Trace_printk fmt' pointer */ \
>  			 VMLINUX_SYMBOL(__stop___trace_bprintk_fmt) = .;
> +#define TRACEPOINT_STR() VMLINUX_SYMBOL(__start___tracepoint_str) = .;	\
> +			 *(__tracepoint_str) /* Trace_printk fmt' pointer */ \
> +			 VMLINUX_SYMBOL(__stop___tracepoint_str) = .;
>  #else
>  #define TRACE_PRINTKS()
> +#define TRACEPOINT_STR()
>  #endif
> 
>  #ifdef CONFIG_FTRACE_SYSCALLS
> @@ -190,7 +194,8 @@
>  	VMLINUX_SYMBOL(__stop___verbose) = .;				\
>  	LIKELY_PROFILE()		       				\
>  	BRANCH_PROFILE()						\
> -	TRACE_PRINTKS()
> +	TRACE_PRINTKS()							\
> +	TRACEPOINT_STR()
> 
>  /*
>   * Data section helpers
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index 4372658..81af18a 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -357,6 +357,40 @@ do {									\
>  		__trace_printk(ip, fmt, ##args);			\
>  } while (0)
> 
> +/**
> + * tracepoint_string - register constant persistent string to trace system
> + * @str - a constant persistent string that will be referenced in tracepoints
> + *
> + * If constant strings are being used in tracepoints, it is faster and
> + * more efficient to just save the pointer to the string and reference
> + * that with a printf "%s" instead of saving the string in the ring buffer
> + * and wasting space and time.
> + *
> + * The problem with the above approach is that userspace tools that read
> + * the binary output of the trace buffers do not have access to the string.
> + * Instead they just show the address of the string which is not very
> + * useful to users.
> + *
> + * With tracepoint_string(), the string will be registered to the tracing
> + * system and exported to userspace via the debugfs/tracing/printk_formats
> + * file that maps the string address to the string text. This way userspace
> + * tools that read the binary buffers have a way to map the pointers to
> + * the ASCII strings they represent.
> + *
> + * The @str used must be a constant string and persistent as it would not
> + * make sense to show a string that no longer exists. But it is still fine
> + * to be used with modules, because when modules are unloaded, if they
> + * had tracepoints, the ring buffers are cleared too. As long as the string
> + * does not change during the life of the module, it is fine to use
> + * tracepoint_string() within a module.
> + */
> +#define tracepoint_string(str)						\
> +	({								\
> +		static const char *___tp_str __tracepoint_string = str; \
> +		___tp_str;						\
> +	})
> +#define __tracepoint_string	__attribute__((section("__tracepoint_str")))
> +
>  #ifdef CONFIG_PERF_EVENTS
>  struct perf_event;
> 
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 4a4f6e1..ba321f1 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -1022,6 +1022,9 @@ extern struct list_head ftrace_events;
>  extern const char *__start___trace_bprintk_fmt[];
>  extern const char *__stop___trace_bprintk_fmt[];
> 
> +extern const char *__start___tracepoint_str[];
> +extern const char *__stop___tracepoint_str[];
> +
>  void trace_printk_init_buffers(void);
>  void trace_printk_start_comm(void);
>  int trace_keep_overwrite(struct tracer *tracer, u32 mask, int set);
> diff --git a/kernel/trace/trace_printk.c b/kernel/trace/trace_printk.c
> index a9077c1..2900817 100644
> --- a/kernel/trace/trace_printk.c
> +++ b/kernel/trace/trace_printk.c
> @@ -244,12 +244,31 @@ static const char **find_next(void *v, loff_t *pos)
>  {
>  	const char **fmt = v;
>  	int start_index;
> +	int last_index;
> 
>  	start_index = __stop___trace_bprintk_fmt - __start___trace_bprintk_fmt;
> 
>  	if (*pos < start_index)
>  		return __start___trace_bprintk_fmt + *pos;
> 
> +	/*
> +	 * The __tracepoint_str section is treated the same as the
> +	 * __trace_printk_fmt section. The difference is that the
> +	 * __trace_printk_fmt section should only be used by trace_printk()
> +	 * in a debugging environment, as if anything exists in that section
> +	 * the trace_prink() helper buffers are allocated, which would just
> +	 * waste space in a production environment.
> +	 *
> +	 * The __tracepoint_str sections on the other hand are used by
> +	 * tracepoints which need to map pointers to their strings to
> +	 * the ASCII text for userspace.
> +	 */
> +	last_index = start_index;
> +	start_index = __stop___tracepoint_str - __start___tracepoint_str;
> +
> +	if (*pos < last_index + start_index)
> +		return __start___tracepoint_str + (*pos - last_index);
> +
>  	return find_next_mod_format(start_index, v, fmt, pos);
>  }
> 
> -- 
> 1.7.10.4
> 
> 

Re: [for-next][PATCH 1/5] tracing: Add __tracepoint_string() to export string pointers

[Steven Rostedt]

On Thu, 22 Aug 2013 10:06:23 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:

> On Thu, Aug 22, 2013 at 12:32:26PM -0400, Steven Rostedt wrote:
> > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > 

> > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 

Since you and I are both using the same commit, I wont be able to
modify it to add you ack. :-/

-- Steve

Re: [for-next][PATCH 1/5] tracing: Add __tracepoint_string() to export string pointers

[Paul E. McKenney]

On Thu, Aug 22, 2013 at 01:27:53PM -0400, Steven Rostedt wrote:
> On Thu, 22 Aug 2013 10:06:23 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Thu, Aug 22, 2013 at 12:32:26PM -0400, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
> > > 
> 
> > > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> > 
> > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Since you and I are both using the same commit, I wont be able to
> modify it to add you ack. :-/

I would rather not rebase, so happy without my ack added!  ;-)

							Thanx, Paul