tracing: More updates for 3.16

PATCH New Steven Rostedt 1 participant 14 messages May 12, 2014

Subsystems (5)

Documentation x86 Tracing ftrace +1 more subsystems

lore

Share & tools

lore

Steven Rostedt

author maintainer

2014-05-12 15:12 UTC #0 lore raw

cover tracing: More updates for 3.16

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

Head SHA1: 1486eb859d525a8be763530c3318293b9b81abc0

Christoph Lameter (1):
tracing: Replace __get_cpu_var uses with this_cpu_ptr

Frederic Weisbecker (1):
tracing: Remove myself as a tracing maintainer

Steven Rostedt (1):
ftrace/x86: Move the mcount/fentry code out of entry_64.S

Steven Rostedt (Red Hat) (10):
ftrace: Have function graph tracer use global_ops for filtering
tracing: Add trace_<tracepoint>_enabled() function
tracing: Add __bitmask() macro to trace events to cpumasks and other bitmasks
ftrace: Remove boolean of hash_enable and hash_disable
ftrace: Write in missing comment from a very old commit
ftrace: Always inline ftrace_hash_empty() helper function ftrace/x86: Get the current mcount addr for add_breakpoint()
ftrace: Make get_ftrace_addr() and get_ftrace_addr_old() global
ftrace: Use the ftrace_addr helper functions to find the ftrace_addr
ftrace: Remove FTRACE_UPDATE_MODIFY_CALL_REGS flag

----

 Documentation/trace/tracepoints.txt |  24 ++++
 MAINTAINERS                         |   1 -
 arch/x86/kernel/Makefile            |   1 +
 arch/x86/kernel/entry_64.S          | 204 ---------------------------------
 arch/x86/kernel/ftrace.c            |  43 +------
 arch/x86/kernel/mcount_64.S         | 217 ++++++++++++++++++++++++++++++++++++
 include/linux/ftrace.h              |   4 +-
 include/linux/ftrace_event.h        |   3 +
 include/linux/kprobes.h             |   2 +-
 include/linux/trace_seq.h           |  10 ++
 include/linux/tracepoint.h          |  10 ++
 include/trace/ftrace.h              |  57 +++++++++-
 kernel/trace/ftrace.c               | 100 +++++++++++------
 kernel/trace/trace.c                |   2 +-
 kernel/trace/trace_output.c         |  41 +++++++
 15 files changed, 436 insertions(+), 283 deletions(-)

Steven Rostedt

author maintainer

2014-05-12 15:12 UTC #1 lore raw

PATCH 1/13 ftrace: Have function graph tracer use global_ops for filtering

From: "Steven Rostedt (Red Hat)" <[email protected]>

Commit 4104d326b670 "ftrace: Remove global function list and call function directly" cleaned up the global_ops filtering and made the code simpler. But it left out function graph filtering which also depended on that code. The function graph filtering still needs to use global_ops as the filter otherwise it wont filter at all.

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

---
 kernel/trace/ftrace.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

kernel/trace/ftrace.c +6 -8

index 34b098bf..9eb1aa0 100644
@@ -5008,12 +5008,6 @@ ftrace_suspend_notifier_call(struct notifier_block *bl, unsigned long state,
 	return NOTIFY_DONE;
 }
 
