All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Ingo Molnar <mingo@redhat.com>, Vojtech Pavlik <vojtech@suse.cz>,
	Jiri Kosina <jkosina@suse.cz>, Jiri Slaby <jslaby@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/2] s390 vs. kprobes on ftrace
Date: Mon, 20 Oct 2014 08:41:28 +0200	[thread overview]
Message-ID: <20141020064128.GA4268@osiris> (raw)
In-Reply-To: <54446D49.1030208@hitachi.com>

Hi Masami,

On Mon, Oct 20, 2014 at 11:02:49AM +0900, Masami Hiramatsu wrote:
> (2014/10/17 17:19), Heiko Carstens wrote:
> > On Thu, Oct 16, 2014 at 02:49:56PM +0900, Masami Hiramatsu wrote:
> >> Hi Heiko,
> >>
> >> (2014/10/16 0:46), Heiko Carstens wrote:
> >>> Hi all,
> >>>
> >>> we would like to implement an architecture specific variant of "kprobes
> >>> on ftrace" without using the current HAVE_KPROBES_ON_FTRACE infrastructure
> >>> which is currently only used by x86.
> > 
> > [...]
> > 
> >> I'm not sure about s390 nor have the machine, so it is very helpful if you
> >> give us a command line level test and show us the result with this patch :)
> >> Fortunately, we already have ftracetest under tools/tesitng/selftest/ftrace/.
> >> You can add the testcase for checking co-existence of kprobes and ftrace on
> >> an entry of a function.
> > 
> > So how about something like below?
> 
> Yes! :) And could you add the results before and after to patch 2/2,
> so that we can see what it changes on s390 ?

The output of the testcase is identical before and after patch 2/2. Maybe I
didn't explain my intention good enough.
Just to explain how mcount/ftrace works on s390 today: if we pass the "-pg"
flag to the compiler a 24 byte(!) block will be added in front of every
function. We patch that block to match the ftrace needs. So an ftrace "nop"
looks like this (simplified):

 0: load return address "24" into register
 6: jump to 24
12: nop
18: nop
24: <function code>

If the function gets enabled for ftrace we will patch the instruction at
offset 6:

 0: load return address "24" into register
 6: jump to ftrace_caller
12: nop
18: nop
24: <function code>

So in fact kprobes and ftrace do work nicely together, since we only patch
the second instruction, while kprobes will put a breakpoint on the first
instruction.

However, what I want to achieve with patch 2/2 is that the code will look
like this:

ftrace disabled:

 0: jump to 24
 6: nop
12: nop
18: nop
24: <function code>

ftrace enabled:

 0: branch to ftrace_caller and save return address into register
 6: nop
12: nop
18: nop
24: <function code>

So, with patch 2/2 the ftrace location of a function now matches the first
instruction of a function and the check within kprobes.c which prevents
putting a kprobe on an ftrace location triggers.

So kprobes and ftrace work with and without patch 2/2, all I want to achieve
is that the overhead of the mcount block gets reduced to a single instruction.
Ultimately we want also a compiler change which only emits a single
instruction, which we can patch; probably similar to "-pg -mfentry" on x86.


  reply	other threads:[~2014-10-20  6:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-15 15:46 [PATCH 0/2] s390 vs. kprobes on ftrace Heiko Carstens
2014-10-15 15:46 ` [PATCH 1/2] kprobes: introduce ARCH_HANDLES_KPROBES_ON_FTRACE Heiko Carstens
2014-10-20  1:55   ` Masami Hiramatsu
2014-10-20 18:53     ` Steven Rostedt
2014-10-21  1:51       ` Masami Hiramatsu
2014-10-15 15:46 ` [PATCH 2/2] s390/ftrace,kprobes: allow to patch first instruction Heiko Carstens
2014-10-16  5:49 ` [PATCH 0/2] s390 vs. kprobes on ftrace Masami Hiramatsu
2014-10-16 10:57   ` Heiko Carstens
2014-10-21  9:37     ` Masami Hiramatsu
2014-10-17  8:19   ` Heiko Carstens
2014-10-17  8:28     ` Heiko Carstens
2014-10-20  2:02     ` Masami Hiramatsu
2014-10-20  6:41       ` Heiko Carstens [this message]
2014-10-17  8:21   ` Heiko Carstens
2014-10-20  1:31     ` Masami Hiramatsu

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=20141020064128.GA4268@osiris \
    --to=heiko.carstens@de.ibm.com \
    --cc=ananth@in.ibm.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=davem@davemloft.net \
    --cc=jkosina@suse.cz \
    --cc=jslaby@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=vojtech@suse.cz \
    /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.