linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [for-next][PATCH 00/16] tracing: Updates for 4.15
@ 2017-10-06 18:06 Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest Steven Rostedt
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

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

Head SHA1: 6171a0310a06a7a0cb83713fa7068bdd4192de19


Colin Ian King (1):
      tracing: Remove redundant unread variable ret

Joel Fernandes (1):
      tracing: Remove obsolete sched_switch tracer selftest

Steven Rostedt (VMware) (7):
      tracing: Reverse the order of trace_types_lock and event_mutex
      ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
      ftrace: Add a ftrace_free_mem() function for modules to use
      ftrace: Allow module init functions to be traced
      ftrace: Save module init functions kallsyms symbols for tracing
      ftrace: Add freeing algorithm to free ftrace_mod_maps
      ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions

Tom Zanussi (7):
      tracing: Exclude 'generic fields' from histograms
      tracing: Remove lookups from tracing_map hitcount
      tracing: Increase tracing map KEYS_MAX size
      tracing: Make traceprobe parsing code reusable
      tracing: Clean up hist_field_flags enum
      tracing: Add hist_field_name() accessor
      tracing: Reimplement log2

----
 include/linux/ftrace.h           |  27 +++++
 include/linux/init.h             |   4 +-
 kernel/kallsyms.c                |  43 +++++++-
 kernel/module.c                  |   2 +
 kernel/trace/ftrace.c            | 230 ++++++++++++++++++++++++++++++++++++++-
 kernel/trace/ring_buffer.c       |  64 +++--------
 kernel/trace/trace.c             |  91 ++++++++++++++++
 kernel/trace/trace.h             |   9 +-
 kernel/trace/trace_events.c      |  31 +++---
 kernel/trace/trace_events_hist.c | 128 +++++++++++++++-------
 kernel/trace/trace_kprobe.c      |  18 +--
 kernel/trace/trace_probe.c       |  86 ---------------
 kernel/trace/trace_probe.h       |   7 --
 kernel/trace/trace_selftest.c    |  32 ------
 kernel/trace/trace_uprobe.c      |   2 +-
 kernel/trace/tracing_map.c       |   3 +-
 kernel/trace/tracing_map.h       |   2 +-
 17 files changed, 521 insertions(+), 258 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
@ 2017-10-06 18:06 ` Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 02/16] tracing: Remove redundant unread variable ret Steven Rostedt
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Ingo Molnar, kernel-team, Joel Fernandes

[-- Attachment #1: 0001-tracing-Remove-obsolete-sched_switch-tracer-selftest.patch --]
[-- Type: text/plain, Size: 2307 bytes --]

From: Joel Fernandes <joelaf@google.com>

Since commit 87d80de2800d087ea833cb79bc13f85ff34ed49f ("tracing: Remove
obsolete sched_switch tracer"), the sched_switch tracer selftest is no longer
used.  This patch removes the same.

Link: http://lkml.kernel.org/r/20170909065517.22262-1-joelaf@google.com

Cc: Ingo Molnar <mingo@redhat.com>
Cc: kernel-team@android.com
Signed-off-by: Joel Fernandes <joelaf@google.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.h          |  2 --
 kernel/trace/trace_selftest.c | 32 --------------------------------
 2 files changed, 34 deletions(-)

diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 652c682707cd..3c078e2ad777 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -738,8 +738,6 @@ extern int trace_selftest_startup_wakeup(struct tracer *trace,
 					 struct trace_array *tr);
 extern int trace_selftest_startup_nop(struct tracer *trace,
 					 struct trace_array *tr);
-extern int trace_selftest_startup_sched_switch(struct tracer *trace,
-					       struct trace_array *tr);
 extern int trace_selftest_startup_branch(struct tracer *trace,
 					 struct trace_array *tr);
 /*
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index b17ec642793b..364f78abdf47 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -1150,38 +1150,6 @@ trace_selftest_startup_wakeup(struct tracer *trace, struct trace_array *tr)
 }
 #endif /* CONFIG_SCHED_TRACER */
 
-#ifdef CONFIG_CONTEXT_SWITCH_TRACER
-int
-trace_selftest_startup_sched_switch(struct tracer *trace, struct trace_array *tr)
-{
-	unsigned long count;
-	int ret;
-
-	/* start the tracing */
-	ret = tracer_init(trace, tr);
-	if (ret) {
-		warn_failed_init_tracer(trace, ret);
-		return ret;
-	}
-
-	/* Sleep for a 1/10 of a second */
-	msleep(100);
-	/* stop the tracing. */
-	tracing_stop();
-	/* check the trace buffer */
-	ret = trace_test_buffer(&tr->trace_buffer, &count);
-	trace->reset(tr);
-	tracing_start();
-
-	if (!ret && !count) {
-		printk(KERN_CONT ".. no entries found ..");
-		ret = -1;
-	}
-
-	return ret;
-}
-#endif /* CONFIG_CONTEXT_SWITCH_TRACER */
-
 #ifdef CONFIG_BRANCH_TRACER
 int
 trace_selftest_startup_branch(struct tracer *trace, struct trace_array *tr)
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 02/16] tracing: Remove redundant unread variable ret
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest Steven Rostedt
@ 2017-10-06 18:06 ` Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 03/16] tracing: Reverse the order of trace_types_lock and event_mutex Steven Rostedt
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Colin Ian King

[-- Attachment #1: 0002-tracing-Remove-redundant-unread-variable-ret.patch --]
[-- Type: text/plain, Size: 1353 bytes --]

From: Colin Ian King <colin.king@canonical.com>

Integer ret is being assigned but never used and hence it is
redundant. Remove it, fixes clang warning:

trace_events_hist.c:1077:3: warning: Value stored to 'ret' is never read

Link: http://lkml.kernel.org/r/20170823112309.19383-1-colin.king@canonical.com

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 1c21d0e2a145..f123b5d0c226 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1062,7 +1062,7 @@ static void hist_trigger_show(struct seq_file *m,
 			      struct event_trigger_data *data, int n)
 {
 	struct hist_trigger_data *hist_data;
-	int n_entries, ret = 0;
+	int n_entries;
 
 	if (n > 0)
 		seq_puts(m, "\n\n");
@@ -1073,10 +1073,8 @@ static void hist_trigger_show(struct seq_file *m,
 
 	hist_data = data->private_data;
 	n_entries = print_entries(m, hist_data);
-	if (n_entries < 0) {
-		ret = n_entries;
+	if (n_entries < 0)
 		n_entries = 0;
-	}
 
 	seq_printf(m, "\nTotals:\n    Hits: %llu\n    Entries: %u\n    Dropped: %llu\n",
 		   (u64)atomic64_read(&hist_data->map->hits),
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 03/16] tracing: Reverse the order of trace_types_lock and event_mutex
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 02/16] tracing: Remove redundant unread variable ret Steven Rostedt
@ 2017-10-06 18:06 ` Steven Rostedt
  2017-10-06 18:06 ` [for-next][PATCH 04/16] ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler Steven Rostedt
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0003-tracing-Reverse-the-order-of-trace_types_lock-and-ev.patch --]
[-- Type: text/plain, Size: 5127 bytes --]

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

In order to make future changes where we need to call
tracing_set_clock() from within an event command, the order of
trace_types_lock and event_mutex must be reversed, as the event command
will hold event_mutex and the trace_types_lock is taken from within
tracing_set_clock().

Link: http://lkml.kernel.org/r/20170921162249.0dde3dca@gandalf.local.home

Requested-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c        |  5 +++++
 kernel/trace/trace_events.c | 31 +++++++++++++++----------------
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 752e5daf0896..5f1ac7d3402c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7687,6 +7687,7 @@ static int instance_mkdir(const char *name)
 	struct trace_array *tr;
 	int ret;
 
+	mutex_lock(&event_mutex);
 	mutex_lock(&trace_types_lock);
 
 	ret = -EEXIST;
@@ -7742,6 +7743,7 @@ static int instance_mkdir(const char *name)
 	list_add(&tr->list, &ftrace_trace_arrays);
 
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	return 0;
 
@@ -7753,6 +7755,7 @@ static int instance_mkdir(const char *name)
 
  out_unlock:
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	return ret;
 
@@ -7765,6 +7768,7 @@ static int instance_rmdir(const char *name)
 	int ret;
 	int i;
 
+	mutex_lock(&event_mutex);
 	mutex_lock(&trace_types_lock);
 
 	ret = -ENODEV;
@@ -7810,6 +7814,7 @@ static int instance_rmdir(const char *name)
 
  out_unlock:
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	return ret;
 }
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 87468398b9ed..ec0f9aa4e151 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1406,8 +1406,8 @@ static int subsystem_open(struct inode *inode, struct file *filp)
 		return -ENODEV;
 
 	/* Make sure the system still exists */
-	mutex_lock(&trace_types_lock);
 	mutex_lock(&event_mutex);
+	mutex_lock(&trace_types_lock);
 	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
 		list_for_each_entry(dir, &tr->systems, list) {
 			if (dir == inode->i_private) {
@@ -1421,8 +1421,8 @@ static int subsystem_open(struct inode *inode, struct file *filp)
 		}
 	}
  exit_loop:
-	mutex_unlock(&event_mutex);
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	if (!system)
 		return -ENODEV;
@@ -2294,15 +2294,15 @@ static void __add_event_to_tracers(struct trace_event_call *call);
 int trace_add_event_call(struct trace_event_call *call)
 {
 	int ret;
-	mutex_lock(&trace_types_lock);
 	mutex_lock(&event_mutex);
+	mutex_lock(&trace_types_lock);
 
 	ret = __register_event(call, NULL);
 	if (ret >= 0)
 		__add_event_to_tracers(call);
 
-	mutex_unlock(&event_mutex);
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 	return ret;
 }
 
@@ -2356,13 +2356,13 @@ int trace_remove_event_call(struct trace_event_call *call)
 {
 	int ret;
 
-	mutex_lock(&trace_types_lock);
 	mutex_lock(&event_mutex);
+	mutex_lock(&trace_types_lock);
 	down_write(&trace_event_sem);
 	ret = probe_remove_event_call(call);
 	up_write(&trace_event_sem);
-	mutex_unlock(&event_mutex);
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	return ret;
 }
