On Wed, 28 May 2025 at 02:23, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> writes:
>
> [...]
>
> >> > diff --git a/kernel/bpf/stream.c b/kernel/bpf/stream.c
> >> > new file mode 100644
> >> > index 000000000000..b9e6f7a43b1b
> >> > --- /dev/null
> >> > +++ b/kernel/bpf/stream.c
> >>
> >> [...]
> >>
> >> > +int bpf_stream_stage_commit(struct bpf_stream_stage *ss, struct bpf_prog *prog,
> >> > + enum bpf_stream_id stream_id)
> >> > +{
> >> > + struct llist_node *list, *head, *tail;
> >> > + struct bpf_stream *stream;
> >> > + int ret;
> >> > +
> >> > + stream = bpf_stream_get(stream_id, prog->aux);
> >> > + if (!stream)
> >> > + return -EINVAL;
> >> > +
> >> > + ret = bpf_stream_consume_capacity(stream, ss->len);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + list = llist_del_all(&ss->log);
> >> > + head = list;
> >> > +
> >> > + if (!list)
> >> > + return 0;
> >> > + while (llist_next(list)) {
> >> > + tail = llist_next(list);
> >> > + list = tail;
> >> > + }
> >> > + llist_add_batch(head, tail, &stream->log);
> >>
> >> If `llist_next(list) == NULL` at entry `tail` is never assigned?
> >
> > The assumption is llist_del_all being non-NULL means llist_next is
> > going to return a non-NULL value at least once.
> > Does that address your concern?
>
> Sorry, maybe I don't understand something.
> Suppose that at entry ss->log is a list with a single element:
>
> ss->log -> 0xAA: { .next = NULL; ... payload ... }
>
> then:
> - list == 0xAA;
> - llist_next(list) == 0x0;
> - loop body never executes.
>
> What do I miss?
Right, I see.
We need to do head = tail = list above.
Then it's equivalent to a single element llist_add.
>
>
> >> > + return 0;
> >> > +}
>
> [...]
