From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 4193FA00E6
	for <public@inbox.dpdk.org>; Wed, 17 Apr 2019 03:45:16 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 3BFAF1B57F;
	Wed, 17 Apr 2019 03:45:15 +0200 (CEST)
Received: from EUR04-VI1-obe.outbound.protection.outlook.com
 (mail-eopbgr80052.outbound.protection.outlook.com [40.107.8.52])
 by dpdk.org (Postfix) with ESMTP id 984BA1B57E
 for <dev@dpdk.org>; Wed, 17 Apr 2019 03:45:13 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; 
 s=selector1-arm-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=XkH4gGjCRFlzAfZ5SYXdE2qeemgnM+pd38BbkzC7rBk=;
 b=D6z09HY4IXHJPAX8n/fw0pV11MFSf+m180Wn/W3XQXhaNQ7FIOrrpz8dP+Oh7gO+w2LdwNGCaSZS7x6Zsmh4PB7jRdNBmWlzR7vW2GxbRc2R/nFFJLdLXiCj6NacVvrxkRULOhPxjwQInrl/KNMWLRgCN9BEzmm+wWUU68znPCo=
Received: from VE1PR08MB5149.eurprd08.prod.outlook.com (20.179.30.152) by
 VE1PR08MB4702.eurprd08.prod.outlook.com (10.255.114.146) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.1792.18; Wed, 17 Apr 2019 01:45:11 +0000
Received: from VE1PR08MB5149.eurprd08.prod.outlook.com
 ([fe80::e0ae:ecad:ec5:8177]) by VE1PR08MB5149.eurprd08.prod.outlook.com
 ([fe80::e0ae:ecad:ec5:8177%2]) with mapi id 15.20.1792.018; Wed, 17 Apr 2019
 01:45:11 +0000
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: Stephen Hemminger <stephen@networkplumber.org>
CC: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
 "paulmck@linux.ibm.com" <paulmck@linux.ibm.com>, "Kovacevic, Marko"
 <marko.kovacevic@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, "Gavin Hu (Arm
 Technology China)" <Gavin.Hu@arm.com>, Dharmik Thakkar
 <Dharmik.Thakkar@arm.com>, Malvika Gupta <Malvika.Gupta@arm.com>, Honnappa
 Nagarahalli <Honnappa.Nagarahalli@arm.com>, nd <nd@arm.com>, nd <nd@arm.com>
Thread-Topic: [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism
Thread-Index: AQHU8XwL9ZW9vcwT20K3DuZSwCCMIqY5FWOAgASrdlSAAIGtIIAAowaAgAAhIcCAAEs7gIAAIfHQ
Date: Wed, 17 Apr 2019 01:45:11 +0000
Message-ID:
 <VE1PR08MB514983BE886E05A7CAB277D298250@VE1PR08MB5149.eurprd08.prod.outlook.com>
References: <20181122033055.3431-1-honnappa.nagarahalli@arm.com>
 <20190412202039.46902-1-honnappa.nagarahalli@arm.com>
 <20190412202039.46902-2-honnappa.nagarahalli@arm.com>
 <20190412150650.3709358e@shemminger-XPS-13-9360>
 <VE1PR08MB51497C4B9E164CD07AC6EB5298280@VE1PR08MB5149.eurprd08.prod.outlook.com>
 <20190412160629.670eacd1@shemminger-XPS-13-9360>
 <2601191342CEEE43887BDE71AB9772580148A97E53@irsmsx105.ger.corp.intel.com>
 <20190415083834.31b38ed3@shemminger-XPS-13-9360>
 <2601191342CEEE43887BDE71AB9772580148A98064@irsmsx105.ger.corp.intel.com>
 <20190415142631.4c250248@shemminger-XPS-13-9360>
 <VE1PR08MB5149AA5C4CAC61DB42E286CF98240@VE1PR08MB5149.eurprd08.prod.outlook.com>
 <20190416075415.76c9d64a@xps13.lan>
 <VE1PR08MB5149930C88AA75010836986598240@VE1PR08MB5149.eurprd08.prod.outlook.com>
 <20190416142205.5d683e8e@xps13.lan>
In-Reply-To: <20190416142205.5d683e8e@xps13.lan>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=Honnappa.Nagarahalli@arm.com; 
x-originating-ip: [217.140.111.135]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 148f61f5-ce6f-43d3-c17e-08d6c2d6545b
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
 RULEID:(2390118)(7020095)(4652040)(8989299)(5600140)(711020)(4605104)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020);
 SRVR:VE1PR08MB4702; 
