> From: Konstantin Ananyev [mailto:konstantin.anan...@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.anan...@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));