[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Ze9ou-OrYO8NZsDb@gpd>
Date: Mon, 11 Mar 2024 21:25:31 +0100
From: Andrea Righi <andrea.righi@...onical.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Andrii Nakryiko <andrii@...nel.org>,
Eduard Zingerman <eddyz87@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>,
Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
David Vernet <void@...ifault.com>, Tejun Heo <tj@...nel.org>,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] libbpf: ringbuf: allow to partially consume items
On Mon, Mar 11, 2024 at 10:55:57AM -0700, Andrii Nakryiko wrote:
> On Sun, Mar 10, 2024 at 8:47 AM Andrea Righi <andrea.righi@...onical.com> wrote:
> >
> > Instead of always consuming all items from a ring buffer in a greedy
> > way, allow to stop when the callback returns a value > 0.
> >
> > This allows to distinguish between an error condition and an intentional
> > stop condition by returning a non-negative non-zero value from the ring
> > buffer callback.
> >
> > This can be useful, for example, to consume just a single item from the
> > ring buffer.
> >
> > Signed-off-by: Andrea Righi <andrea.righi@...onical.com>
> > ---
> > tools/lib/bpf/ringbuf.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
> > index aacb64278a01..dd8908eb3204 100644
> > --- a/tools/lib/bpf/ringbuf.c
> > +++ b/tools/lib/bpf/ringbuf.c
> > @@ -265,6 +265,14 @@ static int64_t ringbuf_process_ring(struct ring *r)
> > return err;
> > }
> > cnt++;
> > + if (err > 0) {
>
> So libbpf already stops at any err < 0 (and sets correct consumer
> pos). So you could already get desired behavior by just returning your
> own error code. If you need count, you'd have to count it yourself
> through custom context, that's a bit of inconvenience.
Yep, that's exactly what I'm doing right now.
To give more context, here's the code:
https://github.com/sched-ext/scx/blob/b7c06b9ed9f72cad83c31e39e9c4e2cfd8683a55/rust/scx_rustland_core/src/bpf.rs#L217
>
> But on the other hand, currently if user callback returns anything > 0
> they keep going and that return value is ignored. Your change will
> break any such user pretty badly. So I'm a bit hesitant to do this.
So, returning a value > 0 should have the same behavior as returning 0?
Why any user callback would return > 0 then?
>
> Is there any reason you can't just return error code (libbpf doesn't
> do anything with it, just passes it back, so it might as well be
> `-cnt`, if you need that).
Sure, I can keep using my special error code to stop. It won't be a
problem for my particular use case.
Actually, one thing that it would be nice to have is a way to consume up
to a certain amount of items, let's say I need to copy multiple items
from the ring buffer to a limited user buffer. But that would require a
new API I guess, in order to pass the max counter... right?
Thanks,
-Andrea
>
> pw-bot: cr
>
> > + /* update consumer pos and return the
> > + * total amount of items consumed.
> > + */
> > + smp_store_release(r->consumer_pos,
> > + cons_pos);
> > + goto done;
> > + }
> > }
> >
> > smp_store_release(r->consumer_pos, cons_pos);
> > --
> > 2.43.0
> >
> >
Powered by blists - more mailing lists