On Thu, Jan 8, 2026 at 3:21 PM Simon Schippers
<[email protected]> wrote:
>
> On 1/8/26 04:23, Jason Wang wrote:
> > On Thu, Jan 8, 2026 at 5:06 AM Simon Schippers
> > <[email protected]> wrote:
> >>
> >> This proposed function checks whether __ptr_ring_zero_tail() was invoked
> >> within the last n calls to __ptr_ring_consume(), which indicates that new
> >> free space was created. Since __ptr_ring_zero_tail() moves the tail to
> >> the head - and no other function modifies either the head or the tail,
> >> aside from the wrap-around case described below - detecting such a
> >> movement is sufficient to detect the invocation of
> >> __ptr_ring_zero_tail().
> >>
> >> The implementation detects this movement by checking whether the tail is
> >> at most n positions behind the head. If this condition holds, the shift
> >> of the tail to its current position must have occurred within the last n
> >> calls to __ptr_ring_consume(), indicating that __ptr_ring_zero_tail() was
> >> invoked and that new free space was created.
> >>
> >> This logic also correctly handles the wrap-around case in which
> >> __ptr_ring_zero_tail() is invoked and the head and the tail are reset
> >> to 0. Since this reset likewise moves the tail to the head, the same
> >> detection logic applies.
> >>
> >> Co-developed-by: Tim Gebauer <[email protected]>
> >> Signed-off-by: Tim Gebauer <[email protected]>
> >> Signed-off-by: Simon Schippers <[email protected]>
> >> ---
> >> include/linux/ptr_ring.h | 13 +++++++++++++
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> >> index a5a3fa4916d3..7cdae6d1d400 100644
> >> --- a/include/linux/ptr_ring.h
> >> +++ b/include/linux/ptr_ring.h
> >> @@ -438,6 +438,19 @@ static inline int ptr_ring_consume_batched_bh(struct
> >> ptr_ring *r,
> >> return ret;
> >> }
> >>
> >> +/* Returns true if the consume of the last n elements has created space
> >> + * in the ring buffer (i.e., a new element can be produced).
> >> + *
> >> + * Note: Because of batching, a successful call to __ptr_ring_consume() /
> >> + * __ptr_ring_consume_batched() does not guarantee that the next call to
> >> + * __ptr_ring_produce() will succeed.
> >
> > This sounds like a bug that needs to be fixed, as it requires the user
> > to know the implementation details. For example, even if
> > __ptr_ring_consume_created_space() returns true, __ptr_ring_produce()
> > may still fail?
>
> No, it should not fail in that case.
> If you only call consume and after that try to produce, *then* it is
> likely to fail because __ptr_ring_zero_tail() is only invoked once per
> batch.
Well, this makes the helper very hard for users.
So I think at least the documentation should specify the meaning of
'n' here. For example, is it the value returned by
ptr_ring_consume_batched()(), and is it required to be called
immediately after ptr_ring_consume_batched()? If it is, the API is
kind of tricky to be used, we should consider to merge two helpers
into a new single helper to ease the user.
What's more, there would be false positives. Considering there's not
many entries in the ring, just after the first zeroing,
__ptr_ring_consume_created_space() will return true, this will lead to
unnecessary wakeups.
And last, the function will always succeed if n is greater than the batch.
>
> >
> > Maybe revert fb9de9704775d?
>
> I disagree, as I consider this to be one of the key features of ptr_ring.
Nope, it's just an optimization and actually it changes the behaviour
that might be noticed by the user.
Before the patch, ptr_ring_produce() is guaranteed to succeed after a
ptr_ring_consume(). After that patch, it's not. We don't see complaint
because the implication is not obvious (e.g more packet dropping).
>
> That said, there are several other implementation details that users need
> to be aware of.
>
> For example, __ptr_ring_empty() must only be called by the consumer. This
> was for example the issue in dc82a33297fc ("veth: apply qdisc
> backpressure on full ptr_ring to reduce TX drops") and the reason why
> 5442a9da6978 ("veth: more robust handing of race to avoid txq getting
> stuck") exists.
At least the behaviour is documented. This is not the case for the
implications of fb9de9704775d.
Thanks
>
> >
> >> + */
> >> +static inline bool __ptr_ring_consume_created_space(struct ptr_ring *r,
> >> + int n)
> >> +{
> >> + return r->consumer_head - r->consumer_tail < n;
> >> +}
> >> +
> >> /* Cast to structure type and call a function without discarding from
> >> FIFO.
> >> * Function must return a value.
> >> * Callers must take consumer_lock.
> >> --
> >> 2.43.0
> >>
> >
> > Thanks
> >
>
