From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id AFA33A00E6 for ; Wed, 17 Apr 2019 16:02:49 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6A2EE1B635; Wed, 17 Apr 2019 16:02:48 +0200 (CEST) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50082.outbound.protection.outlook.com [40.107.5.82]) by dpdk.org (Postfix) with ESMTP id 30CED1B627 for ; Wed, 17 Apr 2019 16:02:47 +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=6w/dcd++5KHT7L62Whcwp9nAwGMhaHYxZlNNMs/drAQ=; b=oadb/a0dwsEaxDvwED8S9AUwOAOgXx/IGs/vEkdXTmGI423q5CpUvfE23P57iDGDwAfHcfU/kKjUR2JjOmGtPPn9ppTZ4qDboCnFbhicpUdtXZD4k4CzdxSf1wpZRseOlFgvEtNPnRjgvzzZancy43HQIsfIPG1GSjmjMjkjSUM= Received: from VE1PR08MB5149.eurprd08.prod.outlook.com (20.179.30.152) by VE1PR08MB4685.eurprd08.prod.outlook.com (10.255.115.74) 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 14:02:45 +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 14:02:45 +0000 From: Honnappa Nagarahalli To: "Ananyev, Konstantin" , Stephen Hemminger CC: "paulmck@linux.ibm.com" , "Kovacevic, Marko" , "dev@dpdk.org" , "Gavin Hu (Arm Technology China)" , Dharmik Thakkar , Malvika Gupta , Honnappa Nagarahalli , nd , nd Thread-Topic: [PATCH v5 1/3] rcu: add RCU library supporting QSBR mechanism Thread-Index: AQHU8XwL9ZW9vcwT20K3DuZSwCCMIqY5FWOAgASrdlSAAIGtIIAAowaAgAAhIcCAAEs7gIAAIfHQgADvCwCAAATdcA== Date: Wed, 17 Apr 2019 14:02:44 +0000 Message-ID: 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> <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> <20190416075415.76c9d64a@xps13.lan> <20190416142205.5d683e8e@xps13.lan> <2601191342CEEE43887BDE71AB9772580148A98F26@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772580148A98F26@irsmsx105.ger.corp.intel.com> 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: 4606c124-4fec-4cb6-c7f5-08d6c33d5d9b 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:VE1PR08MB4685; x-ms-traffictypediagnostic: VE1PR08MB4685: x-ms-exchange-purlcount: 2 nodisclaimer: True x-microsoft-antispam-prvs: x-forefront-prvs: 0010D93EFE x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(396003)(346002)(39860400002)(376002)(136003)(366004)(189003)(199004)(6436002)(14454004)(102836004)(6306002)(966005)(347745004)(97736004)(72206003)(93886005)(8936002)(33656002)(478600001)(26005)(74316002)(81166006)(53936002)(316002)(76176011)(8676002)(9686003)(81156014)(6246003)(66066001)(6116002)(186003)(16799955002)(305945005)(3846002)(2906002)(6506007)(68736007)(5660300002)(55016002)(86362001)(25786009)(7736002)(110136005)(14444005)(106356001)(52536014)(105586002)(486006)(256004)(71200400001)(476003)(7696005)(229853002)(11346002)(71190400001)(4326008)(54906003)(99286004)(446003); DIR:OUT; SFP:1101; SCL:1; SRVR:VE1PR08MB4685; H:VE1PR08MB5149.eurprd08.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A: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: axdBtvCTr4tyTHMpFLpRiqwgbUpPYsKvJLyz7sQWbZKlsvl3R0tf9TNkn+gEbMBAUHtlJeS6wGN1ivIiags7nNt31MXHPpKuH6pSvIC5F64s4gSlRCdn6LvDhnL6423eQbIpdfE88lEBHj52nXGWAtz2eVoF4CImPLJI10EX6Wp9xSWIbVYqQbRaHwC5ZrcanvbzY5Fz3AZhQvHJPuSvEgbB0pvRW2JLzuL4Lwwy+d/BEUT47X4FNslqmoTDCzyGVhn8bncYxzIJ5goaaG4VlW9Q0yPlg2ImB/AKPGWhiRA5z6se6HGXxZj26E2IFQkYOTfWaHtSPDko6iE+kXqp4WLEZj4t9VPe4FTdsqTt0JAfpPJJkBdF9eTkua5fih5+sIGopofdlvcneduPQ76LhGZghEWN11Jh6LiHtWpNdR4= 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: 4606c124-4fec-4cb6-c7f5-08d6c33d5d9b X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2019 14:02:44.9144 (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: VE1PR08MB4685 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190417140244.dGz-1urR5DlDfWPPV6LbSNvHp4-oyrO6VdMQbBV_UJc@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 > addressed. > > > > > > > > > > > > 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 tool [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 > > > > > > 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 rte_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 stru= cture > 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 drop (41%). > > > > Testing on x86 (Xeon Gold 6132 CPU @ 2.60GHz, results *are* pretty > > repeatable) shows performance improvements (7% to 8%) while using > lock-free rte_hash data structure. The test cases which is just spinning > show significant drop (14%, 155%, 231%). > > Konstantin, any thoughts on the results? >=20 > The fact that function show better result than inline (even for hash) is = sort > of surprise to me. It was a surprise to me too and counter-intuitive to my understanding. =20 > Don't have any good explanation off-hand, but the actual numbers for > hash test are huge by itself... >=20 > In general, I still think that sync primitives better to stay inlined - t= here is > no much point to create ones and then figure out that no-one using them > because they are too slow. > Though if there is no real perf difference between inlined and normal - n= o > point to keep it inlined. > About RCU lib, my thought to have inlined version for 19.05 and do > further perf testing with it (as I remember there were suggestions about > using it in l3fwd for guarding routing table or so). Yes, there is more work planned to integrate the library better which might= provide more insight.=20 > If we'll find there is no real difference - move it to not-inlined versio= n in > 19.08. +1. > It is experimental for now - so could be changed without formal ABI > breakage. >=20 > Konstantin >=20 >=20