All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tuomas Tynkkynen <ttynkkynen@nvidia.com>
To: Steven Rostedt <rostedt@goodmis.org>, <linux-kernel@vger.kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [for-next][PATCH 04/21] ftrace: Optimize function graph to be called directly
Date: Thu, 10 Jul 2014 20:45:57 +0300	[thread overview]
Message-ID: <53BED155.9040607@nvidia.com> (raw)
In-Reply-To: <20140703161224.062459692@goodmis.org>

On 03/07/14 19:05, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)"<rostedt@goodmis.org>
>
> Function graph tracing is a bit different than the function tracers, as
> it is processed after either the ftrace_caller or ftrace_regs_caller
> and we only have one place to modify the jump to ftrace_graph_caller,
> the jump needs to happen after the restore of registeres.
>
> The function graph tracer is dependent on the function tracer, where
> even if the function graph tracing is going on by itself, the save and
> restore of registers is still done for function tracing regardless of
> if function tracing is happening, before it calls the function graph
> code.
>
> If there's no function tracing happening, it is possible to just call
> the function graph tracer directly, and avoid the wasted effort to save
> and restore regs for function tracing.
>
> This requires adding new flags to the dyn_ftrace records:
>
>    FTRACE_FL_TRAMP
>    FTRACE_FL_TRAMP_EN
>
> The first is set if the count for the record is one, and the ftrace_ops
> associated to that record has its own trampoline. That way the mcount code
> can call that trampoline directly.
>
> In the future, trampolines can be added to arbitrary ftrace_ops, where you
> can have two or more ftrace_ops registered to ftrace (like kprobes and perf)
> and if they are not tracing the same functions, then instead of doing a
> loop to check all registered ftrace_ops against their hashes, just call the
> ftrace_ops trampoline directly, which would call the registered ftrace_ops
> function directly.
>
> Without this patch perf showed:
>
>    0.05%  hackbench  [kernel.kallsyms]  [k] ftrace_caller
>    0.05%  hackbench  [kernel.kallsyms]  [k] arch_local_irq_save
>    0.05%  hackbench  [kernel.kallsyms]  [k] native_sched_clock
>    0.04%  hackbench  [kernel.kallsyms]  [k] __buffer_unlock_commit
>    0.04%  hackbench  [kernel.kallsyms]  [k] preempt_trace
>    0.04%  hackbench  [kernel.kallsyms]  [k] prepare_ftrace_return
>    0.04%  hackbench  [kernel.kallsyms]  [k] __this_cpu_preempt_check
>    0.04%  hackbench  [kernel.kallsyms]  [k] ftrace_graph_caller
>
> See that the ftrace_caller took up more time than the ftrace_graph_caller
> did.
>
> With this patch:
>
>    0.05%  hackbench  [kernel.kallsyms]  [k] __buffer_unlock_commit
>    0.04%  hackbench  [kernel.kallsyms]  [k] call_filter_check_discard
>    0.04%  hackbench  [kernel.kallsyms]  [k] ftrace_graph_caller
>    0.04%  hackbench  [kernel.kallsyms]  [k] sched_clock
>
> The ftrace_caller is no where to be found and ftrace_graph_caller still
> takes up the same percentage.
>
> Signed-off-by: Steven Rostedt<rostedt@goodmis.org>
> ---
>   arch/x86/kernel/mcount_64.S |   5 +
>   include/linux/ftrace.h      |  19 +++-
>   kernel/trace/ftrace.c       | 242 ++++++++++++++++++++++++++++++++++++++++++--
>   3 files changed, 254 insertions(+), 12 deletions(-)
>

This commit (79922b8) breaks the function graph tracer on today's -next. 
This is on an ARM Tegra board.

I'm using ftrace with this script:

#!/bin/sh
echo function_graph > /d/tracing/current_tracer
echo clear > /d/tracing/trace
echo $$ > /d/tracing/set_ftrace_pid
exec "$@"

...and a simple './trace.sh ls' crashes with no console output. SysRq is 
not responsive either.

  reply	other threads:[~2014-07-10 17:46 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 16:05 [for-next][PATCH 00/21] tracing: Updates for 3.17 Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 01/21] ftrace: Allow no regs if no more callbacks require it Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 02/21] ftrace: Use macros for numbers in ftrace rec shift bits Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 03/21] ftrace: Add ftrace_rec_counter() macro to simplify the code Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 04/21] ftrace: Optimize function graph to be called directly Steven Rostedt
2014-07-10 17:45   ` Tuomas Tynkkynen [this message]
2014-07-10 18:01     ` Steven Rostedt
2014-07-12  3:36     ` Steven Rostedt
2014-07-12  3:37       ` Steven Rostedt
2014-07-14 13:46         ` Tuomas Tynkkynen
2014-07-14 16:14           ` Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 05/21] ftrace: Add trampolines to enabled_functions debug file Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 06/21] tracing: Change trace event sample to use strlcpy instead of strncpy Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 07/21] ftrace: Simplify ftrace_hash_disable/enable path in ftrace_hash_move Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 08/21] tracing: Move the trace_seq_* functions into its own trace_seq.c file Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 09/21] tracing: Clean up trace_seq.c Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 10/21] tracing: Make trace_seq_putmem_hex() more robust Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 11/21] tracing: Remove trace_seq_reserve() Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 12/21] tracing: Remove unnecessary null test before debugfs_remove() Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 13/21] tracing: Add trace_seq_buffer_ptr() helper function Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 14/21] ftrace: Get rid of obsolete global_start_up variable Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 15/21] ftrace: Fix memory leak on failure path in ftrace_allocate_pages() Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 16/21] ftrace: Do not copy hash if O_TRUNC is set Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 17/21] tracing: Convert pr_warning() to pr_warn() in trace_events.c Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 18/21] tracing: Add ftrace_graph_notrace boot parameter Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 19/21] tracing: Improve message of empty set_graph_notrace file Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 20/21] tracing: Improve message of empty set_ftrace_notrace file Steven Rostedt
2014-07-03 16:05 ` [for-next][PATCH 21/21] tracing: Add description of set_graph_notrace to tracing/README Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53BED155.9040607@nvidia.com \
    --to=ttynkkynen@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.