Re: [PATCH net-next v8 3/4] ptr_ring: move free-space check into separate helper

[Michael S. Tsirkin]

On Thu, Mar 12, 2026 at 02:17:16PM +0100, Eric Dumazet wrote:
> On Thu, Mar 12, 2026 at 2:06 PM Simon Schippers
> <simon.schippers@xxxxxxxxxxxxxx> wrote:
> >
> > This patch moves the check for available free space for a new entry into
> > a separate function. As a result, __ptr_ring_produce() remains logically
> > unchanged, while the new helper allows callers to determine in advance
> > whether subsequent __ptr_ring_produce() calls will succeed. This
> > information can, for example, be used to temporarily stop producing until
> > __ptr_ring_peek() indicates that space is available again.
> >
> > Co-developed-by: Tim Gebauer <tim.gebauer@xxxxxxxxxxxxxx>
> > Signed-off-by: Tim Gebauer <tim.gebauer@xxxxxxxxxxxxxx>
> > Signed-off-by: Simon Schippers <simon.schippers@xxxxxxxxxxxxxx>
> > ---
> >  include/linux/ptr_ring.h | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> > index 534531807d95..a5a3fa4916d3 100644
> > --- a/include/linux/ptr_ring.h
> > +++ b/include/linux/ptr_ring.h
> > @@ -96,6 +96,14 @@ static inline bool ptr_ring_full_bh(struct ptr_ring *r)
> >         return ret;
> >  }
> >
> > +static inline int __ptr_ring_produce_peek(struct ptr_ring *r)
> > +{
> > +       if (unlikely(!r->size) || r->queue[r->producer])
> 
> I think this should be
> 
>        if (unlikely(!r->size) || READ_ONCE(r->queue[r->producer]))
> 
> And of course:
> 
> @@ -194,7 +194,7 @@ static inline void *__ptr_ring_peek(struct ptr_ring *r)
>  static inline bool __ptr_ring_empty(struct ptr_ring *r)
>  {
>         if (likely(r->size))
> -               return !r->queue[READ_ONCE(r->consumer_head)];
> +               return !READ_ONCE(r->queue[READ_ONCE(r->consumer_head)]);
>         return true;
>  }


I don't understand why it's necessary. consumer_head etc are
all lock protected.

queue itself is not but we are only checking it for NULL -
it is fine if compiler reads it in many chunks and not all
at once.

> @@ -256,7 +256,7 @@ static inline void __ptr_ring_zero_tail(struct
> ptr_ring *r, int consumer_head)
>          * besides the first one until we write out all entries.
>          */
>         while (likely(head > r->consumer_tail))
> -               r->queue[--head] = NULL;
> +               WRITE_ONCE(r->queue[--head], NULL);
> 
>         r->consumer_tail = consumer_head;
>  }
> 
> 
> Presumably we should fix this in net tree first.


Maybe this one yes but I am not sure at all - KCSAN is happy.


-- 
MST