public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <compudj@krystal.dyndns.org>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Theodore Tso <tytso@mit.edu>,
	Arjan van de Ven <arjan@infradead.org>,
	Christoph Hellwig <hch@lst.de>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	Zhaolei <zhaolei@cn.fujitsu.com>, Li Zefan <lizf@cn.fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Masami Hiramatsu <mhiramat@redhat.com>,
	"Frank Ch. Eigler" <fche@elastic.org>,
	Tom Zanussi <tzanussi@gmail.com>,
	Jiaying Zhang <jiayingz@google.com>,
	Michael Rubin <mrubin@google.com>,
	Martin Bligh <mbligh@google.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Neil Horman <nhorman@tuxdriver.com>,
	Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>,
	Pekka Enberg <penberg@cs.helsinki.fi>
Subject: Re: [PATCH 2/8] tracing: create automated trace defines
Date: Thu, 16 Apr 2009 20:28:31 -0400	[thread overview]
Message-ID: <20090417002831.GC20513@Krystal> (raw)
In-Reply-To: <49E7CADE.1060202@goop.org>

* Jeremy Fitzhardinge (jeremy@goop.org) wrote:
> Mathieu Desnoyers wrote:
>>> Are they?  They're generally considered to be "free", because the 
>>> call  and return are predicted 100% accurately.
>>>
>>>     
>>
>> Adding a simple function call within the tracer fast path, in LTTng, has
>> a very measurable performance impact on the tbench workload. This is why
>> I don't use any function call-based trace clocks in LTTng, but rather my
>> own inline trace clock.
>
> I'm a bit concerned about all the code that tracing puts inline though.   
> It seems it would put quite a lot of icache overhead on the codepath  
> when the tracepoint is disabled, not least because its duplicated in  
> every instance of the tracepoint.  And if the compiler decides to put  
> the unlikely() branch code out of line, then that's the same as making  
> it a function call (except that if it is a function call, all the  
> tracepoints will share the same code, and get a higher likelihood of  
> getting icache hits).
>

"all this code" is actually :

               rcu_read_lock_sched_notrace();                          \
                it_func = rcu_dereference((tp)->funcs);                 \
                if (it_func) {                                          \
                        do {                                            \
                                ((void(*)(proto))(*it_func))(args);     \
                        } while (*(++it_func));                         \
                }                                                       \
                rcu_read_unlock_sched_notrace();                        \

Which does nothing more than disabling preemption and a for loop to
call all the tracepoint handlers. I don't see the big win in laying out
the stack to call this code out-of-line; we would just remove the
preempt disable and the loop, which are minimal compared to most
call stacks.

So basically, tracepoints are already just doing a function call, with a
few more bytes for preempt disable and multiple handler support.

About the compiler deciding to put the unlikely branch out-of-line, I've
never seen any function calls generated just for the sake of saving
those few bytes, that would be crazy of the part of the compiler.
However, it can (and should) freely put the stack setup in the coldest
cache-lines possible, which are reachable by a near jump.

Mathieu