x-ms-traffictypediagnostic: VE1PR08MB4702:
x-ms-exchange-purlcount: 2
nodisclaimer: True
x-microsoft-antispam-prvs: <VE1PR08MB47023181953BE4EB0177022F98250@VE1PR08MB4702.eurprd08.prod.outlook.com>
x-forefront-prvs: 0010D93EFE
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(136003)(346002)(39860400002)(396003)(366004)(376002)(199004)(189003)(52536014)(81156014)(8676002)(3846002)(6116002)(81166006)(86362001)(305945005)(478600001)(71200400001)(14454004)(99286004)(71190400001)(347745004)(72206003)(966005)(68736007)(74316002)(316002)(6246003)(6306002)(33656002)(106356001)(55016002)(7736002)(9686003)(11346002)(6506007)(105586002)(476003)(446003)(6436002)(2906002)(25786009)(93886005)(53936002)(8936002)(186003)(76176011)(54906003)(97736004)(6916009)(102836004)(26005)(5660300002)(229853002)(16799955002)(4326008)(256004)(486006)(7696005)(14444005)(66066001);
 DIR:OUT; SFP:1101; SCL:1; SRVR:VE1PR08MB4702;
 H:VE1PR08MB5149.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; A:1; MX:1; 
received-spf: None (protection.outlook.com: arm.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: fC0beB+ygIJ/juUtvFMBkuiMLNrOqPR44v8DOQ6SypzGKab4vPHc8I9oKhZnMXMXphoFUGh8RBCmzAcVbf17Zo9oWF8alK7dOEiDXb/MUul3tJZi4oNJVUbNPiY3Mq70JPgY0Wv/sblSrhnf94G04gan5pjYD1FAbQxk8JO3ZYyH4btm7BT8ylbwCBYVkw+jhGG2eagcuf5VG2Y9YuVPbkck+JhhzhCM6ddOOF7IoHdDVFddHO+82BxNWRUN47iyW7/2TQlZhVa47l2D0CotcdY0AvfGpSejhUzdb8o5J+U7iMVVrJjx7DkDBcTPHz1njsn6hPQ/6rQDLQ/VSag060+QAJ8bU+nTNWkBN5lfXFwDAogJmk1gJ1Ucsx2T83KKCIDIUdgEYFVpYsk7ah9ihqKQabWNqKm098OsZD84+TE=
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 148f61f5-ce6f-43d3-c17e-08d6c2d6545b
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2019 01:45:11.1982 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR08MB4702
Subject: Re: [dpdk-dev] [PATCH v5 1/3] rcu: add RCU library supporting QSBR
	mechanism
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190417014511.ulnEVEEGZL3kn-5yvnnOSoKqe2KROZn42o1lSQJkyRk@z>

> > > > > > > > > > >
> > > > > > > > > > > After evaluating long term API/ABI issues, I think
> > > > > > > > > > > you need to get rid of almost all use of inline and
> > > > > > > > > > > visible structures. Yes it might be marginally
> > > > > > > > > > > slower, but you thank me
> > > > > the first time you have to fix something.
> > > > > > > > > > >
> > > > > > > > > > Agree, I was planning on another version to address
> > > > > > > > > > this (I am yet
> > > > > to take a look at your patch addressing the ABI).
> > > > > > > > > > The structure visibility definitely needs to be address=
ed.
> > > > > > > > > > For the inline functions, is the plan to convert all
> > > > > > > > > > the inline functions in DPDK? If yes, I think we need
> > > > > > > > > > to consider the performance
> > > > > > > > > difference. May be consider L3-fwd application, change
> > > > > > > > > all the
> > > > > inline functions in its path and run a test?
> > > > > > > > >
> > > > > > > > > Every function that is not in the direct datapath should
> > > > > > > > > not be
> > > > > inline.
> > > > > > > > > Exceptions or things like rx/tx burst, ring
> > > > > > > > > enqueue/dequeue, and packet alloc/free
> > > > I do not understand how DPDK can claim ABI compatibility if we
> > > > have
> > > inline functions (unless we freeze any development in these inline
> > > functions forever).
> > > >
> > > > > > > >
> > > > > > > > Plus synchronization routines: spin/rwlock/barrier, etc.
> > > > > > > > I think rcu should be one of such exceptions - it is just
> > > > > > > > another synchronization mechanism after all (just a bit
> > > > > > > > more
> > > sophisticated).
> > > > > > > > Konstantin
> > > > > > >
> > > > > > > If you look at the other userspace RCU, you wil see that the
> > > > > > > only inlines are the rcu_read_lock,rcu_read_unlock and
> > > > > rcu_reference/rcu_assign_pointer.
> > > > > > >
> > > > > > > The synchronization logic is all real functions.
> > > > > >
> > > > > > In fact, I think urcu provides both flavors:
> > > > > > https://github.com/urcu/userspace-
> > > > > rcu/blob/master/include/urcu/static/
> > > > > > urcu-qsbr.h I still don't understand why we have to treat it
> > > > > > differently then let say spin-lock/ticket-lock or rwlock.
> > > > > > If we gone all the way to create our own version of rcu, we
> > > > > > probably want it to be as fast as possible (I know that main
> > > > > > speedup should come from the fact that readers don't have to
> > > > > > wait for writer to finish, but still...)
> > > > > >
> > > > > > Konstantin
> > > > > >
> > > > >
> > > > > Having locking functions inline is already a problem in current
> releases.
> > > > > The implementation can not be improved without breaking ABI (or
> > > > > doing special workarounds like lock v2)
> > > > I think ABI and inline function discussion needs to be taken up in
> > > > a
> > > different thread.
> > > >
> > > > Currently, I am looking to hide the structure visibility. I looked
> > > > at your
> > > patch [1], it is a different case than what I have in this patch. It
> > > is a pretty generic use case as well (similar situation exists in
> > > other libraries). I think a generic solution should be agreed upon.
> > > >
> > > > If we have to hide the structure content, the handle to QS
> > > > variable
> > > returned to the application needs to be opaque. I suggest using 'void=
 *'
> > > behind which any structure can be used.
> > > >
> > > > typedef void * rte_rcu_qsbr_t;
> > > > typedef void * rte_hash_t;
> > > >
> > > > But it requires typecasting.
> > > >
> > > > [1] http://patchwork.dpdk.org/cover/52609/
> > >
> > > C allows structure to be defined without knowing what is in it
> therefore.
> > >
> > > typedef struct rte_rcu_qsbr rte_rcu_qsbr_t;
> > >
> > > is preferred (or do it without typedef)
> > >
> > > struct rte_rcu_qsbr;
> >
> > I see that rte_hash library uses the same approach (struct rte_hash in
> rte_hash.h, though it is marking as internal). But the ABI Laboratory too=
l
> [1] seems to be reporting incorrect numbers for this library even though
> the internal structure is changed.
> >
> > [1]
> > https://abi-
> laboratory.pro/index.php?view=3Dcompat_report&l=3Ddpdk&v1=3D19.0
> > 2&v2=3Dcurrent&obj=3D66794&kind=3Dabi
>=20
> The problem is rte_hash structure is exposed as part of ABI in
> rte_cuckoo_hash.h This was a mistake.
Do you mean, due to the use of structure with the same name? I am wondering=
 if it is just a tools issue. The application is not supposed to include rt=
e_cuckoo_hash.h.

For the RCU library, we either need to go all functions or leave it the way=
 it is. I do not see a point in trying to hide the internal structure while=
 having inline functions.

I converted the inline functions to function calls.

Testing on Arm platform (results *are* repeatable) shows very minimal drop =
(0.1% to 0.2%) in performance while using lock-free rte_hash data structure=
. But one of the test cases which is just spinning shows good amount of dro=
p (41%).

Testing on x86 (Xeon Gold 6132 CPU @ 2.60GHz, results *are* pretty repeatab=
le) shows performance improvements (7% to 8%) while using lock-free rte_has=
h data structure. The test cases which is just spinning show significant dr=
op (14%, 155%, 231%).

Konstantin, any thoughts on the results?

I will send out V6 which will fix issues reported so far. The function vs i=
nline part is still open, need to close it soon.