Message ID | 20160715043951.32040-1-juhamatti.kuusisaari@coriant.com (mailing list archive) |
---|---|
State | Accepted, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 9BDC83978; Fri, 15 Jul 2016 06:40:32 +0200 (CEST) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20087.outbound.protection.outlook.com [40.107.2.87]) by dpdk.org (Postfix) with ESMTP id 1D51B3977 for <dev@dpdk.org>; Fri, 15 Jul 2016 06:40:31 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=coriant.onmicrosoft.com; s=selector1-coriant-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=ct3100KiIevmggZbDmnb8lxezDjH+HCd440loRYS0e0=; b=oMevl2TldgtsJQ0rj7NN/odFg93jxfy1JtDrk3R9cajgJ6CP656JmHt2fvXb2lucmc9uPQQX/3mwuLsGHS2+5rQw/r5yxnbE6MSbfQ6NFmMQe8K4ZOwRMsqqh+D8zIBfTpcoAwbRLjAiZvfW8cDRE1sGDZx/8CZKV44sloM001o= Received: from HE1PR0401CA0037.eurprd04.prod.outlook.com (2a01:111:e400:c512::47) by AM3PR04MB1268.eurprd04.prod.outlook.com (2a01:111:e400:586c::22) with Microsoft SMTP Server (TLS) id 15.1.539.14; Fri, 15 Jul 2016 04:40:29 +0000 Received: from VE1EUR02FT020.eop-EUR02.prod.protection.outlook.com (2a01:111:f400:7e06::200) by HE1PR0401CA0037.outlook.office365.com (2a01:111:e400:c512::47) with Microsoft SMTP Server (TLS) id 15.1.528.16 via Frontend Transport; Fri, 15 Jul 2016 04:40:29 +0000 Authentication-Results: spf=pass (sender IP is 204.154.129.150) smtp.mailfrom=coriant.com; dpdk.org; dkim=none (message not signed) header.d=none;dpdk.org; dmarc=bestguesspass action=none header.from=coriant.com; Received-SPF: Pass (protection.outlook.com: domain of coriant.com designates 204.154.129.150 as permitted sender) receiver=protection.outlook.com; client-ip=204.154.129.150; helo=usnvwwmspedge02.tellabs-west.tellabsinc.net; Received: from usnvwwmspedge02.tellabs-west.tellabsinc.net (204.154.129.150) by VE1EUR02FT020.mail.protection.outlook.com (10.152.12.102) with Microsoft SMTP Server (TLS) id 15.1.539.16 via Frontend Transport; Fri, 15 Jul 2016 04:40:28 +0000 Received: from usnvwwmspht02.tellabs-west.tellabsinc.net (172.23.211.70) by usnvwwmspedge02.tellabs-west.tellabsinc.net (204.154.131.191) with Microsoft SMTP Server (TLS) id 8.3.389.2; Thu, 14 Jul 2016 23:37:53 -0500 Received: from ubuntu-14-04-lxc-sim-v11.tellabs.fi (172.23.229.50) by usnvwwmspht02.tellabs-west.tellabsinc.net (172.23.211.70) with Microsoft SMTP Server id 8.3.298.1; Thu, 14 Jul 2016 23:40:26 -0500 From: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> To: <dev@dpdk.org> Date: Fri, 15 Jul 2016 07:39:51 +0300 Message-ID: <20160715043951.32040-1-juhamatti.kuusisaari@coriant.com> X-Mailer: git-send-email 2.9.0 MIME-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-HT: Tenant X-Forefront-Antispam-Report: CIP:204.154.129.150; IPV:NLI; CTRY:US; EFV:NLI; SFV:NSPM; SFS:(10009020)(6009001)(7916002)(2980300002)(438002)(199003)(368134002)(189002)(77096005)(189998001)(106466001)(86362001)(16796002)(47776003)(97736004)(305945005)(7846002)(68736007)(81156014)(81166006)(8936002)(8746002)(50226002)(87936001)(110136002)(107886002)(15650500001)(11100500001)(1076002)(586003)(92566002)(36756003)(450100001)(50986999)(356003)(8676002)(48376002)(5003940100001)(2906002)(2351001)(229853001)(53416004)(50466002)(6806005)(19580405001)(19580395003)(33646002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR04MB1268; H:usnvwwmspedge02.tellabs-west.tellabsinc.net; FPR:; SPF:Pass; PTR:InfoDomainNonexistent; A:1; MX:1; LANG:en; X-MS-Office365-Filtering-Correlation-Id: 45ba76cb-5083-4abc-be93-08d3ac6a25c4 X-Microsoft-Exchange-Diagnostics: 1; AM3PR04MB1268; 2:f/Je+hRKxadpDZVltNbCPV/EYKmhiqRQ0RCcYl+JCc9BGPFFsKoBU8t+W98XzpgqMZoiHewWpRh6MuH7DQ62wpT2iajuUw2Fbgaq9HCLQUq0BY6674DD5rrhuQ/oAbxfVK4HeoE487X4eKlPNWrWt/VrkgImd8V8/KGHVZ0qMfacXzWOyf8nZmkMFeFCf1uS; 3:8p3IMFQXBRrbbMMAb1rp36RRJM5mYumErA5aCKDzZbAhHuJqvwfSN69XTz1op4BOdmAXJV/w2ofzK7RuHQgTp+LETxeG/nnONkxC96GitCwiPbHre8e5mKBsZrZXexGw+6kKydrjZV9ZhaMFzhgcFjZWLS4FtG5oOBgzFzKqjeLtvVQQVTkGe4lEmLCWHjJ6lOgsVpgLNO2VImdzKTFhMWo7SsruEkQ5i1D4xI67z5H0sfGn31kGltOMKQ287I88NmtRJD0SAJj9SMxefcPTsw== X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(8251501002); SRVR:AM3PR04MB1268; X-Microsoft-Exchange-Diagnostics: 1; AM3PR04MB1268; 25:G4LUQGtFNG7PFK8kR2241N33VPPQ0rClXpzR4x4lbq6uS8FATrGCWRzssFNomeio802XnguPVAzSxDM9DBU9kqjZnvxZkyr8JF4yRveEZFtpn0pFBtodWozJPDFylG52qRz+QXteRDZb9ZH53hHYuTQuIBAQ3PWZukVFaabZAib2lWRmzbfeaCM9mRLFeOBlSNsE3Z2S3nRwYsTVZveVi9jXM4a0KTsgMjxsvbeTS891vUZk2m46A5hrn40tXD0EOC13yxkKKjxKERwPARN6xLERSe0XVLk3YtH2pTGZq3aAOgIg13IC9VxR4eO5lA/SjmKwvQexTaXjnP36o6A9TqDV3sU1NJIh6+rOZ771EFFCA4SoyWwKjyXpLewbmWgYJwtClcp+gzI6WHUzjthCVaxfBZLW/3mADqpOmcRlrd0kkzIFDguxXUZCSCxNEOS7lniok3IF9xFukxwm5ZHfTue6Y0Vr4eVIkiR6XQKFqhTz7cjcbEQLZFXXA94an0eIMVBC7T1xXWId8eVZ92t5gQ0xTJgh0gjbrVtcEFjrzuefsOzCNQKs3RxTlCumvbVEvO7KYms6DS1AUt/dlcGHR4Ym4pwdGje4lBH1sMf40odTHK13DrEjBsowRCGWn+qqgWKT8pzsS8lbbjky4yFsgxCAsXrNvGcvfUIktAoYrPwWiOAtSP/3B9+QAslCL/OnjGpM8R5zt3eRdtHK2Smd5Om+praQaBN6zsG3oKQeWo2BteW10hi1PQtye7MLpJaNFDsXIrOoyRkWJMMZC4RIN/Wl6j9hQEMf1kg0jyYRA2g= X-Microsoft-Exchange-Diagnostics: 1; AM3PR04MB1268; 31:w7vJbifDyGHvTPjfrBk4rQsn35WLh2zyYr25pQI3MUkJWAhMN0kDYPOXutrXPBRup+YOGeksrEG919B9gYZeE+DDLGduLuxlSC/f9jmfZvzQKp/UVYRZBiitdP8AZIs5UvvBUD8Wh+fXi1mt9aQDFNsWe6KzzFbDz9fcxMY4GMWXgzCnqNa9IIxmPftjx2YBBTcKomrdn1mk5rY0izOOyw==; 20:jqwpoY1iiZEmatpuXy07yXWEch1dZrNl/6jMxZjUloBUhP4j5JvXgYntVfWgdAFXh5D5OhPBcfc3cD/+s27p+oaZrQ6cTdh3LlGbGFS2nmoHiMMcWYA4wxSuRkzxr3CxnTjdUKubE9o0Nzuq83zf8r/3rKQXzqrhZJLI1U/xbsDJxxCgIOaLzLYyv9sAgxs7+HCXMOjdlRT+Rp289UPuKqK74ccBdoEOiyP4ndIX3DQXuIgylRrUKRPARKf2Dv+H2JWzOJqMB/xJ4EBwqJT1T1Z/L1uaHoI0f1OfvC2NmFXUZ5EpVVZ/0dLXIq3Qex0FeKl4oFCSGt83vXizvG3BrEtTS8jPsPMVn84slze3lrKTNXmY1XThd0AOAVKmUEhmt/Vy8pAwY+OhPwR19A4v7hjKIpSHtMnRZZ+G8yeeMWAS7yI6p0IKSSDFjllgxHSIXGWZgxHYuzuGFRlf0VSO7Cr5E274h2W0mbjlWQuu/MOqsCW7iZupL3d6bXZ8/Gya X-Microsoft-Antispam-PRVS: <AM3PR04MB126857BB670C5DBC62E37A4F9D330@AM3PR04MB1268.eurprd04.prod.outlook.com> X-Exchange-Antispam-Report-Test: UriScan:(51653755401839); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(601004)(2401047)(13023025)(8121501046)(5005006)(13017025)(13018025)(13024025)(13015025)(10201501046)(3002001)(6055026); SRVR:AM3PR04MB1268; BCL:0; PCL:0; RULEID:; SRVR:AM3PR04MB1268; X-Microsoft-Exchange-Diagnostics: 1; AM3PR04MB1268; 4:e/OlQKBLzKJwViDF1QZ4R/JWUplNHFm4R6TWAvpp6q/o8GRY1ZN+gKjtX0Fukd7nPMOaWjRoUEEUvZQ0UZRqc9cm3Mt0iFEmoAiuE6zdtgnMt+anBnH+sv1s/g7F5R7uFmsNH40opOP3Bbaej4PFdlnZ6zIJpiuKyQS8FL/YmZEtH97I6R9wQDQSwA04irL8JXXNBtlCcaucYk+oDA+Jy43Gn/qhzJRQCePG/vpJkEPhypvOpz3IhY4jRbRN0iCM0Bk+YpxWYdFohZORhSo0ERMHZ6h29F+ANoKYlKRsdRFiQNSIXRA3AeLryR3LIrn0e2L1BRL2wQA4utQEkkreEPmUkToPWZksWvPKx6gwTmVS1xGUncIXmAAKVES4y65p3L3IzI4L9GPyPOdaiYywW9Z6o0xyKUT88Z+9E2HTFm0HN70e8Mwj2EOf7Hd/ye/Rj/+Zl4+6FzogSlQb3rB0u/bhIFIXM2vvLVx1SXcPT/+x8K4jtYgwbOMwjySwPAiqJ2FO+s2hzcbL18BrVeaHiQ== X-Forefront-PRVS: 00046D390F X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM3PR04MB1268; 23:klir/3BOupulqJh6bTn2axyfAHD1fEG63YqfJA08F?= =?us-ascii?Q?gMDTDKEieUcIn1wKxtIsmhJrLtFq/od57uvQXkDc4MYM92yxYRJOPYuepeVK?= =?us-ascii?Q?egEPWSaInx/yNZDgHEP+Rrl931c4TeyTscrdN+ljNE6og97Zp0xwviapBM2T?= =?us-ascii?Q?bfj80fh5Lt43EZcpTQylqPtXa9t5g7R6rnK1wliKFJ0R3eYyGYpMQeRT3r34?= =?us-ascii?Q?qAQw1vi0PBH6fqzy6PeNrjJM7Tcz8khdr3WmTW4vjfLqF64RG8GRS3wlZ20d?= =?us-ascii?Q?xF4PvV8xyoZnwouOlEhDBU8ZdaUFc9+PJVXvEBGiud5ijfm/vt47SCq/qxgL?= =?us-ascii?Q?vpnFVRihbsVuOvKOgkzrOhLhBIckKE/10MMbOa2ugi34XP47apI0b8VfQnak?= =?us-ascii?Q?+nW4uGW8Tk38IotF2IXXWsoLDhs/EA+no1K5KTCMhhOanc50iDRDQjKc1Kfl?= =?us-ascii?Q?sOiiwAY/VUZc2Et1LoiYiA+2IAKx87IUnOYni6EtBmt4aTX8za1Ks9F8toCI?= =?us-ascii?Q?AbVIM5z/8cGFvNp9UTEWGKMNPtTNPY8Ij3eQ6E2k6+m29pc9gor+FiiUXAml?= =?us-ascii?Q?nHDysW/Yix269+1WleXKAvWGKuP/tqKo5J3q+WU8yB6x/gvIdPuO6TL1fFKA?= =?us-ascii?Q?vjOTkzFcSEUcRq5ggmXbjU3cZ1hNdKJqglzChwKI+aSIinDGk9c+wa4Fkwa4?= =?us-ascii?Q?6zb/h9Ns+PJ33pfJ9C/DWWSFBY9NGDxCyfm7gmcWKYdADsvWh6anURfV5B0K?= =?us-ascii?Q?AEHc3L1bVMThvacT7bNa+SeFq9JJqMp46DavGkog8R13P1h9k1ocyu2C6Xr0?= =?us-ascii?Q?7NtKhjZJHn+4FXoh/8KdS5XQ4Vuap/EJ2BnlOIuFe7ST+gOg/V4iY8sWENGz?= =?us-ascii?Q?MMMRn5+YX6+TBk8UFASxiHAnKQkoPlv3RhjP7S7BZzYwr8q3zQd7hhw7Hv75?= =?us-ascii?Q?NJ3ph9L07sEb4UAfWDXuiByn8J+kAa78k2sbQKgOskcy5oS1KcIXfjtUfGfO?= =?us-ascii?Q?iXmQXsKPJal7S70QivoMPRC3UZEKGqUVkbwJBOLAkThrAIss+NlKrKobrC+A?= =?us-ascii?Q?jTUVdJ4MFGe+i6tnXmPRCGFrXlhcZ0qdcZorA0AqEVLc0rjZsGYtsi8iuRXR?= =?us-ascii?Q?YrEvFm8EwoD6A+GU1ABsSxMwR6x177l6Rzh6LFPWMlBkcOPzij9Ag=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; AM3PR04MB1268; 6:l1gaYIs5Po56gnVCpeXOMvNrxiC7swpVSCcLPU4UPNs+1BhhegdUc405z2L8BDUf1HHKU4nVNuuIoH7+xXjDsQ6RJoh6G1NE8ZTcIm9viW+L6JYW1QVPDE7rXh4q8DRyuAGFNJ5BUXyS2eXhQSSN0NeJ6RykkiBAfX2Uu1NgClQ3FTV8g+dM70KJMSJJMbSztmVniWn4SVssFw4/3sHJZrFIqYRRhG+ZOqc3MDeZ8Wxfu7c7p7ngCFs1NVkOowtsXfvyDPt86Up8KIU64zxaofbkq/VRxomOP+viwTfDdpla6K8U1p01eZW0j1GJPYcPpyJYLOcH/dm4C5jpH82mdw==; 5:0tAsOS24ZoIZXJ8/GgqSYeNdWRzsxrcCX3tkBjWEgyXW5Ul9hnWUGYlkNTq76rjEiVrD2GDnTcunscaqAUlo4cWp+1Rp/cj2IvLABFqNaNZag+A/pN2rmiMSiH1/NO77zOBaHG8WMD3QQcMey15xig==; 24:E3BAlOk7ve2Oz4kvVUd5dSnLzLqKqRv69j4mXT768aA2YgWZgsi0ZhDQEtMNZv/ZbW9rkT3h9PVeTbLoc8EzFtl9OBOV/HLA1/0GS920V2M=; 7:6AJFiGj/VFsSC7GPHavJmfmcumcYdyR+tIhzIZzgwIl3OaVH0myEJ98vw9KGTAM0RJOtbkvtwRB8/h6q0Ff4eRo2gADdci7mkDiI3hiTpqxc5biEw3Ai/nLtso3y/DP+Pqtl+zTpMcizf3gVF0H13pfXgClIb45QZp47m0CLZr70cIK7Xq/LlPqXqo3jS619zRPSDpWJbXZzVS/Qs/f4fvSvK2/lK/tkAeF35Yz+JNgq7FbjoVrJJFzF3khdb9cl SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: coriant.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jul 2016 04:40:28.5470 (UTC) X-MS-Exchange-CrossTenant-Id: 76595477-907e-4695-988b-a6b39087332d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=76595477-907e-4695-988b-a6b39087332d; Ip=[204.154.129.150]; Helo=[usnvwwmspedge02.tellabs-west.tellabsinc.net] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR04MB1268 Subject: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Kuusisaari, Juhamatti (Infinera - FI/Espoo)
July 15, 2016, 4:39 a.m. UTC
Consumer queue dequeuing must be guaranteed to be done fully before
the tail is updated. This is not guaranteed with a read barrier,
changed to a write barrier just before tail update which in practice
guarantees correct order of reads and writes.
Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>
---
lib/librte_ring/rte_ring.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--
2.9.0
============================================================
The information contained in this message may be privileged
and confidential and protected from disclosure. If the reader
of this message is not the intended recipient, or an employee
or agent responsible for delivering this message to the
intended recipient, you are hereby notified that any reproduction,
dissemination or distribution of this communication is strictly
prohibited. If you have received this communication in error,
please notify us immediately by replying to the message and
deleting it from your computer. Thank you. Coriant-Tellabs
============================================================
Comments
> > Consumer queue dequeuing must be guaranteed to be done fully before the tail is updated. This is not guaranteed with a read barrier, > changed to a write barrier just before tail update which in practice guarantees correct order of reads and writes. > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> > --- > lib/librte_ring/rte_ring.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index eb45e41..14920af 100644 > --- a/lib/librte_ring/rte_ring.h > +++ b/lib/librte_ring/rte_ring.h > @@ -748,7 +748,7 @@ __rte_ring_sc_do_dequeue(struct rte_ring *r, void **obj_table, > > /* copy in table */ > DEQUEUE_PTRS(); > - rte_smp_rmb(); > + rte_smp_wmb(); > > __RING_STAT_ADD(r, deq_success, n); > r->cons.tail = cons_next; > -- Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > 2.9.0 >
> > Consumer queue dequeuing must be guaranteed to be done fully before the tail is updated. This is not guaranteed with a read barrier, > > changed to a write barrier just before tail update which in practice guarantees correct order of reads and writes. > > > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> Applied, thanks
On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > Consumer queue dequeuing must be guaranteed to be done fully before the tail is updated. This is not guaranteed with a read barrier, > > > changed to a write barrier just before tail update which in practice guarantees correct order of reads and writes. > > > > > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> > > > > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > Applied, thanks There was ongoing discussion on this http://dpdk.org/ml/archives/dev/2016-July/044168.html This change may not be required as it has the performance impact.
2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: >> > > Consumer queue dequeuing must be guaranteed to be done fully before the tail is updated. This is not guaranteed with a read barrier, >> > > changed to a write barrier just before tail update which in practice guarantees correct order of reads and writes. >> > > >> > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> >> > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> >> >> Applied, thanks > > There was ongoing discussion on this > http://dpdk.org/ml/archives/dev/2016-July/044168.html Sorry Jerin, I forgot this email. The problem is that nobody replied to your email and you did not nack the v2 of this patch. > This change may not be required as it has the performance impact. We need to clearly understand what is the performance impact (numbers and use cases) on one hand, and is there a real bug fixed by this patch on the other hand? Please guys make things clear and we'll revert if needed.
On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > >> > > Consumer queue dequeuing must be guaranteed to be done fully before the tail is updated. This is not guaranteed with a read barrier, > >> > > changed to a write barrier just before tail update which in practice guarantees correct order of reads and writes. > >> > > > >> > > Signed-off-by: Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com> > >> > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > >> > >> Applied, thanks > > > > There was ongoing discussion on this > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > Sorry Jerin, I forgot this email. > The problem is that nobody replied to your email and you did not nack > the v2 of this patch. > > > This change may not be required as it has the performance impact. > > We need to clearly understand what is the performance impact > (numbers and use cases) on one hand, and is there a real bug fixed > by this patch on the other hand? IHMO, there is no real bug here. rte_smb_rmb() provides the LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior LOADS. Thoughts? > > Please guys make things clear and we'll revert if needed.
Hi lads, > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > >> > > Consumer queue dequeuing must be guaranteed to be done fully > > >> > > before the tail is updated. This is not guaranteed with a read barrier, changed to a write barrier just before tail update which in > practice guarantees correct order of reads and writes. > > >> > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > >> > > <juhamatti.kuusisaari@coriant.com> > > >> > > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > >> > > >> Applied, thanks > > > > > > There was ongoing discussion on this > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > Sorry Jerin, I forgot this email. > > The problem is that nobody replied to your email and you did not nack > > the v2 of this patch. It's probably my bad. I acked the patch before Jerin response, and forgot to reply later. > > > > > This change may not be required as it has the performance impact. > > > > We need to clearly understand what is the performance impact (numbers > > and use cases) on one hand, and is there a real bug fixed by this > > patch on the other hand? > > IHMO, there is no real bug here. rte_smb_rmb() provides the LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior > LOADS. Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > Thoughts? Wonder how big is a performance impact? If there is a real one, I suppose we can revert the patch? Konstantin > > > > > Please guys make things clear and we'll revert if needed.
On Sat, Jul 23, 2016 at 10:14:51AM +0000, Ananyev, Konstantin wrote: > Hi lads, > > > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > >> > > Consumer queue dequeuing must be guaranteed to be done fully > > > >> > > before the tail is updated. This is not guaranteed with a read barrier, changed to a write barrier just before tail update which in > > practice guarantees correct order of reads and writes. > > > >> > > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > > >> > > <juhamatti.kuusisaari@coriant.com> > > > >> > > > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > > >> > > > >> Applied, thanks > > > > > > > > There was ongoing discussion on this > > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > > > Sorry Jerin, I forgot this email. > > > The problem is that nobody replied to your email and you did not nack > > > the v2 of this patch. > > It's probably my bad. > I acked the patch before Jerin response, and forgot to reply later. > > > > > > > > This change may not be required as it has the performance impact. > > > > > > We need to clearly understand what is the performance impact (numbers > > > and use cases) on one hand, and is there a real bug fixed by this > > > patch on the other hand? > > > > IHMO, there is no real bug here. rte_smb_rmb() provides the LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior > > LOADS. > > Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... > For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > > > > Thoughts? > > Wonder how big is a performance impact? With this change we need to wait for addtional STORES to be completed to local buffer in addtion to LOADS from ring buffers memory. > If there is a real one, I suppose we can revert the patch? Request to revert this one as their no benifts for other architectures and indeed it creates addtional delay in waiting for STORES to complete in ARM. Lets do the correct thing by reverting it. Jerin > Konstantin > > > > > > > > > Please guys make things clear and we'll revert if needed.
> -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Saturday, July 23, 2016 11:39 AM > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update > > On Sat, Jul 23, 2016 at 10:14:51AM +0000, Ananyev, Konstantin wrote: > > Hi lads, > > > > > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > > >> > > Consumer queue dequeuing must be guaranteed to be done > > > > >> > > fully before the tail is updated. This is not guaranteed > > > > >> > > with a read barrier, changed to a write barrier just before > > > > >> > > tail update which in > > > practice guarantees correct order of reads and writes. > > > > >> > > > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > > > >> > > <juhamatti.kuusisaari@coriant.com> > > > > >> > > > > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > > > >> > > > > >> Applied, thanks > > > > > > > > > > There was ongoing discussion on this > > > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > > > > > Sorry Jerin, I forgot this email. > > > > The problem is that nobody replied to your email and you did not > > > > nack the v2 of this patch. > > > > It's probably my bad. > > I acked the patch before Jerin response, and forgot to reply later. > > > > > > > > > > > This change may not be required as it has the performance impact. > > > > > > > > We need to clearly understand what is the performance impact > > > > (numbers and use cases) on one hand, and is there a real bug fixed > > > > by this patch on the other hand? > > > > > > IHMO, there is no real bug here. rte_smb_rmb() provides the > > > LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior LOADS. > > > > Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... > > For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > > > > > > > Thoughts? > > > > Wonder how big is a performance impact? > > With this change we need to wait for addtional STORES to be completed to local buffer in addtion to LOADS from ring buffers memory. I understand that, just wonder did you see any real performance difference? Probably with ring_perf_autotest/mempool_perf_autotest or something? Konstantin > > > If there is a real one, I suppose we can revert the patch? > > Request to revert this one as their no benifts for other architectures and indeed it creates addtional delay in waiting for STORES to complete > in ARM. > Lets do the correct thing by reverting it. > > Jerin > > > > > Konstantin > > > > > > > > > > > > > Please guys make things clear and we'll revert if needed.
On Sat, Jul 23, 2016 at 11:15:27AM +0000, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > Sent: Saturday, July 23, 2016 11:39 AM > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update > > > > On Sat, Jul 23, 2016 at 10:14:51AM +0000, Ananyev, Konstantin wrote: > > > Hi lads, > > > > > > > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > > > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > > > >> > > Consumer queue dequeuing must be guaranteed to be done > > > > > >> > > fully before the tail is updated. This is not guaranteed > > > > > >> > > with a read barrier, changed to a write barrier just before > > > > > >> > > tail update which in > > > > practice guarantees correct order of reads and writes. > > > > > >> > > > > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > > > > >> > > <juhamatti.kuusisaari@coriant.com> > > > > > >> > > > > > > >> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com> > > > > > >> > > > > > >> Applied, thanks > > > > > > > > > > > > There was ongoing discussion on this > > > > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > > > > > > > Sorry Jerin, I forgot this email. > > > > > The problem is that nobody replied to your email and you did not > > > > > nack the v2 of this patch. > > > > > > It's probably my bad. > > > I acked the patch before Jerin response, and forgot to reply later. > > > > > > > > > > > > > > This change may not be required as it has the performance impact. > > > > > > > > > > We need to clearly understand what is the performance impact > > > > > (numbers and use cases) on one hand, and is there a real bug fixed > > > > > by this patch on the other hand? > > > > > > > > IHMO, there is no real bug here. rte_smb_rmb() provides the > > > > LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior LOADS. > > > > > > Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... > > > For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > > > > > > > > > > Thoughts? > > > > > > Wonder how big is a performance impact? > > > > With this change we need to wait for addtional STORES to be completed to local buffer in addtion to LOADS from ring buffers memory. > > I understand that, just wonder did you see any real performance difference? Yeah... > Probably with ring_perf_autotest/mempool_perf_autotest or something? W/O change RTE>>ring_perf_autotest ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 4 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue (size: 8): 0 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 ### Testing empty dequeue ### SC empty dequeue: 0.35 MC empty dequeue: 0.60 ### Testing using a single lcore ### SP/SC bulk enq/dequeue (size: 8): 0.93 MP/MC bulk enq/dequeue (size: 8): 2.45 SP/SC bulk enq/dequeue (size: 32): 0.58 MP/MC bulk enq/dequeue (size: 32): 0.97 ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 1.89 MP/MC bulk enq/dequeue (size: 8): 4.28 SP/SC bulk enq/dequeue (size: 32): 0.90 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK RTE>> With change RTE>>ring_perf_autotest ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 6 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue (size: 8): 1 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 ### Testing empty dequeue ### SC empty dequeue: 0.35 MC empty dequeue: 0.60 ### Testing using a single lcore ### SP/SC bulk enq/dequeue (size: 8): 1.28 MP/MC bulk enq/dequeue (size: 8): 2.47 SP/SC bulk enq/dequeue (size: 32): 0.64 MP/MC bulk enq/dequeue (size: 32): 0.97 ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 2.08 MP/MC bulk enq/dequeue (size: 8): 4.29 SP/SC bulk enq/dequeue (size: 32): 1.24 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK > Konstantin > > > > > > If there is a real one, I suppose we can revert the patch? > > > > Request to revert this one as their no benifts for other architectures and indeed it creates addtional delay in waiting for STORES to complete > > in ARM. > > Lets do the correct thing by reverting it. > > > > Jerin > > > > > > > > > Konstantin > > > > > > > > > > > > > > > > > Please guys make things clear and we'll revert if needed.
> -----Original Message----- > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > Sent: Saturday, July 23, 2016 12:49 PM > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update > > On Sat, Jul 23, 2016 at 11:15:27AM +0000, Ananyev, Konstantin wrote: > > > > > > > -----Original Message----- > > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > > Sent: Saturday, July 23, 2016 11:39 AM > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti > > > Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to > > > guarantee ordering before tail update > > > > > > On Sat, Jul 23, 2016 at 10:14:51AM +0000, Ananyev, Konstantin wrote: > > > > Hi lads, > > > > > > > > > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > > > > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > > > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > > > > >> > > Consumer queue dequeuing must be guaranteed to be done > > > > > > >> > > fully before the tail is updated. This is not > > > > > > >> > > guaranteed with a read barrier, changed to a write > > > > > > >> > > barrier just before tail update which in > > > > > practice guarantees correct order of reads and writes. > > > > > > >> > > > > > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > > > > > >> > > <juhamatti.kuusisaari@coriant.com> > > > > > > >> > > > > > > > >> > Acked-by: Konstantin Ananyev > > > > > > >> > <konstantin.ananyev@intel.com> > > > > > > >> > > > > > > >> Applied, thanks > > > > > > > > > > > > > > There was ongoing discussion on this > > > > > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > > > > > > > > > Sorry Jerin, I forgot this email. > > > > > > The problem is that nobody replied to your email and you did > > > > > > not nack the v2 of this patch. > > > > > > > > It's probably my bad. > > > > I acked the patch before Jerin response, and forgot to reply later. > > > > > > > > > > > > > > > > > This change may not be required as it has the performance impact. > > > > > > > > > > > > We need to clearly understand what is the performance impact > > > > > > (numbers and use cases) on one hand, and is there a real bug > > > > > > fixed by this patch on the other hand? > > > > > > > > > > IHMO, there is no real bug here. rte_smb_rmb() provides the > > > > > LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior LOADS. > > > > > > > > Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... > > > > For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > > > > > > > > > > > > > Thoughts? > > > > > > > > Wonder how big is a performance impact? > > > > > > With this change we need to wait for addtional STORES to be completed to local buffer in addtion to LOADS from ring buffers memory. > > > > I understand that, just wonder did you see any real performance difference? > > Yeah... Ok, then I don't see any good reason why we shouldn't revert it. I suppose the best way would be to submit a new patch for RC5 to revert the changes. Do you prefer to submit it yourself and I'll ack it or visa-versa? Thanks Konstantin > > > Probably with ring_perf_autotest/mempool_perf_autotest or something? > > W/O change > RTE>>ring_perf_autotest > ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 4 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue > (size: 8): 0 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 > > ### Testing empty dequeue ### > SC empty dequeue: 0.35 > MC empty dequeue: 0.60 > > ### Testing using a single lcore ### > SP/SC bulk enq/dequeue (size: 8): 0.93 > MP/MC bulk enq/dequeue (size: 8): 2.45 > SP/SC bulk enq/dequeue (size: 32): 0.58 > MP/MC bulk enq/dequeue (size: 32): 0.97 > > ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 1.89 MP/MC bulk enq/dequeue (size: 8): 4.28 SP/SC bulk > enq/dequeue (size: 32): 0.90 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK > RTE>> > > With change > RTE>>ring_perf_autotest > ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 6 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue > (size: 8): 1 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 > > ### Testing empty dequeue ### > SC empty dequeue: 0.35 > MC empty dequeue: 0.60 > > ### Testing using a single lcore ### > SP/SC bulk enq/dequeue (size: 8): 1.28 > MP/MC bulk enq/dequeue (size: 8): 2.47 > SP/SC bulk enq/dequeue (size: 32): 0.64 > MP/MC bulk enq/dequeue (size: 32): 0.97 > > ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 2.08 MP/MC bulk enq/dequeue (size: 8): 4.29 SP/SC bulk > enq/dequeue (size: 32): 1.24 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK > > > Konstantin > > > > > > > > > If there is a real one, I suppose we can revert the patch? > > > > > > Request to revert this one as their no benifts for other > > > architectures and indeed it creates addtional delay in waiting for STORES to complete in ARM. > > > Lets do the correct thing by reverting it. > > > > > > Jerin > > > > > > > > > > > > > Konstantin > > > > > > > > > > > > > > > > > > > > > Please guys make things clear and we'll revert if needed.
On Sat, Jul 23, 2016 at 12:32:01PM +0000, Ananyev, Konstantin wrote: > > > > -----Original Message----- > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > Sent: Saturday, July 23, 2016 12:49 PM > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to guarantee ordering before tail update > > > > On Sat, Jul 23, 2016 at 11:15:27AM +0000, Ananyev, Konstantin wrote: > > > > > > > > > > -----Original Message----- > > > > From: Jerin Jacob [mailto:jerin.jacob@caviumnetworks.com] > > > > Sent: Saturday, July 23, 2016 11:39 AM > > > > To: Ananyev, Konstantin <konstantin.ananyev@intel.com> > > > > Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; Juhamatti > > > > Kuusisaari <juhamatti.kuusisaari@coriant.com>; dev@dpdk.org > > > > Subject: Re: [dpdk-dev] [PATCH] lib: change rte_ring dequeue to > > > > guarantee ordering before tail update > > > > > > > > On Sat, Jul 23, 2016 at 10:14:51AM +0000, Ananyev, Konstantin wrote: > > > > > Hi lads, > > > > > > > > > > > On Sat, Jul 23, 2016 at 11:02:33AM +0200, Thomas Monjalon wrote: > > > > > > > 2016-07-23 8:05 GMT+02:00 Jerin Jacob <jerin.jacob@caviumnetworks.com>: > > > > > > > > On Thu, Jul 21, 2016 at 11:26:50PM +0200, Thomas Monjalon wrote: > > > > > > > >> > > Consumer queue dequeuing must be guaranteed to be done > > > > > > > >> > > fully before the tail is updated. This is not > > > > > > > >> > > guaranteed with a read barrier, changed to a write > > > > > > > >> > > barrier just before tail update which in > > > > > > practice guarantees correct order of reads and writes. > > > > > > > >> > > > > > > > > > >> > > Signed-off-by: Juhamatti Kuusisaari > > > > > > > >> > > <juhamatti.kuusisaari@coriant.com> > > > > > > > >> > > > > > > > > >> > Acked-by: Konstantin Ananyev > > > > > > > >> > <konstantin.ananyev@intel.com> > > > > > > > >> > > > > > > > >> Applied, thanks > > > > > > > > > > > > > > > > There was ongoing discussion on this > > > > > > > > http://dpdk.org/ml/archives/dev/2016-July/044168.html > > > > > > > > > > > > > > Sorry Jerin, I forgot this email. > > > > > > > The problem is that nobody replied to your email and you did > > > > > > > not nack the v2 of this patch. > > > > > > > > > > It's probably my bad. > > > > > I acked the patch before Jerin response, and forgot to reply later. > > > > > > > > > > > > > > > > > > > > This change may not be required as it has the performance impact. > > > > > > > > > > > > > > We need to clearly understand what is the performance impact > > > > > > > (numbers and use cases) on one hand, and is there a real bug > > > > > > > fixed by this patch on the other hand? > > > > > > > > > > > > IHMO, there is no real bug here. rte_smb_rmb() provides the > > > > > > LOAD-STORE barrier to make sure tail pointer WRITE happens only after prior LOADS. > > > > > > > > > > Yep, from what I read at the link Jerin provided, indeed it seems rte_smp_rmb() is enough for the arm arch here... > > > > > For ppc, as I can see both rte_smp_rmb()/rte_smp_wmb() emits the same instruction. > > > > > > > > > > > > > > > > > Thoughts? > > > > > > > > > > Wonder how big is a performance impact? > > > > > > > > With this change we need to wait for addtional STORES to be completed to local buffer in addtion to LOADS from ring buffers memory. > > > > > > I understand that, just wonder did you see any real performance difference? > > > > Yeah... > > Ok, then I don't see any good reason why we shouldn't revert it. > I suppose the best way would be to submit a new patch for RC5 to revert the changes. > Do you prefer to submit it yourself and I'll ack it or visa-versa? OK. I will submit it then > Thanks > Konstantin > > > > > > Probably with ring_perf_autotest/mempool_perf_autotest or something? > > > > W/O change > > RTE>>ring_perf_autotest > > ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 4 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue > > (size: 8): 0 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 > > > > ### Testing empty dequeue ### > > SC empty dequeue: 0.35 > > MC empty dequeue: 0.60 > > > > ### Testing using a single lcore ### > > SP/SC bulk enq/dequeue (size: 8): 0.93 > > MP/MC bulk enq/dequeue (size: 8): 2.45 > > SP/SC bulk enq/dequeue (size: 32): 0.58 > > MP/MC bulk enq/dequeue (size: 32): 0.97 > > > > ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 1.89 MP/MC bulk enq/dequeue (size: 8): 4.28 SP/SC bulk > > enq/dequeue (size: 32): 0.90 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK > > RTE>> > > > > With change > > RTE>>ring_perf_autotest > > ### Testing single element and burst enq/deq ### SP/SC single enq/dequeue: 6 MP/MC single enq/dequeue: 16 SP/SC burst enq/dequeue > > (size: 8): 1 MP/MC burst enq/dequeue (size: 8): 2 SP/SC burst enq/dequeue (size: 32): 0 MP/MC burst enq/dequeue (size: 32): 0 > > > > ### Testing empty dequeue ### > > SC empty dequeue: 0.35 > > MC empty dequeue: 0.60 > > > > ### Testing using a single lcore ### > > SP/SC bulk enq/dequeue (size: 8): 1.28 > > MP/MC bulk enq/dequeue (size: 8): 2.47 > > SP/SC bulk enq/dequeue (size: 32): 0.64 > > MP/MC bulk enq/dequeue (size: 32): 0.97 > > > > ### Testing using two physical cores ### SP/SC bulk enq/dequeue (size: 8): 2.08 MP/MC bulk enq/dequeue (size: 8): 4.29 SP/SC bulk > > enq/dequeue (size: 32): 1.24 MP/MC bulk enq/dequeue (size: 32): 1.19 Test OK > > > > > Konstantin > > > > > > > > > > > > If there is a real one, I suppose we can revert the patch? > > > > > > > > Request to revert this one as their no benifts for other > > > > architectures and indeed it creates addtional delay in waiting for STORES to complete in ARM. > > > > Lets do the correct thing by reverting it. > > > > > > > > Jerin > > > > > > > > > > > > > > > > > Konstantin > > > > > > > > > > > > > > > > > > > > > > > > > Please guys make things clear and we'll revert if needed.
diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h index eb45e41..14920af 100644 --- a/lib/librte_ring/rte_ring.h +++ b/lib/librte_ring/rte_ring.h @@ -748,7 +748,7 @@ __rte_ring_sc_do_dequeue(struct rte_ring *r, void **obj_table, /* copy in table */ DEQUEUE_PTRS(); - rte_smp_rmb(); + rte_smp_wmb(); __RING_STAT_ADD(r, deq_success, n); r->cons.tail = cons_next;