linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ming Lei <ming.lei@redhat.com>, Ming Lei <tom.leiming@gmail.com>,
	"Naveen N. Rao" <naveen.n.rao@linux.ibm.com>,
	Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	linux-block <linux-block@vger.kernel.org>
Subject: Re: kprobe: __blkdev_put probe is missed
Date: Tue, 23 Jun 2020 14:28:22 +0900	[thread overview]
Message-ID: <20200623142822.371012c66baf2cc7a631e6a3@kernel.org> (raw)
In-Reply-To: <20200623093801.db9d2ca9c3bfef61ef6a2a58@kernel.org>

On Tue, 23 Jun 2020 09:38:01 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> On Tue, 23 Jun 2020 08:47:06 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > On Mon, 22 Jun 2020 09:01:48 -0400
> > Steven Rostedt <rostedt@goodmis.org> wrote:
> > 
> > > On Mon, 22 Jun 2020 08:27:53 +0800
> > > Ming Lei <ming.lei@redhat.com> wrote:
> > > 
> > > > Can you kprobe guys improve the implementation for covering this case?
> > > > For example, put probe on 3) in case the above situation is recognized.
> > > 
> > > To do so would require solving the halting problem.
> > > 
> > >   https://en.wikipedia.org/wiki/Halting_problem
> > > 
> > > Or perhaps reading the DWARF output of the compiler to determine if it
> > > optimized the location you are looking for.
> > 
> > As far as I can see, gcc-9.3 doesn't generate this information :(
> > Maybe the optimizer forgot to push the tail-call callsite information
> > to dwarf generator when making a recursive tail-call to a loop.
> > 
> > > The first case is impossible to solve, the second would take a lot of
> > > work, (are you going to fund it?)
> > 
> > What I can provide is "--skip-prologue" option for the perf-probe
> > which will be similar to the "-P" option. If the compiler correctly
> > generates the information, we can enable it automatically. But
> > as far as I can see, it doesn't.
> > 
> > [OT] DWARF has its option(and GNU extension) but it seems not correctly
> > implemented yet.
> >  
> > http://www.dwarfstd.org/ShowIssue.php?issue=100909.2
> 
> Oops, sorry, I missed the following sentences.
> 
> "Tail calls are jump-like instructions which transfer control to the start
> of some subprogram, but the call site location address isn't visible in the
> unwind information."
> 
> "Tail recursion is a call to the current function which is compiled as a
> loop into the middle of the current function."
> 
> "The DW_TAG_call_site entries describe normal and tail calls."
> 
> This means, the gcc is correctly implemented and this __blkdev_put() case
> is NOT covered by DT_TAG_call_site.
> So we can not detect it from the debuginfo.

Hmm, BTW, if optimization is further advanced, it is possible that
the loop start position is not always at the beginning of the function.
It is easy to provide --skip-prologue to perf probe but it doesn't
ensure that works always as you expected.

For example,

func()
{
1:
	{ /* block which doesn't executed in tail-recursion call */
	...
	}
2:
	{ /* block which always executed in tail-recursion call */
	...
	}
	func()
}

In this case, it is natural that the optimizer put a jump to 2 instead
of 1. Moreover, if the number of recursion is fixed, the optimizer
can unroll the loop. In that case there are no jumps. 

So, as Steve pointed, strictly speaking, the developer needs to understand
what the source code was compiled into, before tracing/debuging it.

For the perf-probe case, I'm now thinking it is better user to
choose the line in the function explicitly. I wish I had another flag
that there was a tail-recursion, then I can warn users...

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

      reply	other threads:[~2020-06-23  5:28 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-17 10:30 krobe: __blkdev_put probe is missed Ming Lei
2020-06-18 12:54 ` kprobe: " Ming Lei
2020-06-18 13:56   ` Masami Hiramatsu
2020-06-18 23:19     ` Ming Lei
2020-06-19  5:12       ` Masami Hiramatsu
2020-06-19  7:28         ` Ming Lei
2020-06-19 12:19           ` Steven Rostedt
2020-06-19 13:32             ` Ming Lei
2020-06-19 15:35               ` Masami Hiramatsu
2020-06-19 23:28                 ` Ming Lei
2020-06-20  0:59                   ` Steven Rostedt
2020-06-20  1:37                   ` Masami Hiramatsu
2020-06-22  0:27                     ` Ming Lei
2020-06-22  1:34                       ` Masami Hiramatsu
2020-06-22 13:01                       ` Steven Rostedt
2020-06-22 23:47                         ` Masami Hiramatsu
2020-06-23  0:38                           ` Masami Hiramatsu
2020-06-23  5:28                             ` Masami Hiramatsu [this message]

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=20200623142822.371012c66baf2cc7a631e6a3@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=davem@davemloft.net \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=naveen.n.rao@linux.ibm.com \
    --cc=rostedt@goodmis.org \
    --cc=tom.leiming@gmail.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;
as well as URLs for NNTP newsgroup(s).