From mboxrd@z Thu Jan 1 00:00:00 1970 From: gavin hu Subject: [PATCH v1] ring: enforce reading the tails before ring operations Date: Wed, 6 Mar 2019 11:07:41 +0800 Message-ID: <1551841661-42892-1-git-send-email-gavin.hu@arm.com> Cc: nd@arm.com, thomas@monjalon.net, jerinj@marvell.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com, Honnappa.Nagarahalli@arm.com, gavin.hu@arm.com, olivier.matz@6wind.com To: dev@dpdk.org Return-path: Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by dpdk.org (Postfix) with ESMTP id 142A82BA3 for ; Wed, 6 Mar 2019 04:08:06 +0100 (CET) List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" In weak memory models, like arm64, reading the {prod,cons}.tail may get reordered after reading or writing the ring slots, which corrupts the ring and stale data is observed. This issue was reported by NXP on 8-A72 DPAA2 board. The problem is most likely caused by missing the acquire semantics when reading cons.tail (in SP enqueue) or prod.tail (in SC dequeue) which makes it possible to read a stale value from the ring slots. There must be a read fence before writing or reading the ring slots, rte_atomic32_cmpset() provides the same ordering for MP (and MC) case. This patch is to enforce this ordering for SP (and SC) case. Signed-off-by: gavin hu Reviewed-by: Ola Liljedahl Tested-by: Nipun Gupta --- lib/librte_ring/rte_ring_generic.h | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h index ea7dbe5..1bd3dfd 100644 --- a/lib/librte_ring/rte_ring_generic.h +++ b/lib/librte_ring/rte_ring_generic.h @@ -90,9 +90,11 @@ __rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp, return 0; *new_head = *old_head + n; - if (is_sp) - r->prod.head = *new_head, success = 1; - else + if (is_sp) { + r->prod.head = *new_head; + rte_smp_rmb(); + success = 1; + } else success = rte_atomic32_cmpset(&r->prod.head, *old_head, *new_head); } while (unlikely(success == 0)); @@ -158,9 +160,11 @@ __rte_ring_move_cons_head(struct rte_ring *r, unsigned int is_sc, return 0; *new_head = *old_head + n; - if (is_sc) - r->cons.head = *new_head, success = 1; - else + if (is_sc) { + r->cons.head = *new_head; + rte_smp_rmb(); + success = 1; + } else success = rte_atomic32_cmpset(&r->cons.head, *old_head, *new_head); } while (unlikely(success == 0)); -- 2.7.4