-/* Just a place holder for function graph */
-static struct ftrace_ops fgraph_ops __read_mostly = {
-	.func		= ftrace_stub,
-	.flags		= FTRACE_OPS_FL_STUB | FTRACE_OPS_FL_RECURSION_SAFE,
-};
-
 static int ftrace_graph_entry_test(struct ftrace_graph_ent *trace)
 {
 	if (!ftrace_ops_test(&global_ops, trace->func, NULL))

@@ -5076,7 +5070,10 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,
 	ftrace_graph_entry = ftrace_graph_entry_test;
 	update_function_graph_func();
 
-	ret = ftrace_startup(&fgraph_ops, FTRACE_START_FUNC_RET);
+	/* Function graph doesn't use the .func field of global_ops */
+	global_ops.flags |= FTRACE_OPS_FL_STUB;
+
+	ret = ftrace_startup(&global_ops, FTRACE_START_FUNC_RET);
 
 out:
 	mutex_unlock(&ftrace_lock);

@@ -5094,7 +5091,8 @@ void unregister_ftrace_graph(void)
 	ftrace_graph_return = (trace_func_graph_ret_t)ftrace_stub;
 	ftrace_graph_entry = ftrace_graph_entry_stub;
 	__ftrace_graph_entry = ftrace_graph_entry_stub;
-	ftrace_shutdown(&fgraph_ops, FTRACE_STOP_FUNC_RET);
+	ftrace_shutdown(&global_ops, FTRACE_STOP_FUNC_RET);
+	global_ops.flags &= ~FTRACE_OPS_FL_STUB;
 	unregister_pm_notifier(&ftrace_suspend_notifier);
 	unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:12 UTC #2 lore raw

PATCH 2/13 tracing: Remove myself as a tracing maintainer

In reply to Steven Rostedt

From: Frederic Weisbecker <[email protected]>

It has been a while since I last sent a tracing patch. I always keep an eye on tracing evolutions and contributions in general but given how busy I am with nohz, isolation and more generally core cleanups stuff, I seldom have time left to provide deep reviews of tracing patches nor simply for reviews to begin with.

I've been very lucky to start kernel development on a very young subsystem with tons of low hanging fruits back in 2008. Given that it deals with a lot of tricky stuffs all around (sched, timers, irq, preemption, NMIs, SMP, RCU, ....) I basically learned everything there.

Steve has been doing most of the incredible work these last years. Thanks a lot!

Of course consider me always available to help on tracing if any hard days happen.

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

Cc Arnaldo Carvalho de Melo <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

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

---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

MAINTAINERS +0 -1

index e67ea24..120af71 100644
@@ -9041,7 +9041,6 @@ F:	drivers/char/tpm/
 
 TRACING
 M:	Steven Rostedt <[email protected]>
-M:	Frederic Weisbecker <[email protected]>
 M:	Ingo Molnar <[email protected]>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core
 S:	Maintained

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:16 UTC #3 lore raw

PATCH 3/13 tracing: Replace __get_cpu_var uses with this_cpu_ptr

In reply to Steven Rostedt

From: Christoph Lameter <[email protected]>

Replace uses of &__get_cpu_var for address calculation with this_cpu_ptr.

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

Acked-by Masami Hiramatsu <[email protected]>

Signed-off-by Christoph Lameter <[email protected]>

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

---
 include/linux/kprobes.h | 2 +-
 kernel/trace/ftrace.c   | 4 ++--
 kernel/trace/trace.c    | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

include/linux/kprobes.h +1 -1

index 925eaf2..7bd2ad0 100644
@@ -355,7 +355,7 @@ static inline void reset_current_kprobe(void)
 
 static inline struct kprobe_ctlblk *get_kprobe_ctlblk(void)
 {
-	return (&__get_cpu_var(kprobe_ctlblk));
+	return this_cpu_ptr(&kprobe_ctlblk);
 }
 
 int register_kprobe(struct kprobe *p);

kernel/trace/ftrace.c +2 -2

index 9eb1aa0..38e5cf7 100644
@@ -822,7 +822,7 @@ function_profile_call(unsigned long ip, unsigned long parent_ip,
 
 	local_irq_save(flags);
 
-	stat = &__get_cpu_var(ftrace_profile_stats);
+	stat = this_cpu_ptr(&ftrace_profile_stats);
 	if (!stat->hash || !ftrace_profile_enabled)
 		goto out;

@@ -853,7 +853,7 @@ static void profile_graph_return(struct ftrace_graph_ret *trace)
 	unsigned long flags;
 
 	local_irq_save(flags);
-	stat = &__get_cpu_var(ftrace_profile_stats);
+	stat = this_cpu_ptr(&ftrace_profile_stats);
 	if (!stat->hash || !ftrace_profile_enabled)
 		goto out;

kernel/trace/trace.c +1 -1

index 4c392c8..0543169 100644
@@ -1726,7 +1726,7 @@ static void __ftrace_trace_stack(struct ring_buffer *buffer,
 	 */
 	barrier();
 	if (use_stack == 1) {
-		trace.entries		= &__get_cpu_var(ftrace_stack).calls[0];
+		trace.entries		= this_cpu_ptr(ftrace_stack.calls);
 		trace.max_entries	= FTRACE_STACK_MAX_ENTRIES;
 
 		if (regs)

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:12 UTC #4 lore raw

PATCH 4/13 tracing: Add trace_<tracepoint>_enabled() function

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

There are some code paths in the kernel that need to do some preparations before it calls a tracepoint. As that code is worthless overhead when the tracepoint is not enabled, it would be prudent to have that code only run when the tracepoint is active. To accomplish this, all tracepoints now get a static inline function called "trace_<tracepoint-name>_enabled()" which returns true when the tracepoint is enabled and false otherwise.

As an added bonus, that function uses the static_key of the tracepoint such that no branch is needed.

  if (trace_mytracepoint_enabled()) {
	arg = process_tp_arg();
	trace_mytracepoint(arg);

}

Will keep the "process_tp_arg()" (which may be expensive to run) from being executed when the tracepoint isn't enabled.

It's best to encapsulate the tracepoint itself in the if statement just to keep races. For example, if you had:

if (trace_mytracepoint_enabled())

	arg = process_tp_arg();

trace_mytracepoint(arg);

There's a chance that the tracepoint could be enabled just after the if statement, and arg will be undefined when calling the tracepoint.

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

Acked-by Mathieu Desnoyers <[email protected]>

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

---
 Documentation/trace/tracepoints.txt | 24 ++++++++++++++++++++++++
 include/linux/tracepoint.h          | 10 ++++++++++
 2 files changed, 34 insertions(+)

Documentation/trace/tracepoints.txt +24 -0

index 6b018b5..a3efac6 100644
@@ -115,6 +115,30 @@ If the tracepoint has to be used in kernel modules, an
 EXPORT_TRACEPOINT_SYMBOL_GPL() or EXPORT_TRACEPOINT_SYMBOL() can be
 used to export the defined tracepoints.
 
+If you need to do a bit of work for a tracepoint parameter, and
+that work is only used for the tracepoint, that work can be encapsulated
+within an if statement with the following:
+
+	if (trace_foo_bar_enabled()) {
+		int i;
+		int tot = 0;
+
+		for (i = 0; i < count; i++)
+			tot += calculate_nuggets();
+
+		trace_foo_bar(tot);
+	}
+
+All trace_<tracepoint>() calls have a matching trace_<tracepoint>_enabled()
+function defined that returns true if the tracepoint is enabled and
+false otherwise. The trace_<tracepoint>() should always be within the
+block of the if (trace_<tracepoint>_enabled()) to prevent races between
+the tracepoint being enabled and the check being seen.
+
+The advantage of using the trace_<tracepoint>_enabled() is that it uses
+the static_key of the tracepoint to allow the if statement to be implemented
+with jump labels and avoid conditional branches.
+
 Note: The convenience macro TRACE_EVENT provides an alternative way to
       define tracepoints. Check http://lwn.net/Articles/379903,
       http://lwn.net/Articles/381064 and http://lwn.net/Articles/383362

include/linux/tracepoint.h +10 -0

index 9d30ee4..2e2a5f7 100644
@@ -185,6 +185,11 @@ extern void syscall_unregfunc(void);
 	static inline void						\
 	check_trace_callback_type_##name(void (*cb)(data_proto))	\
 	{								\
+	}								\
+	static inline bool						\
+	trace_##name##_enabled(void)					\
+	{								\
+		return static_key_false(&__tracepoint_##name.key);	\
 	}
 
 /*

@@ -230,6 +235,11 @@ extern void syscall_unregfunc(void);
 	}								\
 	static inline void check_trace_callback_type_##name(void (*cb)(data_proto)) \
 	{								\
+	}								\
+	static inline bool						\
+	trace_##name##_enabled(void)					\
+	{								\
+		return false;						\
 	}
 
 #define DEFINE_TRACE_FN(name, reg, unreg)

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:14 UTC #5 lore raw

PATCH 5/13 tracing: Add __bitmask() macro to trace events to cpumasks and other bitmasks

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Being able to show a cpumask of events can be useful as some events may affect only some CPUs. There is no standard way to record the cpumask and converting it to a string is rather expensive during the trace as traces happen in hotpaths. It would be better to record the raw event mask and be able to parse it at print time.

The following macros were added for use with the TRACE_EVENT() macro:

__bitmask()
__assign_bitmask()
__get_bitmask()

To test this, I added this to the sched_migrate_task event, which looked like this:

TRACE_EVENT(sched_migrate_task,

	TP_PROTO(struct task_struct *p, int dest_cpu, const struct cpumask *cpus),

	TP_ARGS(p, dest_cpu, cpus),

	TP_STRUCT__entry(
		__array(	char,	comm,	TASK_COMM_LEN	)
		__field(	pid_t,	pid			)
		__field(	int,	prio			)
		__field(	int,	orig_cpu		)
		__field(	int,	dest_cpu		)
		__bitmask(	cpumask, cpus			)
	),

	TP_fast_assign(
		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
		__entry->pid		= p->pid;
		__entry->prio		= p->prio;
		__entry->orig_cpu	= task_cpu(p);
		__entry->dest_cpu	= dest_cpu;
		__assign_bitmask(cpumask, cpumask_bits(cpus));
	),

	TP_printk("comm=%s pid=%d prio=%d orig_cpu=%d dest_cpu=%d cpumask=%s",
		  __entry->comm, __entry->pid, __entry->prio,
		  __entry->orig_cpu, __entry->dest_cpu,
		  __get_bitmask(cpumask))

);

With the output of:

        ksmtuned-3613  [003] d..2   485.220508: sched_migrate_task: comm=ksmtuned pid=3615 prio=120 orig_cpu=3 dest_cpu=2 cpumask=00000000,0000000f
     migration/1-13    [001] d..5   485.221202: sched_migrate_task: comm=ksmtuned pid=3614 prio=120 orig_cpu=1 dest_cpu=0 cpumask=00000000,0000000f
             awk-3615  [002] d.H5   485.221747: sched_migrate_task: comm=rcu_preempt pid=7 prio=120 orig_cpu=0 dest_cpu=1 cpumask=00000000,000000ff
     migration/2-18    [002] d..5   485.222062: sched_migrate_task: comm=ksmtuned pid=3615 prio=120 orig_cpu=2 dest_cpu=3 cpumask=00000000,0000000f

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

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

---
 include/linux/ftrace_event.h |  3 +++
 include/linux/trace_seq.h    | 10 ++++++++
 include/trace/ftrace.h       | 57 +++++++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace_output.c  | 41 +++++++++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 1 deletion(-)

include/linux/ftrace_event.h +3 -0

index d16da3e..cff3106 100644
@@ -38,6 +38,9 @@ const char *ftrace_print_symbols_seq_u64(struct trace_seq *p,
 								 *symbol_array);
 #endif
 
+const char *ftrace_print_bitmask_seq(struct trace_seq *p, void *bitmask_ptr,
+				     unsigned int bitmask_size);
+
 const char *ftrace_print_hex_seq(struct trace_seq *p,
 				 const unsigned char *buf, int len);

include/linux/trace_seq.h +10 -0

index a32d86e..1361169 100644
@@ -46,6 +46,9 @@ extern int trace_seq_putmem_hex(struct trace_seq *s, const void *mem,
 extern void *trace_seq_reserve(struct trace_seq *s, size_t len);
 extern int trace_seq_path(struct trace_seq *s, const struct path *path);
 
+extern int trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp,
+			     int nmaskbits);
+
 #else /* CONFIG_TRACING */
 static inline int trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 {

@@ -57,6 +60,13 @@ trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary)
 	return 0;
 }
 
+static inline int
+trace_seq_bitmask(struct trace_seq *s, const unsigned long *maskp,
+		  int nmaskbits)
+{
+	return 0;
+}
+
 static inline int trace_print_seq(struct seq_file *m, struct trace_seq *s)
 {
 	return 0;

include/trace/ftrace.h +56 -1

110 lines not on this page · raw message

kernel/trace/trace_output.c +41 -0

53 lines not on this page · raw message

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:14 UTC #6 lore raw

PATCH 6/13 ftrace: Remove boolean of hash_enable and hash_disable

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Commit 4104d326b670 "ftrace: Remove global function list and call function directly" cleaned up the global_ops filtering and made the code simpler, but it left a variable "hash_enable" that was used to know if the hash functions should be updated or not. It was updated if the global_ops did not override them. As the global_ops are now no different than any other ftrace_ops, the hash always gets updated and there's no reason to use the hash_enable boolean.

The same goes for hash_disable used in ftrace_shutdown().

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

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

kernel/trace/ftrace.c +3 -6

index 38e5cf7..2c99d1f 100644
@@ -2042,7 +2042,6 @@ static void ftrace_startup_enable(int command)
 
 static int ftrace_startup(struct ftrace_ops *ops, int command)
 {
-	bool hash_enable = true;
 	int ret;
 
 	if (unlikely(ftrace_disabled))

@@ -2056,8 +2055,8 @@ static int ftrace_startup(struct ftrace_ops *ops, int command)
 	command |= FTRACE_UPDATE_CALLS;
 
 	ops->flags |= FTRACE_OPS_FL_ENABLED;
-	if (hash_enable)
-		ftrace_hash_rec_enable(ops, 1);
+
+	ftrace_hash_rec_enable(ops, 1);
 
 	ftrace_startup_enable(command);

@@ -2066,7 +2065,6 @@ static int ftrace_startup(struct ftrace_ops *ops, int command)
 
 static int ftrace_shutdown(struct ftrace_ops *ops, int command)
 {
-	bool hash_disable = true;
 	int ret;
 
 	if (unlikely(ftrace_disabled))

@@ -2084,8 +2082,7 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command)
 	 */
 	WARN_ON_ONCE(ftrace_start_up < 0);
 
-	if (hash_disable)
-		ftrace_hash_rec_disable(ops, 1);
+	ftrace_hash_rec_disable(ops, 1);
 
 	if (!global_start_up)
 		ops->flags &= ~FTRACE_OPS_FL_ENABLED;

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:14 UTC #7 lore raw

PATCH 7/13 ftrace: Write in missing comment from a very old commit

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Back in 2011 Commit ed926f9b35cda "ftrace: Use counters to enable functions to trace" changed the way ftrace accounts for enabled and disabled traced functions. There was a comment started as:

	/*
	 *
	 */

But never finished. Well, that's rather useless. I probably forgot to save the file before committing it. And it passed review from all this time.

Anyway, better late than never. I updated the comment to express what is happening in that somewhat complex code.

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

---
 kernel/trace/ftrace.c | 7 +++++++
 1 file changed, 7 insertions(+)

kernel/trace/ftrace.c +7 -0

index 2c99d1f..61f39f8 100644
@@ -1552,7 +1552,14 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
 			in_other_hash = !!ftrace_lookup_ip(other_hash, rec->ip);
 
 			/*
+			 * If filter_hash is set, we want to match all functions
+			 * that are in the hash but not in the other hash.
 			 *
+			 * If filter_hash is not set, then we are decrementing.
+			 * That means we match anything that is in the hash
+			 * and also in the other_hash. That is, we need to turn
+			 * off functions in the other hash because they are disabled
+			 * by this hash.
 			 */
 			if (filter_hash && in_hash && !in_other_hash)
 				match = 1;

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:14 UTC #8 lore raw

PATCH 8/13 ftrace: Always inline ftrace_hash_empty() helper function

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

The ftrace_hash_empty() function is a simple test:

	return !hash || !hash->count;

But gcc seems to want to make it a call. As this is in an extreme hot path of the function tracer, there's no reason it needs to be a call. I only wrote it to be a helper function anyway, otherwise it would have been inlined manually.

Force gcc to inline it, as it could have also been a macro.

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

---
 kernel/trace/ftrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/trace/ftrace.c +1 -1

index 61f39f8..98fa931 100644
@@ -1105,7 +1105,7 @@ struct ftrace_page {
 static struct ftrace_page	*ftrace_pages_start;
 static struct ftrace_page	*ftrace_pages;
 
-static bool ftrace_hash_empty(struct ftrace_hash *hash)
+static bool __always_inline ftrace_hash_empty(struct ftrace_hash *hash)
 {
 	return !hash || !hash->count;
 }

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:13 UTC #9 lore raw

PATCH 9/13 ftrace/x86: Get the current mcount addr for add_breakpoint()

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

The add_breakpoint() code in the ftrace updating gets the address of what the call will become, but if the mcount address is changing from regs to non-regs ftrace_caller or vice versa, it will use what the record currently is.

This is rather silly as the code should always use what is currently there regardless of if it's changing the regs function or just converting to a nop.

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

---
 arch/x86/kernel/ftrace.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

arch/x86/kernel/ftrace.c +2 -4

index 3d837b0..4b3c195 100644
@@ -380,9 +380,9 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)
 	unsigned long ftrace_addr;
 	int ret;
 
-	ret = ftrace_test_record(rec, enable);
+	ftrace_addr = get_ftrace_old_addr(rec);
 
-	ftrace_addr = get_ftrace_addr(rec);
+	ret = ftrace_test_record(rec, enable);
 
 	switch (ret) {
 	case FTRACE_UPDATE_IGNORE:

@@ -394,8 +394,6 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)
 
 	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
-		ftrace_addr = get_ftrace_old_addr(rec);
-		/* fall through */
 	case FTRACE_UPDATE_MAKE_NOP:
 		/* converting a call to a nop */
 		return add_brk_on_call(rec, ftrace_addr);

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:14 UTC #10 lore raw

PATCH 10/13 ftrace: Make get_ftrace_addr() and get_ftrace_addr_old() global

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

Move and rename get_ftrace_addr() and get_ftrace_addr_old() to ftrace_get_addr_new() and ftrace_get_addr_curr() respectively.

This moves these two helper functions in the generic code out from the arch specific code, and renames them to have a better generic name. This will allow other archs to use them as well as makes it a bit easier to work on getting separate trampolines for different functions.

ftrace_get_addr_new() returns the trampoline address that the mcount call address will be converted to.

ftrace_get_addr_curr() returns the trampoline address of what the mcount call address currently jumps to.

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

---
 arch/x86/kernel/ftrace.c | 36 +++++-------------------------------
 include/linux/ftrace.h   |  2 ++
 kernel/trace/ftrace.c    | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 31 deletions(-)

arch/x86/kernel/ftrace.c +5 ~~-31~~

index 4b3c195..5ef43ce 100644
@@ -349,38 +349,12 @@ static int add_brk_on_nop(struct dyn_ftrace *rec)
 	return add_break(rec->ip, old);
 }
 
-/*
- * If the record has the FTRACE_FL_REGS set, that means that it
- * wants to convert to a callback that saves all regs. If FTRACE_FL_REGS
- * is not not set, then it wants to convert to the normal callback.
- */
-static unsigned long get_ftrace_addr(struct dyn_ftrace *rec)
-{
-	if (rec->flags & FTRACE_FL_REGS)
-		return (unsigned long)FTRACE_REGS_ADDR;
-	else
-		return (unsigned long)FTRACE_ADDR;
-}
-
-/*
- * The FTRACE_FL_REGS_EN is set when the record already points to
- * a function that saves all the regs. Basically the '_EN' version
- * represents the current state of the function.
- */
-static unsigned long get_ftrace_old_addr(struct dyn_ftrace *rec)
-{
-	if (rec->flags & FTRACE_FL_REGS_EN)
-		return (unsigned long)FTRACE_REGS_ADDR;
-	else
-		return (unsigned long)FTRACE_ADDR;
-}
-
 static int add_breakpoints(struct dyn_ftrace *rec, int enable)
 {
 	unsigned long ftrace_addr;
 	int ret;
 
-	ftrace_addr = get_ftrace_old_addr(rec);
+	ftrace_addr = ftrace_get_addr_curr(rec);
 
 	ret = ftrace_test_record(rec, enable);

@@ -438,14 +412,14 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
 		 * If not, don't touch the breakpoint, we make just create
 		 * a disaster.
 		 */
-		ftrace_addr = get_ftrace_addr(rec);
+		ftrace_addr = ftrace_get_addr_new(rec);
 		nop = ftrace_call_replace(ip, ftrace_addr);
 
 		if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) == 0)
 			goto update;
 
 		/* Check both ftrace_addr and ftrace_old_addr */
-		ftrace_addr = get_ftrace_old_addr(rec);
+		ftrace_addr = ftrace_get_addr_curr(rec);
 		nop = ftrace_call_replace(ip, ftrace_addr);
 
 		if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) != 0)

@@ -489,7 +463,7 @@ static int add_update(struct dyn_ftrace *rec, int enable)
 
 	ret = ftrace_test_record(rec, enable);
 
-	ftrace_addr  = get_ftrace_addr(rec);
+	ftrace_addr  = ftrace_get_addr_new(rec);
 
 	switch (ret) {
 	case FTRACE_UPDATE_IGNORE:

@@ -536,7 +510,7 @@ static int finish_update(struct dyn_ftrace *rec, int enable)
 
 	ret = ftrace_update_record(rec, enable);
 
-	ftrace_addr = get_ftrace_addr(rec);
+	ftrace_addr = ftrace_get_addr_new(rec);
 
 	switch (ret) {
 	case FTRACE_UPDATE_IGNORE:

include/linux/ftrace.h +2 -0

index f0ff2c2..2f8cbff 100644
@@ -400,6 +400,8 @@ int ftrace_update_record(struct dyn_ftrace *rec, int enable);
 int ftrace_test_record(struct dyn_ftrace *rec, int enable);
 void ftrace_run_stop_machine(int command);
 unsigned long ftrace_location(unsigned long ip);
+unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec);
+unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec);
 
 extern ftrace_func_t ftrace_trace_function;

kernel/trace/ftrace.c +36 -0

index 98fa931..e825fde 100644
@@ -1755,6 +1755,42 @@ int ftrace_test_record(struct dyn_ftrace *rec, int enable)
 	return ftrace_check_record(rec, enable, 0);
 }
 
+/**
+ * ftrace_get_addr_new - Get the call address to set to
+ * @rec:  The ftrace record descriptor
+ *
+ * If the record has the FTRACE_FL_REGS set, that means that it
+ * wants to convert to a callback that saves all regs. If FTRACE_FL_REGS
+ * is not not set, then it wants to convert to the normal callback.
+ *
+ * Returns the address of the trampoline to set to
+ */
+unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec)
+{
+	if (rec->flags & FTRACE_FL_REGS)
+		return (unsigned long)FTRACE_REGS_ADDR;
+	else
+		return (unsigned long)FTRACE_ADDR;
+}
+
+/**
+ * ftrace_get_addr_curr - Get the call address that is already there
+ * @rec:  The ftrace record descriptor
+ *
+ * The FTRACE_FL_REGS_EN is set when the record already points to
+ * a function that saves all the regs. Basically the '_EN' version
+ * represents the current state of the function.
+ *
+ * Returns the address of the trampoline that is currently being called
+ */
+unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec)
+{
+	if (rec->flags & FTRACE_FL_REGS_EN)
+		return (unsigned long)FTRACE_REGS_ADDR;
+	else
+		return (unsigned long)FTRACE_ADDR;
+}
+
 static int
 __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
 {

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:13 UTC #11 lore raw

PATCH 11/13 ftrace: Use the ftrace_addr helper functions to find the ftrace_addr

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

With the moving of the functions that determine what the mcount call site should be replaced with into the generic code, there is a few places in the generic code that can use them instead of hard coding it as it does.

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

---
 kernel/trace/ftrace.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

kernel/trace/ftrace.c +5 ~~-10~~

index e825fde..52c2b53 100644
@@ -1798,12 +1798,12 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
 	unsigned long ftrace_addr;
 	int ret;
 
-	ret = ftrace_update_record(rec, enable);
+	ftrace_addr = ftrace_get_addr_new(rec);
 
-	if (rec->flags & FTRACE_FL_REGS)
-		ftrace_addr = (unsigned long)FTRACE_REGS_ADDR;
-	else
-		ftrace_addr = (unsigned long)FTRACE_ADDR;
+	/* This needs to be done before we call ftrace_update_record */
+	ftrace_old_addr = ftrace_get_addr_curr(rec);
+
+	ret = ftrace_update_record(rec, enable);
 
 	switch (ret) {
 	case FTRACE_UPDATE_IGNORE:

@@ -1817,11 +1817,6 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
 
 	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
-		if (rec->flags & FTRACE_FL_REGS)
-			ftrace_old_addr = (unsigned long)FTRACE_ADDR;
-		else
-			ftrace_old_addr = (unsigned long)FTRACE_REGS_ADDR;
-
 		return ftrace_modify_call(rec, ftrace_old_addr, ftrace_addr);
 	}

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:13 UTC #12 lore raw

PATCH 12/13 ftrace: Remove FTRACE_UPDATE_MODIFY_CALL_REGS flag

In reply to Steven Rostedt

From: "Steven Rostedt (Red Hat)" <[email protected]>

As the decision to what needs to be done (converting a call to the ftrace_caller to ftrace_caller_regs or to convert from ftrace_caller_regs to ftrace_caller) can easily be determined from the rec->flags of FTRACE_FL_REGS and FTRACE_FL_REGS_EN, there's no need to have the ftrace_check_record() return either a UPDATE_MODIFY_CALL_REGS or a UPDATE_MODIFY_CALL. Just he latter is enough. This added flag causes more complexity than is required. Remove it.

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

---
 arch/x86/kernel/ftrace.c |  3 ---
 include/linux/ftrace.h   |  2 --
 kernel/trace/ftrace.c    | 13 ++++---------
 3 files changed, 4 insertions(+), 14 deletions(-)

arch/x86/kernel/ftrace.c +0 -3

index 5ef43ce..89de3ea 100644
@@ -366,7 +366,6 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)
 		/* converting nop to call */
 		return add_brk_on_nop(rec);
 
