Re: [PATCH bpf-next v3 1/2] trace_helpers.c: Add helpers to poll multiple perf FDs for events

[Daniel Borkmann <daniel@iogearbox.net>]

Hi Toke,

On 06/05/2018 01:14 PM, Toke Høiland-Jørgensen wrote:
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>

Please no empty commit message. Not sure why from the previous patch
you removed it here.

> ---
>  tools/testing/selftests/bpf/trace_helpers.c |   47 ++++++++++++++++++++++++++-
>  tools/testing/selftests/bpf/trace_helpers.h |    4 ++
>  2 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
> index 3868dcb63420..1e62d89f34cf 100644
> --- a/tools/testing/selftests/bpf/trace_helpers.c
> +++ b/tools/testing/selftests/bpf/trace_helpers.c
> @@ -88,7 +88,7 @@ static int page_size;
>  static int page_cnt = 8;
>  static struct perf_event_mmap_page *header;
>  
> -int perf_event_mmap(int fd)
> +int perf_event_mmap_header(int fd, struct perf_event_mmap_page **header)
>  {
>  	void *base;
>  	int mmap_size;
> @@ -102,10 +102,15 @@ int perf_event_mmap(int fd)
>  		return -1;
>  	}
>  
> -	header = base;
> +	*header = base;
>  	return 0;
>  }
>  
> +int perf_event_mmap(int fd)
> +{
> +	return perf_event_mmap_header(fd, &header);
> +}
> +
>  static int perf_event_poll(int fd)
>  {
>  	struct pollfd pfd = { .fd = fd, .events = POLLIN };
> @@ -163,3 +168,41 @@ int perf_event_poller(int fd, perf_event_print_fn output_fn)
>  
>  	return ret;
>  }
> +
> +int perf_event_poller_multi(int *fds, struct perf_event_mmap_page **headers,
> +			    int num_fds, perf_event_print_fn output_fn)
> +{
> +	enum bpf_perf_event_ret ret;
> +	struct pollfd *pfds;
> +	void *buf = NULL;
> +	size_t len = 0;
> +	int i;
> +
> +	pfds = malloc(sizeof(*pfds) * num_fds);
> +	if (!pfds)
> +		return -1;

Also, just noticed here you mix -1 as return code with LIBBPF_* return
codes. Would be better not not overlap such usage.

> +	memset(pfds, 0, sizeof(*pfds) * num_fds);
> +	for (i = 0; i < num_fds; i++) {
> +		pfds[i].fd = fds[i];
> +		pfds[i].events = POLLIN;
> +	}
> +
> +	for (;;) {
> +		poll(pfds, num_fds, 1000);
> +		for (i = 0; i < num_fds; i++) {
> +			if (pfds[i].revents) {
> +				ret = bpf_perf_event_read_simple(headers[i], page_cnt * page_size,
> +								page_size, &buf, &len,
> +								bpf_perf_event_print,
> +								output_fn);
> +				if (ret != LIBBPF_PERF_EVENT_CONT)
> +					break;
> +			}
> +		}
> +	}
> +	free(buf);
> +	free(pfds);
> +
> +	return ret;

Thanks,
Daniel