[PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf_event given a struct file

[PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf_event given a struct file

[Yonghong Song]

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.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/perf_event.h | 5 +++++
 kernel/events/core.c       | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index e71e99e..b5c1ad3 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -868,6 +868,7 @@ extern void perf_event_exit_task(struct task_struct *child);
 extern void perf_event_free_task(struct task_struct *task);
 extern void perf_event_delayed_put(struct task_struct *task);
 extern struct file *perf_event_get(unsigned int fd);
+extern struct perf_event *perf_get_event(struct file *file);
 extern const struct perf_event_attr *perf_event_attrs(struct perf_event *event);
 extern void perf_event_print_debug(void);
 extern void perf_pmu_disable(struct pmu *pmu);
@@ -1289,6 +1290,10 @@ static inline void perf_event_exit_task(struct task_struct *child)	{ }
 static inline void perf_event_free_task(struct task_struct *task)	{ }
 static inline void perf_event_delayed_put(struct task_struct *task)	{ }
 static inline struct file *perf_event_get(unsigned int fd)	{ return ERR_PTR(-EINVAL); }
+static inline struct perf_event *perf_get_event(struct file *file)
+{
+	return ERR_PTR(-EINVAL);
+}
 static inline const struct perf_event_attr *perf_event_attrs(struct perf_event *event)
 {
 	return ERR_PTR(-EINVAL);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 67612ce..1e3cddb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -11212,6 +11212,14 @@ struct file *perf_event_get(unsigned int fd)
 	return file;
 }
 
+struct perf_event *perf_get_event(struct file *file)
+{
+	if (file->f_op != &perf_fops)
+		return ERR_PTR(-EINVAL);
+
+	return file->private_data;
+}
+
 const struct perf_event_attr *perf_event_attrs(struct perf_event *event)
 {
 	if (!event)
-- 
2.9.5

Re: [PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf_event given a struct file

[Peter Zijlstra]

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.

Re: [PATCH bpf-next v2 1/7] perf/core: add perf_get_event() to return perf_event given a struct file

[Yonghong Song]

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