From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yonghong Song Subject: Re: [PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf_event given a struct file Date: Fri, 18 May 2018 09:32:09 -0700 Message-ID: References: <20180518053253.2908444-1-yhs@fb.com> <20180518071822.GC12217@hirez.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , , To: Peter Zijlstra Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:53204 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751197AbeERQcx (ORCPT ); Fri, 18 May 2018 12:32:53 -0400 In-Reply-To: <20180518071822.GC12217@hirez.programming.kicks-ass.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 5/18/18 12:18 AM, Peter Zijlstra wrote: > On Thu, May 17, 2018 at 10:32:53PM -0700, Yonghong Song wrote: >> A new extern function, perf_get_event(), is added to return a perf event >> given a struct file. This function will be used in later patches. > > Can't you do a narrower interface? Like return the prog. I'm not too > keen on random !perf code frobbing around inside the event. Hi, Peter, My initial implementation (not upstreamed) actually have the whole function bpf_get_perf_event_info() in the events/core.c. In that case, the "struct file *" pointer is passed. This way, the event pointer does not need to go to kernel/bpf/syscall.c or kernel/trace/bpf_trace.c. I dropped this mechanism since it added more codes in the events/core.c file, and I felt that such query code might clutter events/core.c. The function bpf_get_perf_event_info() is now placed in kernel/trace/bpf_trace.c. Just getting bpf prog pointer is not enough as it does not provide enough attachment information. Getting such information requires poking into event/tp_event etc. Currently we have this extern function exposed by events/core.c: extern struct perf_event *perf_get_event(struct file *file); We could make the result value "const" like extern const struct perf_event *perf_get_event(struct file *file); This will make it clear that we do not change "event" fields, and merely poking at it. Please let me know your preference. Thanks! Yonghong