-	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
 	case FTRACE_UPDATE_MAKE_NOP:
 		/* converting a call to a nop */

@@ -469,7 +468,6 @@ static int add_update(struct dyn_ftrace *rec, int enable)
 	case FTRACE_UPDATE_IGNORE:
 		return 0;
 
-	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
 	case FTRACE_UPDATE_MAKE_CALL:
 		/* converting nop to call */

@@ -516,7 +514,6 @@ static int finish_update(struct dyn_ftrace *rec, int enable)
 	case FTRACE_UPDATE_IGNORE:
 		return 0;
 
-	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
 	case FTRACE_UPDATE_MAKE_CALL:
 		/* converting nop to call */

include/linux/ftrace.h +0 -2

index 2f8cbff..3e6dfb3 100644
@@ -362,14 +362,12 @@ enum {
  *  IGNORE           - The function is already what we want it to be
  *  MAKE_CALL        - Start tracing the function
  *  MODIFY_CALL      - Stop saving regs for the function
- *  MODIFY_CALL_REGS - Start saving regs for the function
  *  MAKE_NOP         - Stop tracing the function
  */
 enum {
 	FTRACE_UPDATE_IGNORE,
 	FTRACE_UPDATE_MAKE_CALL,
 	FTRACE_UPDATE_MODIFY_CALL,
-	FTRACE_UPDATE_MODIFY_CALL_REGS,
 	FTRACE_UPDATE_MAKE_NOP,
 };

kernel/trace/ftrace.c +4 -9

index 52c2b53..cc07b7f 100644
@@ -1701,19 +1701,15 @@ static int ftrace_check_record(struct dyn_ftrace *rec, int enable, int update)
 		/*
 		 * If this record is being updated from a nop, then
 		 *   return UPDATE_MAKE_CALL.
-		 * Otherwise, if the EN flag is set, then return
-		 *   UPDATE_MODIFY_CALL_REGS to tell the caller to convert
-		 *   from the non-save regs, to a save regs function.
 		 * Otherwise,
 		 *   return UPDATE_MODIFY_CALL to tell the caller to convert
-		 *   from the save regs, to a non-save regs function.
+		 *   from the save regs, to a non-save regs function or
+		 *   vice versa.
 		 */
 		if (flag & FTRACE_FL_ENABLED)
 			return FTRACE_UPDATE_MAKE_CALL;
-		else if (rec->flags & FTRACE_FL_REGS_EN)
-			return FTRACE_UPDATE_MODIFY_CALL_REGS;
-		else
-			return FTRACE_UPDATE_MODIFY_CALL;
+
+		return FTRACE_UPDATE_MODIFY_CALL;
 	}
 
 	if (update) {

@@ -1815,7 +1811,6 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
 	case FTRACE_UPDATE_MAKE_NOP:
 		return ftrace_make_nop(NULL, rec, ftrace_addr);
 
-	case FTRACE_UPDATE_MODIFY_CALL_REGS:
 	case FTRACE_UPDATE_MODIFY_CALL:
 		return ftrace_modify_call(rec, ftrace_old_addr, ftrace_addr);
 	}

