RE: [PATCH v1 1/4] ring: introduce extra run-time checks

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com]
> Sent: Wednesday, 21 May 2025 13.14
> 
> Add RTE_ASSERT() to check that different move_tail() flavors
> return meaningful  *entries value.
> It also helps to ensure that inside move_tail(), it uses correct
> head/tail values.
> 
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> ---
>  lib/ring/rte_ring_c11_pvt.h      | 2 +-
>  lib/ring/rte_ring_elem_pvt.h     | 8 ++++++--
>  lib/ring/rte_ring_hts_elem_pvt.h | 8 ++++++--
>  lib/ring/rte_ring_rts_elem_pvt.h | 8 ++++++--
>  lib/ring/soring.c                | 2 ++
>  5 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/ring/rte_ring_c11_pvt.h b/lib/ring/rte_ring_c11_pvt.h
> index b9388af0da..0845cd6dcf 100644
> --- a/lib/ring/rte_ring_c11_pvt.h
> +++ b/lib/ring/rte_ring_c11_pvt.h
> @@ -104,10 +104,10 @@ __rte_ring_headtail_move_head(struct
> rte_ring_headtail *d,
>  			n = (behavior == RTE_RING_QUEUE_FIXED) ?
>  					0 : *entries;
> 
> +		*new_head = *old_head + n;
>  		if (n == 0)
>  			return 0;
> 
> -		*new_head = *old_head + n;
>  		if (is_st) {
>  			d->head = *new_head;
>  			success = 1;

Is there a need to assign a value to *new_head if n==0?

I don't think your suggestion is multi-thread safe.
If d->head moves, the value in *new_head will be incorrect.

Instead, suggest:

-		if (n == 0)
-			return 0;

		*new_head = *old_head + n;
		if (is_st) {
			d->head = *new_head;
			success = 1;
		} else
			/* on failure, *old_head is updated */
			success = rte_atomic_compare_exchange_strong_explicit(
					&d->head, old_head, *new_head,
					rte_memory_order_relaxed,
					rte_memory_order_relaxed);
	} while (unlikely(success == 0));