From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT Date: Thu, 8 Nov 2018 18:04:44 +0000 Message-ID: References: <20181106205246.567448-1-songliubraving@fb.com> <20181106205246.567448-2-songliubraving@fb.com> <20181107084057.GG9781@hirez.programming.kicks-ass.net> <31067290-4B66-4AA1-8027-607397BC0264@fb.com> <20181108150028.GU9761@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Netdev , lkml , Kernel Team , "ast@kernel.org" , "daniel@iogearbox.net" , "acme@kernel.org" To: Peter Zijlstra Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:45090 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726801AbeKIDmJ (ORCPT ); Thu, 8 Nov 2018 22:42:09 -0500 In-Reply-To: <20181108150028.GU9761@hirez.programming.kicks-ass.net> Content-Language: en-US Content-ID: <7ED8500B60234A4285502EC3EE9CD718@namprd15.prod.outlook.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi Peter, > On Nov 8, 2018, at 7:00 AM, Peter Zijlstra wrote: >=20 > On Wed, Nov 07, 2018 at 06:25:04PM +0000, Song Liu wrote: >>=20 >>=20 >>> On Nov 7, 2018, at 12:40 AM, Peter Zijlstra wrot= e: >>>=20 >>> On Tue, Nov 06, 2018 at 12:52:42PM -0800, Song Liu wrote: >>>> For better performance analysis of BPF programs, this patch introduces >>>> PERF_RECORD_BPF_EVENT, a new perf_event_type that exposes BPF program >>>> load/unload information to user space. >>>>=20 >>>> /* >>>> * Record different types of bpf events: >>>> * enum perf_bpf_event_type { >>>> * PERF_BPF_EVENT_UNKNOWN =3D 0, >>>> * PERF_BPF_EVENT_PROG_LOAD =3D 1, >>>> * PERF_BPF_EVENT_PROG_UNLOAD =3D 2, >>>> * }; >>>> * >>>> * struct { >>>> * struct perf_event_header header; >>>> * u16 type; >>>> * u16 flags; >>>> * u32 id; // prog_id or map_id >>>> * }; >>>> */ >>>> PERF_RECORD_BPF_EVENT =3D 17, >>>>=20 >>>> PERF_RECORD_BPF_EVENT contains minimal information about the BPF progr= am. >>>> Perf utility (or other user space tools) should listen to this event a= nd >>>> fetch more details about the event via BPF syscalls >>>> (BPF_PROG_GET_FD_BY_ID, BPF_OBJ_GET_INFO_BY_FD, etc.). >>>=20 >>> Why !? You're failing to explain why it cannot provide the full >>> information there. >>=20 >> Aha, I missed this part. I will add the following to next version. Pleas= e >> let me know if anything is not clear. >=20 >>=20 >> This design decision is picked for the following reasons. First, BPF=20 >> programs could be loaded-and-jited and/or unloaded before/during/after=20 >> perf-record run. Once a BPF programs is unloaded, it is impossible to=20 >> recover details of the program. It is impossible to provide the=20 >> information through a simple key (like the build ID). Second, BPF prog >> annotation is under fast developments. Multiple informations will be=20 >> added to bpf_prog_info in the next few releases. Including all the >> information of a BPF program in the perf ring buffer requires frequent=20 >> changes to the perf ABI, and thus makes it very difficult to manage=20 >> compatibility of perf utility.=20 >=20 > So I don't agree with that reasoning. If you want symbol information > you'll just have to commit to some form of ABI. That bpf_prog_info is an > ABI too. At the beginning of the perf-record run, perf need to query bpf_prog_info=20 of already loaded BPF programs. Therefore, we need to commit to the=20 bpf_prog_info ABI. If we also include full information of the BPF program=20 in the perf ring buffer, we will commit to TWO ABIs.=20 Also, perf-record write the event to perf.data file, so the data need to be= =20 serialized. This is implemented in patch 4/5. To include the data in the=20 ring buffer, we will need another piece of code in the kernel to do the same serialization work. =20 On the other hand, processing BPF load/unload events synchronously should not introduce too much overhead for meaningful use cases. If many BPF progs are being loaded/unloaded within short period of time, it is not the steady state that profiling works care about.=20 Would these resolve your concerns?=20 Thanks, Song