Hi Lazaros,
On Thu, Mar 17, 2016 at 4:49 PM, Lazaros Koromilas <l at nofutznetworks.com>
wrote:
> Issuing a zero objects dequeue with a single consumer has no effect.
> Doing so with multiple consumers, can get more than one thread to succeed
> the compare-and-set operation and observe starvation or even deadlock in
> the while loop that checks for preceding dequeues. The problematic piece
> of code when n = 0:
>
> cons_next = cons_head + n;
> success = rte_atomic32_cmpset(&r->cons.head, cons_head, cons_next);
>
> The same is possible on the enqueue path.
>
> Signed-off-by: Lazaros Koromilas <l at nofutznetworks.com>
> ---
> lib/librte_ring/rte_ring.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h
> index 943c97c..eb45e41 100644
> --- a/lib/librte_ring/rte_ring.h
> +++ b/lib/librte_ring/rte_ring.h
> @@ -431,6 +431,11 @@ __rte_ring_mp_do_enqueue(struct rte_ring *r, void *
> const *obj_table,
> uint32_t mask = r->prod.mask;
> int ret;
>
> + /* Avoid the unnecessary cmpset operation below, which is also
> + * potentially harmful when n equals 0. */
> + if (n == 0)
>
What about using unlikely here?
> + return 0;
> +
> /* move prod.head atomically */
> do {
> /* Reset n to the initial burst count */
> @@ -618,6 +623,11 @@ __rte_ring_mc_do_dequeue(struct rte_ring *r, void
> **obj_table,
> unsigned i, rep = 0;
> uint32_t mask = r->prod.mask;
>
> + /* Avoid the unnecessary cmpset operation below, which is also
> + * potentially harmful when n equals 0. */
> + if (n == 0)
>
Also here.
> + return 0;
> +
> /* move cons.head atomically */
> do {
> /* Restore n as it may change every loop */
> --
> 1.9.1
>
>
