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, hpa@zytor.com, tglx@linutronix.de,
	rostedt@goodmis.org, andi@firstfloor.org, rth@redhat.com,
	mhiramat@redhat.com
Subject: Re: [RFC PATCH 0/6] jump label v3
Date: Wed, 18 Nov 2009 19:54:24 -0800 (PST)	[thread overview]
Message-ID: <20091119035424.B3E8B1E2C@magilla.sf.frob.com> (raw)
In-Reply-To: Jason Baron's message of  Wednesday, 18 November 2009 17:43:25 -0500 <cover.1258580048.git.jbaron@redhat.com>

This looks like another good iteration, still imperfect but getting there.

There are three aspects of this work that are largely separable.

1. the actual instruction-poking for runtime switching

   Masami, Mathieu, et al are hashing this out and I have stopped trying to
   keep track of the details.

2. optimal compiled hot path code

   You and Richard have been working on this in gcc and we know the state
   of it now.  When we get the cold labels feature done, it will be ideal
   for -O(2?).  But people mostly use -Os and there no block reordering
   gets done now (I think perhaps this even means likely/unlikely don't
   really change which path is the straight line, just the source order
   of the blocks still determines it).  So we hope for more incremental
   improvements here, and maybe even really optimal code for -O2 soon.
   But at least for -Os it may not be better than "unconditional jump
   around" as the "straight line" path in the foreseeable future.  As
   noted, that alone is still a nice savings over the status quo for the
   disabled case.  (You gave an "average cycles saved" for this vs a load
   and test, but do you have any comparisons of how those two compare to
   no tracepoint at all?)

3. bookkeeping magic to find all the jumps to enable for a given tracepoint

   Here you have a working first draft, but it looks pretty clunky.
   That strcmp just makes me gag.  For a first version that's still
   pretty simple, I think it should be trivial to use a pointer
   comparison there.  For tracepoints, it can be the address of the
   struct tracepoint.  For the general case, it can be the address of
   the global that would be flag variable in case of no asm goto support.

   For more incremental improvements, we could cut down on running
   through the entire table for every switch.  If there are many
   different switches (as there are already for many different
   tracepoints), then you really just want to run through the
   insn-patch list for the particular switch when you toggle it.  

   It's possible to group this all statically at link time, but all
   the linker magic hacking required to get that to go is probably
   more trouble than it's worth.  

   A simple hack is to run through the big unsorted table at boot time
   and turn it into a contiguous table for each switch.  Then
   e.g. hang each table off the per-switch global variable by the same
   name that in a no-asm-goto build would be the simple global flag.


Finally, for using this for general purposes unrelated to tracepoints,
I envision something like:

	DECLARE_MOSTLY_NOT(foobar);

	foo(int x, int y)
	{
		if (x > y && mostly_not(foobar))
			do_foobar(x - y);
	}

	... set_mostly_not(foobar, onoff);

where it's:

#define DECLARE_MOSTLY_NOT(name) ... __something_##name
#define mostly_not(name) ({ int _doit = 0; __label__ _yes; \
			    JUMP_LABEL(name, _yes, __something_##name); \
			    if (0) _yes: __cold _doit = 1; \
			    unlikely (_doit); })

I don't think we've tried to figure out how well this compiles yet.
But it shows the sort of thing that we can do to expose this feature
in a way that's simple and unrestrictive for kernel code to use casually.


Thanks,
Roland

  parent reply	other threads:[~2009-11-19  3:55 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-18 22:43 Jason Baron
2009-11-18 22:43 ` [RFC PATCH 1/6] jump label v3 - kprobes/x86: Cleanup RELATIVEJUMP_INSTRUCTION to RELATIVEJUMP_OPCODE Jason Baron
2009-11-18 22:43 ` [RFC PATCH 2/6] jump label v3 - x86: Introduce generic jump patching without stop_machine Jason Baron
2009-11-19  0:28   ` Mathieu Desnoyers
2009-11-19  0:58     ` Paul E. McKenney
2009-11-19  1:22       ` Steven Rostedt
2009-11-19  1:39         ` Paul E. McKenney
2009-11-19  1:57       ` Mathieu Desnoyers
2009-11-19  4:16         ` Paul E. McKenney
2009-11-19 14:04     ` Masami Hiramatsu
2009-11-19 16:03       ` Mathieu Desnoyers
2009-11-20  1:00         ` Masami Hiramatsu
2009-11-21 15:32           ` Mathieu Desnoyers
2009-11-21  1:11     ` Masami Hiramatsu
2009-11-21 15:38       ` Mathieu Desnoyers
2009-11-20 21:54   ` H. Peter Anvin
2009-11-21  0:06     ` Masami Hiramatsu
2009-11-21  0:19       ` H. Peter Anvin
2009-11-21 16:21       ` Mathieu Desnoyers
2009-11-21 21:55         ` Masami Hiramatsu
2009-11-22  1:46           ` Mathieu Desnoyers
2009-11-21 16:12     ` Mathieu Desnoyers
2009-11-18 22:43 ` [RFC PATCH 3/6] jump label v3 - move opcode defs Jason Baron
2009-11-18 22:43 ` [RFC PATCH 4/6] jump label v3 - base patch Jason Baron
2009-11-18 23:38   ` [PATCH] notifier atomic call chain notrace Mathieu Desnoyers
2009-11-19  0:02     ` Paul E. McKenney
2009-11-19  3:59     ` Masami Hiramatsu
2009-11-19 16:48     ` Jason Baron
2009-11-18 22:43 ` [RFC PATCH 5/6] jump label v3 - add module support Jason Baron
2009-11-18 22:43 ` [RFC PATCH 6/6] jump label v3 - tracepoint support Jason Baron
2009-11-18 22:51 ` [RFC PATCH 0/6] jump label v3 H. Peter Anvin
2009-11-18 23:07   ` Roland McGrath
2009-11-18 23:18     ` H. Peter Anvin
2009-11-19  3:54 ` Roland McGrath [this message]
2009-11-19 21:55   ` Jason Baron

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=20091119035424.B3E8B1E2C@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=andi@firstfloor.org \
    --cc=hpa@zytor.com \
    --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®