Re: [GIT PULL 0/4] perf/annotate loop detection V2, fixes

[Linus Torvalds]

On Fri, Apr 27, 2012 at 9:46 AM, Arnaldo Carvalho de Melo
<acme@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/