linux-kernel - Re: [PATCH 1/2] perf stat: refactor aggregation code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABPqkBR=H5NOTjjEXRzC-tU3K+tbkYHYTzBvjyZyn3-Xu68UVw@mail.gmail.com>
Date:	Wed, 13 Feb 2013 10:38:09 +0100
From:	Stephane Eranian <eranian@...gle.com>
To:	Namhyung Kim <namhyung@...nel.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"ak@...ux.intel.com" <ak@...ux.intel.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Jiri Olsa <jolsa@...hat.com>,
	Namhyung Kim <namhyung.kim@....com>
Subject: Re: [PATCH 1/2] perf stat: refactor aggregation code

On Wed, Feb 13, 2013 at 8:50 AM, Namhyung Kim <namhyung@...nel.org> wrote:
> Hi Stephane,
>
> On Tue, 12 Feb 2013 15:09:27 +0100, Stephane Eranian wrote:
>> Refactor aggregation code by introducing
>> a single aggr_mode variable and an enum
>> for aggregation.
>>
>> Also refactor cpumap code having to do
>> with cpu to socket mappings. All in preparation
>> for extended modes, such as cpu -> core.
>>
>> Also fix socket aggregation and ensure
>> that sockets are printed in increasing order.
> [snip]
>> -static void print_aggr_socket(char *prefix)
>> +static void print_aggr(char *prefix)
>>  {
>>       struct perf_evsel *counter;
>> +     int cpu, s, s2, id, nr;
>>       u64 ena, run, val;
>> -     int cpu, s, s2, sock, nr;
>>
>> -     if (!sock_map)
>> +     if (!(aggr_map || aggr_get_id))
>>               return;
>>
>> -     for (s = 0; s < sock_map->nr; s++) {
>> -             sock = cpu_map__socket(sock_map, s);
>> +     for (s = 0; s < aggr_map->nr; s++) {
>> +             id = aggr_map->map[s];
>>               list_for_each_entry(counter, &evsel_list->entries, node) {
>>                       val = ena = run = 0;
>>                       nr = 0;
> ...
>> @@ -1073,14 +1081,20 @@ static void print_stat(int argc, const char **argv)
>>               fprintf(output, ":\n\n");
>>       }
>>
>> -     if (aggr_socket)
>> -             print_aggr_socket(NULL);
>> -     else if (no_aggr) {
>> -             list_for_each_entry(counter, &evsel_list->entries, node)
>> -                     print_counter(counter, NULL);
>> -     } else {
>> +     switch (aggr_mode) {
>> +     case AGGR_SOCKET:
>> +             print_aggr(NULL);
>> +             break;
>
> This line should look like this IMHO:
>
>                 list_for_each_entry(counter, &evsel_list->entries, node)
>                         print_aggr(counter, NULL);
>
> and the equivalent loop in the print_aggr() should be removed.  This is
> for consistency with other formats if multiple events counted.
>
Sounds good. I will make the change.

> For instance, it'd sorting on events first and then sockets like:
>
>   #      time socket cpus     counts events
>            t0 S0     4        XXXXXX cycles
>            t0 S1     4        XXXXXX cycles
>            t0 S0     4          YYYY cache-misses
>            t0 S1     4          YYYY cache-misses
>            t1 S0     4        ZZZZZZ cycles
>   ...
>
> But current code looks like sorting on sockets first instead.
>
>   #      time socket cpus     counts events
>            t0 S0     4        XXXXXX cycles
>            t0 S0     4          YYYY cache-misses
>            t0 S1     4        XXXXXX cycles
>            t0 S1     4          YYYY cache-misses
>            t1 S0     4        ZZZZZZ cycles
>   ...
>
> Thanks,
> Namhyung
>
>
>> +     case AGGR_GLOBAL:
>>               list_for_each_entry(counter, &evsel_list->entries, node)
>>                       print_counter_aggr(counter, NULL);
>> +             break;
>> +     case AGGR_NONE:
>> +             list_for_each_entry(counter, &evsel_list->entries, node)
>> +                     print_counter(counter, NULL);
>> +             break;
>> +     default:
>> +             break;
>>       }
--
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/