mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Jason Baron <jbaron@redhat.com>
Cc: linux-kernel@vger.kernel.org, mingo@elte.hu,
	mathieu.desnoyers@polymtl.ca, tglx@linutronix.de,
	rostedt@goodmis.org, ak@suse.de, rth@redhat.com,
	mhiramat@redhat.com
Subject: Re: [PATCH 0/4] jump label patches
Date: Mon,  5 Oct 2009 22:39:15 -0700 (PDT)	[thread overview]
Message-ID: <20091006053915.D9D0928@magilla.sf.frob.com> (raw)
In-Reply-To: Jason Baron's message of  Thursday, 24 September 2009 19:17:45 -0400 <cover.1253831945.git.jbaron@redhat.com>

I am, of course, fully in favor of this hack.  This version raises a new
concern for me vs what we had discussed before.  I don't know what the
conclusion about this should be, but I think it should be aired.

In the previous plan, we had approximately:

	asm goto ("1:" P6_NOP5
		  ".pushsection __jump_table\n"
		  _ASM_PTR "1b, %l[do_trace]\n"
		  ".popsection" : : : do_trace);
	if (0) { do_trace: ... tracing_path(); ... }
	... hot_path(); ...

That is, the straight-line code path is a 5-byte nop.  To enable the
"static if" at runtime, we replace that with a "jmp .Ldo_trace".
So, disabled:

	0x1:	nopl
	0x6:	hot path
	...
	0x100:	ret		# or jmp somewhere else, whatever
	...
	0x234:	tracing path	# never reached
	...
	0x250:	jmp 0x6

and enabled:

	0x1:	jmp 0x234
	0x6:	hot path
	...
	0x100:	ret
	...
	0x234:	tracing path
	...
	0x250:	jmp 0x6


In your new plan, instead we now have approximately:

	asm goto ("1: jmp %l[dont_trace]\n"
		  ".pushsection __jump_table\n"
		  _ASM_PTR "1b, %l[dont_trace]\n"
		  ".popsection" : : : dont_trace);
	... tracing path ...
	dont_trace:
	... hot_path(); ...

That is, we've inverted the sense of the control flow: the straight-line
code path is the tracing path, and in default "disabled" state we jump
around the tracing path to get to the hot path.
So, disabled:

	0x1:	jmp 0x1f
	0x3:	tracing path	# never reached
	...
	0x1f:	hot path
	...
	0x119:	ret

and enabled:

	0x1:	jmp 0x3
	0x3:	tracing path
	...
	0x1f:	hot path
	...
	0x119:	ret


As I understand it, the point of the exercise is to optimize the "disabled"
case to as close as possible to what we'd get with no tracing path compiled
in at all.  In the first example (with "nopl"), it's easy to see how that
is what we presume is pretty close to epsilon addition: the execution cost
of the 5-byte nop, plus the indirect effects of those 5 bytes polluting the
I-cache.  We only really know when we measure, but that just seems likely
to be minimally obtrustive.

In the second example (with "jmp around"), I really wonder what the actual
overhead is.  There's the cost of the jmp itself, plus maybe whatever extra
jumps do to branch predictions or pipelines or whatnots of which I know not
much, plus the entire tracing path being right there adjacent using up the
I-cache space that would otherwise be keeping more of the hot path hot.
I'm sure others on the list have more insight than I do into what the
specific performance impacts we can expect from one code sequence or the
other on various chips.

Of course, a first important point is what the actual compiled code
sequences look like.  I'm hoping Richard (who implemented the compiler
feature for us) can help us with making sure our expectations jibe with the
code we'll really get.  There's no benefit in optimizing our asm not to
introduce a jump into the hot path if the compiler actually generates the
tracing path first and gives the hot path a "jmp" around it anyway.

The code example above assumes that "if (0)" is enough for the compiler to
put that code fork (where the "do_trace:" label is) somewhere out of the
straight-line path rather than jumping around it.  Going on the "belt and
suspenders" theory as to being thoroughly explicit to the compiler what we
intend, I'd go for:

	if (__builtin_expect(0,0)) do_trace: __attribute__((cold)) { ... }

But we need Richard et al to tell us what actually makes a difference to
the compiler's optimizer, and will reliably continue to do so in the future.


Thanks,
Roland

  parent reply	other threads:[~2009-10-06  5:40 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-24 23:17 Jason Baron
2009-09-24 23:17 ` [PATCH 1/4] jump label - make init_kernel_text() global Jason Baron
2009-10-01 11:20   ` Ingo Molnar
2009-10-01 12:58     ` Mathieu Desnoyers
2009-10-01 20:39     ` Jason Baron
2009-10-03 10:43       ` Ingo Molnar
2009-10-03 12:39         ` Mathieu Desnoyers
2009-10-07  1:54           ` Steven Rostedt
2009-10-07  2:32             ` Mathieu Desnoyers
2009-10-07  3:10               ` Masami Hiramatsu
2009-10-07  3:23                 ` Mathieu Desnoyers
2009-10-07  3:29               ` Mathieu Desnoyers
2009-10-07 12:56               ` Steven Rostedt
2009-10-07 13:35                 ` Mathieu Desnoyers
2009-09-24 23:17 ` [PATCH 2/4] jump label - base patch Jason Baron
2009-09-25  0:49   ` Roland McGrath
2009-09-26 10:21     ` Steven Rostedt
2009-10-01 11:36   ` Ingo Molnar
2009-09-24 23:17 ` [PATCH 3/4] jump label - add module support Jason Baron
2009-09-24 23:18 ` [PATCH 4/4] jump label - tracepoint implementation Jason Baron
2009-10-06  5:39 ` Roland McGrath [this message]
2009-10-06 14:07   ` [PATCH 0/4] jump label patches Jason Baron
2009-10-06 23:24   ` Richard Henderson
2009-10-07  0:14     ` Roland McGrath
2009-10-07 15:35       ` Richard Henderson
2009-10-06  6:04 ` Roland McGrath
2009-10-06 14:09   ` Steven Rostedt
2009-10-06 14:13   ` Masami Hiramatsu
2009-10-06 14:30     ` Mathieu Desnoyers

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=20091006053915.D9D0928@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=ak@suse.de \
    --cc=jbaron@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=rostedt@goodmis.org \
    --cc=rth@redhat.com \
    --cc=tglx@linutronix.de \
    /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

all inboxes | Powered by JetHome®