@@ -2424,8 +2424,8 @@ static int trace_module_notify(struct notifier_block *self,
 {
 	struct module *mod = data;
 
-	mutex_lock(&trace_types_lock);
 	mutex_lock(&event_mutex);
+	mutex_lock(&trace_types_lock);
 	switch (val) {
 	case MODULE_STATE_COMING:
 		trace_module_add_events(mod);
@@ -2434,8 +2434,8 @@ static int trace_module_notify(struct notifier_block *self,
 		trace_module_remove_events(mod);
 		break;
 	}
-	mutex_unlock(&event_mutex);
 	mutex_unlock(&trace_types_lock);
+	mutex_unlock(&event_mutex);
 
 	return 0;
 }
@@ -2950,24 +2950,24 @@ create_event_toplevel_files(struct dentry *parent, struct trace_array *tr)
  * creates the event hierachry in the @parent/events directory.
  *
  * Returns 0 on success.
+ *
+ * Must be called with event_mutex held.
  */
 int event_trace_add_tracer(struct dentry *parent, struct trace_array *tr)
 {
 	int ret;
 
-	mutex_lock(&event_mutex);
+	lockdep_assert_held(&event_mutex);
 
 	ret = create_event_toplevel_files(parent, tr);
 	if (ret)
-		goto out_unlock;
+		goto out;
 
 	down_write(&trace_event_sem);
 	__trace_add_event_dirs(tr);
 	up_write(&trace_event_sem);
 
- out_unlock:
-	mutex_unlock(&event_mutex);
-
+ out:
 	return ret;
 }
 
@@ -2996,9 +2996,10 @@ early_event_add_tracer(struct dentry *parent, struct trace_array *tr)
 	return ret;
 }
 
+/* Must be called with event_mutex held */
 int event_trace_del_tracer(struct trace_array *tr)
 {
-	mutex_lock(&event_mutex);
+	lockdep_assert_held(&event_mutex);
 
 	/* Disable any event triggers and associated soft-disabled events */
 	clear_event_triggers(tr);
@@ -3019,8 +3020,6 @@ int event_trace_del_tracer(struct trace_array *tr)
 
 	tr->event_dir = NULL;
 
-	mutex_unlock(&event_mutex);
-
 	return 0;
 }
 
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 04/16] ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (2 preceding siblings ...)
  2017-10-06 18:06 ` [for-next][PATCH 03/16] tracing: Reverse the order of trace_types_lock and event_mutex Steven Rostedt
@ 2017-10-06 18:06 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 05/16] tracing: Exclude generic fields from histograms Steven Rostedt
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:06 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

[-- Attachment #1: 0004-ring-buffer-Rewrite-trace_recursive_-un-lock-to-be-s.patch --]
[-- Type: text/plain, Size: 4091 bytes --]

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

The current method to prevent the ring buffer from entering into a recursize
loop is to use a bitmask and set the bit that maps to the current context
(normal, softirq, irq or NMI), and if that bit was already set, it is
considered a recursive loop.

New code is being added that may require the ring buffer to be entered a
second time in the current context. The recursive locking prevents that from
happening. Instead of mapping a bitmask to the current context, just allow 4
levels of nesting in the ring buffer. This matches the 4 context levels that
it can already nest. It is highly unlikely to have more than two levels,
thus it should be fine when we add the second entry into the ring buffer. If
that proves to be a problem, we can always up the number to 8.

An added benefit is that reading preempt_count() to get the current level
adds a very slight but noticeable overhead. This removes that need.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ring_buffer.c | 64 ++++++++++++----------------------------------
 1 file changed, 17 insertions(+), 47 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 81279c6602ff..f6ee9b1ef62a 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -2538,61 +2538,29 @@ rb_wakeups(struct ring_buffer *buffer, struct ring_buffer_per_cpu *cpu_buffer)
  * The lock and unlock are done within a preempt disable section.
  * The current_context per_cpu variable can only be modified
  * by the current task between lock and unlock. But it can
- * be modified more than once via an interrupt. To pass this
- * information from the lock to the unlock without having to
- * access the 'in_interrupt()' functions again (which do show
- * a bit of overhead in something as critical as function tracing,
- * we use a bitmask trick.
+ * be modified more than once via an interrupt. There are four
+ * different contexts that we need to consider.
  *
- *  bit 0 =  NMI context
- *  bit 1 =  IRQ context
- *  bit 2 =  SoftIRQ context
- *  bit 3 =  normal context.
+ *  Normal context.
+ *  SoftIRQ context
+ *  IRQ context
+ *  NMI context
  *
- * This works because this is the order of contexts that can
- * preempt other contexts. A SoftIRQ never preempts an IRQ
- * context.
- *
- * When the context is determined, the corresponding bit is
- * checked and set (if it was set, then a recursion of that context
- * happened).
- *
- * On unlock, we need to clear this bit. To do so, just subtract
- * 1 from the current_context and AND it to itself.
- *
- * (binary)
- *  101 - 1 = 100
- *  101 & 100 = 100 (clearing bit zero)
- *
- *  1010 - 1 = 1001
- *  1010 & 1001 = 1000 (clearing bit 1)
- *
- * The least significant bit can be cleared this way, and it
- * just so happens that it is the same bit corresponding to
- * the current context.
+ * If for some reason the ring buffer starts to recurse, we
+ * only allow that to happen at most 4 times (one for each
+ * context). If it happens 5 times, then we consider this a
+ * recusive loop and do not let it go further.
  */
 
 static __always_inline int
 trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer)
 {
-	unsigned int val = cpu_buffer->current_context;
-	int bit;
-
-	if (in_interrupt()) {
-		if (in_nmi())
-			bit = RB_CTX_NMI;
-		else if (in_irq())
-			bit = RB_CTX_IRQ;
-		else
-			bit = RB_CTX_SOFTIRQ;
-	} else
-		bit = RB_CTX_NORMAL;
-
-	if (unlikely(val & (1 << bit)))
+	if (cpu_buffer->current_context >= 4)
 		return 1;
 
-	val |= (1 << bit);
-	cpu_buffer->current_context = val;
+	cpu_buffer->current_context++;
+	/* Interrupts must see this update */
+	barrier();
 
 	return 0;
 }
@@ -2600,7 +2568,9 @@ trace_recursive_lock(struct ring_buffer_per_cpu *cpu_buffer)
 static __always_inline void
 trace_recursive_unlock(struct ring_buffer_per_cpu *cpu_buffer)
 {
-	cpu_buffer->current_context &= cpu_buffer->current_context - 1;
+	/* Don't let the dec leak out */
+	barrier();
+	cpu_buffer->current_context--;
 }
 
 /**
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 05/16] tracing: Exclude generic fields from histograms
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (3 preceding siblings ...)
  2017-10-06 18:06 ` [for-next][PATCH 04/16] ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 06/16] tracing: Remove lookups from tracing_map hitcount Steven Rostedt
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0005-tracing-Exclude-generic-fields-from-histograms.patch --]
[-- Type: text/plain, Size: 1392 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

There are a small number of 'generic fields' (comm/COMM/cpu/CPU) that
are found by trace_find_event_field() but are only meant for
filtering.  Specifically, they unlike normal fields, they have a size
of 0 and thus wreak havoc when used as a histogram key.

Exclude these (return -EINVAL) when used as histogram keys.

Link: http://lkml.kernel.org/r/956154cbc3e8a4f0633d619b886c97f0f0edf7b4.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index f123b5d0c226..121d56850f24 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -450,7 +450,7 @@ static int create_val_field(struct hist_trigger_data *hist_data,
 	}
 
 	field = trace_find_event_field(file->event_call, field_name);
-	if (!field) {
+	if (!field || !field->size) {
 		ret = -EINVAL;
 		goto out;
 	}
@@ -548,7 +548,7 @@ static int create_key_field(struct hist_trigger_data *hist_data,
 		}
 
 		field = trace_find_event_field(file->event_call, field_name);
-		if (!field) {
+		if (!field || !field->size) {
 			ret = -EINVAL;
 			goto out;
 		}
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 06/16] tracing: Remove lookups from tracing_map hitcount
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (4 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 05/16] tracing: Exclude generic fields from histograms Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 07/16] tracing: Increase tracing map KEYS_MAX size Steven Rostedt
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0006-tracing-Remove-lookups-from-tracing_map-hitcount.patch --]
[-- Type: text/plain, Size: 1047 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

Lookups inflate the hitcount, making it essentially useless.  Only
inserts and updates should really affect the hitcount anyway, so
explicitly filter lookups out.

Link: http://lkml.kernel.org/r/c8d9dc39d269a8abf88bf4102d0dfc65deb0fc7f.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/tracing_map.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index 305039b122fa..07e75344725b 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -428,7 +428,8 @@ __tracing_map_insert(struct tracing_map *map, void *key, bool lookup_only)
 
 		if (test_key && test_key == key_hash && entry->val &&
 		    keys_match(key, entry->val->key, map->key_size)) {
-			atomic64_inc(&map->hits);
+			if (!lookup_only)
+				atomic64_inc(&map->hits);
 			return entry->val;
 		}
 
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 07/16] tracing: Increase tracing map KEYS_MAX size
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (5 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 06/16] tracing: Remove lookups from tracing_map hitcount Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 08/16] tracing: Make traceprobe parsing code reusable Steven Rostedt
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0007-tracing-Increase-tracing-map-KEYS_MAX-size.patch --]
[-- Type: text/plain, Size: 970 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

The current default for the number of subkeys in a compound key is 2,
which is too restrictive.  Increase it to a more realistic value of 3.

Link: http://lkml.kernel.org/r/b6952cca06d1f912eba33804a6fd6069b3847d44.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/tracing_map.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/tracing_map.h b/kernel/trace/tracing_map.h
index 618838f5f30a..f0975110b967 100644
--- a/kernel/trace/tracing_map.h
+++ b/kernel/trace/tracing_map.h
@@ -5,7 +5,7 @@
 #define TRACING_MAP_BITS_MAX		17
 #define TRACING_MAP_BITS_MIN		7
 
-#define TRACING_MAP_KEYS_MAX		2
+#define TRACING_MAP_KEYS_MAX		3
 #define TRACING_MAP_VALS_MAX		3
 #define TRACING_MAP_FIELDS_MAX		(TRACING_MAP_KEYS_MAX + \
 					 TRACING_MAP_VALS_MAX)
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 08/16] tracing: Make traceprobe parsing code reusable
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (6 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 07/16] tracing: Increase tracing map KEYS_MAX size Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 09/16] tracing: Clean up hist_field_flags enum Steven Rostedt
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0008-tracing-Make-traceprobe-parsing-code-reusable.patch --]
[-- Type: text/plain, Size: 9214 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

traceprobe_probes_write() and traceprobe_command() actually contain
nothing that ties them to kprobes - the code is generically useful for
similar types of parsing elsewhere, so separate it out and move it to
trace.c/trace.h.

Other than moving it, the only change is in naming:
traceprobe_probes_write() becomes trace_parse_run_command() and
traceprobe_command() becomes trace_run_command().

Link: http://lkml.kernel.org/r/ae5c26ea40c196a8986854d921eb6e713ede7e3f.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace.c        | 86 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace.h        |  7 ++++
 kernel/trace/trace_kprobe.c | 18 +++++-----
 kernel/trace/trace_probe.c  | 86 ---------------------------------------------
 kernel/trace/trace_probe.h  |  7 ----
 kernel/trace/trace_uprobe.c |  2 +-
 6 files changed, 103 insertions(+), 103 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5f1ac7d3402c..73e67b68c53b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -8281,6 +8281,92 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
 }
 EXPORT_SYMBOL_GPL(ftrace_dump);
 
