[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFyZ+pMRn_Na_o5A3yWeUbXj2J_FVS8jbHgdD-UfPbOD5g@mail.gmail.com>
Date: Fri, 27 Apr 2012 10:12:45 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Arnaldo Carvalho de Melo <acme@...radead.org>
Cc: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
David Ahern <dsahern@...il.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Hagen Paul Pfeifer <hagen@...u.net>,
Mike Galbraith <efault@....de>,
Namhyung Kim <namhyung@...il.com>,
Paul Mackerras <paulus@...ba.org>,
Peter Zijlstra <peterz@...radead.org>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [GIT PULL 0/4] perf/annotate loop detection V2, fixes
On Fri, Apr 27, 2012 at 9:46 AM, Arnaldo Carvalho de Melo
<acme@...radead.org> wrote:
> Em Fri, Apr 27, 2012 at 09:35:58AM -0700, Linus Torvalds escreveu:
>> Btw, don't get me wrong. I really like the changes. It's just that now
>> that the asm is almost readable, the remaining stupid default decode
>> format details just show up so much more clearly.
>
> Hey, I love the comments and suggestions, keep them coming when you feel
> like doing it.
I found another problem, and I think this one is more fundamental.
The "loop detection" is completely and utterly broken.
It seems to think that a backwards jump implies a loop. But that's not
at all true.
In fact, many backwards jumps are the *reverse* of loops. They are due
to *cold* code, that is totally uninteresting, and that was done
out-of-line. The backwards jump is not a loop at all, it's a jump back
to the hot code. In fact, it's often a jump back to the *exit* of a
function, when the cold code returns an error value (but the actual
code to do the "return" part was generated earlier as part of the hot
normal case code).
So making a big deal out of it as if it was a loop can be actively
wrong and misleading.
(And yes, I'm looking at an example of that right now -
__d_lookup_rcu() has this, for example)
Now, it's often nice to see the line to find the branch target
(whether it's a loop or not), but you don't show them for forwards
branches, you only show them for backwards branches, as if the
backwards branches were somehow more important. But they really really
aren't.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists