From mboxrd@z Thu Jan 1 00:00:00 1970 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 94922F94 for ; Thu, 7 Mar 2019 07:46:13 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A8F07EBD; Wed, 6 Mar 2019 22:46:12 -0800 (PST) Received: from net-arm-c2400.shanghai.arm.com (net-arm-c2400.shanghai.arm.com [10.169.40.66]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id EA8943F703; Wed, 6 Mar 2019 22:46:10 -0800 (PST) From: gavin hu To: gavin.hu@arm.com, dev@dpdk.org Cc: nd@arm.com, thomas@monjalon.net, jerinj@marvell.com, hemant.agrawal@nxp.com, nipun.gupta@nxp.com, Honnappa.Nagarahalli@arm.com, olivier.matz@6wind.com, i.maximets@samsung.com Date: Thu, 7 Mar 2019 14:45:37 +0800 Message-Id: <1551941137-33250-1-git-send-email-gavin.hu@arm.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1551841661-42892-1-git-send-email-gavin.hu@arm.com> References: <1551841661-42892-1-git-send-email-gavin.hu@arm.com> Subject: [dpdk-dev] [PATCH v2] ring: enforce reading the tails before ring operations X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Mar 2019 06:46:13 -0000 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. For MP (and MC) case, rte_atomic32_cmpset() already provides the required ordering. This patch is to prevent reading and writing the ring slots get reordered before reading {prod,cons}.tail 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