[PATCH bpf-next v1] libbpf: Add wakeup_events to creation options

[PATCH bpf-next v1] libbpf: Add wakeup_events to creation options

[Jon Doron]

From: Jon Doron <jond@wiz.io>

Add option to set when the perf buffer should wake up, by default the
perf buffer becomes signaled for every event that is being pushed to it.

In case of a high throughput of events it will be more efficient to wake
up only once you have X events ready to be read.

So your application can wakeup once and drain the entire perf buffer.

Signed-off-by: Jon Doron <jond@wiz.io>
---
 tools/lib/bpf/libbpf.c | 4 ++--
 tools/lib/bpf/libbpf.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index eed5cec6f510..6b30ff13922b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -11719,8 +11719,8 @@ struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
 	attr.config = PERF_COUNT_SW_BPF_OUTPUT;
 	attr.type = PERF_TYPE_SOFTWARE;
 	attr.sample_type = PERF_SAMPLE_RAW;
-	attr.sample_period = 1;
-	attr.wakeup_events = 1;
+	attr.sample_period = OPTS_GET(opts, wakeup_events, 1);
+	attr.wakeup_events = OPTS_GET(opts, wakeup_events, 1);
 
 	p.attr = &attr;
 	p.sample_cb = sample_cb;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 8777ff21ea1d..2e4bdfc58c82 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -1246,6 +1246,7 @@ typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
 /* common use perf buffer options */
 struct perf_buffer_opts {
 	size_t sz;
+	__u32 wakeup_events;
 };
 #define perf_buffer_opts__last_field sz
 
-- 
2.39.1

Re: [PATCH bpf-next v1] libbpf: Add wakeup_events to creation options

[Jiri Olsa]

On Wed, Feb 01, 2023 at 11:00:09AM +0200, Jon Doron wrote:
> From: Jon Doron <jond@wiz.io>
> 
> Add option to set when the perf buffer should wake up, by default the
> perf buffer becomes signaled for every event that is being pushed to it.
> 
> In case of a high throughput of events it will be more efficient to wake
> up only once you have X events ready to be read.
> 
> So your application can wakeup once and drain the entire perf buffer.
> 
> Signed-off-by: Jon Doron <jond@wiz.io>
> ---
>  tools/lib/bpf/libbpf.c | 4 ++--
>  tools/lib/bpf/libbpf.h | 1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index eed5cec6f510..6b30ff13922b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11719,8 +11719,8 @@ struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
>  	attr.config = PERF_COUNT_SW_BPF_OUTPUT;
>  	attr.type = PERF_TYPE_SOFTWARE;
>  	attr.sample_type = PERF_SAMPLE_RAW;
> -	attr.sample_period = 1;
> -	attr.wakeup_events = 1;
> +	attr.sample_period = OPTS_GET(opts, wakeup_events, 1);

hm, but I think we still want every event.. setting sample_period to X
would store only every X-th bpf-output event, no?

jirka

> +	attr.wakeup_events = OPTS_GET(opts, wakeup_events, 1);
>  
>  	p.attr = &attr;
>  	p.sample_cb = sample_cb;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 8777ff21ea1d..2e4bdfc58c82 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -1246,6 +1246,7 @@ typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
>  /* common use perf buffer options */
>  struct perf_buffer_opts {
>  	size_t sz;
> +	__u32 wakeup_events;
>  };
>  #define perf_buffer_opts__last_field sz
>  
> -- 
> 2.39.1
> 

Re: [PATCH bpf-next v1] libbpf: Add wakeup_events to creation options

[Andrii Nakryiko]

On Wed, Feb 1, 2023 at 11:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 01, 2023 at 11:00:09AM +0200, Jon Doron wrote:
> > From: Jon Doron <jond@wiz.io>
> >
> > Add option to set when the perf buffer should wake up, by default the
> > perf buffer becomes signaled for every event that is being pushed to it.
> >
> > In case of a high throughput of events it will be more efficient to wake
> > up only once you have X events ready to be read.
> >
> > So your application can wakeup once and drain the entire perf buffer.
> >
> > Signed-off-by: Jon Doron <jond@wiz.io>
> > ---
> >  tools/lib/bpf/libbpf.c | 4 ++--
> >  tools/lib/bpf/libbpf.h | 1 +
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index eed5cec6f510..6b30ff13922b 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -11719,8 +11719,8 @@ struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
> >       attr.config = PERF_COUNT_SW_BPF_OUTPUT;
> >       attr.type = PERF_TYPE_SOFTWARE;
> >       attr.sample_type = PERF_SAMPLE_RAW;
> > -     attr.sample_period = 1;
> > -     attr.wakeup_events = 1;
> > +     attr.sample_period = OPTS_GET(opts, wakeup_events, 1);
>
> hm, but I think we still want every event.. setting sample_period to X
> would store only every X-th bpf-output event, no?