+int trace_run_command(const char *buf, int (*createfn)(int, char **))
+{
+	char **argv;
+	int argc, ret;
+
+	argc = 0;
+	ret = 0;
+	argv = argv_split(GFP_KERNEL, buf, &argc);
+	if (!argv)
+		return -ENOMEM;
+
+	if (argc)
+		ret = createfn(argc, argv);
+
+	argv_free(argv);
+
+	return ret;
+}
+
+#define WRITE_BUFSIZE  4096
+
+ssize_t trace_parse_run_command(struct file *file, const char __user *buffer,
+				size_t count, loff_t *ppos,
+				int (*createfn)(int, char **))
+{
+	char *kbuf, *buf, *tmp;
+	int ret = 0;
+	size_t done = 0;
+	size_t size;
+
+	kbuf = kmalloc(WRITE_BUFSIZE, GFP_KERNEL);
+	if (!kbuf)
+		return -ENOMEM;
+
+	while (done < count) {
+		size = count - done;
+
+		if (size >= WRITE_BUFSIZE)
+			size = WRITE_BUFSIZE - 1;
+
+		if (copy_from_user(kbuf, buffer + done, size)) {
+			ret = -EFAULT;
+			goto out;
+		}
+		kbuf[size] = '\0';
+		buf = kbuf;
+		do {
+			tmp = strchr(buf, '\n');
+			if (tmp) {
+				*tmp = '\0';
+				size = tmp - buf + 1;
+			} else {
+				size = strlen(buf);
+				if (done + size < count) {
+					if (buf != kbuf)
+						break;
+					/* This can accept WRITE_BUFSIZE - 2 ('\n' + '\0') */
+					pr_warn("Line length is too long: Should be less than %d\n",
+						WRITE_BUFSIZE - 2);
+					ret = -EINVAL;
+					goto out;
+				}
+			}
+			done += size;
+
+			/* Remove comments */
+			tmp = strchr(buf, '#');
+
+			if (tmp)
+				*tmp = '\0';
+
+			ret = trace_run_command(buf, createfn);
+			if (ret)
+				goto out;
+			buf += size;
+
+		} while (done < count);
+	}
+	ret = done;
+
+out:
+	kfree(kbuf);
+
+	return ret;
+}
+
 __init static int tracer_alloc_buffers(void)
 {
 	int ring_buf_size;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 3c078e2ad777..f8343eb3c1f9 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1752,6 +1752,13 @@ void trace_printk_start_comm(void);
 int trace_keep_overwrite(struct tracer *tracer, u32 mask, int set);
 int set_tracer_flag(struct trace_array *tr, unsigned int mask, int enabled);
 
+#define MAX_EVENT_NAME_LEN	64
+
+extern int trace_run_command(const char *buf, int (*createfn)(int, char**));
+extern ssize_t trace_parse_run_command(struct file *file,
+		const char __user *buffer, size_t count, loff_t *ppos,
+		int (*createfn)(int, char**));
+
 /*
  * Normal trace_printk() and friends allocates special buffers
  * to do the manipulation, as well as saves the print formats
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8a907e12b6b9..af6134f2e597 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -907,8 +907,8 @@ static int probes_open(struct inode *inode, struct file *file)
 static ssize_t probes_write(struct file *file, const char __user *buffer,
 			    size_t count, loff_t *ppos)
 {
-	return traceprobe_probes_write(file, buffer, count, ppos,
-			create_trace_kprobe);
+	return trace_parse_run_command(file, buffer, count, ppos,
+				       create_trace_kprobe);
 }
 
 static const struct file_operations kprobe_events_ops = {
@@ -1433,9 +1433,9 @@ static __init int kprobe_trace_self_tests_init(void)
 
 	pr_info("Testing kprobe tracing: ");
 
-	ret = traceprobe_command("p:testprobe kprobe_trace_selftest_target "
-				  "$stack $stack0 +0($stack)",
-				  create_trace_kprobe);
+	ret = trace_run_command("p:testprobe kprobe_trace_selftest_target "
+				"$stack $stack0 +0($stack)",
+				create_trace_kprobe);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on probing function entry.\n");
 		warn++;
@@ -1455,8 +1455,8 @@ static __init int kprobe_trace_self_tests_init(void)
 		}
 	}
 
-	ret = traceprobe_command("r:testprobe2 kprobe_trace_selftest_target "
-				  "$retval", create_trace_kprobe);
+	ret = trace_run_command("r:testprobe2 kprobe_trace_selftest_target "
+				"$retval", create_trace_kprobe);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on probing function return.\n");
 		warn++;
@@ -1526,13 +1526,13 @@ static __init int kprobe_trace_self_tests_init(void)
 			disable_trace_kprobe(tk, file);
 	}
 
-	ret = traceprobe_command("-:testprobe", create_trace_kprobe);
+	ret = trace_run_command("-:testprobe", create_trace_kprobe);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on deleting a probe.\n");
 		warn++;
 	}
 
-	ret = traceprobe_command("-:testprobe2", create_trace_kprobe);
+	ret = trace_run_command("-:testprobe2", create_trace_kprobe);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on deleting a probe.\n");
 		warn++;
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 52478f033f88..d59357308677 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -623,92 +623,6 @@ void traceprobe_free_probe_arg(struct probe_arg *arg)
 	kfree(arg->comm);
 }
 
-int traceprobe_command(const char *buf, int (*createfn)(int, char **))
-{
-	char **argv;
-	int argc, ret;
-
-	argc = 0;
-	ret = 0;
-	argv = argv_split(GFP_KERNEL, buf, &argc);
-	if (!argv)
-		return -ENOMEM;
-
-	if (argc)
-		ret = createfn(argc, argv);
-
-	argv_free(argv);
-
-	return ret;
-}
-
-#define WRITE_BUFSIZE  4096
-
-ssize_t traceprobe_probes_write(struct file *file, const char __user *buffer,
-				size_t count, loff_t *ppos,
-				int (*createfn)(int, char **))
-{
-	char *kbuf, *buf, *tmp;
-	int ret = 0;
-	size_t done = 0;
-	size_t size;
-
-	kbuf = kmalloc(WRITE_BUFSIZE, GFP_KERNEL);
-	if (!kbuf)
-		return -ENOMEM;
-
-	while (done < count) {
-		size = count - done;
-
-		if (size >= WRITE_BUFSIZE)
-			size = WRITE_BUFSIZE - 1;
-
-		if (copy_from_user(kbuf, buffer + done, size)) {
-			ret = -EFAULT;
-			goto out;
-		}
-		kbuf[size] = '\0';
-		buf = kbuf;
-		do {
-			tmp = strchr(buf, '\n');
-			if (tmp) {
-				*tmp = '\0';
-				size = tmp - buf + 1;
-			} else {
-				size = strlen(buf);
-				if (done + size < count) {
-					if (buf != kbuf)
-						break;
-					/* This can accept WRITE_BUFSIZE - 2 ('\n' + '\0') */
-					pr_warn("Line length is too long: Should be less than %d\n",
-						WRITE_BUFSIZE - 2);
-					ret = -EINVAL;
-					goto out;
-				}
-			}
-			done += size;
-
-			/* Remove comments */
-			tmp = strchr(buf, '#');
-
-			if (tmp)
-				*tmp = '\0';
-
-			ret = traceprobe_command(buf, createfn);
-			if (ret)
-				goto out;
-			buf += size;
-
-		} while (done < count);
-	}
-	ret = done;
-
-out:
-	kfree(kbuf);
-
-	return ret;
-}
-
 static int __set_print_fmt(struct trace_probe *tp, char *buf, int len,
 			   bool is_return)
 {
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 903273c93e61..fb66e3eaa192 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -42,7 +42,6 @@
 
 #define MAX_TRACE_ARGS		128
 #define MAX_ARGSTR_LEN		63
-#define MAX_EVENT_NAME_LEN	64
 #define MAX_STRING_SIZE		PATH_MAX
 
 /* Reserved field names */
@@ -356,12 +355,6 @@ extern void traceprobe_free_probe_arg(struct probe_arg *arg);
 
 extern int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset);
 
-extern ssize_t traceprobe_probes_write(struct file *file,
-		const char __user *buffer, size_t count, loff_t *ppos,
-		int (*createfn)(int, char**));
-
-extern int traceprobe_command(const char *buf, int (*createfn)(int, char**));
-
 /* Sum up total data length for dynamic arraies (strings) */
 static nokprobe_inline int
 __get_data_size(struct trace_probe *tp, struct pt_regs *regs)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 4525e0271a53..b34965e62fb9 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -651,7 +651,7 @@ static int probes_open(struct inode *inode, struct file *file)
 static ssize_t probes_write(struct file *file, const char __user *buffer,
 			    size_t count, loff_t *ppos)
 {
-	return traceprobe_probes_write(file, buffer, count, ppos, create_trace_uprobe);
+	return trace_parse_run_command(file, buffer, count, ppos, create_trace_uprobe);
 }
 
 static const struct file_operations uprobe_events_ops = {
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 09/16] tracing: Clean up hist_field_flags enum
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (7 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 08/16] tracing: Make traceprobe parsing code reusable Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 10/16] tracing: Add hist_field_name() accessor Steven Rostedt
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0009-tracing-Clean-up-hist_field_flags-enum.patch --]
[-- Type: text/plain, Size: 1580 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

As we add more flags, specifying explicit integers for the flag values
becomes more unwieldy and error-prone - switch them over to left-shift
values.

Link: http://lkml.kernel.org/r/e644e4fb7665aec015f4a2d84a2f990d3dd5b8a1.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 121d56850f24..0c7ec3048624 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -110,16 +110,16 @@ DEFINE_HIST_FIELD_FN(u8);
 #define HIST_KEY_SIZE_MAX	(MAX_FILTER_STR_VAL + HIST_STACKTRACE_SIZE)
 
 enum hist_field_flags {
-	HIST_FIELD_FL_HITCOUNT		= 1,
-	HIST_FIELD_FL_KEY		= 2,
-	HIST_FIELD_FL_STRING		= 4,
-	HIST_FIELD_FL_HEX		= 8,
-	HIST_FIELD_FL_SYM		= 16,
-	HIST_FIELD_FL_SYM_OFFSET	= 32,
-	HIST_FIELD_FL_EXECNAME		= 64,
-	HIST_FIELD_FL_SYSCALL		= 128,
-	HIST_FIELD_FL_STACKTRACE	= 256,
-	HIST_FIELD_FL_LOG2		= 512,
+	HIST_FIELD_FL_HITCOUNT		= 1 << 0,
+	HIST_FIELD_FL_KEY		= 1 << 1,
+	HIST_FIELD_FL_STRING		= 1 << 2,
+	HIST_FIELD_FL_HEX		= 1 << 3,
+	HIST_FIELD_FL_SYM		= 1 << 4,
+	HIST_FIELD_FL_SYM_OFFSET	= 1 << 5,
+	HIST_FIELD_FL_EXECNAME		= 1 << 6,
+	HIST_FIELD_FL_SYSCALL		= 1 << 7,
+	HIST_FIELD_FL_STACKTRACE	= 1 << 8,
+	HIST_FIELD_FL_LOG2		= 1 << 9,
 };
 
 struct hist_trigger_attrs {
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 10/16] tracing: Add hist_field_name() accessor
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (8 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 09/16] tracing: Clean up hist_field_flags enum Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 11/16] tracing: Reimplement log2 Steven Rostedt
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0010-tracing-Add-hist_field_name-accessor.patch --]
[-- Type: text/plain, Size: 6125 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

In preparation for hist_fields that won't be strictly based on
trace_event_fields, add a new hist_field_name() accessor to allow that
flexibility and update associated users.

Link: http://lkml.kernel.org/r/5b5a2d36dde067cbbe2434b10f06daac27b7dbd5.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 67 +++++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 0c7ec3048624..34edf5fd85fd 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -146,6 +146,23 @@ struct hist_trigger_data {
 	struct tracing_map		*map;
 };
 
