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 5014BA05D3 for ; Tue, 23 Apr 2019 03:08:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 94A1E1B485; Tue, 23 Apr 2019 03:08:32 +0200 (CEST) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20060.outbound.protection.outlook.com [40.107.2.60]) by dpdk.org (Postfix) with ESMTP id 26DE31B203 for ; Tue, 23 Apr 2019 03:08:31 +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=nQOgcZeF+yqZanpGGCAqLFcxb02B69TbN1bmbCRBoXk=; b=FF9HwGaIu22DpgFqw9FwWxj9WHXGTy3OSE8UaQ4lrOI6VvHJ8ksg/nXho8Mv+qRfVNmkHAvPwB6oaUyB2KmNrnchSOCbnnRDObA/kGI7ZjCCGMtETrcZazl3M0LzntRNDJ5hfOullm3jFaEIk94Ce5eT0ZaL888Iu5GCrRm67YA= Received: from VE1PR08MB5149.eurprd08.prod.outlook.com (20.179.30.152) by VE1PR08MB4943.eurprd08.prod.outlook.com (10.255.158.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1813.16; Tue, 23 Apr 2019 01:08:29 +0000 Received: from VE1PR08MB5149.eurprd08.prod.outlook.com ([fe80::9b6:3403:4386:78]) by VE1PR08MB5149.eurprd08.prod.outlook.com ([fe80::9b6:3403:4386:78%2]) with mapi id 15.20.1813.017; Tue, 23 Apr 2019 01:08:29 +0000 From: Honnappa Nagarahalli To: "paulmck@linux.ibm.com" CC: "konstantin.ananyev@intel.com" , "stephen@networkplumber.org" , "marko.kovacevic@intel.com" , "dev@dpdk.org" , "Gavin Hu (Arm Technology China)" , Dharmik Thakkar , Malvika Gupta , Honnappa Nagarahalli , nd , nd Thread-Topic: [PATCH v6 1/3] rcu: add RCU library supporting QSBR mechanism Thread-Index: AQHU9uTV75Dh46NGFki80//uZnyP8qZIv4zw Date: Tue, 23 Apr 2019 01:08:28 +0000 Message-ID: References: <20181122033055.3431-1-honnappa.nagarahalli@arm.com> <20190417041359.45335-1-honnappa.nagarahalli@arm.com> <20190417041359.45335-2-honnappa.nagarahalli@arm.com> <20190419191929.GG14111@linux.ibm.com> In-Reply-To: <20190419191929.GG14111@linux.ibm.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: a939d292-fa68-40dc-530d-08d6c7883256 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600141)(711020)(4605104)(4618075)(2017052603328)(7193020); SRVR:VE1PR08MB4943; x-ms-traffictypediagnostic: VE1PR08MB4943: nodisclaimer: True x-microsoft-antispam-prvs: x-forefront-prvs: 0016DEFF96 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(376002)(396003)(136003)(366004)(39860400002)(346002)(189003)(199004)(1730700003)(9686003)(93886005)(8676002)(55016002)(81166006)(81156014)(71190400001)(2351001)(33656002)(66066001)(8936002)(7736002)(229853002)(71200400001)(6436002)(478600001)(5640700003)(97736004)(305945005)(72206003)(52536014)(6916009)(2501003)(486006)(14444005)(316002)(6246003)(25786009)(256004)(14454004)(446003)(6116002)(5660300002)(74316002)(86362001)(66446008)(11346002)(3846002)(68736007)(2906002)(7696005)(186003)(73956011)(6506007)(102836004)(76116006)(4326008)(26005)(66946007)(76176011)(54906003)(476003)(53936002)(64756008)(66556008)(99286004)(66476007); DIR:OUT; SFP:1101; SCL:1; SRVR:VE1PR08MB4943; 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: hDZYUCFNsa36ZJo8BJ4hn9+Q+dUSSFUYLpmeMcQ5u5fIdvWpR+6eoNWf71hHxlJOwT5xN9rQL3VCzkIhXQVvYtKciB6uU1c5AbMqbH8mBNV+O6iXFgqZAbMy9EE2BYvXb4AmdEmcJujNEByA1BHdOGPtAO9lnXHkTcplUHsY6T/CjuRt2B4qNE168VV5ILbxDuhyV2ahR92SrhNi6wR62pfPOVAb3ZyR+9gM6TrouonOz/PQ540EVlqOjZHVErCyKHiLTl6PJFlMF6LxXhUsQfcABLiMp7ZSkz+rQ93S1n76hljnCgaWx7C6ogk8psNOxg48gXu5fFXV+DbajKISL8ZieKTq9IliabfR6rhQs4mjeK5IG+VCyPAFy32kFakb/qeXf9PaUs00kZb7E6LZE1YBwg/hYpQbOf0kv/PkKtU= 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: a939d292-fa68-40dc-530d-08d6c7883256 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Apr 2019 01:08:29.1600 (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: VE1PR08MB4943 Subject: Re: [dpdk-dev] [PATCH v6 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: <20190423010828.lp5MOIO-QPHIo-pKOVlNBlP9gANwl5LZ6qneBwls5E8@z> >=20 > On Tue, Apr 16, 2019 at 11:13:57PM -0500, Honnappa Nagarahalli wrote: > > Add RCU library supporting quiescent state based memory reclamation > method. > > This library helps identify the quiescent state of the reader threads > > so that the writers can free the memory associated with the lock less > > data structures. > > > > Signed-off-by: Honnappa Nagarahalli > > Reviewed-by: Steve Capper > > Reviewed-by: Gavin Hu > > Reviewed-by: Ola Liljedahl > > Acked-by: Konstantin Ananyev >=20 > Looks much better! >=20 > One more suggestion below, on rte_rcu_qsbr_thread_offline(). >=20 > Thanx, Paul >=20 > > diff --git a/lib/librte_rcu/rte_rcu_qsbr.h > > b/lib/librte_rcu/rte_rcu_qsbr.h new file mode 100644 index > > 000000000..73fa3354e > > --- /dev/null > > +++ b/lib/librte_rcu/rte_rcu_qsbr.h > > @@ -0,0 +1,629 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright (c) 2018 Arm Limited > > + */ > > + > > +#ifndef _RTE_RCU_QSBR_H_ > > +#define _RTE_RCU_QSBR_H_ > > + > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Add a registered reader thread, to the list of threads reporting > > +their > > + * quiescent state on a QS variable. > > + * > > + * This is implemented as a lock-free function. It is multi-thread > > + * safe. > > + * > > + * Any registered reader thread that wants to report its quiescent > > +state must > > + * call this API before calling rte_rcu_qsbr_quiescent. This can be > > +called > > + * during initialization or as part of the packet processing loop. > > + * > > + * The reader thread must call rte_rcu_thread_offline API, before > > + * calling any functions that block, to ensure that > > +rte_rcu_qsbr_check > > + * API does not wait indefinitely for the reader thread to update its = QS. > > + * > > + * The reader thread must call rte_rcu_thread_online API, after the > > +blocking > > + * function call returns, to ensure that rte_rcu_qsbr_check API > > + * waits for the reader thread to update its quiescent state. > > + * > > + * @param v > > + * QS variable > > + * @param thread_id > > + * Reader thread with this thread ID will report its quiescent state= on > > + * the QS variable. > > + */ > > +static __rte_always_inline void __rte_experimental > > +rte_rcu_qsbr_thread_online(struct rte_rcu_qsbr *v, unsigned int > > +thread_id) { > > + uint64_t t; > > + > > + RTE_ASSERT(v !=3D NULL && thread_id < v->max_threads); > > + > > + /* Copy the current value of token. > > + * The fence at the end of the function will ensure that > > + * the following will not move down after the load of any shared > > + * data structure. > > + */ > > + t =3D __atomic_load_n(&v->token, __ATOMIC_RELAXED); > > + > > + /* __atomic_store_n(cnt, __ATOMIC_RELAXED) is used to ensure > > + * 'cnt' (64b) is accessed atomically. > > + */ > > + __atomic_store_n(&v->qsbr_cnt[thread_id].cnt, > > + t, __ATOMIC_RELAXED); > > + > > + /* The subsequent load of the data structure should not > > + * move above the store. Hence a store-load barrier > > + * is required. > > + * If the load of the data structure moves above the store, > > + * writer might not see that the reader is online, even though > > + * the reader is referencing the shared data structure. > > + */ > > +#ifdef RTE_ARCH_X86_64 > > + /* rte_smp_mb() for x86 is lighter */ > > + rte_smp_mb(); > > +#else > > + __atomic_thread_fence(__ATOMIC_SEQ_CST); > > +#endif > > +} > > + > > +/** > > + * @warning > > + * @b EXPERIMENTAL: this API may change without prior notice > > + * > > + * Remove a registered reader thread from the list of threads > > +reporting their > > + * quiescent state on a QS variable. > > + * > > + * This is implemented as a lock-free function. It is multi-thread > > + * safe. > > + * > > + * This can be called during initialization or as part of the packet > > + * processing loop. > > + * > > + * The reader thread must call rte_rcu_thread_offline API, before > > + * calling any functions that block, to ensure that > > +rte_rcu_qsbr_check > > + * API does not wait indefinitely for the reader thread to update its = QS. > > + * > > + * @param v > > + * QS variable > > + * @param thread_id > > + * rte_rcu_qsbr_check API will not wait for the reader thread with > > + * this thread ID to report its quiescent state on the QS variable. > > + */ > > +static __rte_always_inline void __rte_experimental > > +rte_rcu_qsbr_thread_offline(struct rte_rcu_qsbr *v, unsigned int > > +thread_id) { > > + RTE_ASSERT(v !=3D NULL && thread_id < v->max_threads); >=20 > I suggest adding an assertion that v->qsbr_cnt[thread_id].lock_cnt is equ= al to > zero. This makes it easier to find a misplaced rte_rcu_qsbr_thread_offli= ne(). > Similar situation as the assertion that you added to rte_rcu_qsbr_quiesce= nt(). >=20 Agree, will add that. I think there is value in adding similar check to rte= _rcu_qsbr_thread_online and rte_rcu_qsbr_thread_unregister as well. Adding a check to rte_rcu_qsbr_thread_register does not hurt. > > + > > + /* The reader can go offline only after the load of the > > + * data structure is completed. i.e. any load of the > > + * data strcture can not move after this store. > > + */ > > + > > + __atomic_store_n(&v->qsbr_cnt[thread_id].cnt, > > + RTE_QSBR_CNT_THR_OFFLINE, __ATOMIC_RELEASE); } > > +