Re: [PATCHSET 00/17] perf report: Add -F option for specifying output fields (v4)

[Namhyung Kim]

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.

Thanks,
Namhyung
--
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/