seems like benchs/bench_ringbufs.c sets both sample_period and
wakeup_events, but it would be nice to make sure we do not lose events
with such configuration.

Let's add a selftest for this feature.

>
> jirka
>
> > +     attr.wakeup_events = OPTS_GET(opts, wakeup_events, 1);
> >
> >       p.attr = &attr;
> >       p.sample_cb = sample_cb;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 8777ff21ea1d..2e4bdfc58c82 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -1246,6 +1246,7 @@ typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
> >  /* common use perf buffer options */
> >  struct perf_buffer_opts {
> >       size_t sz;
> > +     __u32 wakeup_events;
> >  };
> >  #define perf_buffer_opts__last_field sz

you need to update perf_buffer_opts__last_field to wakeup_events as well


> >
> > --
> > 2.39.1
> >

Re: [PATCH bpf-next v1] libbpf: Add wakeup_events to creation options

[Jon Doron]

On 01/02/2023, Andrii Nakryiko wrote:
>On Wed, Feb 1, 2023 at 11:57 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>>
>> On Wed, Feb 01, 2023 at 11:00:09AM +0200, Jon Doron wrote:
>> > From: Jon Doron <jond@wiz.io>
>> >
>> > Add option to set when the perf buffer should wake up, by default the
>> > perf buffer becomes signaled for every event that is being pushed to it.
>> >
>> > In case of a high throughput of events it will be more efficient to wake
>> > up only once you have X events ready to be read.
>> >
>> > So your application can wakeup once and drain the entire perf buffer.
>> >
>> > Signed-off-by: Jon Doron <jond@wiz.io>
>> > ---
>> >  tools/lib/bpf/libbpf.c | 4 ++--
>> >  tools/lib/bpf/libbpf.h | 1 +
>> >  2 files changed, 3 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> > index eed5cec6f510..6b30ff13922b 100644
>> > --- a/tools/lib/bpf/libbpf.c
>> > +++ b/tools/lib/bpf/libbpf.c
>> > @@ -11719,8 +11719,8 @@ struct perf_buffer *perf_buffer__new(int map_fd, size_t page_cnt,
>> >       attr.config = PERF_COUNT_SW_BPF_OUTPUT;
>> >       attr.type = PERF_TYPE_SOFTWARE;
>> >       attr.sample_type = PERF_SAMPLE_RAW;
>> > -     attr.sample_period = 1;
>> > -     attr.wakeup_events = 1;
>> > +     attr.sample_period = OPTS_GET(opts, wakeup_events, 1);
>>
>> hm, but I think we still want every event.. setting sample_period to X
>> would store only every X-th bpf-output event, no?
>
>seems like benchs/bench_ringbufs.c sets both sample_period and
>wakeup_events, but it would be nice to make sure we do not lose events
>with such configuration.
>
>Let's add a selftest for this feature.
>

That's how it works, we wont be losing any events

>>
>> jirka
>>
>> > +     attr.wakeup_events = OPTS_GET(opts, wakeup_events, 1);
>> >
>> >       p.attr = &attr;
>> >       p.sample_cb = sample_cb;
>> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> > index 8777ff21ea1d..2e4bdfc58c82 100644
>> > --- a/tools/lib/bpf/libbpf.h
>> > +++ b/tools/lib/bpf/libbpf.h
>> > @@ -1246,6 +1246,7 @@ typedef void (*perf_buffer_lost_fn)(void *ctx, int cpu, __u64 cnt);
>> >  /* common use perf buffer options */
>> >  struct perf_buffer_opts {
>> >       size_t sz;
>> > +     __u32 wakeup_events;
>> >  };
>> >  #define perf_buffer_opts__last_field sz
>
>you need to update perf_buffer_opts__last_field to wakeup_events as well
>
>

Done

>> >
>> > --
>> > 2.39.1
>> >