From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id EB914A059A; Sat, 11 Apr 2020 01:10:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 39B611D183; Sat, 11 Apr 2020 01:10:53 +0200 (CEST) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2045.outbound.protection.outlook.com [40.107.21.45]) by dpdk.org (Postfix) with ESMTP id C75301D178 for ; Sat, 11 Apr 2020 01:10:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zodGIao6YO8QsIZIegPQmOn8LCP+e3laYUhbxDvc7Qc=; b=NDqhW4PHJH4g9zY1QCNBm8sSMneORsuBnBzVhNjRJaG+xbhz+MjWxZiOmFTMgVpea11VUkgj+Qri0anfs4ZM4NyhpkkwFbJWeKZj08UW53VcxLY2tewnp46drjxSWTU2dvN+KZtACdCosgoSF5T+V6QKHhlwaHh7mlR77/yxZ/M= Received: from AM5PR0701CA0017.eurprd07.prod.outlook.com (2603:10a6:203:51::27) by DB6PR08MB2677.eurprd08.prod.outlook.com (2603:10a6:6:1c::22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.17; Fri, 10 Apr 2020 23:10:50 +0000 Received: from VE1EUR03FT051.eop-EUR03.prod.protection.outlook.com (2603:10a6:203:51:cafe::ef) by AM5PR0701CA0017.outlook.office365.com (2603:10a6:203:51::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2921.12 via Frontend Transport; Fri, 10 Apr 2020 23:10:50 +0000 Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dpdk.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dpdk.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT051.mail.protection.outlook.com (10.152.19.75) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2900.15 via Frontend Transport; Fri, 10 Apr 2020 23:10:49 +0000 Received: ("Tessian outbound af37c2b81632:v50"); Fri, 10 Apr 2020 23:10:49 +0000 X-CR-MTA-TID: 64aa7808 Received: from dccff3b26426.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id BDED9B87-D5A1-4564-B991-D9ABB0E584A0.1; Fri, 10 Apr 2020 23:10:44 +0000 Received: from EUR01-DB5-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id dccff3b26426.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Fri, 10 Apr 2020 23:10:44 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Xa1MSz7sE5cdqEuOhbrTvaDU8adH1v4+/qbZZT9EawNNbyS2eDrD9L/FWaxC11NdB3ojVXMqyhQNU5yjyCmZc8m/f/KqL5h6Y3NyMNbWxhKwhPub3uWD2WlOwZ8toEYA5XSEabvgNwL6ZYB+2VHHhHzzFLdZyATiycQSTnCFCaLfdIX+NaItSkuZfhyjQkRJgPdLR4zKZv0fHH/pcC1FMewExoZNuah2Qu3BUUz8U8WozLT4lowFM64jbTm2SZb83J218oYw1n9AGwV756cmKjpjTJDNDDDm/cUE0jYiAMAtQveeqXsINxM+/kzhe99JiEU0PR1rlX39PbO7vRNb8w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zodGIao6YO8QsIZIegPQmOn8LCP+e3laYUhbxDvc7Qc=; b=M1UO8ZbApNLRxbfrNP0hweUEERNybQY5Pg8VJWRN/zrbw4pO5/QcWNle+GKkGzNNV6nMdAZeB5PjVkFKg3j/izerTfqv7ylmbwFG8+dFhBm537n+v3TLTXHgkjjmrNNLqOvtizRW74hIXPTOjKKezuLHgE8mpJ2XETUsnA6Y1PNiakZmr0/uE2gXlv9+SkpTfpCHZoLKVg/dkUEPmzKOC7H/b/MRwwbWt27Spifi3FntcDqQMi4uC2xGa1XNyAl/GVkkkn7C8jS/qCo2bB96AyfOosAm/JQZAW/LGiTZyRBWjnj+FsYVuYY3Bf/Eo/d4zTIOEDvFswrJ+A+fcYL02A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=zodGIao6YO8QsIZIegPQmOn8LCP+e3laYUhbxDvc7Qc=; b=NDqhW4PHJH4g9zY1QCNBm8sSMneORsuBnBzVhNjRJaG+xbhz+MjWxZiOmFTMgVpea11VUkgj+Qri0anfs4ZM4NyhpkkwFbJWeKZj08UW53VcxLY2tewnp46drjxSWTU2dvN+KZtACdCosgoSF5T+V6QKHhlwaHh7mlR77/yxZ/M= Received: from AM6PR08MB4644.eurprd08.prod.outlook.com (2603:10a6:20b:c9::10) by AM6PR08MB3480.eurprd08.prod.outlook.com (2603:10a6:20b:44::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Fri, 10 Apr 2020 23:10:42 +0000 Received: from AM6PR08MB4644.eurprd08.prod.outlook.com ([fe80::c17e:19ec:d94b:bc23]) by AM6PR08MB4644.eurprd08.prod.outlook.com ([fe80::c17e:19ec:d94b:bc23%3]) with mapi id 15.20.2900.015; Fri, 10 Apr 2020 23:10:42 +0000 From: Honnappa Nagarahalli To: "Ananyev, Konstantin" , "dev@dpdk.org" CC: "david.marchand@redhat.com" , "jielong.zjl@antfin.com" , nd , Honnappa Nagarahalli , nd Thread-Topic: [PATCH v3 3/9] ring: introduce RTS ring mode Thread-Index: AQHWCd9WRTTZTeHr3kKWZbgjl/2b76huljRQgAJS+oCAAgY3UA== Date: Fri, 10 Apr 2020 23:10:41 +0000 Message-ID: References: <20200402220959.29885-1-konstantin.ananyev@intel.com> <20200403174235.23308-1-konstantin.ananyev@intel.com> <20200403174235.23308-4-konstantin.ananyev@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ts-tracking-id: 78f55ddf-1ac6-4065-885f-88dd46bc9055.0 x-checkrecipientchecked: true Authentication-Results-Original: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; x-originating-ip: [70.113.25.165] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: c3daa445-e974-494d-217d-08d7dda468a7 x-ms-traffictypediagnostic: AM6PR08MB3480:|AM6PR08MB3480:|DB6PR08MB2677: x-ms-exchange-transport-forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true nodisclaimer: true x-ms-oob-tlc-oobclassifiers: OLM:4502;OLM:4502; x-forefront-prvs: 0369E8196C X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM6PR08MB4644.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(376002)(366004)(346002)(396003)(39860400002)(136003)(86362001)(66946007)(33656002)(64756008)(5660300002)(66556008)(52536014)(66476007)(71200400001)(66446008)(110136005)(2906002)(478600001)(76116006)(9686003)(81156014)(4326008)(7696005)(316002)(55016002)(30864003)(186003)(8936002)(26005)(54906003)(6506007)(8676002)(21314003)(559001)(579004); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: fV5InfHXO/UwivosVdlaiqHCXsjGEd9Gw2KzaiQkw2kHjWRhW+o1asL8sRmb1SmissPoHFo7MeRkx0xfi9B5Vzd1XOrET154/ugNfIJm2nt144n2awu08st+XVxfGm/U0NUK6txkyLQ73AS97+6NlJUUbwsZBz+G/0EViZxy2uH21CuZPHdSmnkHCt6ARFXDqETiEz8WHRymsSb3/00eN2dAm0HNNu5FTxOOm4Rpw2AuP9+ImdZWJsU94guTTai0Cw3Ha+G7VddR0uttpt35EdWxCwnen9B3DagLSJkU4LZJdb8Fj3pyxUXV14iD1dlNvNhvCliMTTE9OUfqxKx/fNQxK8BbxnND7ko10HOVh+8UzQOcmP1DSJLQ4FLQPDGVJ3HiJGvdpdq0Qt45jcimlp/mrk11sLhgBXQhQSLm63aUE18FkNht9/J97eCnazLqUgVvU1mGW4cDwY4DhKBblSqRI5hlqNxhcAJXZ904rtsHUdXu61Sm/gy54ol5sB1A x-ms-exchange-antispam-messagedata: dBLnRQEaBAcwcE+Ir2S1sKTkFcLWhZxUUUcwfxglSVPVXoGXi6nz1+jvCOHNCfzk+NXzsjKHqqa7ojbxwy3/xSxEqMuShFtpEQtYK9lqQzHqwmOM+7DJMlUpapbaIceOi7yiiUvCMsiJvgPigXnCqA== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3480 Original-Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT051.eop-EUR03.prod.protection.outlook.com X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(376002)(346002)(396003)(39860400002)(136003)(46966005)(54906003)(47076004)(7696005)(36906005)(4326008)(9686003)(86362001)(81166007)(30864003)(52536014)(70206006)(316002)(70586007)(55016002)(26826003)(186003)(110136005)(82740400003)(81156014)(8936002)(356005)(6506007)(8676002)(2906002)(478600001)(33656002)(5660300002)(336012)(26005)(21314003); DIR:OUT; SFP:1101; X-MS-Office365-Filtering-Correlation-Id-Prvs: 5b27de03-4516-470a-d2f9-08d7dda46426 X-Forefront-PRVS: 0369E8196C X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: RnQ6XjuI6V8ahX+Jz+YskgH/OqjEIOtIAJ4wDopqFS1KO/7ANpY7ZYdTSVu7pCbTDHmjD6kkYP5esVXmj7fFsN51XzXve0d2gxNkcg6Sb1PAxGRzgH97iZM+zwILtNtWw+a5jNycCvXbsXkjUQbpla8rlIvXGyyb7SjyjY3IGuxqjZySXH2sS0kk9PccC83pnX2k8Kp1caM+sl3xVJi8bPIlpD7mRb8z0uttJMo5wxjIB0QgwuN93lKPLYPfWQ48GclHTZVgwWsr1BoyeedCHMEpFIVMCS9STBK7ibz4BDl/ORbEECl7RFAvZ7rOR0ITejGjtMGz3e2Py2egcl28itPR13v7Nm3kFoV4YkIbkc9vWU6yrDKQJvwba0MF3InIcSVVP7Qpir00lK4NThjDAhNaQ9nlvFOrsNlLx7pOREZ16ck0Usb/VTsQddMv/YAp6S85mxJvfy1xeB9SH3wHmsANRP0vw7+z8OOnpGLJh6jvnLG/UmZ6LYo+A07VqBzB0ksRrnzOrStG+lZAhAdnL3bw3wckV/3+Atjslx9Q5pk= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2020 23:10:49.4911 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: c3daa445-e974-494d-217d-08d7dda468a7 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2677 Subject: Re: [dpdk-dev] [PATCH v3 3/9] ring: introduce RTS ring mode 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" > Subject: RE: [PATCH v3 3/9] ring: introduce RTS ring mode >=20 > > > Introduce relaxed tail sync (RTS) mode for MT ring synchronization. > > > Aim to reduce stall times in case when ring is used on overcommited > > > cpus (multiple active threads on the same cpu). > > > The main difference from original MP/MC algorithm is that tail value > > > is increased not by every thread that finished enqueue/dequeue, but > > > only by the last one. > > > That allows threads to avoid spinning on ring tail value, leaving > > > actual tail value change to the last thread in the update queue. > > > > > > check-abi.sh reports what I believe is a false-positive about ring > > > cons/prod changes. As a workaround, devtools/libabigail.abignore is > > > updated to suppress *struct ring* related errors. > > This can be removed from the commit message. > > > > > > > > Signed-off-by: Konstantin Ananyev > > > --- > > > devtools/libabigail.abignore | 7 + > > > lib/librte_ring/Makefile | 5 +- > > > lib/librte_ring/meson.build | 5 +- > > > lib/librte_ring/rte_ring.c | 100 +++++++- > > > lib/librte_ring/rte_ring.h | 110 ++++++++- > > > lib/librte_ring/rte_ring_elem.h | 86 ++++++- > > > lib/librte_ring/rte_ring_rts.h | 316 +++++++++++++++++++++++= ++ > > > lib/librte_ring/rte_ring_rts_elem.h | 205 ++++++++++++++++ > > > lib/librte_ring/rte_ring_rts_generic.h | 210 ++++++++++++++++ > > > 9 files changed, 1015 insertions(+), 29 deletions(-) create mode > > > 100644 lib/librte_ring/rte_ring_rts.h create mode 100644 > > > lib/librte_ring/rte_ring_rts_elem.h > > > create mode 100644 lib/librte_ring/rte_ring_rts_generic.h > > > > > > diff --git a/devtools/libabigail.abignore > > > b/devtools/libabigail.abignore index a59df8f13..cd86d89ca 100644 > > > --- a/devtools/libabigail.abignore > > > +++ b/devtools/libabigail.abignore > > > @@ -11,3 +11,10 @@ > > > type_kind =3D enum > > > name =3D rte_crypto_asym_xform_type > > > changed_enumerators =3D > RTE_CRYPTO_ASYM_XFORM_TYPE_LIST_END > > > +; Ignore updates of ring prod/cons > > > +[suppress_type] > > > + type_kind =3D struct > > > + name =3D rte_ring > > > +[suppress_type] > > > + type_kind =3D struct > > > + name =3D rte_event_ring > > Does this block the reporting of these structures forever? >=20 > Till we'll have a fix in libabigail, then we can remove these lines. > I don't know any better alternative. David, does this block all issues in the future for rte_ring library? >=20 > > > > > diff --git a/lib/librte_ring/Makefile b/lib/librte_ring/Makefile > > > index 917c560ad..8f5c284cc 100644 > > > --- a/lib/librte_ring/Makefile > > > +++ b/lib/librte_ring/Makefile > > > @@ -18,6 +18,9 @@ SRCS-$(CONFIG_RTE_LIBRTE_RING) :=3D rte_ring.c > > > SYMLINK-$(CONFIG_RTE_LIBRTE_RING)-include :=3D rte_ring.h \ > > > rte_ring_elem.h \ > > > rte_ring_generic.h \ > > > - rte_ring_c11_mem.h > > > + rte_ring_c11_mem.h \ > > > + rte_ring_rts.h \ > > > + rte_ring_rts_elem.h \ > > > + rte_ring_rts_generic.h > > > > > > include $(RTE_SDK)/mk/rte.lib.mk > > > diff --git a/lib/librte_ring/meson.build > > > b/lib/librte_ring/meson.build index f2f3ccc88..612936afb 100644 > > > --- a/lib/librte_ring/meson.build > > > +++ b/lib/librte_ring/meson.build > > > @@ -5,7 +5,10 @@ sources =3D files('rte_ring.c') headers =3D files('= rte_ring.h', > > > 'rte_ring_elem.h', > > > 'rte_ring_c11_mem.h', > > > - 'rte_ring_generic.h') > > > + 'rte_ring_generic.h', > > > + 'rte_ring_rts.h', > > > + 'rte_ring_rts_elem.h', > > > + 'rte_ring_rts_generic.h') > > > > > > # rte_ring_create_elem and rte_ring_get_memsize_elem are > > > experimental allow_experimental_apis =3D true diff --git > > > a/lib/librte_ring/rte_ring.c b/lib/librte_ring/rte_ring.c index > > > fa5733907..222eec0fb 100644 > > > --- a/lib/librte_ring/rte_ring.c > > > +++ b/lib/librte_ring/rte_ring.c > > > @@ -45,6 +45,9 @@ EAL_REGISTER_TAILQ(rte_ring_tailq) > > > /* true if x is a power of 2 */ > > > #define POWEROF2(x) ((((x)-1) & (x)) =3D=3D 0) > > > > > > +/* by default set head/tail distance as 1/8 of ring capacity */ > > > +#define HTD_MAX_DEF 8 > > > + > > > /* return the size of memory occupied by a ring */ ssize_t > > > rte_ring_get_memsize_elem(unsigned int esize, unsigned int count) @@ > > > - > > > 79,11 +82,84 @@ rte_ring_get_memsize(unsigned int count) > > > return rte_ring_get_memsize_elem(sizeof(void *), count); } > > > > > > +/* > > > + * internal helper function to reset prod/cons head-tail values. > > > + */ > > > +static void > > > +reset_headtail(void *p) > > > +{ > > > + struct rte_ring_headtail *ht; > > > + struct rte_ring_rts_headtail *ht_rts; > > > + > > > + ht =3D p; > > > + ht_rts =3D p; > > > + > > > + switch (ht->sync_type) { > > > + case RTE_RING_SYNC_MT: > > > + case RTE_RING_SYNC_ST: > > > + ht->head =3D 0; > > > + ht->tail =3D 0; > > > + break; > > > + case RTE_RING_SYNC_MT_RTS: > > > + ht_rts->head.raw =3D 0; > > > + ht_rts->tail.raw =3D 0; > > > + break; > > > + default: > > > + /* unknown sync mode */ > > > + RTE_ASSERT(0); > > > + } > > > +} > > > + > > > void > > > rte_ring_reset(struct rte_ring *r) > > > { > > > - r->prod.head =3D r->cons.head =3D 0; > > > - r->prod.tail =3D r->cons.tail =3D 0; > > > + reset_headtail(&r->prod); > > > + reset_headtail(&r->cons); > > > +} > > > + > > > +/* > > > + * helper function, calculates sync_type values for prod and cons > > > + * based on input flags. Returns zero at success or negative > > > + * errno value otherwise. > > > + */ > > > +static int > > > +get_sync_type(uint32_t flags, enum rte_ring_sync_type *prod_st, > > > + enum rte_ring_sync_type *cons_st) > > > +{ > > > + static const uint32_t prod_st_flags =3D > > > + (RING_F_SP_ENQ | RING_F_MP_RTS_ENQ); > > > + static const uint32_t cons_st_flags =3D > > > + (RING_F_SC_DEQ | RING_F_MC_RTS_DEQ); > > > + > > > + switch (flags & prod_st_flags) { > > > + case 0: > > > + *prod_st =3D RTE_RING_SYNC_MT; > > > + break; > > > + case RING_F_SP_ENQ: > > > + *prod_st =3D RTE_RING_SYNC_ST; > > > + break; > > > + case RING_F_MP_RTS_ENQ: > > > + *prod_st =3D RTE_RING_SYNC_MT_RTS; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + switch (flags & cons_st_flags) { > > > + case 0: > > > + *cons_st =3D RTE_RING_SYNC_MT; > > > + break; > > > + case RING_F_SC_DEQ: > > > + *cons_st =3D RTE_RING_SYNC_ST; > > > + break; > > > + case RING_F_MC_RTS_DEQ: > > > + *cons_st =3D RTE_RING_SYNC_MT_RTS; > > > + break; > > > + default: > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > } > > > > > > int > > > @@ -100,16 +176,20 @@ rte_ring_init(struct rte_ring *r, const char > > > *name, unsigned count, > > > RTE_BUILD_BUG_ON((offsetof(struct rte_ring, prod) & > > > RTE_CACHE_LINE_MASK) !=3D 0); > > > > > > + RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, sync_type) !=3D > > > + offsetof(struct rte_ring_rts_headtail, sync_type)); > > > + RTE_BUILD_BUG_ON(offsetof(struct rte_ring_headtail, tail) !=3D > > > + offsetof(struct rte_ring_rts_headtail, tail.val.pos)); > > > + > > > /* init the ring structure */ > > > memset(r, 0, sizeof(*r)); > > > ret =3D strlcpy(r->name, name, sizeof(r->name)); > > > if (ret < 0 || ret >=3D (int)sizeof(r->name)) > > > return -ENAMETOOLONG; > > > r->flags =3D flags; > > > - r->prod.sync_type =3D (flags & RING_F_SP_ENQ) ? > > > - RTE_RING_SYNC_ST : RTE_RING_SYNC_MT; > > > - r->cons.sync_type =3D (flags & RING_F_SC_DEQ) ? > > > - RTE_RING_SYNC_ST : RTE_RING_SYNC_MT; > > > + ret =3D get_sync_type(flags, &r->prod.sync_type, &r->cons.sync_type= ); > > > + if (ret !=3D 0) > > > + return ret; > > > > > > if (flags & RING_F_EXACT_SZ) { > > > r->size =3D rte_align32pow2(count + 1); @@ -126,8 +206,12 > @@ > > > rte_ring_init(struct rte_ring *r, const char *name, unsigned count, > > > r->mask =3D count - 1; > > > r->capacity =3D r->mask; > > > } > > > - r->prod.head =3D r->cons.head =3D 0; > > > - r->prod.tail =3D r->cons.tail =3D 0; > > > + > > > + /* set default values for head-tail distance */ > > > + if (flags & RING_F_MP_RTS_ENQ) > > > + rte_ring_set_prod_htd_max(r, r->capacity / HTD_MAX_DEF); > > > + if (flags & RING_F_MC_RTS_DEQ) > > > + rte_ring_set_cons_htd_max(r, r->capacity / HTD_MAX_DEF); > > > > > > return 0; > > > } > > > diff --git a/lib/librte_ring/rte_ring.h b/lib/librte_ring/rte_ring.h > > > index > > > d4775a063..f6f084d79 100644 > > > --- a/lib/librte_ring/rte_ring.h > > > +++ b/lib/librte_ring/rte_ring.h > > > @@ -48,6 +48,7 @@ extern "C" { > > > #include > > > #include > > > #include > > > +#include > > > > > > #define RTE_TAILQ_RING_NAME "RTE_RING" > > > > > > @@ -65,10 +66,13 @@ enum rte_ring_queue_behavior { enum > > > rte_ring_sync_type { > > > RTE_RING_SYNC_MT, /**< multi-thread safe (default mode) */ > > > RTE_RING_SYNC_ST, /**< single thread only */ > > > +#ifdef ALLOW_EXPERIMENTAL_API > > > + RTE_RING_SYNC_MT_RTS, /**< multi-thread relaxed tail sync */ > > > #endif > > > }; > > > > > > /** > > > - * structure to hold a pair of head/tail values and other metadata. > > > + * structures to hold a pair of head/tail values and other metadata. > > > * Depending on sync_type format of that structure might be differen= t, > > > * but offset for *sync_type* and *tail* values should remain the sa= me. > > > */ > > > @@ -84,6 +88,21 @@ struct rte_ring_headtail { > > > }; > > > }; > > > > > > +union rte_ring_ht_poscnt { > > nit, this is specific to RTS, may be change this to rte_ring_rts_ht_pos= cnt? >=20 > Ok. >=20 > > > > > + uint64_t raw; > > > + struct { > > > + uint32_t cnt; /**< head/tail reference counter */ > > > + uint32_t pos; /**< head/tail position */ > > > + } val; > > > +}; > > > + > > > +struct rte_ring_rts_headtail { > > > + volatile union rte_ring_ht_poscnt tail; > > > + enum rte_ring_sync_type sync_type; /**< sync type of prod/cons */ > > > + uint32_t htd_max; /**< max allowed distance between head/tail */ > > > + volatile union rte_ring_ht_poscnt head; }; > > > + > > > /** > > > * An RTE ring structure. > > > * > > > @@ -111,11 +130,21 @@ struct rte_ring { > > > char pad0 __rte_cache_aligned; /**< empty cache line */ > > > > > > /** Ring producer status. */ > > > - struct rte_ring_headtail prod __rte_cache_aligned; > > > + RTE_STD_C11 > > > + union { > > > + struct rte_ring_headtail prod; > > > + struct rte_ring_rts_headtail rts_prod; > > > + } __rte_cache_aligned; > > > + > > > char pad1 __rte_cache_aligned; /**< empty cache line */ > > > > > > /** Ring consumer status. */ > > > - struct rte_ring_headtail cons __rte_cache_aligned; > > > + RTE_STD_C11 > > > + union { > > > + struct rte_ring_headtail cons; > > > + struct rte_ring_rts_headtail rts_cons; > > > + } __rte_cache_aligned; > > > + > > > char pad2 __rte_cache_aligned; /**< empty cache line */ }; > > > > > > @@ -132,6 +161,9 @@ struct rte_ring { #define RING_F_EXACT_SZ > > > 0x0004 #define RTE_RING_SZ_MASK (0x7fffffffU) /**< Ring size mask > > > */ > > > > > > +#define RING_F_MP_RTS_ENQ 0x0008 /**< The default enqueue is "MP > RTS". > > > +*/ #define RING_F_MC_RTS_DEQ 0x0010 /**< The default dequeue is > "MC > > > +RTS". */ > > > + > > > #define __IS_SP RTE_RING_SYNC_ST > > > #define __IS_MP RTE_RING_SYNC_MT > > > #define __IS_SC RTE_RING_SYNC_ST > > > @@ -461,6 +493,10 @@ rte_ring_sp_enqueue_bulk(struct rte_ring *r, > > > void * const *obj_table, > > > RTE_RING_SYNC_ST, free_space); > > > } > > > > > > +#ifdef ALLOW_EXPERIMENTAL_API > > > +#include > > > +#endif > > > + > > > /** > > > * Enqueue several objects on a ring. > > > * > > > @@ -484,8 +520,21 @@ static __rte_always_inline unsigned int > > > rte_ring_enqueue_bulk(struct rte_ring *r, void * const *obj_table, > > > unsigned int n, unsigned int *free_space) { > > > - return __rte_ring_do_enqueue(r, obj_table, n, > > > RTE_RING_QUEUE_FIXED, > > > - r->prod.sync_type, free_space); > > > + switch (r->prod.sync_type) { > > > + case RTE_RING_SYNC_MT: > > > + return rte_ring_mp_enqueue_bulk(r, obj_table, n, free_space); > > > + case RTE_RING_SYNC_ST: > > > + return rte_ring_sp_enqueue_bulk(r, obj_table, n, free_space); > > Have you validated if these affect the performance for the existing API= s? >=20 > I run ring_pmd_perf_autotest > (AFAIK, that's the only one of our perf tests that calls > rte_ring_enqueue/dequeue), and didn't see any real difference in perf > numbers. >=20 > > I am also wondering why should we support these new modes in the legacy > APIs? >=20 > Majority of DPDK users still do use legacy API, and I am not sure all of = them > will be happy to switch to _elem_ one manually. > Plus I can't see how we can justify that after let say: > rte_ring_init(ring, ..., RING_F_MP_HTS_ENQ | RING_F_MC_HTS_DEQ); returns > with success valid call to rte_ring_enqueue(ring,...) should fail. Agree, I think the only way right now is through documentation. >=20 > > I think users should move to use rte_ring_xxx_elem APIs. If users want = to > use RTS/HTS it will be a good time for them to move to new APIs. >=20 > If they use rte_ring_enqueue/dequeue all they have to do - just change fl= ags > in ring_create/ring_init call. > With what you suggest - they have to change every > rte_ring_enqueue/dequeue to rte_ring_elem_enqueue/dequeue. > That's much bigger code churn. But these are just 1 to 1 mapped. I would think, there are not a whole lot= of them in the application code, may be ~10 lines? I think the bigger factor for the user here is the algorithm changes in rte= _ring library. Bigger effort for the users would be testing rather than cod= e changes in the applications.=20 >=20 > > They anyway have to test their code for RTS/HTS, might as well make the > change to new APIs and test both. > > It will be less code to maintain for the community as well. >=20 > That's true, right now there is a lot of duplication between _elem_ and l= egacy > code. > Actually the only real diff between them - actual copying of the objects= . > But I thought we are going to deal with that, just by changing one day a= ll > legacy API to wrappers around _elem_ calls, i.e something like: >=20 > static __rte_always_inline unsigned int > rte_ring_enqueue_bulk(struct rte_ring *r, void * const *obj_table, > unsigned int n, unsigned int *free_space) { > return rte_ring_enqueue_elem_bulk(r, obj_table, sizeof(uintptr_t), n, > free_space); } >=20 > That way users will switch to new API automatically, without any extra ef= fort > for them, and we will be able to remove legacy code. > Do you have some other thoughts here how to deal with this legacy/elem > conversion? Yes, that is what I was thinking, but had not considered any addition of ne= w APIs. But, I am wondering if we should look at deprecation? If we decide to depre= cate, it would be good to avoid making the users of RTS/HTS do the work twi= ce (once to make the switch to RTS/HTS and then another to _elem_ APIs). One thing we can do is to implement the wrappers you mentioned above for RT= S/HTS now. I also it is worth considering to switch to these wrappers 20.05= so that come 20.11, we have a code base that has gone through couple of re= leases' testing. =20 >=20 > > > > > #ifdef > > > +ALLOW_EXPERIMENTAL_API > > > + case RTE_RING_SYNC_MT_RTS: > > > + return rte_ring_mp_rts_enqueue_bulk(r, obj_table, n, > > > + free_space); > > > +#endif > > > + } > > > + > > > + /* valid ring should never reach this point */ > > > + RTE_ASSERT(0); > > > + return 0; > > > } > > > > > > > > > #ifdef __cplusplus > > > diff --git a/lib/librte_ring/rte_ring_elem.h > > > b/lib/librte_ring/rte_ring_elem.h index 28f9836e6..5de0850dc 100644 > > > --- a/lib/librte_ring/rte_ring_elem.h > > > +++ b/lib/librte_ring/rte_ring_elem.h > > > @@ -542,6 +542,8 @@ rte_ring_sp_enqueue_bulk_elem(struct rte_ring > > > *r, const void *obj_table, > > > RTE_RING_QUEUE_FIXED, __IS_SP, free_space); } > > > > > > +#include > > > > > > #ifdef __cplusplus > > > diff --git a/lib/librte_ring/rte_ring_rts.h > > > b/lib/librte_ring/rte_ring_rts.h new file mode 100644 index > > > 000000000..18404fe48 > > > --- /dev/null > > > +++ b/lib/librte_ring/rte_ring_rts.h > > IMO, we should not provide these APIs. >=20 > You mean only _elem_ ones, as discussed above? Yes >=20 > > > > > @@ -0,0 +1,316 @@ > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > + * > > > + * Copyright (c) 2010-2017 Intel Corporation > > nit, the year should change to 2020? Look at others too. >=20 > ack, will do. >=20 > > > > > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org > > > + * All rights reserved. > > > + * Derived from FreeBSD's bufring.h > > > + * Used as BSD-3 Licensed with permission from Kip Macy. > > > + */ > > > + > > > +#ifndef _RTE_RING_RTS_H_ > > > +#define _RTE_RING_RTS_H_ > > > + > > > +/** > > > + * @file rte_ring_rts.h > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > + * It is not recommended to include this file directly. > > > + * Please include instead. > > > + * > > > + * Contains functions for Relaxed Tail Sync (RTS) ring mode. > > > + * The main idea remains the same as for our original MP/MC > > > > ^^^ the > > > +synchronization > > > + * mechanism. > > > + * The main difference is that tail value is increased not > > > + * by every thread that finished enqueue/dequeue, > > > + * but only by the last one doing enqueue/dequeue. > > should we say 'current last' or 'last thread at a given instance'? > > > > > + * That allows threads to skip spinning on tail value, > > > + * leaving actual tail value change to last thread in the update que= ue. > > nit, I understand what you mean by 'update queue' here. IMO, we should > remove it as it might confuse some. > > > > > + * RTS requires 2 64-bit CAS for each enqueue(/dequeue) operation: > > > + * one for head update, second for tail update. > > > + * As a gain it allows thread to avoid spinning/waiting on tail valu= e. > > > + * In comparision original MP/MC algorithm requires one 32-bit CAS > > > + * for head update and waiting/spinning on tail value. > > > + * > > > + * Brief outline: > > > + * - introduce refcnt for both head and tail. > > Suggesting using the same names as used in the structures. > > > > > + * - increment head.refcnt for each head.value update > > > + * - write head:value and head:refcnt atomically (64-bit CAS) > > > + * - move tail.value ahead only when tail.refcnt + 1 =3D=3D > > > + head.refcnt > > May be add '(indicating that this is the last thread updating the tail)= ' > > > > > + * - increment tail.refcnt when each enqueue/dequeue op finishes > > May be add 'otherwise' at the beginning. > > > > > + * (no matter is tail:value going to change or not) > > nit ^^ if > > > + * - write tail.value and tail.recnt atomically (64-bit CAS) > > > + * > > > + * To avoid producer/consumer starvation: > > > + * - limit max allowed distance between head and tail value (HTD_MA= X). > > > + * I.E. thread is allowed to proceed with changing head.value, > > > + * only when: head.value - tail.value <=3D HTD_MAX > > > + * HTD_MAX is an optional parameter. > > > + * With HTD_MAX =3D=3D 0 we'll have fully serialized ring - > > > + * i.e. only one thread at a time will be able to enqueue/dequeue > > > + * to/from the ring. > > > + * With HTD_MAX >=3D ring.capacity - no limitation. > > > + * By default HTD_MAX =3D=3D ring.capacity / 8. > > > + */ > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +#include > > > + > > > + > > > +#endif /* _RTE_RING_RTS_H_ */ > > > diff --git a/lib/librte_ring/rte_ring_rts_elem.h > > > b/lib/librte_ring/rte_ring_rts_elem.h > > > new file mode 100644 > > > index 000000000..71a331b23 > > > --- /dev/null > > > +++ b/lib/librte_ring/rte_ring_rts_elem.h > > > @@ -0,0 +1,205 @@ > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > + * > > > + * Copyright (c) 2010-2017 Intel Corporation > > > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org > > > + * All rights reserved. > > > + * Derived from FreeBSD's bufring.h > > > + * Used as BSD-3 Licensed with permission from Kip Macy. > > > + */ > > > + > > > +#ifndef _RTE_RING_RTS_ELEM_H_ > > > +#define _RTE_RING_RTS_ELEM_H_ > > > + > > > +/** > > > + * @file rte_ring_rts_elem.h > > > + * @b EXPERIMENTAL: this API may change without prior notice > > > + * > > > + * It is not recommended to include this file directly. > > > + * Please include instead. > > > + * Contains *ring_elem* functions for Relaxed Tail Sync (RTS) ring m= ode. > > > + * for more details please refer to . > > > + */ > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +#include > > > + > > > +/** > > > + * @internal Enqueue several objects on the RTS ring. > > > + * > > > + * @param r > > > + * A pointer to the ring structure. > > > + * @param obj_table > > > + * A pointer to a table of void * pointers (objects). > > > + * @param n > > > + * The number of objects to add in the ring from the obj_table. > > > + * @param behavior > > > + * RTE_RING_QUEUE_FIXED: Enqueue a fixed number of items from a > ring > > > + * RTE_RING_QUEUE_VARIABLE: Enqueue as many items as possible > from > > > ring > > > + * @param free_space > > > + * returns the amount of space after the enqueue operation has fin= ished > > > + * @return > > > + * Actual number of objects enqueued. > > > + * If behavior =3D=3D RTE_RING_QUEUE_FIXED, this will be 0 or n on= ly. > > > + */ > > > +static __rte_always_inline unsigned int > > > +__rte_ring_do_rts_enqueue_elem(struct rte_ring *r, void * const > > > +*obj_table, > > obj_table should be of type 'const void * obj_table' (looks like copy p= aste > error). Please check the other APIs below too. > > > > > + uint32_t esize, uint32_t n, enum rte_ring_queue_behavior behavior, > > 'esize' is not documented in the comments above the function. You can > > copy the header from rte_ring_elem.h file. Please check other APIs as w= ell. >=20 > Ack to both, will fix. >=20 > > > > > + uint32_t *free_space) > > > +{ > > > + uint32_t free, head; > > > + > > > + n =3D __rte_ring_rts_move_prod_head(r, n, behavior, &head, &free); > > > + > > > + if (n !=3D 0) { > > > + __rte_ring_enqueue_elems(r, head, obj_table, esize, n); > > > + __rte_ring_rts_update_tail(&r->rts_prod); > > > + } > > > + > > > + if (free_space !=3D NULL) > > > + *free_space =3D free - n; > > > + return n; > > > +} > > > + > > > + > > > +#endif /* _RTE_RING_RTS_ELEM_H_ */ > > > diff --git a/lib/librte_ring/rte_ring_rts_generic.h > > > b/lib/librte_ring/rte_ring_rts_generic.h > > > new file mode 100644 > > > index 000000000..f88460d47 > > > --- /dev/null > > > +++ b/lib/librte_ring/rte_ring_rts_generic.h > > I do not know the benefit to providing the generic version. Do you know > why this was done in the legacy APIs? >=20 > I think at first we had generic API only, then later C11 was added. > As I remember, C11 one on IA was measured as a bit slower then generic, > so it was decided to keep both. >=20 > > If there is no performance difference between generic and C11 versions, > should we just skip the generic version? > > The oldest compiler in CI are GCC 4.8.5 and Clang 3.4.2 and C11 built-i= ns > are supported earlier than these compiler versions. > > I feel the code is growing exponentially in rte_ring library and we sho= uld try > to cut non-value-ass code/APIs aggressively. >=20 > I'll check is there perf difference for RTS and HTS between generic and C= 11 > versions on IA. > Meanwhile please have a proper look at C11 implementation, I am not that > familiar with C11 atomics yet. ok > If there would be no problems with it and no noticeable diff in performan= ce - > I am ok to have for RTS/HTS modes C11 version only. >=20 > > > > > @@ -0,0 +1,210 @@ > > > +/* SPDX-License-Identifier: BSD-3-Clause > > > + * > > > + * Copyright (c) 2010-2017 Intel Corporation > > > + * Copyright (c) 2007-2009 Kip Macy kmacy@freebsd.org > > > + * All rights reserved. > > > + * Derived from FreeBSD's bufring.h > > > + * Used as BSD-3 Licensed with permission from Kip Macy. > > > + */ > > > + > > > +#ifndef _RTE_RING_RTS_GENERIC_H_ > > > +#define _RTE_RING_RTS_GENERIC_H_ > > > + > > > +/** > > > + * @file rte_ring_rts_generic.h > > > + * It is not recommended to include this file directly, > > > + * include instead. > > > + * Contains internal helper functions for Relaxed Tail Sync (RTS) ri= ng > mode. > > > + * For more information please refer to . > > > + */