Message-Id: <20200623142822.371012c66baf2cc7a631e6a3@kernel.org>
Date:   Tue, 23 Jun 2020 14:28:22 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Masami Hiramatsu <mhiramat@...nel.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ming Lei <ming.lei@...hat.com>,
        Ming Lei <tom.leiming@...il.com>,
        "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        linux-block <linux-block@...r.kernel.org>
Subject: Re: kprobe: __blkdev_put probe is missed

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

> On Tue, 23 Jun 2020 08:47:06 +0900
> Masami Hiramatsu <mhiramat@...nel.org> wrote:
> 
> > On Mon, 22 Jun 2020 09:01:48 -0400
> > Steven Rostedt <rostedt@...dmis.org> wrote:
> > 
> > > On Mon, 22 Jun 2020 08:27:53 +0800
> > > Ming Lei <ming.lei@...hat.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@...nel.org>

Powered by blists - more mailing lists