On Wed, Dec 06, 2017 at 12:56:36PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 05, 2017 at 10:31:28PM -0800, Yonghong Song wrote:
> > Commit e87c6bc3852b ("bpf: permit multiple bpf attachments
> > for a single perf event") added support to attach multiple
> > bpf programs to a single perf event.
> > Commit 2541517c32be ("tracing, perf: Implement BPF programs
> > attached to kprobes") utilized the existing perf ioctl
> > interface and added the command PERF_EVENT_IOC_SET_BPF
> > to attach a bpf program to a tracepoint.
> > 
> > This patch adds a new ioctl
> > command, given a perf event fd, to query the bpf program array
> > attached to the same perf tracepoint event.
> > 
> > The new uapi ioctl command:
> >   PERF_EVENT_IOC_QUERY_BPF
> > 
> > The new uapi/linux/perf_event.h structure:
> >   struct perf_event_query_bpf {
> >        __u64        prog_ids;
> >        __u32        prog_cnt;
> >   };
> > 
> > The usage:
> >   struct perf_event_query_bpf query;
> >   query.prog_ids = (__u64)usr_prog_ids_buf;
> >   query.prog_cnt = usr_prog_ids_buf_len;
> >   err = ioctl(pmu_efd, PERF_EVENT_IOC_QUERY_BPF, &query);
> > 
> > Signed-off-by: Yonghong Song <y...@fb.com>
> > Acked-by: Alexei Starovoitov <a...@kernel.org>
> 
> Can you please fix that example to make it clear that prog_ids is in
> fact a pointer to an array of size prog_cnt. Ideally also describing
> what the type of array is.
> 
> In fact, would not something like:
> 
>       struct perf_event_query_bpf {
>               __u32 len;
>               __u32 __reserved;

I suppose we could use this field to store the number of entries
returned, retaining the len to indicate how large the structure is.

>               __u64 ids[0];
>       };
> 
> be a much clearer interface?
> 
> Also, you forgot to tell us why we need this interface at all.

Reply via email to