From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2755E467AE; Wed, 21 May 2025 14:14:17 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E05B1427C7; Wed, 21 May 2025 14:14:16 +0200 (CEST) Received: from dkmailrelay1.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id C3B23427BC for ; Wed, 21 May 2025 14:14:15 +0200 (CEST) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesys.local [192.168.4.10]) by dkmailrelay1.smartsharesystems.com (Postfix) with ESMTP id 8AD6F206C2; Wed, 21 May 2025 14:14:15 +0200 (CEST) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v1 1/4] ring: introduce extra run-time checks X-MimeOLE: Produced By Microsoft Exchange V6.5 Date: Wed, 21 May 2025 14:14:12 +0200 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35E9FC7E@smartserver.smartshare.dk> In-Reply-To: <20250521111432.207936-2-konstantin.ananyev@huawei.com> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v1 1/4] ring: introduce extra run-time checks Thread-Index: AdvKQZ5glFN0P7lRSfGbFsDxUUNRXQABIw9w References: <20250521111432.207936-1-konstantin.ananyev@huawei.com> <20250521111432.207936-2-konstantin.ananyev@huawei.com> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Konstantin Ananyev" , Cc: , , , X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com] > Sent: Wednesday, 21 May 2025 13.14 >=20 > 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. >=20 > Signed-off-by: Konstantin Ananyev > --- > 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(-) >=20 > 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 =3D (behavior =3D=3D RTE_RING_QUEUE_FIXED) ? > 0 : *entries; >=20 > + *new_head =3D *old_head + n; > if (n =3D=3D 0) > return 0; >=20 > - *new_head =3D *old_head + n; > if (is_st) { > d->head =3D *new_head; > success =3D 1; Is there a need to assign a value to *new_head if n=3D=3D0? 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 =3D=3D 0) - return 0; *new_head =3D *old_head + n; if (is_st) { d->head =3D *new_head; success =3D 1; } else /* on failure, *old_head is updated */ success =3D rte_atomic_compare_exchange_strong_explicit( &d->head, old_head, *new_head, rte_memory_order_relaxed, rte_memory_order_relaxed); } while (unlikely(success =3D=3D 0));