>
>    J
>

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

  reply	other threads:[~2009-04-17  0:28 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-14 17:23 [PATCH 0/8] [GIT PULL] TRACE_EVENT for modules Steven Rostedt
2009-04-14 17:23 ` [PATCH 1/8] tracing: consolidate trace and trace_event headers Steven Rostedt
2009-04-14 21:51   ` Frederic Weisbecker
2009-04-14 22:04     ` Steven Rostedt
2009-04-14 17:23 ` [PATCH 2/8] tracing: create automated trace defines Steven Rostedt
2009-04-14 23:44   ` Jeremy Fitzhardinge
2009-04-15  1:45     ` Mathieu Desnoyers
2009-04-15 16:07       ` Jeremy Fitzhardinge
2009-04-16  2:34         ` Mathieu Desnoyers
2009-04-16  2:56           ` Jeremy Fitzhardinge
2009-04-16 23:44             ` Mathieu Desnoyers
2009-04-17  0:03               ` Jeremy Fitzhardinge
2009-04-17  0:13                 ` Mathieu Desnoyers
2009-04-17  0:18                   ` Jeremy Fitzhardinge
2009-04-17  0:28                     ` Mathieu Desnoyers [this message]
2009-04-17  0:43                       ` Jeremy Fitzhardinge
2009-04-17  3:05                         ` [PATCH] tracepoints : let subsystem nop-out the tracepoints at build time Mathieu Desnoyers
2009-04-20  7:12               ` [PATCH 2/8] tracing: create automated trace defines Andi Kleen
2009-04-21 15:51                 ` Mathieu Desnoyers
2009-04-21 17:18                   ` Jeremy Fitzhardinge
2009-04-21 17:21                     ` Steven Rostedt
2009-04-21 17:43                       ` Jeremy Fitzhardinge
2009-04-21 20:28                       ` Andi Kleen
2009-04-21 21:17                         ` Steven Rostedt
2009-04-21 21:23                           ` Frank Ch. Eigler
2009-04-21 21:33                             ` Steven Rostedt
2009-04-22  5:47                               ` Mathieu Desnoyers
2009-04-22  6:07                           ` Andi Kleen
2009-04-22  6:24                             ` Steven Rostedt
2009-04-22  7:26                               ` Andi Kleen
2009-04-15  7:04   ` Zhaolei
2009-04-14 17:23 ` [PATCH 3/8] tracing: make trace_seq operations available for core kernel Steven Rostedt
2009-04-14 19:12   ` Peter Zijlstra
2009-04-15  2:19     ` Steven Rostedt
2009-04-14 17:23 ` [PATCH 4/8] tracing/events: move declarations from trace directory to core include Steven Rostedt
2009-04-14 17:23 ` [PATCH 5/8] tracing/events: move the ftrace event tracing code to core Steven Rostedt
2009-04-14 19:23   ` Peter Zijlstra
2009-04-15  2:25     ` Steven Rostedt
2009-04-15  3:40       ` Jiaying Zhang
2009-04-14 17:23 ` [PATCH 6/8] tracing/events: convert event call sites to use a link list Steven Rostedt
2009-04-14 17:23 ` [PATCH 7/8] tracing/events: add export symbols for trace events in modules Steven Rostedt
2009-04-14 17:23 ` [PATCH 8/8] tracing/events: add support for modules to TRACE_EVENT Steven Rostedt
2009-04-15  3:22   ` Rusty Russell
2009-04-14 18:15 ` [PATCH 0/8] [GIT PULL] TRACE_EVENT for modules Ingo Molnar
2009-04-14 18:25   ` Ingo Molnar
2009-04-14 18:21 ` Ingo Molnar
2009-04-14 18:33   ` Steven Rostedt
2009-04-14 18:35     ` Ingo Molnar
2009-04-14 21:04 ` Theodore Tso
2009-04-14 21:23   ` Steven Rostedt
2009-04-14 21:59     ` Steven Rostedt
2009-04-14 21:29   ` Frank Ch. Eigler
2009-04-14 22:00     ` Steven Rostedt
2009-04-16 16:53     ` Christoph Hellwig
2009-04-14 21:48   ` Jeremy Fitzhardinge
2009-04-14 21:55     ` Steven Rostedt
2009-04-14 22:33       ` Jeremy Fitzhardinge
2009-04-15  8:29         ` Ingo Molnar
2009-04-16  2:29           ` Mathieu Desnoyers
2009-04-16 16:52   ` Christoph Hellwig

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=20090417002831.GC20513@Krystal \
    --to=compudj@krystal.dyndns.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@infradead.org \
    --cc=eduard.munteanu@linux360.ro \
    --cc=fche@elastic.org \
    --cc=fweisbec@gmail.com \
    --cc=hch@lst.de \
    --cc=jeremy@goop.org \
    --cc=jiayingz@google.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizf@cn.fujitsu.com \
    --cc=mbligh@google.com \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=mrubin@google.com \
    --cc=nhorman@tuxdriver.com \
    --cc=penberg@cs.helsinki.fi \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tytso@mit.edu \
    --cc=tzanussi@gmail.com \
    --cc=zhaolei@cn.fujitsu.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox