From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM02-SN1-obe.outbound.protection.outlook.com (mail-sn1nam02on0080.outbound.protection.outlook.com [104.47.36.80]) by dpdk.org (Postfix) with ESMTP id 429E01B5EA for ; Thu, 2 Nov 2017 18:23:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=CAVIUMNETWORKS.onmicrosoft.com; s=selector1-cavium-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=Qm7NVQ+HMO+sgcPmvVzJYPT8mP8A1VKN8HrpAuhUvT0=; b=PC2j6+W89e7vCb/ZUcia2vUXBePGktInMc6EIlkS0wgS09cqNaQtchvR0FmyxcM/P5CaZCrx5WjYPy9fZ4ZEPjaszKJUJLI5iUHWDMErzz0iS9zyOJhJgNazQs/Bo83hgQ71ESJUhCR+28BOBg9yhTTqMEcjGOtSyA+SR8XysvQ= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Jerin.JacobKollanukkaran@cavium.com; Received: from jerin (106.201.55.141) by CY1PR07MB2521.namprd07.prod.outlook.com (10.167.16.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.197.13; Thu, 2 Nov 2017 17:23:52 +0000 Date: Thu, 2 Nov 2017 22:53:38 +0530 From: Jerin Jacob To: Jia He Cc: dev@dpdk.org, olivier.matz@6wind.com, konstantin.ananyev@intel.com, bruce.richardson@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com, jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, jia.he@hxt-semitech.com Message-ID: <20171102172337.GB1478@jerin> References: <1509612210-5499-1-git-send-email-hejianet@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1509612210-5499-1-git-send-email-hejianet@gmail.com> User-Agent: Mutt/1.9.1 (2017-09-22) X-Originating-IP: [106.201.55.141] X-ClientProxiedBy: MA1PR01CA0106.INDPRD01.PROD.OUTLOOK.COM (10.174.56.150) To CY1PR07MB2521.namprd07.prod.outlook.com (10.167.16.12) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 959ab790-6274-41da-dd81-08d522167f59 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(2017052603199); SRVR:CY1PR07MB2521; X-Microsoft-Exchange-Diagnostics: 1; CY1PR07MB2521; 3:11W0a62qIWzvg7PSOzUQheRoG8WeGmNxijpBVpjKj1/lty410ZB3X78qTDE6QhWL0uRHyP7xWTXGoQvhtmBn+b+y9DJL6zqHxZ70mmaxMu5SXHxF45gABFdBZIOia+lKZUPgXCuel5BUHyuDwx4QCxJuvI/B+p76t/Y61bkAtO3RspgZTjD0vDq1wJCjfwwO1QI5ibkvF+eWn4WpdXbdtLXT6DjnpqlEuV71M4hMZ9IXz9RKuR0laN/yZmbnZH6n; 25:1Vs4ZoQn06Q9pnobqDuikgANKQNuDycxQiMyef+8nWEO4wVmeJ5r9W5BOdkxCn3dIlqE8AZNJlOVjMENpqAJghVq3v3iSjPQIJDPUsnVUfaycZn8NW6z3TS7wE+XiF8ltzftZasGDKRePD6T6G+KhGMOjuG5weibpvbNZQM3SmkSx8Fpw2BNVx06DCRVnXlkHNsEMsqtWYHZa3jAFCGZZIJze4a0ig13h+1HIvdo98N5yC9JAM5+GBEIj82G+UNOWCzRJe2yVKRnbAJxGkiTAT4BfATywqFvtCgTmd1YJ+yG4lXYKNBP8dv1Fprpn0Ke9rb7YCyEECsdYsvwNGRzJA==; 31:Hwtg9hC39OtCmJzWr3UklK34jz1gCWINSnQ2ru7Gam2+nx933quGJl+VxWEaRC2M/nTlWBkka+klAnwDevB79zMl0PmuvABUHsQZ8KafiCgBPe2vEbaS1hcf8BiaqWZ4vzmJoxnbI5FBotAp7NpAlY8Ahd2U/alBmTma9fsVjoS+dsx5AqEGCZs2AHssvGHY6ECDh8GGBYWQj2PEBP4HplEMMWh+FB66InrSbRmgxSE= X-MS-TrafficTypeDiagnostic: CY1PR07MB2521: X-Microsoft-Exchange-Diagnostics: 1; CY1PR07MB2521; 20:fzlPzzanOIzOh5OoeY1L2ef0Pw2j+C2aWC/I2fxP+bcfxNTtMNYrf+zM0kT2kYrkdGXFABuW1bte9ROfNwMNI1G2iMeE/AsZrEFOT7H3QyROKvaMabc22MVonw9fWyhB9ULOvbndrU/GRAyMteMBlRVx5vzyDcaBSKDr4ymaALq6p4Gfe7Hz72BbFjfakYrwKJ4T5lQ3zjG0L0nBZdIKYthl1vKdCrLFhT/ivEZ7N+6nU8Q6/C46Q1vQGdpINU+qXZzmpDo4nIyXp+BtHqcSj/EtXzLfKpaTaUN9fUhrZuRwW/KcWWWLDxbgyz37ey90rwMIXqNy6/xd/fQFnwbmLpwighm9GohLGAxeoHyBhuS/U4F4WOlnsFQ7ggNeUx0MeBOcojlOmWhVIEWd4+ZAYGYXD/dPaiHCoJV+Q33My0z0L4EGmQyI1/vxmjWQMqN7zz/b+aiGu47DOb8fDaDKPiH6JJ00AjPQByz77eo6gZNTw1KvwgF+vwaGYXdB5B9nhSeaZ503jC+w+mhfDIKn4iVoTwl4zUgpKB+bX8IWgHH7YG7uv0Axl1HWK7lf1HstBIlLXy4UmxiDQEy587LRAJSfBYMoK7Yn0gZolKwJDE0= X-Exchange-Antispam-Report-Test: UriScan:(180628864354917)(185117386973197)(228905959029699); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(93006095)(10201501046)(3231020)(100000703101)(100105400095)(3002001)(6041248)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123555025)(20161123558100)(20161123564025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:CY1PR07MB2521; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:CY1PR07MB2521; X-Microsoft-Exchange-Diagnostics: 1; CY1PR07MB2521; 4:4n1nnpXOr1LBkYBFO7q2s8Eir/k+KBMY42BJP32kmjgEvxNHeuWxcqiZLPF8OpWplcgrAjwEJwYl3HWYmCd9EO/qcWc71f+o9nZdUX7EFxTghxVKshVaX0Muo8Pvwtdz/RatyvLjSlWeU+HheVC4NTmudtNPY4fL87BCnLZj0GwFp1Fs3OSdYJpahNYIKa5PPE/54QXAM5xEIB/WL2n+nsXybtApqu/8mTl3eO99M6cxPcltOJl9tBk2UckDP5zkzDt3i2ibohb65JunkyD6GWcWKOqBQvqate5kr2FVWQ4jSVXZJAH3ct0MZWWJgtnUam+HJ9i65NqpIKkj8/oR40ciJlX//V74Qyy1ZOrb4qBaq3toSheHuDAjPMXzDYXQ X-Forefront-PRVS: 047999FF16 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6009001)(346002)(376002)(189002)(13464003)(54534003)(199003)(72206003)(23726003)(68736007)(1076002)(5009440100003)(6116002)(54356999)(16526018)(189998001)(81166006)(81156014)(2906002)(8676002)(316002)(16586007)(97736004)(76176999)(58126008)(8656006)(8936002)(2950100002)(6916009)(42882006)(6666003)(4326008)(3846002)(50986999)(7416002)(33656002)(105586002)(106356001)(50466002)(101416001)(33716001)(55016002)(66066001)(478600001)(25786009)(1411001)(47776003)(39060400002)(6246003)(6496005)(5660300001)(53936002)(229853002)(305945005)(7736002)(83506002)(9686003)(18370500001); DIR:OUT; SFP:1101; SCL:1; SRVR:CY1PR07MB2521; H:jerin; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; CY1PR07MB2521; 23:eWRpBCb6jNysCIyh9fhM2AzzIs63gmgPryduGVGTe?= =?us-ascii?Q?PodY6PznRMhg+dv8F+6HRQxLmIdPvNhIcm3aCWLgXZr08wXdNaU9/qrU1RFX?= =?us-ascii?Q?DY/6RLfaSYds26K2mSnF3UPq1tCrb9bz4G+x5e9i2rXuIzGxh2MtMSQKM0k+?= =?us-ascii?Q?gZCOa4BfpfanpE5zYv0Xge9duggsmjo3qwNGWfooVGCyGOZi3T/ufGrCrzIy?= =?us-ascii?Q?9ycvLcBywu6/BYdjX3qb1j4rNVV6jtkAU3LuggDu4v2q8PJw0pUorJKHghqF?= =?us-ascii?Q?Zb4mpDW05pwbxndqAPxuKXLtF6bAVcoUQ+EGPl3DfZNA7Ajp/X9eLLRLjAl2?= =?us-ascii?Q?UFogSlUC9w1t/ATdf6lCYDtmW4t+61inT5d/h+vsa9UqOYyBcwR274MzRJ07?= =?us-ascii?Q?dzUfw5cM66o+mURbd+EEniwlnU6C4OsbaHJoam0gVsDgoI9vFMK4EUJD8rmm?= =?us-ascii?Q?q2OBcUB2OMMsZgw3YNWk1DNesLpUBgcw8Pb00yj8+buxZ0xQBg0ceODQbyDl?= =?us-ascii?Q?02kcXWFabAFrhaNFevZGEBcWGcbzutNBCSn0onOmKXhYTskIDp7Z5bywkG+Q?= =?us-ascii?Q?t2aMzg/LaeApEVBcfwyg7prVADQh2LtKhiz9nCNiEObiKtIWF8uk3+hgbRiH?= =?us-ascii?Q?rX6cfvj/OOIF+q2M1I1cCfprv7PNPOareZAch4hVkihPsyFuEJQl1oKez2m7?= =?us-ascii?Q?Fss/ZMYEpKHrJqZiSMoNEXQ/nezC1h/pKja9Qr5tr3M5LRZFmYqF7rK7zegW?= =?us-ascii?Q?nuptrghsk7zGqWFhzwk0pig9oXO2kRhfsd3mebB6oAv0Z193mwtiAeNDnOv9?= =?us-ascii?Q?C4ajGsLnhnfmn1NxaRnz1cY3UiBDn+qDsF8xvZCazT+02msgzB2CfmrgT/SQ?= =?us-ascii?Q?8jnO8d3yG1TWD69Z6fpX7BezcIZN8N6aFHAZEnSKfKHQEsMjbSPyFF6+u0dv?= =?us-ascii?Q?jUfLNFXYzgejLDrbUa77oCLSZJ1zubOmOoboEF0u78Vwydn8Uzxt3AskxVp/?= =?us-ascii?Q?14W9OyDzbP/0cOICpAfwTmcNyHhpb8AADWZrWHpycoCeVA389gNxssgx0f/5?= =?us-ascii?Q?ow13mZuBRr4/Fhe17tnCH13c73dhF/0snGtcB946QdJl3ninFRfwWV2MPLqd?= =?us-ascii?Q?o+9EyoSYid3ue3EplV19/8Zzi4wVEIRg2dnqNvS9s8gwrlaNBAaBumsjjEOJ?= =?us-ascii?Q?wbnHpvSQDP++buhyjXQLUS9YIXf0+AuCuG67nVW3Q//QnHfR8YUGE4XInD45?= =?us-ascii?Q?m3tkvCnvqxIStRuB6Cm8JCgofg7kT48w05eGYpE1K5EgNPfjJsATFAK1NHWm?= =?us-ascii?Q?5j0Qv2ENfDLcMJ/85RcvEv0eqNWo/tCN0VaB2jRcAXL?= X-Microsoft-Exchange-Diagnostics: 1; CY1PR07MB2521; 6:pCpaVxA9zkE/MhEAq0UQ66bg/4GlAYQHkq/Fvfx2uz2MopheGEoGjz+GJ6nAIMYU74DIeuuZg+0kYUZMvy9+xdjOC5QfpxHpc8NMysfhbnCdEWXnRkIyMtpCanmX963m04cjbkPTcbzwIs5t5LkNpByj/wm6xEsg4n49T/95qfE4xWHZvk1R4/NQXL5hZE7sB42GvBL5lgRcc+FkA0wEyWJ3C2edd0gJlEypQyoOdTnZa23xX895B1FLgYB4/a8HzIWzpj7s1HH5pj4twoBGWPwd0unRjdRRUjZY15vjKYLA2ZyGom3l6ICPbiuGX7jszdP5IckkMEOBiMtYrM1IaoSc6JZOhp71U1xmYUw23wo=; 5:XBJ7JRujDwNZclmfht/tLxJaFlYCuYKHPg4it3keadhqhHkPhDUgBZ3oBGayK0tCdA/6AjKmT8frs0dPE/IuMicGHEITEvPtxVEj17bFrSh90NqKVbeVHfOASoJpBxhP8Uojr/K8sny5vIKRVQJb67/j8tphPDnROxIqbPxH3hI=; 24:v1dNiwfdMjQBQkxQGKXPuneQ08LC+pvjl7mZTqC2Dq/mPDA0aZ2gHfD5+WXIsvAm0m7aPFP+BnluPKgDSKPzyd+0UGQTN0aaxYrX2+HX9CA=; 7:U2EEkhP3DSWJIGZuUQwV5eTOCLPqok27WBncefzhco0NN9vYUnQjsEh8/FTwZ+dLfMWVdQvkbDDtrIteqSE4HIwruoEINF1bkJImeTUIC+T52g1dDm70B/HX6W++1t/isQTelmHxO1uUraSiFBexMpvBV36ZM1qdJ8V/O35kBeowcpD52ZdzNa95LH7xI3RFK90wd9Cu6Cf36Vr9qOD2R29a8ZIFBtFUKEIoanAWXWpTI447tfCqzHXtkC8AipEX SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2017 17:23:52.9638 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 959ab790-6274-41da-dd81-08d522167f59 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR07MB2521 Subject: Re: [dpdk-dev] [PATCH v2] ring: guarantee ordering of cons/prod loading when doing 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, 02 Nov 2017 17:23:58 -0000 -----Original Message----- > Date: Thu, 2 Nov 2017 08:43:30 +0000 > From: Jia He > To: jerin.jacob@caviumnetworks.com, dev@dpdk.org, olivier.matz@6wind.com > Cc: konstantin.ananyev@intel.com, bruce.richardson@intel.com, > jianbo.liu@arm.com, hemant.agrawal@nxp.com, Jia He , > jie2.liu@hxt-semitech.com, bing.zhao@hxt-semitech.com, > jia.he@hxt-semitech.com > Subject: [PATCH v2] ring: guarantee ordering of cons/prod loading when doing > X-Mailer: git-send-email 2.7.4 > > We watched a rte panic of mbuf_autotest in our qualcomm arm64 server. > As for the possible race condition, please refer to [1]. Hi Jia, In addition to Konstantin comments. Please find below some review comments. > > Furthermore, there are 2 options as suggested by Jerin: > 1. use rte_smp_rmb > 2. use load_acquire/store_release(refer to [2]). > CONFIG_RTE_ATOMIC_ACQUIRE_RELEASE_BARRIER_PREFER is provided, and by I think, The better name would be CONFIG_RTE_RING_USE_C11_MEM_MODEL or something like that. > default it is n; > > --- > Changelog: > V2: let users choose whether using load_acquire/store_release > V1: rte_smp_rmb() between 2 loads > > Signed-off-by: Jia He > Signed-off-by: jie2.liu@hxt-semitech.com > Signed-off-by: bing.zhao@hxt-semitech.com > Signed-off-by: jia.he@hxt-semitech.com > Suggested-by: jerin.jacob@caviumnetworks.com > --- > lib/librte_ring/Makefile | 4 +++- > lib/librte_ring/rte_ring.h | 38 ++++++++++++++++++++++++------ > lib/librte_ring/rte_ring_arm64.h | 48 ++++++++++++++++++++++++++++++++++++++ > lib/librte_ring/rte_ring_generic.h | 45 +++++++++++++++++++++++++++++++++++ > 4 files changed, 127 insertions(+), 8 deletions(-) > create mode 100644 lib/librte_ring/rte_ring_arm64.h > create mode 100644 lib/librte_ring/rte_ring_generic.h > > diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile > index e34d9d9..fa57a86 100644 > --- a/lib/librte_ring/Makefile > +++ b/lib/librte_ring/Makefile > @@ -45,6 +45,8 @@ LIBABIVER := 1 > SRCS-$(CONFIG_RTE_LIBRTE_RING) := rte_ring.c > > # install includes > -SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h > +SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include := rte_ring.h \ > + rte_ring_arm64.h \ It is really not specific to arm64. We could rename it to rte_ring_c11_mem.h or something like that to reflect the implementation based on c11 memory model. > + rte_ring_generic.h > > include $(RTE_SDK)/mk/rte.lib.mk > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > index 5e9b3b7..943b1f9 100644 > --- a/lib/librte_ring/rte_ring.h > +++ b/lib/librte_ring/rte_ring.h > @@ -103,6 +103,18 @@ extern "C" { > #include > #include > > +/* In those strong memory models (e.g. x86), there is no need to add rmb() > + * between load and load. > + * In those weak models(powerpc/arm), there are 2 choices for the users > + * 1.use rmb() memory barrier > + * 2.use one-direcion load_acquire/store_release barrier > + * It depends on performance test results. */ > +#ifdef RTE_ARCH_ARM64 s/RTE_ARCH_ARM64/RTE_RING_USE_C11_MEM_MODEL and update the generic arm64 config. By that way it can used by another architecture like ppc if they choose to do so. > +#include "rte_ring_arm64.h" > +#else > +#include "rte_ring_generic.h" > +#endif > + > #define RTE_TAILQ_RING_NAME "RTE_RING" > > enum rte_ring_queue_behavior { > @@ -368,7 +380,7 @@ update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val, > while (unlikely(ht->tail != old_val)) > rte_pause(); > > - ht->tail = new_val; > + arch_rte_atomic_store(&ht->tail, new_val, __ATOMIC_RELEASE); > } > > /** > @@ -408,7 +420,8 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp, > /* Reset n to the initial burst count */ > n = max; > > - *old_head = r->prod.head; > + *old_head = arch_rte_atomic_load(&r->prod.head, > + __ATOMIC_ACQUIRE); Same as Konstantin comments, i.e move to this function to c11 memory model header file > const uint32_t cons_tail = r->cons.tail; > /* > * The subtraction is done between two unsigned 32bits value > @@ -430,8 +443,10 @@ __rte_ring_move_prod_head(struct rte_ring *r, int is_sp, > if (is_sp) > r->prod.head = *new_head, success = 1; > else > - success = rte_atomic32_cmpset(&r->prod.head, > - *old_head, *new_head); > + success = arch_rte_atomic32_cmpset(&r->prod.head, > + old_head, *new_head, > + 0, __ATOMIC_ACQUIRE, > + __ATOMIC_RELAXED); > } while (unlikely(success == 0)); > return n; > } > @@ -470,7 +485,10 @@ __rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table, > goto end; > > ENQUEUE_PTRS(r, &r[1], prod_head, obj_table, n, void *); > + > +#ifndef RTE_ATOMIC_ACQUIRE_RELEASE_BARRIER_PREFER This #define will be removed if the function moves. > rte_smp_wmb(); > +#endif > > update_tail(&r->prod, prod_head, prod_next, is_sp); > end: > @@ -516,7 +534,8 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc, > /* Restore n as it may change every loop */ > n = max; > > - *old_head = r->cons.head; > + *old_head = arch_rte_atomic_load(&r->cons.head, > + __ATOMIC_ACQUIRE); > const uint32_t prod_tail = r->prod.tail; > /* The subtraction is done between two unsigned 32bits value > * (the result is always modulo 32 bits even if we have > @@ -535,8 +554,10 @@ __rte_ring_move_cons_head(struct rte_ring *r, int is_sc, > if (is_sc) > r->cons.head = *new_head, success = 1; > else > - success = rte_atomic32_cmpset(&r->cons.head, *old_head, > - *new_head); > + success = arch_rte_atomic32_cmpset(&r->cons.head, > + old_head, *new_head, > + 0, __ATOMIC_ACQUIRE, > + __ATOMIC_RELAXED); > } while (unlikely(success == 0)); > return n; > } > @@ -575,7 +596,10 @@ __rte_ring_do_dequeue(struct rte_ring *r, void **obj_table, > goto end; > > DEQUEUE_PTRS(r, &r[1], cons_head, obj_table, n, void *); > + > +#ifndef RTE_ATOMIC_ACQUIRE_RELEASE_BARRIER_PREFER This #define will be removed if the function moves. > rte_smp_rmb(); > +#endif