linux-kernel - Re: [PATCH] perf data: Add JSON export

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 for Android: free password hash cracker in your pocket

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

Message-ID: <0831d8ee-12d5-dd28-dde4-a46e4781a76f@codeweavers.com>
Date:   Tue, 6 Apr 2021 05:31:51 -0400
From:   Nicholas Fraser <nfraser@...eweavers.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Kan Liang <kan.liang@...ux.intel.com>,
        Stephane Eranian <eranian@...gle.com>,
        Tan Xiaojun <tanxiaojun@...wei.com>,
        Jin Yao <yao.jin@...ux.intel.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf data: Add JSON export

Hi Jiri,

Thanks again for the review. I've fixed the issues you mentioned. Some notes
below:

On 2021-04-01 8:15 a.m., Jiri Olsa wrote:
> I recall you did not add support for walltime clock,
> don't you need it to sync with other events?

Not necessarily. As long as the perf recording and the GPU trace are made from
the same clock, the events should line up; the real time doesn't matter. I've
added it anyway though since it's good to have. It now outputs "clockid",
"clock-time" and "real-time" in the headers if clock info was included in the
recording.

On 2021-04-01 9:18 a.m., Jiri Olsa wrote:
> also I will not push hard for test, becase we don't have any for CTF ;-)
> but if you could think of any, that'd be great

I'm not sure what kind of tests would be useful. We could include a small
perf.data file and its JSON output and verify that they match. This is probably
pointless because almost any change to the JSON code would be expected to
change the output so we'd just be resetting the test on each change.

> I was wondering how to make this \t mess more readable,
> how about you define function like output_json:
> 
> 	output_json(FILE, level, field, format, ...);
> 
> and use it:
> 
> 	output_json(c->out, 3, "data-offset", "PRIu64", header->data_offset);
> 	output_json(c->out, 3, "data-size", "PRIu64", header->data_size);
> 	output_json(c->out, 3, "feat-offset", PRIu64, header->feat_offset);
> 
> similar way as we do for pr_debug -> eprintf

I've cleaned up the output with some additional helper functions. This is
essentially turning into its own mini JSON encoder which I was hoping to avoid
but I suppose it's necessary to make the code maintainable.

I'll be sending a new patch shortly.

Nick