From: Don Zickus <dzickus@redhat.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@redhat.com>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Ingo Molnar <mingo@kernel.org>, Paul Mackerras <paulus@samba.org>,
Namhyung Kim <namhyung.kim@lge.com>,
LKML <linux-kernel@vger.kernel.org>,
David Ahern <dsahern@gmail.com>, Andi Kleen <andi@firstfloor.org>
Subject: Re: [PATCHSET 00/17] perf report: Add -F option for specifying output fields (v4)
Date: Wed, 30 Apr 2014 09:35:55 -0400 [thread overview]
Message-ID: <20140430133554.GB61249@redhat.com> (raw)
In-Reply-To: <874n1bpuzh.fsf@sejong.aot.lge.com>
On Wed, Apr 30, 2014 at 08:38:10AM +0900, Namhyung Kim wrote:
> Hi Don,
>
> On Tue, 29 Apr 2014 13:27:35 -0400, Don Zickus wrote:
> > On Tue, Apr 29, 2014 at 10:13:35AM +0900, Namhyung Kim wrote:
> >> > /*
> >> > * Addresses with no major/minor numbers are assumed to be
> >> > * anonymous in userspace. Sort those on pid then address.
> >> > *
> >> > * The kernel and non-zero major/minor mapped areas are
> >> > * assumed to be unity mapped. Sort those on address.
> >> > */
> >> >
> >> > if ((left->cpumode != PERF_RECORD_MISC_KERNEL) &&
> >> > !l_map->maj && !l_map->min && !l_map->ino &&
> >> > !l_map->ino_generation) {
> >> > /* userspace anonymous */
> >> >
> >> > if (left->thread->pid_ > right->thread->pid_) return -1;
> >> > if (left->thread->pid_ < right->thread->pid_) return 1;
> >>
> >> Isn't it necessary to check whether the address is in a same map in case
> >> of anon pages? I mean the daddr.al_addr is a map-relative offset so it
> >> might have same value for different maps.
> >
> > That's why I sort on pids here. Because the anon address might have the
> > same value for different maps. The thought was to group all the pid
> > addresses together to keep things seperated.
> >
> > Do you see a different way to solve the problem? I am not sure al_addr
> > vs. addr will make much difference here.
>
> I'm not saying to get rid of the pid check, I'm saying that it might
> need to add another check for maps (i.e. start address) as there might
> be many maps in a single address space.
Hmm, I guess I would need to see an example. While I agree there might be
many maps in a single address space (/proc/<pid>/maps demonstrates that),
I understood them to map to a unique location (ie no overlap) unless they
are shared.
I am willing to believe I missed scenario when sorting, I just can't think
of it (so I wouldn't know how to fix it). That's why I was looking for an
example to make it more obvious to me. Sorry for being slow..
Cheers,
Don
next prev parent reply other threads:[~2014-04-30 13:36 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-16 3:05 [PATCHSET 00/17] perf report: Add -F option for specifying output fields (v4) Namhyung Kim
2014-04-16 3:05 ` [PATCH 01/17] perf tools: Add ->cmp(), ->collapse() and ->sort() to perf_hpp_fmt Namhyung Kim
2014-04-16 3:05 ` [PATCH 02/17] perf tools: Convert sort entries to hpp formats Namhyung Kim
2014-04-16 3:05 ` [PATCH 03/17] perf tools: Use hpp formats to sort hist entries Namhyung Kim
2014-04-16 3:05 ` [PATCH 04/17] perf tools: Support event grouping in hpp ->sort() Namhyung Kim
2014-04-16 3:05 ` [PATCH 05/17] perf tools: Use hpp formats to sort final output Namhyung Kim
2014-04-16 3:05 ` [PATCH 06/17] perf tools: Consolidate output field handling to hpp format routines Namhyung Kim
2014-04-16 3:05 ` [PATCH 07/17] perf ui: Get rid of callback from __hpp__fmt() Namhyung Kim
2014-04-16 3:05 ` [PATCH 08/17] perf tools: Allow hpp fields to be sort keys Namhyung Kim
2014-04-16 3:05 ` [PATCH 09/17] perf tools: Consolidate management of default sort orders Namhyung Kim
2014-04-16 3:05 ` [PATCH 10/17] perf tools: Call perf_hpp__init() before setting up GUI browsers Namhyung Kim
2014-04-16 3:05 ` [PATCH 11/17] perf report: Add -F option to specify output fields Namhyung Kim
2014-04-16 3:05 ` [PATCH 12/17] perf tools: Add ->sort() member to struct sort_entry Namhyung Kim
2014-04-16 3:05 ` [PATCH 13/17] perf report/tui: Fix a bug when --fields/sort is given Namhyung Kim
2014-04-16 3:05 ` [PATCH 14/17] perf top: Add --fields option to specify output fields Namhyung Kim
2014-04-16 3:05 ` [PATCH 15/17] perf diff: Add missing setup_output_field() Namhyung Kim
2014-04-16 3:05 ` [PATCH 16/17] perf tools: Skip elided sort entries Namhyung Kim
2014-04-16 3:05 ` [PATCH 17/17] perf hists: Reset width of output fields with header length Namhyung Kim
2014-04-22 21:16 ` [PATCHSET 00/17] perf report: Add -F option for specifying output fields (v4) Don Zickus
2014-04-23 6:15 ` Namhyung Kim
2014-04-23 12:58 ` Don Zickus
2014-04-24 13:41 ` Namhyung Kim
2014-04-24 21:00 ` Don Zickus
2014-04-25 7:58 ` Namhyung Kim
2014-04-28 19:46 ` Don Zickus
2014-04-29 1:13 ` Namhyung Kim
2014-04-29 17:27 ` Don Zickus
2014-04-29 23:38 ` Namhyung Kim
2014-04-30 13:35 ` Don Zickus [this message]
2014-05-07 3:05 ` Namhyung Kim
2014-05-07 15:22 ` Don Zickus
2014-05-09 6:11 ` Namhyung Kim
2014-05-09 13:33 ` Don Zickus
2014-05-04 17:53 ` Jiri Olsa
2014-05-07 3:09 ` Namhyung Kim
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=20140430133554.GB61249@redhat.com \
--to=dzickus@redhat.com \
--cc=a.p.zijlstra@chello.nl \
--cc=acme@kernel.org \
--cc=andi@firstfloor.org \
--cc=dsahern@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung.kim@lge.com \
--cc=namhyung@kernel.org \
--cc=paulus@samba.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.