Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Song Liu <songliubraving@fb.com>
To:     Stephane Eranian <eranian@google.com>
CC:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@redhat.com>,
        Alexey Budankov <alexey.budankov@linux.intel.com>,
        Jiri Olsa <jolsa@kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Andi Kleen <ak@linux.intel.com>
Subject: Re: [RFC/PATCH 00/14] perf record: Add support to store data in
 directory
Thread-Topic: [RFC/PATCH 00/14] perf record: Add support to store data in
 directory
Thread-Index: AQHUwjtlRyzpVjG3JkKhkHGKKI1AuqXa++yAgAAQ1YA=
Date:   Mon, 11 Feb 2019 20:30:22 +0000
Message-ID: <90265D59-C5B9-4AD2-B5EA-0ADC9BEF7C79@fb.com>
References: <6bf24b7d-2bd3-8091-cf49-363c91e4e864@linux.intel.com>
 <20190204114144.GC18141@krava>
 <CABPqkBQpQX67NOysim7s-R84nAYipXJ+DgR=xYPi6zWQWgq4Zg@mail.gmail.com>
 <20190204192721.GI5593@kernel.org>
 <CABPqkBTcxkKkxBwLfToenAk0ubp-o08xVq=XqpkfzYHTNqgBkQ@mail.gmail.com>
 <20190204202818.GC4794@krava>
 <CABPqkBQTpt0dhinJZc+mJ-JNvkU2M9DVx1G65ngWxZxk-eGBVA@mail.gmail.com>
 <20190205133727.GF4794@krava> <20190211101957.GB14253@krava>
 <CABPqkBSkyHRaXN-oVLuJJ=aPEBfqOAunRUReHsxESnC=KKQuPg@mail.gmail.com>
 <20190211185510.GF3269@kernel.org>
 <CABPqkBTOM2gLvv89ywkpWDKniS2L37XhKPjNLtR8WCY3VuYXzw@mail.gmail.com>
In-Reply-To: <CABPqkBTOM2gLvv89ywkpWDKniS2L37XhKPjNLtR8WCY3VuYXzw@mail.gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8420259236415E44B8AD523B42318266@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 30614848-31de-411f-69bb-08d6905fbf2b
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Feb 2019 20:30:22.1608
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1790
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-02-11_14:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Feb 11, 2019, at 11:30 AM, Stephane Eranian <eranian@google.com> wrote=
:
>=20
> Arnaldo,
>=20
> On Mon, Feb 11, 2019 at 10:55 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>>=20
>> Em Mon, Feb 11, 2019 at 10:34:16AM -0800, Stephane Eranian escreveu:
>>> Jiri,
>>>=20
>>> On Mon, Feb 11, 2019 at 2:20 AM Jiri Olsa <jolsa@redhat.com> wrote:
>>>>=20
>>>> On Tue, Feb 05, 2019 at 02:37:27PM +0100, Jiri Olsa wrote:
>>>>> On Mon, Feb 04, 2019 at 02:44:37PM -0800, Stephane Eranian wrote:
>>>>>> Jiri,
>>>>>>=20
>>>>>> While you're looking at the output format, I think it would be good
>>>>>> time to simplify the code handling perf.data file.
>>>>>> Today, perf record can emit in two formats: file mode or pipe mode.
>>>>>> This adds complexity in the code and
>>>>>> is error prone as the file mode path is tested more than the pipe mo=
de
>>>>>> path. We have run into multiple issues with
>>>>>> the pipe mode in recent years. There is no real reason why we need t=
o
>>>>>> maintain two formats. If I recall, the pipe format
>>>>>> was introduced because on pipes you cannot lseek to update the heade=
rs
>>>>>> and therefore some of the information present as tables
>>>>>> updated on the fly needed to be generated as pseudo records by the
>>>>>> tool. I believe that the pipe format covers all the needs and could
>>>>>> supersede the file mode format. That would simplify code in perf
>>>>>> record and eliminate the risk of errors when new headers
>>>>>> are introduced.
>>>>>=20
>>>>> yep, I think we have almost all the features covered for pipe mode,
>>>>> and we have all necessary events to describe events features
>>>>>=20
>>>>> so with some effort we could switch off the superfluos file header
>>>>> and use only events to describe events ;-) make sense, I'll check
>>>>> on it
>>>>=20
>>>> so following features are not synthesized:
>>>>=20
>>>>        FEAT_OPN(TRACING_DATA,  tracing_data,   false),
>>>>        FEAT_OPN(BUILD_ID,      build_id,       false),
>>>>        FEAT_OPN(BRANCH_STACK,  branch_stack,   false),
>>>>        FEAT_OPN(AUXTRACE,      auxtrace,       false),
>>>>        FEAT_OPN(STAT,          stat,           false),
>>>>        FEAT_OPN(CACHE,         cache,          true),
>>>>=20
>>> What do you need for BRANCH_STACK?
>>>=20
>>>> I think all could be added and worked around with exception
>>>> of BUILD_ID, which we store at the end (after processing
>>>> all data) and we need it early in the report phase
>>>>=20
>>> Buildids are injected after the fact via perf inject when in pipe mode.
>>>=20
>>>> maybe it's time to re-think that buildid -> mmap event
>>>> association again, because it's pain in current implementation
>>>> as well
>>>>=20
>>> Sure, but what do you propose?
>>=20
>> this keeps resurfacing, the idea is to have the building go together
>> with the PERF_RECORD_MMAP3 event, i.e. as part of setting up an
>> executable mapping the loader would get the buildid and ask the kernel
>> to keep it aroung, then when a PERF_RECORD_MMAP needs to be issued, it
>> can include the build id, so tooling will not need to get it.
>>=20
> And how would the dynamic loader (ld.so) communicate the buildid to the k=
ernel?
> How would that work for statically linked binaries.
> I think you're say the kernel would parse the ELF header looking for
> that note section
> and extract the buildid from there. Is that what you are proposing?

We have kernel parses ELF header for BUILD-ID in BPF side. You can=20
find the code in stack_map_get_build_id_offset() and functions called
by it.=20

>=20
>> Alternatively, we would have a separate thread to process
>> PERF_RECORD_MMAP events, and as soon as it gets one from the kernel,
>> augment it straight away with the build-id it reads from the ELF file,
>> i.e. no need to have the kernel provide it, do it just like we do with
>> PERF_RECORD_BPF_EVENT, which reminds me Song probably already posted
>> thise bits...
>>=20
> But that would not work in pipe mode, wouldn't it?
> Unless that thread intercepts everything pushed to the pipe looking
> for MMAP records.

For PERF_RECORD_BPF_EVENT, I am adding a separate thread, which only
listen to PERF_RECORD_BPF_EVENT with watermark of 1. This means,=20
each PERF_RECORD_BPF_EVENT is sent to two ring buffers. One of them
got written to the pipe, the other is only processed by the listening
thread. Please see https://patchwork.ozlabs.org/patch/1039091/ for=20
details.=20

Thanks,
Song

>=20
>>>> looks like bpf code is actualy getting build ids and storing
>>>> it for the callchains in kernel.. we can check if we can do
>>>> something similar for mmap events
>>>>=20
>>>> jirka
>>=20
>> --
>>=20
>> - Arnaldo