Signature · 1 line

1.8.5.3

Steven Rostedt

author maintainer

2014-05-12 15:13 UTC #13 lore raw

PATCH 13/13 ftrace/x86: Move the mcount/fentry code out of entry_64.S

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

As the mcount code gets more complex, it really does not belong in the entry.S file. By moving it into its own file "mcount.S" keeps things a bit cleaner.

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

Acked-by Thomas Gleixner <[email protected]>

Acked-by Jiri Kosina <[email protected]>

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

---
 arch/x86/kernel/Makefile    |   1 +
 arch/x86/kernel/entry_64.S  | 204 -----------------------------------------
 arch/x86/kernel/mcount_64.S | 217 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 218 insertions(+), 204 deletions(-)
 create mode 100644 arch/x86/kernel/mcount_64.S

arch/x86/kernel/Makefile +1 -0

index f4d9600..db7f41d 100644
@@ -26,6 +26,7 @@ obj-$(CONFIG_IRQ_WORK)  += irq_work.o
 obj-y			+= probe_roms.o
 obj-$(CONFIG_X86_32)	+= i386_ksyms_32.o
 obj-$(CONFIG_X86_64)	+= sys_x86_64.o x8664_ksyms_64.o
+obj-$(CONFIG_X86_64)	+= mcount_64.o
 obj-y			+= syscall_$(BITS).o vsyscall_gtod.o
 obj-$(CONFIG_X86_64)	+= vsyscall_64.o
 obj-$(CONFIG_X86_64)	+= vsyscall_emu_64.o

arch/x86/kernel/entry_64.S +0 ~~-204~~

216 lines not on this page · raw message

arch/x86/kernel/mcount_64.S new file +217 -0

217 lines not on this page · raw message

Signature · 1 line

1.8.5.3