+static const char *hist_field_name(struct hist_field *field,
+				   unsigned int level)
+{
+	const char *field_name = "";
+
+	if (level > 1)
+		return field_name;
+
+	if (field->field)
+		field_name = field->field->name;
+
+	if (field_name == NULL)
+		field_name = "";
+
+	return field_name;
+}
+
 static hist_field_fn_t select_value_fn(int field_size, int field_is_signed)
 {
 	hist_field_fn_t fn = NULL;
@@ -653,7 +670,6 @@ static int is_descending(const char *str)
 static int create_sort_keys(struct hist_trigger_data *hist_data)
 {
 	char *fields_str = hist_data->attrs->sort_key_str;
-	struct ftrace_event_field *field = NULL;
 	struct tracing_map_sort_key *sort_key;
 	int descending, ret = 0;
 	unsigned int i, j;
@@ -670,7 +686,9 @@ static int create_sort_keys(struct hist_trigger_data *hist_data)
 	}
 
 	for (i = 0; i < TRACING_MAP_SORT_KEYS_MAX; i++) {
+		struct hist_field *hist_field;
 		char *field_str, *field_name;
+		const char *test_name;
 
 		sort_key = &hist_data->sort_keys[i];
 
@@ -703,8 +721,10 @@ static int create_sort_keys(struct hist_trigger_data *hist_data)
 		}
 
 		for (j = 1; j < hist_data->n_fields; j++) {
-			field = hist_data->fields[j]->field;
-			if (field && (strcmp(field_name, field->name) == 0)) {
+			hist_field = hist_data->fields[j];
+			test_name = hist_field_name(hist_field, 0);
+
+			if (strcmp(field_name, test_name) == 0) {
 				sort_key->field_idx = j;
 				descending = is_descending(field_str);
 				if (descending < 0) {
@@ -952,6 +972,7 @@ hist_trigger_entry_print(struct seq_file *m,
 	struct hist_field *key_field;
 	char str[KSYM_SYMBOL_LEN];
 	bool multiline = false;
+	const char *field_name;
 	unsigned int i;
 	u64 uval;
 
@@ -963,26 +984,27 @@ hist_trigger_entry_print(struct seq_file *m,
 		if (i > hist_data->n_vals)
 			seq_puts(m, ", ");
 
+		field_name = hist_field_name(key_field, 0);
+
 		if (key_field->flags & HIST_FIELD_FL_HEX) {
 			uval = *(u64 *)(key + key_field->offset);
-			seq_printf(m, "%s: %llx",
-				   key_field->field->name, uval);
+			seq_printf(m, "%s: %llx", field_name, uval);
 		} else if (key_field->flags & HIST_FIELD_FL_SYM) {
 			uval = *(u64 *)(key + key_field->offset);
 			sprint_symbol_no_offset(str, uval);
-			seq_printf(m, "%s: [%llx] %-45s",
-				   key_field->field->name, uval, str);
+			seq_printf(m, "%s: [%llx] %-45s", field_name,
+				   uval, str);
 		} else if (key_field->flags & HIST_FIELD_FL_SYM_OFFSET) {
 			uval = *(u64 *)(key + key_field->offset);
 			sprint_symbol(str, uval);
-			seq_printf(m, "%s: [%llx] %-55s",
-				   key_field->field->name, uval, str);
+			seq_printf(m, "%s: [%llx] %-55s", field_name,
+				   uval, str);
 		} else if (key_field->flags & HIST_FIELD_FL_EXECNAME) {
 			char *comm = elt->private_data;
 
 			uval = *(u64 *)(key + key_field->offset);
-			seq_printf(m, "%s: %-16s[%10llu]",
-				   key_field->field->name, comm, uval);
+			seq_printf(m, "%s: %-16s[%10llu]", field_name,
+				   comm, uval);
 		} else if (key_field->flags & HIST_FIELD_FL_SYSCALL) {
 			const char *syscall_name;
 
@@ -991,8 +1013,8 @@ hist_trigger_entry_print(struct seq_file *m,
 			if (!syscall_name)
 				syscall_name = "unknown_syscall";
 
-			seq_printf(m, "%s: %-30s[%3llu]",
-				   key_field->field->name, syscall_name, uval);
+			seq_printf(m, "%s: %-30s[%3llu]", field_name,
+				   syscall_name, uval);
 		} else if (key_field->flags & HIST_FIELD_FL_STACKTRACE) {
 			seq_puts(m, "stacktrace:\n");
 			hist_trigger_stacktrace_print(m,
@@ -1000,15 +1022,14 @@ hist_trigger_entry_print(struct seq_file *m,
 						      HIST_STACKTRACE_DEPTH);
 			multiline = true;
 		} else if (key_field->flags & HIST_FIELD_FL_LOG2) {
-			seq_printf(m, "%s: ~ 2^%-2llu", key_field->field->name,
+			seq_printf(m, "%s: ~ 2^%-2llu", field_name,
 				   *(u64 *)(key + key_field->offset));
 		} else if (key_field->flags & HIST_FIELD_FL_STRING) {
-			seq_printf(m, "%s: %-50s", key_field->field->name,
+			seq_printf(m, "%s: %-50s", field_name,
 				   (char *)(key + key_field->offset));
 		} else {
 			uval = *(u64 *)(key + key_field->offset);
-			seq_printf(m, "%s: %10llu", key_field->field->name,
-				   uval);
+			seq_printf(m, "%s: %10llu", field_name, uval);
 		}
 	}
 
@@ -1021,13 +1042,13 @@ hist_trigger_entry_print(struct seq_file *m,
 		   tracing_map_read_sum(elt, HITCOUNT_IDX));
 
 	for (i = 1; i < hist_data->n_vals; i++) {
+		field_name = hist_field_name(hist_data->fields[i], 0);
+
 		if (hist_data->fields[i]->flags & HIST_FIELD_FL_HEX) {
-			seq_printf(m, "  %s: %10llx",
-				   hist_data->fields[i]->field->name,
+			seq_printf(m, "  %s: %10llx", field_name,
 				   tracing_map_read_sum(elt, i));
 		} else {
-			seq_printf(m, "  %s: %10llu",
-				   hist_data->fields[i]->field->name,
+			seq_printf(m, "  %s: %10llu", field_name,
 				   tracing_map_read_sum(elt, i));
 		}
 	}
@@ -1140,7 +1161,9 @@ static const char *get_hist_field_flags(struct hist_field *hist_field)
 
 static void hist_field_print(struct seq_file *m, struct hist_field *hist_field)
 {
-	seq_printf(m, "%s", hist_field->field->name);
+	const char *field_name = hist_field_name(hist_field, 0);
+
+	seq_printf(m, "%s", field_name);
 	if (hist_field->flags) {
 		const char *flags_str = get_hist_field_flags(hist_field);
 
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 11/16] tracing: Reimplement log2
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (9 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 10/16] tracing: Add hist_field_name() accessor Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 12/16] ftrace: Add a ftrace_free_mem() function for modules to use Steven Rostedt
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Tom Zanussi

[-- Attachment #1: 0011-tracing-Reimplement-log2.patch --]
[-- Type: text/plain, Size: 3864 bytes --]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

log2 as currently implemented applies only to u64 trace_event_field
derived fields, and assumes that anything it's applied to is a u64
field.

To prepare for synthetic fields like latencies, log2 should be
applicable to those as well, so take the opportunity now to fix the
current problems as well as expand to more general uses.

log2 should be thought of as a chaining function rather than a field
type.  To enable this as well as possible future function
implementations, add a hist_field operand array into the hist_field
definition for this purpose, and make use of it to implement the log2
'function'.

Link: http://lkml.kernel.org/r/b47f93fc0b87b36eccf716b0c018f3a71e1f1111.1506105045.git.tom.zanussi@linux.intel.com

Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 34edf5fd85fd..1e1558c99d56 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -28,12 +28,16 @@ struct hist_field;
 
 typedef u64 (*hist_field_fn_t) (struct hist_field *field, void *event);
 
+#define HIST_FIELD_OPERANDS_MAX	2
+
 struct hist_field {
 	struct ftrace_event_field	*field;
 	unsigned long			flags;
 	hist_field_fn_t			fn;
 	unsigned int			size;
 	unsigned int			offset;
+	unsigned int                    is_signed;
+	struct hist_field		*operands[HIST_FIELD_OPERANDS_MAX];
 };
 
 static u64 hist_field_none(struct hist_field *field, void *event)
@@ -71,7 +75,9 @@ static u64 hist_field_pstring(struct hist_field *hist_field, void *event)
 
 static u64 hist_field_log2(struct hist_field *hist_field, void *event)
 {
-	u64 val = *(u64 *)(event + hist_field->field->offset);
+	struct hist_field *operand = hist_field->operands[0];
+
+	u64 val = operand->fn(operand, event);
 
 	return (u64) ilog2(roundup_pow_of_two(val));
 }
@@ -156,6 +162,8 @@ static const char *hist_field_name(struct hist_field *field,
 
 	if (field->field)
 		field_name = field->field->name;
+	else if (field->flags & HIST_FIELD_FL_LOG2)
+		field_name = hist_field_name(field->operands[0], ++level);
 
 	if (field_name == NULL)
 		field_name = "";
@@ -357,8 +365,20 @@ static const struct tracing_map_ops hist_trigger_elt_comm_ops = {
 	.elt_init	= hist_trigger_elt_comm_init,
 };
 
-static void destroy_hist_field(struct hist_field *hist_field)
+static void destroy_hist_field(struct hist_field *hist_field,
+			       unsigned int level)
 {
+	unsigned int i;
+
+	if (level > 2)
+		return;
+
+	if (!hist_field)
+		return;
+
+	for (i = 0; i < HIST_FIELD_OPERANDS_MAX; i++)
+		destroy_hist_field(hist_field->operands[i], level + 1);
+
 	kfree(hist_field);
 }
 
@@ -385,7 +405,10 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field,
 	}
 
 	if (flags & HIST_FIELD_FL_LOG2) {
+		unsigned long fl = flags & ~HIST_FIELD_FL_LOG2;
 		hist_field->fn = hist_field_log2;
+		hist_field->operands[0] = create_hist_field(field, fl);
+		hist_field->size = hist_field->operands[0]->size;
 		goto out;
 	}
 
@@ -405,7 +428,7 @@ static struct hist_field *create_hist_field(struct ftrace_event_field *field,
 		hist_field->fn = select_value_fn(field->size,
 						 field->is_signed);
 		if (!hist_field->fn) {
-			destroy_hist_field(hist_field);
+			destroy_hist_field(hist_field, 0);
 			return NULL;
 		}
 	}
@@ -422,7 +445,7 @@ static void destroy_hist_fields(struct hist_trigger_data *hist_data)
 
 	for (i = 0; i < TRACING_MAP_FIELDS_MAX; i++) {
 		if (hist_data->fields[i]) {
-			destroy_hist_field(hist_data->fields[i]);
+			destroy_hist_field(hist_data->fields[i], 0);
 			hist_data->fields[i] = NULL;
 		}
 	}
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 12/16] ftrace: Add a ftrace_free_mem() function for modules to use
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (10 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 11/16] tracing: Reimplement log2 Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 13/16] ftrace: Allow module init functions to be traced Steven Rostedt
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

[-- Attachment #1: 0012-ftrace-Add-a-ftrace_free_mem-function-for-modules-to.patch --]
[-- Type: text/plain, Size: 1977 bytes --]

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

In order to be able to trace module init functions, the module code needs to
tell ftrace what is being freed when the init sections are freed. Use the
code that the main init calls to tell ftrace to free the main init sections.
This requires passing in a start and end address to free.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/ftrace.h |  2 ++
 kernel/trace/ftrace.c  | 14 +++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 2e028854bac7..47fc404ad233 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -151,8 +151,10 @@ struct ftrace_ops_hash {
 };
 
 void ftrace_free_init_mem(void);
+void ftrace_free_mem(void *start, void *end);
 #else
 static inline void ftrace_free_init_mem(void) { }
+static inline void ftrace_free_mem(void *start, void *end) { }
 #endif
 
 /*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6abfafd7f173..84cb5928665a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5868,10 +5868,10 @@ void ftrace_module_init(struct module *mod)
 }
 #endif /* CONFIG_MODULES */
 
-void __init ftrace_free_init_mem(void)
+void ftrace_free_mem(void *start_ptr, void *end_ptr)
 {
-	unsigned long start = (unsigned long)(&__init_begin);
-	unsigned long end = (unsigned long)(&__init_end);
+	unsigned long start = (unsigned long)(start_ptr);
+	unsigned long end = (unsigned long)(end_ptr);
 	struct ftrace_page **last_pg = &ftrace_pages_start;
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
@@ -5913,6 +5913,14 @@ void __init ftrace_free_init_mem(void)
 	mutex_unlock(&ftrace_lock);
 }
 
+void __init ftrace_free_init_mem(void)
+{
+	void *start = (void *)(&__init_begin);
+	void *end = (void *)(&__init_end);
+
+	ftrace_free_mem(start, end);
+}
+
 void __init ftrace_init(void)
 {
 	extern unsigned long __start_mcount_loc[];
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 13/16] ftrace: Allow module init functions to be traced
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (11 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 12/16] ftrace: Add a ftrace_free_mem() function for modules to use Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 14/16] ftrace: Save module init functions kallsyms symbols for tracing Steven Rostedt
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jessica Yu, Rusty Russell

[-- Attachment #1: 0013-ftrace-Allow-module-init-functions-to-be-traced.patch --]
[-- Type: text/plain, Size: 2662 bytes --]

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

Allow for module init sections to be traced as well as core kernel init
sections. Now that filtering modules functions can be stored, for when they
are loaded, it makes sense to be able to trace them.

Cc: Jessica Yu <jeyu@kernel.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/init.h  | 4 +---
 kernel/module.c       | 2 ++
 kernel/trace/ftrace.c | 6 ++++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/linux/init.h b/include/linux/init.h
index 94769d687cf0..a779c1816437 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -39,7 +39,7 @@
 
 /* These are for everybody (although not all archs will actually
    discard it in modules) */
-#define __init		__section(.init.text) __cold __inittrace __latent_entropy
+#define __init		__section(.init.text) __cold  __latent_entropy
 #define __initdata	__section(.init.data)
 #define __initconst	__section(.init.rodata)
 #define __exitdata	__section(.exit.data)
@@ -68,10 +68,8 @@
 
 #ifdef MODULE
 #define __exitused
-#define __inittrace notrace
 #else
 #define __exitused  __used
-#define __inittrace
 #endif
 
 #define __exit          __section(.exit.text) __exitused __cold notrace
diff --git a/kernel/module.c b/kernel/module.c
index de66ec825992..58bca427ac3f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3473,6 +3473,8 @@ static noinline int do_init_module(struct module *mod)
 	if (!mod->async_probe_requested && (current->flags & PF_USED_ASYNC))
 		async_synchronize_full();
 
+	ftrace_free_mem(mod->init_layout.base, mod->init_layout.base +
+			mod->init_layout.size);
 	mutex_lock(&module_mutex);
 	/* Drop initial reference. */
 	module_put(mod);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 84cb5928665a..d7297e866e4a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5752,7 +5752,8 @@ void ftrace_release_mod(struct module *mod)
 	last_pg = &ftrace_pages_start;
 	for (pg = ftrace_pages_start; pg; pg = *last_pg) {
 		rec = &pg->records[0];
-		if (within_module_core(rec->ip, mod)) {
+		if (within_module_core(rec->ip, mod) ||
+		    within_module_init(rec->ip, mod)) {
 			/*
 			 * As core pages are first, the first
 			 * page should never be a module page.
@@ -5821,7 +5822,8 @@ void ftrace_module_enable(struct module *mod)
 		 * not part of this module, then skip this pg,
 		 * which the "break" will do.
 		 */
-		if (!within_module_core(rec->ip, mod))
+		if (!within_module_core(rec->ip, mod) &&
+		    !within_module_init(rec->ip, mod))
 			break;
 
 		cnt = 0;
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 14/16] ftrace: Save module init functions kallsyms symbols for tracing
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (12 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 13/16] ftrace: Allow module init functions to be traced Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps Steven Rostedt
  2017-10-06 18:07 ` [for-next][PATCH 16/16] ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions Steven Rostedt
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

[-- Attachment #1: 0014-ftrace-Save-module-init-functions-kallsyms-symbols-f.patch --]
[-- Type: text/plain, Size: 11208 bytes --]

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

If function tracing is active when the module init functions are freed, then
store them to be referenced by kallsyms. As module init functions can now be
traced on module load, they were useless:

 ># echo ':mod:snd_seq' > set_ftrace_filter
 ># echo function > current_tracer
 ># modprobe snd_seq
 ># cat trace
 # tracer: function
 #
 #                              _-----=> irqs-off
 #                             / _----=> need-resched
 #                            | / _---=> hardirq/softirq
 #                            || / _--=> preempt-depth
 #                            ||| /     delay
 #           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
 #              | |       |   ||||       |         |
         modprobe-2786  [000] ....  3189.037874: 0xffffffffa0860000 <-do_one_initcall
         modprobe-2786  [000] ....  3189.037876: 0xffffffffa086004d <-0xffffffffa086000f
         modprobe-2786  [000] ....  3189.037876: 0xffffffffa086010d <-0xffffffffa0860018
         modprobe-2786  [000] ....  3189.037877: 0xffffffffa086011a <-0xffffffffa0860021
         modprobe-2786  [000] ....  3189.037877: 0xffffffffa0860080 <-0xffffffffa086002a
         modprobe-2786  [000] ....  3189.039523: 0xffffffffa0860400 <-0xffffffffa0860033
         modprobe-2786  [000] ....  3189.039523: 0xffffffffa086038a <-0xffffffffa086041c
         modprobe-2786  [000] ....  3189.039591: 0xffffffffa086038a <-0xffffffffa0860436
         modprobe-2786  [000] ....  3189.039657: 0xffffffffa086038a <-0xffffffffa0860450
         modprobe-2786  [000] ....  3189.039719: 0xffffffffa0860127 <-0xffffffffa086003c
         modprobe-2786  [000] ....  3189.039742: snd_seq_create_kernel_client <-0xffffffffa08601f6

When the output is shown, the kallsyms for the module init functions have
already been freed, and the output of the trace can not convert them to
their function names.

Now this looks like this:

 # tracer: function
 #
 #                              _-----=> irqs-off
 #                             / _----=> need-resched
 #                            | / _---=> hardirq/softirq
 #                            || / _--=> preempt-depth
 #                            ||| /     delay
 #           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
 #              | |       |   ||||       |         |
         modprobe-2463  [002] ....   174.243237: alsa_seq_init <-do_one_initcall
         modprobe-2463  [002] ....   174.243239: client_init_data <-alsa_seq_init
         modprobe-2463  [002] ....   174.243240: snd_sequencer_memory_init <-alsa_seq_init
         modprobe-2463  [002] ....   174.243240: snd_seq_queues_init <-alsa_seq_init
         modprobe-2463  [002] ....   174.243240: snd_sequencer_device_init <-alsa_seq_init
         modprobe-2463  [002] ....   174.244860: snd_seq_info_init <-alsa_seq_init
         modprobe-2463  [002] ....   174.244861: create_info_entry <-snd_seq_info_init
         modprobe-2463  [002] ....   174.244936: create_info_entry <-snd_seq_info_init
         modprobe-2463  [002] ....   174.245003: create_info_entry <-snd_seq_info_init
         modprobe-2463  [002] ....   174.245072: snd_seq_system_client_init <-alsa_seq_init
         modprobe-2463  [002] ....   174.245094: snd_seq_create_kernel_client <-snd_seq_system_client_init

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/ftrace.h |  20 ++++++-
 kernel/kallsyms.c      |   5 ++
 kernel/module.c        |   2 +-
 kernel/trace/ftrace.c  | 146 ++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 168 insertions(+), 5 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 47fc404ad233..202b40784c4e 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -51,6 +51,21 @@ static inline void early_trace_init(void) { }
 struct module;
 struct ftrace_hash;
 
+#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_MODULES) && \
+	defined(CONFIG_DYNAMIC_FTRACE)
+const char *
+ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
+		   unsigned long *off, char **modname, char *sym);
+#else
+static inline const char *
+ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
+		   unsigned long *off, char **modname, char *sym)
+{
+	return NULL;
+}
+#endif
+
+
 #ifdef CONFIG_FUNCTION_TRACER
 
 extern int ftrace_enabled;
@@ -151,10 +166,10 @@ struct ftrace_ops_hash {
 };
 
 void ftrace_free_init_mem(void);
-void ftrace_free_mem(void *start, void *end);
+void ftrace_free_mem(struct module *mod, void *start, void *end);
 #else
 static inline void ftrace_free_init_mem(void) { }
-static inline void ftrace_free_mem(void *start, void *end) { }
+static inline void ftrace_free_mem(struct module *mod, void *start, void *end) { }
 #endif
 
 /*
@@ -272,6 +287,7 @@ static inline int ftrace_nr_registered_ops(void)
 static inline void clear_ftrace_function(void) { }
 static inline void ftrace_kill(void) { }
 static inline void ftrace_free_init_mem(void) { }
+static inline void ftrace_free_mem(struct module *mod, void *start, void *end) { }
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #ifdef CONFIG_STACK_TRACER
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 127e7cfafa55..976ecb9275d9 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -24,6 +24,7 @@
 #include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/filter.h>
+#include <linux/ftrace.h>
 #include <linux/compiler.h>
 
 #include <asm/sections.h>
@@ -337,6 +338,10 @@ const char *kallsyms_lookup(unsigned long addr,
 	if (!ret)
 		ret = bpf_address_lookup(addr, symbolsize,
 					 offset, modname, namebuf);
+
+	if (!ret)
+		ret = ftrace_mod_address_lookup(addr, symbolsize,
+						offset, modname, namebuf);
 	return ret;
 }
 
diff --git a/kernel/module.c b/kernel/module.c
index 58bca427ac3f..279a469dc375 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3473,7 +3473,7 @@ static noinline int do_init_module(struct module *mod)
 	if (!mod->async_probe_requested && (current->flags & PF_USED_ASYNC))
 		async_synchronize_full();
 
-	ftrace_free_mem(mod->init_layout.base, mod->init_layout.base +
+	ftrace_free_mem(mod, mod->init_layout.base, mod->init_layout.base +
 			mod->init_layout.size);
 	mutex_lock(&module_mutex);
 	/* Drop initial reference. */
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index d7297e866e4a..86dbbfb353db 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5675,6 +5675,21 @@ static int ftrace_process_locs(struct module *mod,
 	return ret;
 }
 
+struct ftrace_mod_func {
+	struct list_head	list;
+	char			*name;
+	unsigned long		ip;
+	unsigned int		size;
+};
+
+struct ftrace_mod_map {
+	struct list_head	list;
+	struct module		*mod;
+	unsigned long		start_addr;
+	unsigned long		end_addr;
+	struct list_head	funcs;
+};
+
 #ifdef CONFIG_MODULES
 
 #define next_to_ftrace_page(p) container_of(p, struct ftrace_page, next)
@@ -5868,9 +5883,123 @@ void ftrace_module_init(struct module *mod)
 	ftrace_process_locs(mod, mod->ftrace_callsites,
 			    mod->ftrace_callsites + mod->num_ftrace_callsites);
 }
+
+static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
+				struct dyn_ftrace *rec)
+{
+	struct ftrace_mod_func *mod_func;
+	unsigned long symsize;
+	unsigned long offset;
+	char str[KSYM_SYMBOL_LEN];
+	char *modname;
+	const char *ret;
+
+	ret = kallsyms_lookup(rec->ip, &symsize, &offset, &modname, str);
+	if (!ret)
+		return;
+
+	mod_func = kmalloc(sizeof(*mod_func), GFP_KERNEL);
+	if (!mod_func)
+		return;
+
+	mod_func->name = kstrdup(str, GFP_KERNEL);
+	if (!mod_func->name) {
+		kfree(mod_func);
+		return;
+	}
+
+	mod_func->ip = rec->ip - offset;
+	mod_func->size = symsize;
+
+	list_add_rcu(&mod_func->list, &mod_map->funcs);
+}
+
+static LIST_HEAD(ftrace_mod_maps);
+
+static struct ftrace_mod_map *
+allocate_ftrace_mod_map(struct module *mod,
+			unsigned long start, unsigned long end)
+{
+	struct ftrace_mod_map *mod_map;
+
+	mod_map = kmalloc(sizeof(*mod_map), GFP_KERNEL);
+	if (!mod_map)
+		return NULL;
+
+	mod_map->mod = mod;
+	mod_map->start_addr = start;
+	mod_map->end_addr = end;
+
+	INIT_LIST_HEAD_RCU(&mod_map->funcs);
+
+	list_add_rcu(&mod_map->list, &ftrace_mod_maps);
+
+	return mod_map;
+}
+
+static const char *
+ftrace_func_address_lookup(struct ftrace_mod_map *mod_map,
+			   unsigned long addr, unsigned long *size,
+			   unsigned long *off, char *sym)
+{
+	struct ftrace_mod_func *found_func =  NULL;
+	struct ftrace_mod_func *mod_func;
+
+	list_for_each_entry_rcu(mod_func, &mod_map->funcs, list) {
+		if (addr >= mod_func->ip &&
+		    addr < mod_func->ip + mod_func->size) {
+			found_func = mod_func;
+			break;
+		}
+	}
+
+	if (found_func) {
+		if (size)
+			*size = found_func->size;
+		if (off)
+			*off = addr - found_func->ip;
+		if (sym)
+			strlcpy(sym, found_func->name, KSYM_NAME_LEN);
+
+		return found_func->name;
+	}
+
+	return NULL;
+}
+
+const char *
+ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
+		   unsigned long *off, char **modname, char *sym)
+{
+	struct ftrace_mod_map *mod_map;
+	const char *ret = NULL;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod_map, &ftrace_mod_maps, list) {
+		ret = ftrace_func_address_lookup(mod_map, addr, size, off, sym);
+		if (ret) {
+			if (modname)
+				*modname = mod_map->mod->name;
+			break;
+		}
+	}
+	preempt_enable();
+
+	return ret;
+}
+
+#else
+static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
+				struct dyn_ftrace *rec) { }
+static inline struct ftrace_mod_map *
+allocate_ftrace_mod_map(struct module *mod,
+			unsigned long start, unsigned long end)
+{
+	return NULL;
+}
 #endif /* CONFIG_MODULES */
 
-void ftrace_free_mem(void *start_ptr, void *end_ptr)
+void ftrace_free_mem(struct module *mod, void *start_ptr, void *end_ptr)
 {
 	unsigned long start = (unsigned long)(start_ptr);
 	unsigned long end = (unsigned long)(end_ptr);
@@ -5878,6 +6007,7 @@ void ftrace_free_mem(void *start_ptr, void *end_ptr)
 	struct ftrace_page *pg;
 	struct dyn_ftrace *rec;
 	struct dyn_ftrace key;
+	struct ftrace_mod_map *mod_map = NULL;
 	int order;
 
 	key.ip = start;
@@ -5885,6 +6015,14 @@ void ftrace_free_mem(void *start_ptr, void *end_ptr)
 
 	mutex_lock(&ftrace_lock);
 
+	/*
+	 * If we are freeing module init memory, then check if
+	 * any tracer is active. If so, we need to save a mapping of
+	 * the module functions being freed with the address.
+	 */
+	if (mod && ftrace_ops_list != &ftrace_list_end)
+		mod_map = allocate_ftrace_mod_map(mod, start, end);
+
 	for (pg = ftrace_pages_start; pg; last_pg = &pg->next, pg = *last_pg) {
 		if (end < pg->records[0].ip ||
 		    start >= (pg->records[pg->index - 1].ip + MCOUNT_INSN_SIZE))
@@ -5895,6 +6033,10 @@ void ftrace_free_mem(void *start_ptr, void *end_ptr)
 			      ftrace_cmp_recs);
 		if (!rec)
 			continue;
+
+		if (mod_map)
+			save_ftrace_mod_rec(mod_map, rec);
+
 		pg->index--;
 		ftrace_update_tot_cnt--;
 		if (!pg->index) {
@@ -5920,7 +6062,7 @@ void __init ftrace_free_init_mem(void)
 	void *start = (void *)(&__init_begin);
 	void *end = (void *)(&__init_end);
 
-	ftrace_free_mem(start, end);
+	ftrace_free_mem(NULL, start, end);
 }
 
 void __init ftrace_init(void)
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (13 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 14/16] ftrace: Save module init functions kallsyms symbols for tracing Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  2017-10-07  6:41   ` Joel Fernandes (Google)
  2017-10-06 18:07 ` [for-next][PATCH 16/16] ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions Steven Rostedt
  15 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

[-- Attachment #1: 0015-ftrace-Add-freeing-algorithm-to-free-ftrace_mod_maps.patch --]
[-- Type: text/plain, Size: 2935 bytes --]

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

The ftrace_mod_map is a descriptor to save module init function names in
case they were traced, and the trace output needs to reference the function
name from the function address. But after the function is unloaded, it
the maps should be freed, as the rest of the function names are as well.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 86dbbfb353db..a5824408bed9 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5683,6 +5683,7 @@ struct ftrace_mod_func {
 };
 
 struct ftrace_mod_map {
+	struct rcu_head		rcu;
 	struct list_head	list;
 	struct module		*mod;
 	unsigned long		start_addr;
@@ -5694,6 +5695,8 @@ struct ftrace_mod_map {
 
 #define next_to_ftrace_page(p) container_of(p, struct ftrace_page, next)
 
+static LIST_HEAD(ftrace_mod_maps);
+
 static int referenced_filters(struct dyn_ftrace *rec)
 {
 	struct ftrace_ops *ops;
@@ -5747,8 +5750,26 @@ static void clear_mod_from_hashes(struct ftrace_page *pg)
 	mutex_unlock(&trace_types_lock);
 }
 
+static void ftrace_free_mod_map(struct rcu_head *rcu)
+{
+	struct ftrace_mod_map *mod_map = container_of(rcu, struct ftrace_mod_map, rcu);
+	struct ftrace_mod_func *mod_func;
+	struct ftrace_mod_func *n;
+
+	/* All the contents of mod_map are now not visible to readers */
+	list_for_each_entry_safe(mod_func, n, &mod_map->funcs, list) {
+		kfree(mod_func->name);
+		list_del(&mod_func->list);
+		kfree(mod_func);
+	}
+
+	kfree(mod_map);
+}
+
 void ftrace_release_mod(struct module *mod)
 {
+	struct ftrace_mod_map *mod_map;
+	struct ftrace_mod_map *n;
 	struct dyn_ftrace *rec;
 	struct ftrace_page **last_pg;
 	struct ftrace_page *tmp_page = NULL;
@@ -5760,6 +5781,14 @@ void ftrace_release_mod(struct module *mod)
 	if (ftrace_disabled)
 		goto out_unlock;
 
+	list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) {
+		if (mod_map->mod == mod) {
+			list_del_rcu(&mod_map->list);
+			call_rcu_sched(&mod_map->rcu, ftrace_free_mod_map);
+			break;
+		}
+	}
+
 	/*
 	 * Each module has its own ftrace_pages, remove
 	 * them from the list.
@@ -5914,8 +5943,6 @@ static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
 	list_add_rcu(&mod_func->list, &mod_map->funcs);
 }
 
-static LIST_HEAD(ftrace_mod_maps);
-
 static struct ftrace_mod_map *
 allocate_ftrace_mod_map(struct module *mod,
 			unsigned long start, unsigned long end)
@@ -5974,6 +6001,7 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
 	struct ftrace_mod_map *mod_map;
 	const char *ret = NULL;
 
+	/* mod_map is freed via call_rcu_sched() */
 	preempt_disable();
 	list_for_each_entry_rcu(mod_map, &ftrace_mod_maps, list) {
 		ret = ftrace_func_address_lookup(mod_map, addr, size, off, sym);
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [for-next][PATCH 16/16] ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions
  2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
                   ` (14 preceding siblings ...)
  2017-10-06 18:07 ` [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps Steven Rostedt
@ 2017-10-06 18:07 ` Steven Rostedt
  15 siblings, 0 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-06 18:07 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton

[-- Attachment #1: 0016-ftrace-kallsyms-Have-proc-kallsyms-show-saved-mod-in.patch --]
[-- Type: text/plain, Size: 5511 bytes --]

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

If a module is loaded while tracing is enabled, then there's a possibility
that the module init functions were traced. These functions have their name
and address stored by ftrace such that it can translate the function address
that is written into the buffer into a human readable function name.

As userspace tools may be doing the same, they need a way to map function
names to their address as well. This is done through reading /proc/kallsyms.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/ftrace.h |  9 +++++++++
 kernel/kallsyms.c      | 38 ++++++++++++++++++++++++++++++++------
 kernel/trace/ftrace.c  | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 6 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 202b40784c4e..346f8294e40a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -56,6 +56,9 @@ struct ftrace_hash;
 const char *
 ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
 		   unsigned long *off, char **modname, char *sym);
+int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *value,
+			   char *type, char *name,
+			   char *module_name, int *exported);
 #else
 static inline const char *
 ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
@@ -63,6 +66,12 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
 {
 	return NULL;
 }
+static inline int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *value,
+					 char *type, char *name,
+					 char *module_name, int *exported)
+{
+	return -1;
+}
 #endif
 
 
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 976ecb9275d9..1966fe1c2b57 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -479,6 +479,7 @@ EXPORT_SYMBOL(__print_symbol);
 struct kallsym_iter {
 	loff_t pos;
 	loff_t pos_mod_end;
+	loff_t pos_ftrace_mod_end;
 	unsigned long value;
 	unsigned int nameoff; /* If iterating in core kernel symbols. */
 	char type;
@@ -501,11 +502,25 @@ static int get_ksymbol_mod(struct kallsym_iter *iter)
 	return 1;
 }
 
+static int get_ksymbol_ftrace_mod(struct kallsym_iter *iter)
+{
+	int ret = ftrace_mod_get_kallsym(iter->pos - iter->pos_mod_end,
+					 &iter->value, &iter->type,
+					 iter->name, iter->module_name,
+					 &iter->exported);
+	if (ret < 0) {
+		iter->pos_ftrace_mod_end = iter->pos;
+		return 0;
+	}
+
+	return 1;
+}
+
 static int get_ksymbol_bpf(struct kallsym_iter *iter)
 {
 	iter->module_name[0] = '\0';
 	iter->exported = 0;
-	return bpf_get_kallsym(iter->pos - iter->pos_mod_end,
+	return bpf_get_kallsym(iter->pos - iter->pos_ftrace_mod_end,
 			       &iter->value, &iter->type,
 			       iter->name) < 0 ? 0 : 1;
 }
@@ -530,20 +545,31 @@ static void reset_iter(struct kallsym_iter *iter, loff_t new_pos)
 	iter->name[0] = '\0';
 	iter->nameoff = get_symbol_offset(new_pos);
 	iter->pos = new_pos;
-	if (new_pos == 0)
+	if (new_pos == 0) {
 		iter->pos_mod_end = 0;
+		iter->pos_ftrace_mod_end = 0;
+	}
 }
 
 static int update_iter_mod(struct kallsym_iter *iter, loff_t pos)
 {
 	iter->pos = pos;
 
-	if (iter->pos_mod_end > 0 &&
-	    iter->pos_mod_end < iter->pos)
+	if (iter->pos_ftrace_mod_end > 0 &&
+	    iter->pos_ftrace_mod_end < iter->pos)
 		return get_ksymbol_bpf(iter);
 
-	if (!get_ksymbol_mod(iter))
-		return get_ksymbol_bpf(iter);
+	if (iter->pos_mod_end > 0 &&
+	    iter->pos_mod_end < iter->pos) {
+		if (!get_ksymbol_ftrace_mod(iter))
+			return get_ksymbol_bpf(iter);
+		return 1;
+	}
+
+	if (!get_ksymbol_mod(iter)) {
+		if (!get_ksymbol_ftrace_mod(iter))
+			return get_ksymbol_bpf(iter);
+	}
 
 	return 1;
 }
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a5824408bed9..9e99bd55732e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5689,6 +5689,7 @@ struct ftrace_mod_map {
 	unsigned long		start_addr;
 	unsigned long		end_addr;
 	struct list_head	funcs;
+	unsigned int		num_funcs;
 };
 
 #ifdef CONFIG_MODULES
@@ -5940,6 +5941,8 @@ static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
 	mod_func->ip = rec->ip - offset;
 	mod_func->size = symsize;
 
+	mod_map->num_funcs++;
+
 	list_add_rcu(&mod_func->list, &mod_map->funcs);
 }
 
@@ -5956,6 +5959,7 @@ allocate_ftrace_mod_map(struct module *mod,
 	mod_map->mod = mod;
 	mod_map->start_addr = start;
 	mod_map->end_addr = end;
+	mod_map->num_funcs = 0;
 
 	INIT_LIST_HEAD_RCU(&mod_map->funcs);
 
@@ -6016,6 +6020,42 @@ ftrace_mod_address_lookup(unsigned long addr, unsigned long *size,
 	return ret;
 }
 
+int ftrace_mod_get_kallsym(unsigned int symnum, unsigned long *value,
+			   char *type, char *name,
+			   char *module_name, int *exported)
+{
+	struct ftrace_mod_map *mod_map;
+	struct ftrace_mod_func *mod_func;
+
+	preempt_disable();
+	list_for_each_entry_rcu(mod_map, &ftrace_mod_maps, list) {
+
+		if (symnum >= mod_map->num_funcs) {
+			symnum -= mod_map->num_funcs;
+			continue;
+		}
+
+		list_for_each_entry_rcu(mod_func, &mod_map->funcs, list) {
+			if (symnum > 1) {
+				symnum--;
+				continue;
+			}
+
+			*value = mod_func->ip;
+			*type = 'T';
+			strlcpy(name, mod_func->name, KSYM_NAME_LEN);
+			strlcpy(module_name, mod_map->mod->name, MODULE_NAME_LEN);
+			*exported = 1;
+			preempt_enable();
+			return 0;
+		}
+		WARN_ON(1);
+		break;
+	}
+	preempt_enable();
+	return -ERANGE;
+}
+
 #else
 static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
 				struct dyn_ftrace *rec) { }
-- 
2.13.2

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-06 18:07 ` [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps Steven Rostedt
@ 2017-10-07  6:41   ` Joel Fernandes (Google)
  2017-10-07 13:32     ` Steven Rostedt
  0 siblings, 1 reply; 25+ messages in thread
From: Joel Fernandes (Google) @ 2017-10-07  6:41 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton

Hi Steve,

On Fri, Oct 6, 2017 at 11:07 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> The ftrace_mod_map is a descriptor to save module init function names in
> case they were traced, and the trace output needs to reference the function
> name from the function address. But after the function is unloaded, it
> the maps should be freed, as the rest of the function names are as well.

Just checking for my understanding of this patch - wouldn't this also
mean that if there were any look ups of the init functions that may be
needed at trace output time, then those look ups wont be possible any
more after module is unloaded?

I guess having a reference somehow on the ftrace_mod_map descriptor if
there are any entries in the trace buffer that need it can help
prevent that but it could be too expensive for not much return since
most likely the user wouldn't unload modules before trace collection
in normal usage.

thanks,

- Joel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-07  6:41   ` Joel Fernandes (Google)
@ 2017-10-07 13:32     ` Steven Rostedt
  2017-10-08  4:52       ` Joel Fernandes (Google)
  0 siblings, 1 reply; 25+ messages in thread
From: Steven Rostedt @ 2017-10-07 13:32 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton, Jessica Yu

On Fri, 6 Oct 2017 23:41:25 -0700
"Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:

> Hi Steve,
> 
> On Fri, Oct 6, 2017 at 11:07 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
> >
> > The ftrace_mod_map is a descriptor to save module init function names in
> > case they were traced, and the trace output needs to reference the function
> > name from the function address. But after the function is unloaded, it
> > the maps should be freed, as the rest of the function names are as well.  
> 
> Just checking for my understanding of this patch - wouldn't this also
> mean that if there were any look ups of the init functions that may be
> needed at trace output time, then those look ups wont be possible any
> more after module is unloaded?

Yes. That's true for all functions in the module. When a module is
unloaded, all references to it in kallsyms is also freed. Try it on a
current kernel. Trace a function in a module, then unload that module.
The trace data will just show the ip hex address of the module function
after that.

> 
> I guess having a reference somehow on the ftrace_mod_map descriptor if
> there are any entries in the trace buffer that need it can help
> prevent that but it could be too expensive for not much return since
> most likely the user wouldn't unload modules before trace collection
> in normal usage.

Right, I have thought about this, and I haven't come up with an
inexpensive way to do this. As this has been the default operation of
all module functions, and I haven't heard much complaining about it (I
think I may have had a single complaint), I didn't put too much effort
into it.

I need to look at the approach that Jessica sent me. Perhaps there's
ways to have all module function names be saved by a trace. But mapping
which trace buffer has these names may be difficult.

-- Steve

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-07 13:32     ` Steven Rostedt
@ 2017-10-08  4:52       ` Joel Fernandes (Google)
  2017-10-08  8:42         ` Joel Fernandes
  2017-10-09  6:02         ` Joel Fernandes
  0 siblings, 2 replies; 25+ messages in thread
From: Joel Fernandes (Google) @ 2017-10-08  4:52 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
	Jessica Yu, Joel Fernandes

Hi Steve,

On Sat, Oct 7, 2017 at 6:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 6 Oct 2017 23:41:25 -0700
> "Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:
>
>> Hi Steve,
>>
>> On Fri, Oct 6, 2017 at 11:07 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>> >
>> > The ftrace_mod_map is a descriptor to save module init function names in
>> > case they were traced, and the trace output needs to reference the function
>> > name from the function address. But after the function is unloaded, it
>> > the maps should be freed, as the rest of the function names are as well.
>>
>> Just checking for my understanding of this patch - wouldn't this also
>> mean that if there were any look ups of the init functions that may be
>> needed at trace output time, then those look ups wont be possible any
>> more after module is unloaded?
>
> Yes. That's true for all functions in the module. When a module is
> unloaded, all references to it in kallsyms is also freed. Try it on a
> current kernel. Trace a function in a module, then unload that module.
> The trace data will just show the ip hex address of the module function
> after that.
>

I tried your core branch and I see some weirdness with the filters:

Here's the code for the module:
================================================
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/init.h>

void bar(void)
{
    printk(KERN_INFO "bar!\n");
}

void foo(void)
{
    printk(KERN_INFO "foo!\n");
    bar();
}

static int __init hello_init(void)
{
    printk(KERN_INFO "Hello world!\n");
    foo();
    return 0;
}

static void __exit hello_cleanup(void)
{
    printk(KERN_INFO "Cleaning up module.\n");
}

module_init(hello_init);
module_exit(hello_cleanup);
================================================

Following is a run with function tracing, during the first run I see
foo and bar are setup in the filters. After that when I unload and
load the module, I see that only "bar" is in the filters:

bash-4.3# echo '*:mod:test' > /d/tracing/set_ftrace_filter
bash-4.3# echo function > /d/tracing/current_tracer
bash-4.3# cat /d/tracing/set_ftrace_filter
*:mod:test
bash-4.3# modprobe test
bash-4.3# cat /d/tracing/trace
# tracer: function
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
        modprobe-1050  [003] ....     1.653000: hello_init <-do_one_initcall
bash-4.3# cat /d/tracing/set_ftrace_filter
bar [test]
foo [test]
bash-4.3# rmmod test
bash-4.3# cat /d/tracing/set_ftrace_filter
bash-4.3# sleep 2
bash-4.3# modprobe test
bash-4.3# cat /d/tracing/trace
# tracer: function
#
#                              _-----=> irqs-off
#                             / _----=> need-resched
#                            | / _---=> hardirq/softirq
#                            || / _--=> preempt-depth
#                            ||| /     delay
#           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
#              | |       |   ||||       |         |
        modprobe-1050  [003] ....     1.653000: bar <-do_one_initcall
bash-4.3# cat /d/tracing/set_ftrace_filter
bar [test]                                       <--- bar is still in
the filter list
bash-4.3#

Interestingly this also shows that the symbol conversion can be
unreliable if another module was loaded into the same address, in this
case 'bar' was loaded at the same address as 'hello_init' the second
time so during the second cat of the trace file, it shows a different
symbol name.

Also could you let me know what is the correct behavior of the filters
after a module being traced is unloaded, are the filters supposed to
be setup again after the module is unloaded, before the module can be
loaded again?

Also I wasn't able to see the call from hello_init -> bar and bar ->
foo so I'm not fully sure if that's a bug or I did something wrong.
I'm happy to try out anything you suggest.

>> I guess having a reference somehow on the ftrace_mod_map descriptor if
>> there are any entries in the trace buffer that need it can help
>> prevent that but it could be too expensive for not much return since
>> most likely the user wouldn't unload modules before trace collection
>> in normal usage.
>
> Right, I have thought about this, and I haven't come up with an
> inexpensive way to do this. As this has been the default operation of
> all module functions, and I haven't heard much complaining about it (I
> think I may have had a single complaint), I didn't put too much effort
> into it.

> I need to look at the approach that Jessica sent me. Perhaps there's

Yes, I think that approach is better. But I think it would have the
same issue (that if the init sections are freed and say used by other
loaded modules), then the trace output would incorrectly show a
different symbol name than the one from the module at the time the
trace record was generated.

thanks,

- Joel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-08  4:52       ` Joel Fernandes (Google)
@ 2017-10-08  8:42         ` Joel Fernandes
  2017-10-08 18:42           ` Steven Rostedt
  2017-10-09  6:02         ` Joel Fernandes
  1 sibling, 1 reply; 25+ messages in thread
From: Joel Fernandes @ 2017-10-08  8:42 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
	Jessica Yu, Joel Fernandes (Google)

Hi Steve,

> "Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:
[..]
> Also could you let me know what is the correct behavior of the filters
> after a module being traced is unloaded, are the filters supposed to
> be setup again after the module is unloaded, before the module can be
> loaded again?

Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know
what you think, thanks. (its only lightly tested - checked that the filters are preserved,
will do more testing tomorrow):

---8<---
From: Joel Fernandes <joelaf@google.com>
Subject: [PATCH RFC] ftrace: Preserve filters across loading/unloading of module

When a module is removed, the filters associated with it is cleared, however it
doesn't make sense to clear it in the following work flow:

1. setup filters (:mod:<mod-name>)
2. load module
3. unload module
4. load module

In step 4, the filters would have gone. Since the recently added mechanism to
do step 1 before 2 (deferred execution of the module command string) is already
present, we can reuse the same mechanism to preserve the filters. This patch
does that and avoids losing the filters and having to set them up again.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Joel Fernandes <joelaf@google.com>
---
 kernel/trace/ftrace.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6abfafd7f173..4979fd2ef466 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5693,7 +5693,9 @@ static int referenced_filters(struct dyn_ftrace *rec)
 }
 
 static void
-clear_mod_from_hash(struct ftrace_page *pg, struct ftrace_hash *hash)
+deactivate_mod_from_hash(struct module *mod, struct ftrace_page *pg,
+			 struct trace_array *tr, int enable,
+			 struct ftrace_hash *hash)
 {
 	struct ftrace_func_entry *entry;
 	struct dyn_ftrace *rec;
@@ -5712,11 +5714,22 @@ clear_mod_from_hash(struct ftrace_page *pg, struct ftrace_hash *hash)
 		 */
 		if (entry)
 			entry->ip = 0;
+
+		/*
+		 * The hash has been made cleared, however lets save it as a
+		 * string for the next time the module is loaded.
+		 */
+		if (entry) {
+			char func[KSYM_SYMBOL_LEN];
+			kallsyms_lookup(rec->ip, NULL, NULL, NULL, func);
+			ftrace_add_mod(tr, func, mod->name, enable);
+		}
 	}
 }
 
 /* Clear any records from hashs */
-static void clear_mod_from_hashes(struct ftrace_page *pg)
+static void deactivate_mod_from_hashes(struct module *mod,
+				       struct ftrace_page *pg)
 {
 	struct trace_array *tr;
 
@@ -5725,8 +5738,10 @@ static void clear_mod_from_hashes(struct ftrace_page *pg)
 		if (!tr->ops || !tr->ops->func_hash)
 			continue;
 		mutex_lock(&tr->ops->func_hash->regex_lock);
-		clear_mod_from_hash(pg, tr->ops->func_hash->filter_hash);
-		clear_mod_from_hash(pg, tr->ops->func_hash->notrace_hash);
+		deactivate_mod_from_hash(mod, pg, tr, 1,
+					 tr->ops->func_hash->filter_hash);
+		deactivate_mod_from_hash(mod, pg, tr, 0,
+					 tr->ops->func_hash->notrace_hash);
 		mutex_unlock(&tr->ops->func_hash->regex_lock);
 	}
 	mutex_unlock(&trace_types_lock);
@@ -5778,7 +5793,7 @@ void ftrace_release_mod(struct module *mod)
 	for (pg = tmp_page; pg; pg = tmp_page) {
 
 		/* Needs to be called outside of ftrace_lock */
-		clear_mod_from_hashes(pg);
+		deactivate_mod_from_hashes(mod, pg);
 
 		order = get_count_order(pg->size / ENTRIES_PER_PAGE);
 		free_pages((unsigned long)pg->records, order);
-- 
2.14.2.920.gcf0c67979c-goog

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-08  8:42         ` Joel Fernandes
@ 2017-10-08 18:42           ` Steven Rostedt
  2017-10-08 18:56             ` Joel Fernandes
  2017-10-09  6:07             ` Joel Fernandes
  0 siblings, 2 replies; 25+ messages in thread
From: Steven Rostedt @ 2017-10-08 18:42 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
	Jessica Yu, Joel Fernandes (Google)

On Sun, 8 Oct 2017 01:42:15 -0700
Joel Fernandes <joelaf@google.com> wrote:

> Hi Steve,
> 
> > "Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:  
> [..]
> > Also could you let me know what is the correct behavior of the filters
> > after a module being traced is unloaded, are the filters supposed to
> > be setup again after the module is unloaded, before the module can be
> > loaded again?  
> 
> Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know
> what you think, thanks. (its only lightly tested - checked that the filters are preserved,
> will do more testing tomorrow):

They should not be preserved, it's too non-deterministic.

I'm wondering why the setting of the ip to zero didn't keep it from
showing up again. I'll have to investigate that.

-- Steve

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-08 18:42           ` Steven Rostedt
@ 2017-10-08 18:56             ` Joel Fernandes
  2017-10-09  6:07             ` Joel Fernandes
  1 sibling, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2017-10-08 18:56 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
	Jessica Yu, Joel Fernandes (Google)

Hi Steve,

On Sun, Oct 8, 2017 at 11:42 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 8 Oct 2017 01:42:15 -0700
[..]
>> > "Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:
>> [..]
>> > Also could you let me know what is the correct behavior of the filters
>> > after a module being traced is unloaded, are the filters supposed to
>> > be setup again after the module is unloaded, before the module can be
>> > loaded again?
>>
>> Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know
>> what you think, thanks. (its only lightly tested - checked that the filters are preserved,
>> will do more testing tomorrow):
>
> They should not be preserved, it's too non-deterministic.

Could you elaborate more on non-deterministic?

Say a user setup filters with '*:mod:<mod-name>' and unloads and
loads, then the filter carrying forward for the second load helps
avoid having them to set it up again.

OTOH I agree its not a big deal to set up the filters again, and
probably not worth save it if it complicates the code too much to
handle all cases.

> I'm wondering why the setting of the ip to zero didn't keep it from
> showing up again. I'll have to investigate that.

Ok. Happy to help test anything you want to try :) I *think* the 'bar'
function that showed up the second time around is actually the address
of the init function in the previous load, and probably the new code
you added switched it on but I have yet to debug it (you'll probably
beat me to it since you wrote this code ;)).

thanks,

- Joel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-08  4:52       ` Joel Fernandes (Google)
  2017-10-08  8:42         ` Joel Fernandes
@ 2017-10-09  6:02         ` Joel Fernandes
  1 sibling, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2017-10-09  6:02 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: Steven Rostedt, Linux Kernel Mailing List, Ingo Molnar,
	Andrew Morton, Jessica Yu

Hi Steve,

On Sat, Oct 7, 2017 at 9:52 PM, Joel Fernandes (Google)
<joel.opensrc@gmail.com> wrote:
[..]
>>>
>>> On Fri, Oct 6, 2017 at 11:07 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>> > From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>>> >
>>> > The ftrace_mod_map is a descriptor to save module init function names in
>>> > case they were traced, and the trace output needs to reference the function
>>> > name from the function address. But after the function is unloaded, it
>>> > the maps should be freed, as the rest of the function names are as well.
>>>
>>> Just checking for my understanding of this patch - wouldn't this also
>>> mean that if there were any look ups of the init functions that may be
>>> needed at trace output time, then those look ups wont be possible any
>>> more after module is unloaded?
>>
>> Yes. That's true for all functions in the module. When a module is
>> unloaded, all references to it in kallsyms is also freed. Try it on a
>> current kernel. Trace a function in a module, then unload that module.
>> The trace data will just show the ip hex address of the module function
>> after that.
>>
>
> I tried your core branch and I see some weirdness with the filters:
>
> Here's the code for the module:
> ================================================
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
>
> void bar(void)
> {
>     printk(KERN_INFO "bar!\n");
> }
>
> void foo(void)
> {
>     printk(KERN_INFO "foo!\n");
>     bar();
> }
>
> static int __init hello_init(void)
> {
>     printk(KERN_INFO "Hello world!\n");
>     foo();
>     return 0;
> }
>
> static void __exit hello_cleanup(void)
> {
>     printk(KERN_INFO "Cleaning up module.\n");
> }
>
> module_init(hello_init);
> module_exit(hello_cleanup);
> ================================================
[..]
> Also I wasn't able to see the call from hello_init -> bar and bar ->
> foo so I'm not fully sure if that's a bug or I did something wrong.
> I'm happy to try out anything you suggest.

This was I think because of function inlining, using 'noinline' on the
foo and bar functions in the above module makes it all work properly
(tested on ftrace/core branch).

thanks,

- Joel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
  2017-10-08 18:42           ` Steven Rostedt
  2017-10-08 18:56             ` Joel Fernandes
@ 2017-10-09  6:07             ` Joel Fernandes
  1 sibling, 0 replies; 25+ messages in thread
From: Joel Fernandes @ 2017-10-09  6:07 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Linux Kernel Mailing List, Ingo Molnar, Andrew Morton,
	Jessica Yu, Joel Fernandes (Google)

Hi Steve,

On Sun, Oct 8, 2017 at 11:42 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > "Joel Fernandes (Google)" <joel.opensrc@gmail.com> wrote:
>> > Also could you let me know what is the correct behavior of the filters
>> > after a module being traced is unloaded, are the filters supposed to
>> > be setup again after the module is unloaded, before the module can be
>> > loaded again?
>>
>> Actually I learnt that it doesn't get preserved and wrote a patch to fix that, let me know
>> what you think, thanks. (its only lightly tested - checked that the filters are preserved,
>> will do more testing tomorrow):
>
> They should not be preserved, it's too non-deterministic.
>
> I'm wondering why the setting of the ip to zero didn't keep it from
> showing up again. I'll have to investigate that.
>

I found that the entries weren't being cleared for the init functions
as the records weren't available at module unload time.
Posted a patch to fix this: https://lkml.org/lkml/2017/10/8/196

thanks,

- Joel

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2017-10-09  6:07 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-06 18:06 [for-next][PATCH 00/16] tracing: Updates for 4.15 Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 01/16] tracing: Remove obsolete sched_switch tracer selftest Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 02/16] tracing: Remove redundant unread variable ret Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 03/16] tracing: Reverse the order of trace_types_lock and event_mutex Steven Rostedt
2017-10-06 18:06 ` [for-next][PATCH 04/16] ring-buffer: Rewrite trace_recursive_(un)lock() to be simpler Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 05/16] tracing: Exclude generic fields from histograms Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 06/16] tracing: Remove lookups from tracing_map hitcount Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 07/16] tracing: Increase tracing map KEYS_MAX size Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 08/16] tracing: Make traceprobe parsing code reusable Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 09/16] tracing: Clean up hist_field_flags enum Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 10/16] tracing: Add hist_field_name() accessor Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 11/16] tracing: Reimplement log2 Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 12/16] ftrace: Add a ftrace_free_mem() function for modules to use Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 13/16] ftrace: Allow module init functions to be traced Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 14/16] ftrace: Save module init functions kallsyms symbols for tracing Steven Rostedt
2017-10-06 18:07 ` [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps Steven Rostedt
2017-10-07  6:41   ` Joel Fernandes (Google)
2017-10-07 13:32     ` Steven Rostedt
2017-10-08  4:52       ` Joel Fernandes (Google)
2017-10-08  8:42         ` Joel Fernandes
2017-10-08 18:42           ` Steven Rostedt
2017-10-08 18:56             ` Joel Fernandes
2017-10-09  6:07             ` Joel Fernandes
2017-10-09  6:02         ` Joel Fernandes
2017-10-06 18:07 ` [for-next][PATCH 16/16] ftrace/kallsyms: Have /proc/kallsyms show saved mod init functions Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).