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:     Peter Zijlstra <peterz@infradead.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "acme@kernel.org" <acme@kernel.org>,
        Kernel Team <Kernel-team@fb.com>
Subject: Re: [PATCH perf,bpf 0/5] reveal invisible bpf programs
Thread-Topic: [PATCH perf,bpf 0/5] reveal invisible bpf programs
Thread-Index: AQHUgdQ11TRJMN8CIkSD/uQZUfyfEaVbiN6AgACRn4CABhB7AIAB3ygA
Date:   Tue, 27 Nov 2018 19:25:03 +0000
Message-ID: <EDEF2248-6909-44FE-8819-3D2349DBFB73@fb.com>
References: <20181121195502.3259930-1-songliubraving@fb.com>
 <20181122093219.GK2131@hirez.programming.kicks-ass.net>
 <71189F83-A09F-4A03-95EC-694D37FD7675@fb.com>
 <20181126145004.GO2113@hirez.programming.kicks-ass.net>
In-Reply-To: <20181126145004.GO2113@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="us-ascii"
Content-ID: <FB4DFCFC9337BD49BEC8D3C7CDF23482@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 4012e2f4-c219-428e-7c0b-08d6549e07ee
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Nov 2018 19:25:03.3045
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR15MB1837
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-27_16:,,
 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 Nov 26, 2018, at 6:50 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>=20
> On Thu, Nov 22, 2018 at 06:13:32PM +0000, Song Liu wrote:
>> Hi Peter,
>>=20
>>> On Nov 22, 2018, at 1:32 AM, Peter Zijlstra <peterz@infradead.org> wrot=
e:
>>>=20
>>> On Wed, Nov 21, 2018 at 11:54:57AM -0800, Song Liu wrote:
>>>> Changes RFC -> PATCH v1:
>>>>=20
>>>> 1. In perf-record, poll vip events in a separate thread;
>>>> 2. Add tag to bpf prog name;
>>>> 3. Small refactorings.
>>>>=20
>>>> Original cover letter (with minor revisions):
>>>>=20
>>>> This is to follow up Alexei's early effort to show bpf programs
>=20
>>>> In this version, PERF_RECORD_BPF_EVENT is introduced to send real time=
 BPF
>>>> load/unload events to user space. In user space, perf-record is modifi=
ed
>>>> to listen to these events (through a dedicated ring buffer) and genera=
te
>>>> detailed information about the program (struct bpf_prog_info_event). T=
hen,
>>>> perf-report translates these events into proper symbols.
>>>>=20
>>>> With this set, perf-report will show bpf program as:
>>>>=20
>>>>  18.49%     0.16%  test  [kernel.vmlinux]    [k] ksys_write
>>>>  18.01%     0.47%  test  [kernel.vmlinux]    [k] vfs_write
>>>>  17.02%     0.40%  test  bpf_prog            [k] bpf_prog_07367f7ba80d=
f72b_
>>>>  16.97%     0.10%  test  [kernel.vmlinux]    [k] __vfs_write
>>>>  16.86%     0.12%  test  [kernel.vmlinux]    [k] comm_write
>>>>  16.67%     0.39%  test  [kernel.vmlinux]    [k] bpf_probe_read
>>>>=20
>>>> Note that, the program name is still work in progress, it will be clea=
ner
>>>> with function types in BTF.
>>>>=20
>>>> Please share your comments on this.
>>>=20
>>> So I see:
>>>=20
>>> kernel/bpf/core.c:void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>>>=20
>>> which should already provide basic symbol information for extant eBPF
>>> programs, right?
>>=20
>> Right, if the BPF program is still loaded when perf-report runs, symbols=
=20
>> are available.=20
>=20
> Good, that is not something that was clear. The Changelog seems to imply
> we need this new stuff in order to observe symbols.
>=20
>>> And (AFAIK) perf uses /proc/kcore for annotate on the current running
>>> kernel (if not, it really should, given alternatives, jump_labels and
>>> all other other self-modifying code).
>>>=20
>>> So this fancy new stuff is only for the case where your profile spans
>>> eBPF load/unload events (which should be relatively rare in the normal
>>> case, right), or when you want source annotated asm output (I normally
>>> don't bother with that).
>>=20
>> This patch set adds two pieces of information:
>> 1. At the beginning of perf-record, save info of existing BPF programs;
>> 2. Gather information of BPF programs load/unload during perf-record.=20
>>=20
>> (1) is all in user space. It is necessary to show symbols of BPF program
>> that are unloaded _after_ perf-record. (2) needs PERF_RECORD_BPF_EVENT=20
>> from the ring buffer. It covers BPF program loaded during perf-record=20
>> (perf record -- bpf_test).=20
>=20
> I'm saying that if you given them symbols; most people won't need any of
> that ever.
>=20
> And just tracking kallsyms is _much_ cheaper than either 1 or 2. Alexei
> was talking fairly big amounts of data per BPF prog. Dumping and saving
> that sounds like pointless overhead for 99% of the users.

If annotation is not needed, we have the option to reduce the amount of=20
data per bpf prog by not requesting JITed binaries. Would a flag to tune
this solve the concern of dumping and saving too much data?

Thanks,
Song