Re: [dpdk-dev] [PATCH v5 7/9] ring: introduce peek style API

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>
> 
> 
> >
> > > > diff --git a/lib/librte_ring/rte_ring_generic.h
> > > > b/lib/librte_ring/rte_ring_generic.h
> > > > index 953cdbbd5..9f5fdf13b 100644
> > > > --- a/lib/librte_ring/rte_ring_generic.h
> > > > +++ b/lib/librte_ring/rte_ring_generic.h
> > > Changes in this file are not required as we agreed to implement only C11
> for new features.
> >
> > Right, will remove.
> 
> Actually no, spoke too early before thinking properly We do need these
> functions in rte_ring_generic.h for SP/SC _start_/_finish_.
> Konstantin
The peek APIs are new functionality. So the peek APIs in legacy format should be wrappers around _elem_ APIs. That is what I see in the code as well:
rte_ring_peek.h has this:
static __rte_always_inline void
rte_ring_dequeue_finish(struct rte_ring *r, unsigned int n)
{
        rte_ring_dequeue_elem_finish(r, n);
}

I think, I gave you incomplete feedback earlier.
Actually, __rte_ring_st_get_tail and __rte_ring_st_set_head_tail should be in a new file named rte_ring_peek_c11_mem.h. This file should be included in rte_ring_peek.h (same way you have done for RTS and HTS). Then remove both these functions from rte_ring_generic.h and rte_ring_c11_mem.h.

> 
> >
> > >
> > > > @@ -10,6 +10,54 @@
> > > >  #ifndef _RTE_RING_GENERIC_H_
> > > >  #define _RTE_RING_GENERIC_H_
> > > >
> > > > +/**
> > > > + * @internal get current tail value.
> > > > + * This function should be used only for single thread
> producer/consumer.
> > > > + * Check that user didn't request to move tail above the head.
> > > > + * In that situation:
> > > > + * - return zero, that will cause abort any pending changes and
> > > > + *   return head to its previous position.
> > > > + * - throw an assert in debug mode.
> > > > + */
> > > > +static __rte_always_inline uint32_t __rte_ring_st_get_tail(struct
> > > > +rte_ring_headtail *ht, uint32_t *tail,
> > > > +	uint32_t num)
> > > > +{
> > > > +	uint32_t h, n, t;
> > > > +
> > > > +	h = ht->head;
> > > > +	t = ht->tail;
> > > > +	n = h - t;
> > > > +
> > > > +	RTE_ASSERT(n >= num);
> > > > +	num = (n >= num) ? num : 0;
> > > > +
> > > > +	*tail = h;
> > > > +	return num;
> > > > +}
> > > > +
> > > > +/**
> > > > + * @internal set new values for head and tail.
> > > > + * This function should be used only for single thread
> producer/consumer.
> > > > + * Should be used only in conjunction with __rte_ring_st_get_tail.
> > > > + */
> > > > +static __rte_always_inline void
> > > > +__rte_ring_st_set_head_tail(struct rte_ring_headtail *ht, uint32_t tail,
> > > > +	uint32_t num, uint32_t enqueue)
> > > > +{
> > > > +	uint32_t pos;
> > > > +
> > > > +	pos = tail + num;
> > > > +
> > > > +	if (enqueue)
> > > > +		rte_smp_wmb();
> > > > +	else
> > > > +		rte_smp_rmb();
> > > > +
> > > > +	ht->head = pos;
> > > > +	ht->tail = pos;
> > > > +}
> > > > +
> > > >  static __rte_always_inline void
> > > >  update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t
> new_val,
> > > >  		uint32_t single, uint32_t enqueue) diff --git
> > > > a/lib/librte_ring/rte_ring_peek.h
> > > > b/lib/librte_ring/rte_ring_peek.h new file mode 100644 index
> > > > 000000000..2d06888b6
> > > > --- /dev/null