mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: mingo@kernel.org, hpa@zytor.com, paulus@samba.org,
	eranian@google.com, linux-kernel@vger.kernel.org, efault@gmx.de,
	peterz@infradead.org, namhyung@gmail.com, fweisbec@gmail.com,
	dsahern@gmail.com, tglx@linutronix.de,
	linux-tip-commits@vger.kernel.org
Subject: Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view
Date: Fri, 13 Apr 2012 21:59:21 -0300	[thread overview]
Message-ID: <20120414005921.GA22114@infradead.org> (raw)
In-Reply-To: <CA+55aFxnv5Wrix=uREQsEg+tSBE8SFvy0v_76csv3hrnmPYG2A@mail.gmail.com>

Em Fri, Apr 13, 2012 at 11:25:50AM -0700, Linus Torvalds escreveu:
> On Fri, Apr 13, 2012 at 11:07 AM, tip-bot for Arnaldo Carvalho de Melo
> <acme@redhat.com> wrote:
> >
> > The offset view will be the default as soon as operations that deal with
> > offsets in a function are handled accodringly, i.e. in offset view the
> > above will become:
> >
> >     2f:       jne    __list_del_entry+0x84
> > <SNIP>
> >     84:       mov    %rdi,%rcx
> 
> Oh, please please please make this happen.

I will in the next days. Glad you like it.
 
> Also, if at all possible, please mark offsets that are referenced by a
> branch some way. I do think that the "size of instruction" can be a
> very valid piece of information when looking at instruction-level
> profiles, but quite frankly, I'm also 100% sure that the fact that an
> instruction is a branch target is *more* important.
> 
> Now, fancy arrows etc might be too hard to do (especially without
> cluttering things up too much), but marking just the targets would
> already be lovely. IOW, turn this mess:
> 
> 
>          :        ffffffff810d7e80 <kmem_cache_free>:
>     1.91 :        ffffffff810d7e80:       push   %rbp
>     0.05 :        ffffffff810d7e81:       mov    %rsp,%rbp
>     2.02 :        ffffffff810d7e84:       push   %r12
>     0.00 :        ffffffff810d7e86:       mov    %rdi,%r12
>     0.55 :        ffffffff810d7e89:       push   %rbx
>     0.00 :        ffffffff810d7e8a:       mov    %rsi,%rdi
>     1.20 :        ffffffff810d7e8d:       mov    %rsi,%rbx
>     1.64 :        ffffffff810d7e90:       callq  ffffffff8102a730 <__phys_addr>
>     0.05 :        ffffffff810d7e95:       movabs $0xffffea0000000000,%rdi
>     0.22 :        ffffffff810d7e9f:       shr    $0xc,%rax
>     0.11 :        ffffffff810d7ea3:       shl    $0x6,%rax
>     1.64 :        ffffffff810d7ea7:       lea    (%rax,%rdi,1),%rdi
>    42.80 :        ffffffff810d7eab:       mov    (%rdi),%rax
>     2.02 :        ffffffff810d7eae:       test   $0x80,%ah
>     0.00 :        ffffffff810d7eb1:       jne    ffffffff810d7ef6
> <kmem_cache_free+0x76>
>     0.22 :        ffffffff810d7eb3:       mov    0x8(%rbp),%r9
>     7.14 :        ffffffff810d7eb7:       mov    (%r12),%rax
>     0.65 :        ffffffff810d7ebb:       add    %gs:0xcb88,%rax
>     4.58 :        ffffffff810d7ec4:       mov    0x8(%rax),%rdx
> 
> (which has tons of unnecessary white-space too - shades of G+) into

Grin ;-)

Yeah, even the : column isn't needed I think.
 
> <kmem_cache_free>:
>     1.91 :          push   %rbp
>     0.05 :          mov    %rsp,%rbp
>     2.02 :          push   %r12
>     0.00 :          mov    %rdi,%r12
>     0.55 :          push   %rbx
>     0.00 :          mov    %rsi,%rdi
>     1.20 :          mov    %rsi,%rbx
>     1.64 :          callq  ffffffff8102a730 <__phys_addr>
>     0.05 :          movabs $0xffffea0000000000,%rdi
>     0.22 :          shr    $0xc,%rax
>     0.11 :          shl    $0x6,%rax
>     1.64 :          lea    (%rax,%rdi,1),%rdi
>    42.80 :          mov    (%rdi),%rax
>     2.02 :          test   $0x80,%ah
>     0.00 :          jne    kmem_cache_free+0x76
>     0.22 : 0x33:    mov    0x8(%rbp),%r9
>     7.14 : 0x37:    mov    (%r12),%rax
>     0.65 :          add    %gs:0xcb88,%rax
>     4.58 :          mov    0x8(%rax),%rdx
>    ....
> 
> or something. I think the "callq  ffffffff8102a730 <__phys_addr>"
> could also be prettified with *zero* downside.

Yeah, no need to see the address in default mode.
 
> Sure, leave some magic keycombination to get the "full information", but ..

Yeah, we have o to toggle offset view + s to toggle source code view,
etc. 

>                       Linus

  parent reply	other threads:[~2012-04-14  0:59 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-13 18:07 tip-bot for Arnaldo Carvalho de Melo
2012-04-13 18:25 ` Linus Torvalds
2012-04-13 18:30   ` Linus Torvalds
2012-04-14  1:04     ` Arnaldo Carvalho de Melo
2012-04-14  8:55       ` Ingo Molnar
2012-04-14 16:28         ` Arnaldo Carvalho de Melo
2012-04-16  6:25           ` Masami Hiramatsu
2012-04-16 15:02             ` Linus Torvalds
2012-04-16 15:21               ` Peter Zijlstra
2012-04-18  0:43                 ` Masami Hiramatsu
2012-04-15 10:56       ` Peter Zijlstra
2012-04-14  0:59   ` Arnaldo Carvalho de Melo [this message]
2012-04-14  8:46     ` Ingo Molnar
2012-04-14 16:26       ` Arnaldo Carvalho de Melo
2012-04-15  5:29       ` Mike Galbraith
2012-04-15  6:57         ` Ingo Molnar

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=20120414005921.GA22114@infradead.org \
    --to=acme@redhat.com \
    --cc=dsahern@gmail.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@gmail.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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

all inboxes | Powered by JetHome®