From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM05-DM3-obe.outbound.protection.outlook.com (mail-eopbgr730086.outbound.protection.outlook.com [40.107.73.86]) by dpdk.org (Postfix) with ESMTP id F3DD02965; Tue, 17 Jul 2018 05:34:31 +0200 (CEST) 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:X-MS-Exchange-SenderADCheck; bh=zYxtb+D8xHtpRae9NzhlHbu1sSJRUo4t0/RZbNmcXE0=; b=aeWv/ESxIYH6o1mzIUbTQEFXmWCVPV65YXPcgBdnxnnQNho1BNlA3QEd41cbHzTLbllA0iKSmcYe7/4qQ8CRQfKSTqZF0BaujXfo2twB2MmBCr2Cq1Rc53bypErR5qJRqqoK7TZ44usDwdJMB6GHzC9whHkjPqaQuSRmlsXihik= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Jerin.JacobKollanukkaran@cavium.com; Received: from jerin (122.167.121.153) by SN6PR07MB5005.namprd07.prod.outlook.com (2603:10b6:805:ac::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.952.18; Tue, 17 Jul 2018 03:34:26 +0000 Date: Tue, 17 Jul 2018 09:04:13 +0530 From: Jerin Jacob To: Takeshi Yoshimura Cc: dev@dpdk.org, stable@dpdk.org, olivier.matz@6wind.com, chaozhu@linux.vnet.ibm.com, konstantin.ananyev@intel.com Message-ID: <20180717033410.GA3344@jerin> References: <20180712024414.4756-1-t.yoshimura8869@gmail.com> <20180712170839.GA11626@jerin> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Originating-IP: [122.167.121.153] X-ClientProxiedBy: SG2PR06CA0204.apcprd06.prod.outlook.com (2603:1096:4:1::36) To SN6PR07MB5005.namprd07.prod.outlook.com (2603:10b6:805:ac::31) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 91683ef1-80be-475d-9912-08d5eb963378 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989117)(5600053)(711020)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(2017052603328)(7153060)(7193020); SRVR:SN6PR07MB5005; X-Microsoft-Exchange-Diagnostics: 1; SN6PR07MB5005; 3:wk23YAn8IIigXProItw4aHEti0WqRKRy4PzwTeHQDnSHeKZz5Xp8qgVNK9WVaSt0g2nblZVjcrlxn2o04TI5xstDd5P6p87sBjQgSrKDgvn6zvzXne+QxZubJvPgGiFaFOaaPF+gH/wEhZTprhxXF7Js47cKhkdAgvvooiOMnJYlY0rYCHHqKr/fom4jElxlM4Ad6jPabXlMT27arMm+tsAG+eNoP6dm60TgWmm+i7DQMUuvk5NCa6QV5uXCWQBu; 25:E0maFcm5U6y70Bgz1pV96yvbe/tBWFVkV5UUDhmopt2NBY2KzwyEy4qizOYE85i+zCEqo4CJ8FS1yFwhS8h4N9dm9Ty22sxMIP6yKys8Wo4MsV0nZ2W0H++VqG4UlNB7aVCr/SKOdox8sthdLoCWK7eXjDbFK7biLDMa81N9jlq64IqLs++i0RlyAlNXToI8Ol/a//3J6tOKuW776aYA0z6NjwzcoUcgRCwsJWIOlmuCCHhhbb8oafg03KCoA+BK2a5wM4Y1och7dSCTcRQzLDFSOdqir86Fp/qUg1F+Y05X22lPGs90UKz1XaXJP0QaE1ahw9/PkTrG5XN+O51uCg==; 31:V02yoD4snWVprY+1NBb0sSMpUWVEJAjq42o+6qGNCeXlkFgXlIOW/Snu8XBmKtl0Z8YZRoyMENUryvbVIhwaSJwcFLd/RdD/VQpP1pmIrvpnB/035rpKmGfHS0QOdtY6oq5n+wV8Waqumk9tI6uVyOV1Qm1mq98ZCvpV8+cOkDY5WfaMV1r/5QwQm2337a0ogeWerRZap7LF3ui9ltbTt3WKwmxCRVy4Qev+3wPcqMA= X-MS-TrafficTypeDiagnostic: SN6PR07MB5005: X-Microsoft-Exchange-Diagnostics: 1; SN6PR07MB5005; 20:gg0JIuF61YsqqR9PiGXb5O5bETPTeWMn55Zh5EmlEVktBUzLAWDc3J4QWPQ1eT7OOVMK6YfA5bA45DLrA6CSC5TLiWBwLDyQVm76FfD0YErbs5pcfwstNUf3RoI6Oy8f2HRolgvfjkP9ZIwKDOQtr3dQhLSfJfjRFJVrUU6rB/S28AZHgXDu85Vsut88EA1nL8oLopKyi5LFyOmm6F3UJatYuhZ5nW/wohmWHvlGYZGnupUt66Q5UCtPk+Y2JXWP6dT+O6Dzjz6wQnI3Be8JlPfZILlZeZbkEOTApcgWpp4Cq/7n9R48Cv5/rsz1JW6wErRZM6pru6bckO+y3OUCX83DtCuyTKdDwxmsprNuHa7eu6OLkoUdimx4mFWx7l03jbNYX76UyftBL1drW11odnfZA0yVYrizTgnbk9dI4xHb0gB/pl7zOUlMBb9CBQ9iAp8rS4TPAEjicIkNLZZbShXU2dCcEtiov96OokCerS6GLw9xsquVhcHR0oPnmBESIitQILSJ2f9ZfaN4bMca6Bd5zbqPQxjAF838YjwPV/fN1p7dhHhUQlPIItnodaBeZhL3AkEgI35z7GWpCyqY9kCmdauL73Yzd5I3lMAyY9c= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(166708455590820)(85827821059158)(104084551191319)(228905959029699); X-MS-Exchange-SenderADCheck: 1 X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3002001)(10201501046)(93006095)(3231311)(944501410)(52105095)(149027)(150027)(6041310)(20161123564045)(20161123560045)(20161123562045)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(6072148)(201708071742011)(7699016); SRVR:SN6PR07MB5005; BCL:0; PCL:0; RULEID:; SRVR:SN6PR07MB5005; X-Microsoft-Exchange-Diagnostics: 1; SN6PR07MB5005; 4:IcIjiGZ8LLFp5fVxQrQ9ZMoKiAPh5sQV0tA79WtAJwQVxsNZ3moWX3FOZV0XvbeYXlX6erI9ldjWX5sqVvV9L+BvIc5fhPYDz9cHUQVqCD53u5c6dntOFbf9v8d3LfPJi6KgYnQp/L17bulDwBGYKXn+llukIHIYdaYNmgfLb9LiRp/4ShXtRO0feFIXX20vR659NHJN3ntGiLuaDB9YkzFtjdx+Od93LEJt1qscNA4WkJZ/1YQeBH/y9xqrBajDvHXNdBUwxeOSk6NG1pJPzudMLxky1GKAVtrKcBKNMGiD60bG19jPKhZnD37GcC09olrmHs9Wb/zu9z+/2zFQACi5iju2YZs7lQKJNk736LdhdxHvlGkMfIIVIhkxzTh+474pR2Wk7qRY6T41c7oowBdd5BSNiher2mgxS+sc7B8= X-Forefront-PRVS: 073631BD3D X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(39860400002)(396003)(136003)(376002)(366004)(346002)(199004)(189003)(13464003)(478600001)(42882007)(4326008)(58126008)(50466002)(55016002)(39060400002)(2906002)(47776003)(5660300001)(68736007)(25786009)(66066001)(305945005)(6246003)(186003)(16526019)(6496006)(16586007)(53936002)(7736002)(26005)(229853002)(14444005)(33656002)(97736004)(72206003)(6666003)(966005)(8676002)(1076002)(8936002)(33896004)(81156014)(76176011)(446003)(486006)(33716001)(81166006)(6116002)(386003)(44832011)(106356001)(956004)(6916009)(3846002)(316002)(11346002)(105586002)(23726003)(9686003)(476003)(52116002)(6306002)(18370500001); DIR:OUT; SFP:1101; SCL:1; SRVR:SN6PR07MB5005; H:jerin; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; Received-SPF: None (protection.outlook.com: cavium.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; SN6PR07MB5005; 23:PlWzY/YH8orbSYSg7J7IJuWqxf4aq1UEerO5KUbIG?= =?us-ascii?Q?x+Cj6LPvnmN8NlpLZV5oMzmcaf0sIks8Y8JhTAsJmPFsco/w09pn678MheBY?= =?us-ascii?Q?FJtFIt4X1ANXxsL87vcAKvEp25oDtE4b0jAcW+GvuaG2z1bxZXKJJi8QEBK8?= =?us-ascii?Q?8r41wMWz4Hgge9yKoMRLt2iMOTP6D+oirmRQJCfw/tUJxwMLuVAWItNjk32R?= =?us-ascii?Q?8H7aBWdNJGD9F6xeEO3iPGGLfUBTE0nH36jgluTZ6hoCl6RQCMYsvbkyCNua?= =?us-ascii?Q?Mw+cboXDrBC75cMb/q6gjB8UpzuOS3QmJiwUUxdeVO4ovoURDqUDBuOp6xSE?= =?us-ascii?Q?KKgaJeq2LKgIRL0yB6uInVYPG1CcM/sXD4vTPFbphpm8rFGGyI7t+22sfUyb?= =?us-ascii?Q?ik+SNkGiFs46sccy4ovkBU0bTGpxZHGxLR/jbAZ2XruTccLjbAAy8jCpyv4u?= =?us-ascii?Q?18XNLsaG6/7nEoUcWzq4rRgBG+zwxGIOJZyOP1ANyvJ/nzX23GfFjje0OQZd?= =?us-ascii?Q?IS09O+9kH/bGc9Ra1ME6Jg/oUG5AMba2el1dbjMhTdVmaZl/BWX64vLG51B7?= =?us-ascii?Q?AsGDTMb1P2LW9dD7rdDmXWqPP4Hvk3wZlRiEUDDvsbu9XIVRAG73yzZDup50?= =?us-ascii?Q?r04HgrRzuTAK5rdNK8QffaVIZhoHfNF9YVMamnFuYaJpE527b1Tf+WW5oMwT?= =?us-ascii?Q?W2ft8CPx+ql/+SmtXnvSJtqhIiPjneiV1GdUe52zkuXzIWAgRbjiXa1epxCX?= =?us-ascii?Q?OPC3sLddhH45Bml6f2f+gOkS6xDPugoi/s7PKJRBv3bonOZ3C9EnD6DuA+sC?= =?us-ascii?Q?wOx4hgYUcyPbVH1eEnuBLFy5iVeBHwJS/hTRHIQHEfLR5GEXQCT1UyJ10kAg?= =?us-ascii?Q?rUwuwhkC3SJUHVXSaraMACdta2iduqvJ6tsg1PXShClPLckj48XHYMK44nTx?= =?us-ascii?Q?Szi12EXVAeyvHJZIt5x9CKK6MlwP39mF+F1ptXF9ocrKzZIJeD8u4ql136f1?= =?us-ascii?Q?9jGIby8syjbo3sNaVI2OGN9JIk/8h8pxxDAjjo5YEjerm50BjmowPsC/AWkF?= =?us-ascii?Q?Xayb0cesJiOeJiSvdHeauZVBINMDkS5psPjUMJj3lXnLiKH2nWNnctzsIYMP?= =?us-ascii?Q?bwCuAkbE+qyv0/BhWnaTONaT2djDdMGd/0uDkearxy+bG1mjPCRNpAjXEa7T?= =?us-ascii?Q?+xcn6rnpKpLFL1IziyFoh4ibiiR+UkhWhUbj1Gmp70n2ldDKaLx+ZtP4HlGB?= =?us-ascii?Q?+tMzSc0SM5bTZFsc7ZPrTT4OGGkySGxdB0tPMP4U3OcW2By1eHi1NlmWvjWd?= =?us-ascii?Q?/VtHSRhyrfaCFDNrB4LKZ0xRrIpOLk8nP5GBYpYdpq7UImfT10wk2HsEpqnv?= =?us-ascii?Q?Ci+0bajUseyrbxplSIkOjQVJ4ZVPQEVFLGaY/9P7PECeFO2?= X-Microsoft-Antispam-Message-Info: wRu1TqqghgHcTADPvBbEie9r+/FpFR+ebMvIfzWVbYC0v4O59PVBqrcQFL4wNKwr5tBuqdiUSH+SeU1vLRrxx6x1NQuraKPp1egsemW5HGNKZtAiyoMv4nynmD+ULrMnphDSTLG2uzkJTH1e2zY3AGHSit2PVisMTbjDErKx426NKnuWOJhANaQDxufAhg6FYOQ5k5AfKNjgp4HjzMK9eiWWn1MHU53Br6Qb/cKWFrfZRb5uiKE+hXf5tdUcAJ3rlJnpOYYI022MAbFoSnBK5prYEO2ZYH/MCuxhd3asNx5LO9zHZJpW94Ci8gOSAZ8MyiIfkv1iTC/jH3kWt8CZlRnWRKfjwgt4o1RvfulbgEk= X-Microsoft-Exchange-Diagnostics: 1; SN6PR07MB5005; 6:ibNfCpMBcy9cbI7Sj5gkyAnjPGV4QaifOK7wpvYUtbWI8uoSiDc9+STBH2JXIdAGXbVC4Sam4WMUeaLUSpihpctHOqTUN+Q7iCf66Do4RnXqbGe8fllGYzcAm86O4qW+Jk/M44rR34ekOONJS1LLB1oUL8R79LlX8TSpX06UFgKuC48MGFeOyaGaGuoa4ZyeDEjRfJbAitV5+pF0OCtPzFG/14SEaNacC2xgTtMXKJ1eZLvwbYMuoGbYMe+phUFv8L21OmRWQpSReNp263FoXXsDypYcGa84Y26vm4bylNlBZIF85pO3+4+PEgE0/xe13jQEdJ8pCUZdO+vWKDVgsnPUeG3+yV7nRdlb6tD5sWeSt9LX1ALTNOIXoL01PI1c9+yUAFjoyagxaMDfske2CqiZ61rfRno+5klITY0ZXteXqRtDk7oZN9NURdMOcv0Adw5wNx3F7713WQoTj038Jw==; 5:ZhmqOIBC/+G7/tSB3yPQ7YJgkIs83vwThv9O3MXUcGvS/TBFOYQR3QjLZ/NK9FBRHQOHv0inw018HiT0JXDXxgXbgm4fT2OrtwEDvtZ/DmcMtvTNwNeMEhFll4qePRsH0/v7n/05XWvhZtLHTT2HxVRe92okkVtByZELgBM6sww=; 24:rPmkQGcoA0UFznmCEsQlxNTNo/+Nxr/8QlZjtss91lD9n8VSR6MLqD9wbnJOZbLXrat9UnmbnuoaaWe6GeepaCwQXUiTjVHoIXqfwmdicPQ= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; SN6PR07MB5005; 7:ZqLvSC4J7XqswENuQS+3+7/2LMSjvatCa/qin5xLOPx0mWHs3I2rnNsRD9qzYl+YUyaOWAGSWYxj+NgtVBYBk6gxiY4KTQeUwDL7RqEPccRKHi1Tk1uThOR0qZX8klRpPYGw5lEEdlWn5RT7bLCZuTT48t9eIdIH2I6uWEvcsWkaZnIVqmP8x433TLJ7kb1rl3He40W+pBF4va9fnl22IEtVv1S7GZXA8emfyrJX2q3kCUpKqq5d8MylvZLRCYda X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jul 2018 03:34:26.2131 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 91683ef1-80be-475d-9912-08d5eb963378 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 711e4ccf-2e9b-4bcf-a551-4094005b6194 X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR07MB5005 Subject: Re: [dpdk-dev] [PATCH] rte_ring: fix racy dequeue/enqueue in ppc64 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: Tue, 17 Jul 2018 03:34:32 -0000 -----Original Message----- > Date: Tue, 17 Jul 2018 11:54:18 +0900 > From: Takeshi Yoshimura > To: Jerin Jacob > Cc: dev@dpdk.org, stable@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] rte_ring: fix racy dequeue/enqueue in ppc64 > Cc: olivier.matz@6wind.com Cc: chaozhu@linux.vnet.ibm.com Cc: konstantin.ananyev@intel.com > > > Adding rte_smp_rmb() cause performance regression on non x86 platforms. > > Having said that, load-load barrier can be expressed very well with C11 memory > > model. I guess ppc64 supports C11 memory model. If so, > > Could you try CONFIG_RTE_RING_USE_C11_MEM_MODEL=y for ppc64 and check > > original issue? > > Yes, the performance regression happens on non-x86 with single > producer/consumer. > The average latency of an enqueue was increased from 21 nsec to 24 nsec in my > simple experiment. But, I think it is worth it. That varies to machine to machine. What is the burst size etc. > > > I also tested C11 rte_ring, however, it caused the same race condition in ppc64. > I tried to fix the C11 problem as well, but I also found the C11 > rte_ring had other potential > incorrect choices of memory orders, which caused another race > condition in ppc64. Does it happens on all ppc64 machines? Or on a specific machine? Is following tests are passing on your system without the patch? test/test/test_ring_perf.c test/test/test_ring.c > > For example, > __ATOMIC_ACQUIRE is passed to __atomic_compare_exchange_n(), but > I am not sure why the load-acquire is used for the compare exchange. It correct as per C11 acquire and release semantics. > Also in update_tail, the pause can be called before the data copy because > of ht->tail load without atomic_load_n. > > The memory order is simply difficult, so it might take a bit longer > time to check > if the code is correct. I think I can fix the C11 rte_ring as another patch. > > >> > >> SPDK blobfs encountered a crash around rte_ring dequeues in ppc64. > >> It uses a single consumer and multiple producers for a rte_ring. > >> The problem was a load-load reorder in rte_ring_sc_dequeue_bulk(). > > > > Adding rte_smp_rmb() cause performance regression on non x86 platforms. > > Having said that, load-load barrier can be expressed very well with C11 memory > > model. I guess ppc64 supports C11 memory model. If so, > > Could you try CONFIG_RTE_RING_USE_C11_MEM_MODEL=y for ppc64 and check > > original issue? > > > >> > >> The reordered loads happened on r->prod.tail in There is rte_smp_rmb() just before reading r->prod.tail in ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ _rte_ring_move_cons_head(). Would that not suffice the requirement? Can you check adding compiler barrier and see is compiler is reordering the stuff? DPDK's ring implementation is based freebsd's ring implementation, I don't see need for such barrier https://github.com/freebsd/freebsd/blob/master/sys/sys/buf_ring.h If it is something specific to ppc64 or a specific ppc64 machine, we could add a compile option as it is arch specific to avoid performance impact on other architectures. > >> __rte_ring_move_cons_head() (rte_ring_generic.h) and ring[idx] in > >> DEQUEUE_PTRS() (rte_ring.h). They have a load-load control > >> dependency, but the code does not satisfy it. Note that they are > >> not reordered if __rte_ring_move_cons_head() with is_sc != 1 because > >> cmpset invokes a read barrier. > >> > >> The paired stores on these loads are in ENQUEUE_PTRS() and > >> update_tail(). Simplified code around the reorder is the following. > >> > >> Consumer Producer > >> load idx[ring] > >> store idx[ring] > >> store r->prod.tail > >> load r->prod.tail > >> > >> In this case, the consumer loads old idx[ring] and confirms the load > >> is valid with the new r->prod.tail. > >> > >> I added a read barrier in the case where __IS_SC is passed to > >> __rte_ring_move_cons_head(). I also fixed __rte_ring_move_prod_head() > >> to avoid similar problems with a single producer. > >> > >> Cc: stable@dpdk.org > >> > >> Signed-off-by: Takeshi Yoshimura > >> --- > >> lib/librte_ring/rte_ring_generic.h | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/lib/librte_ring/rte_ring_generic.h b/lib/librte_ring/rte_ring_generic.h > >> index ea7dbe5b9..477326180 100644 > >> --- a/lib/librte_ring/rte_ring_generic.h > >> +++ b/lib/librte_ring/rte_ring_generic.h > >> @@ -90,9 +90,10 @@ __rte_ring_move_prod_head(struct rte_ring *r, unsigned int is_sp, > >> return 0; > >> > >> *new_head = *old_head + n; > >> - if (is_sp) > >> + if (is_sp) { > >> + rte_smp_rmb(); > >> r->prod.head = *new_head, success = 1; > >> - else > >> + } else > >> success = rte_atomic32_cmpset(&r->prod.head, > >> *old_head, *new_head); > >> } while (unlikely(success == 0)); > >> @@ -158,9 +159,10 @@ __rte_ring_move_cons_head(struct rte_ring *r, unsigned int is_sc, > >> return 0; > >> > >> *new_head = *old_head + n; > >> - if (is_sc) > >> + if (is_sc) { > >> + rte_smp_rmb(); > >> r->cons.head = *new_head, success = 1; > >> - else > >> + } else > >> success = rte_atomic32_cmpset(&r->cons.head, *old_head, > >> *new_head); > >> } while (unlikely(success == 0)); > >> -- > >> 2.17.1 > >>