From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00068.outbound.protection.outlook.com [40.107.0.68]) by dpdk.org (Postfix) with ESMTP id 2B4314F94 for ; Mon, 15 Oct 2018 22:15:38 +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=KWxzH7XmLir85TgH6YogqWFeQ7PoMGKiWwipOJrGEwQ=; b=g6/JEDEQ1A0eaj5rAJ+DKvdd4KrnFIR3LabX9cEPzzjuFuCaA6ALSfV8VPZiWB5ZbUYzhhBxA1aw3foOY6DDquJKaBEacT7W/hUsr5UdUTAe6AAC1RzG+pYG2xGprYu8Mwy+4vzc0j6kadoBaEFeXPMthO1fwY0CjWyST8e+T6I= Received: from AM6PR08MB3672.eurprd08.prod.outlook.com (20.177.115.29) by AM6PR08MB3559.eurprd08.prod.outlook.com (20.177.114.216) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1228.26; Mon, 15 Oct 2018 20:15:36 +0000 Received: from AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::f423:e46a:a03c:e928]) by AM6PR08MB3672.eurprd08.prod.outlook.com ([fe80::f423:e46a:a03c:e928%3]) with mapi id 15.20.1228.027; Mon, 15 Oct 2018 20:15:36 +0000 From: Honnappa Nagarahalli To: "Wang, Yipeng1" , "Richardson, Bruce" , "De Lara Guarch, Pablo" CC: "dev@dpdk.org" , Dharmik Thakkar , "Gavin Hu (Arm Technology China)" , nd , "Gobriel, Sameh" Thread-Topic: [PATCH v3 1/7] hash: separate multi-writer from rw-concurrency Thread-Index: AQHUYfVZDSV5+6g5JUelBh5TJP2IcqUcWAaQgAQ0aLA= Date: Mon, 15 Oct 2018 20:15:36 +0000 Message-ID: References: <1539325918-125438-1-git-send-email-honnappa.nagarahalli@arm.com> <1539325918-125438-2-git-send-email-honnappa.nagarahalli@arm.com> In-Reply-To: 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-microsoft-exchange-diagnostics: 1; AM6PR08MB3559; 6:96AUqJcEczUWMbqPqmJjWhmbFj2vPkoI/guig4kt+AAkq+exclHLh1iS/vK06W30emUqVFLYOf87iDQuZdzKcAo7SFO7CZJbWDPC4KFgjii1XeLoDp8Dz7UoqYDkR22+DYpm8lc5nIAYBeWjohWGZnYdQzm5174jHBtLOjoW8e7Ch3Y9Qb8xTH7sMbfCNK9BhUKzr8br8pR06aIP8chQIMdbHtWmydt8AtHFY08KgC+jEHDFy4YU5wJej15KYuUYM3CZg5O0AjvdBlk3HYBRQ9pCEEH/h7vSv6BI79cg3l8hwDzCSVvP2O9swyjKXvFq/YzgIZJuT4FKjby4YCyb7yzxfV5UnsjVjpklTPsOtXVe161KJ+w6WR2gXlhOmeHfAZW+zSX6skqyFlaOh75on6p41WaPEX8GyfSxTNcSm482bp7EecFDq31OOPPH8PDRc7QmlQM2hXewzH1jWxOp8g==; 5:1fEkWISYuyLg5IL5wQhItG+VQ+1X/wkT3pmtVw6BXWiBxO9c2R7hKOSZDmK6tSlpDQegVA8cLOkYb02pX/x9QlFP9A0tD/Jej7APWV6e2GKdc/Vf9TOc/fjitJasB8XNwHIdTm75Ff0937Fy5jF9gbvxCIYYNkqgH273ZEOmPq0=; 7:FMlj1aSxfgGXNF+lRf/NXeK/2fDgmCpHERWGab6lk2oebyu2iouka4T7qsngM8Ww902s4wc999WiGrqzjthWtktUoTbfG3ML///U5h7eXiduKQ+s4TY/vXQJhVKlSNOlXgUq1s8DUnlbl06C1zGExJW2cu2V+9SRsGDbgaCa9EMNN5tA0pROvTqxXzm7WTMKuMyHAfR0TwS4/coQimyQ8py1UBigLsVY+r4GD0c1BXvT9Lxi6GnUsJ7rFS1XxhT5 x-ms-exchange-antispam-srfa-diagnostics: SOS;SOR; x-ms-office365-filtering-correlation-id: a077171e-91a6-44bf-3e58-08d632daf829 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:AM6PR08MB3559; x-ms-traffictypediagnostic: AM6PR08MB3559: nodisclaimer: True x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(228905959029699)(278428928389397)(180628864354917); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3002001)(3231355)(944501410)(52105095)(10201501046)(93006095)(93001095)(6055026)(149066)(150057)(6041310)(20161123560045)(20161123558120)(20161123562045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051); SRVR:AM6PR08MB3559; BCL:0; PCL:0; RULEID:; SRVR:AM6PR08MB3559; x-forefront-prvs: 0826B2F01B x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(39860400002)(396003)(366004)(136003)(376002)(346002)(199004)(189003)(2900100001)(4326008)(14444005)(256004)(5250100002)(66066001)(345774005)(478600001)(8936002)(81156014)(72206003)(97736004)(14454004)(33656002)(106356001)(105586002)(81166006)(8676002)(68736007)(71200400001)(71190400001)(9686003)(229853002)(7736002)(316002)(6246003)(446003)(2906002)(53936002)(6506007)(186003)(3846002)(99286004)(110136005)(54906003)(74316002)(5660300001)(11346002)(7696005)(305945005)(476003)(76176011)(6116002)(86362001)(55016002)(102836004)(486006)(6436002)(26005)(25786009); DIR:OUT; SFP:1101; SCL:1; SRVR:AM6PR08MB3559; H:AM6PR08MB3672.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-microsoft-antispam-message-info: TIA4Hf0BmD6iKpGSBB486h/rsCpFOmJ9ONwRSgYmD3hMrOHb+ZSinXKZHxmWaEzSiIcPs36G/QF8T8BW6prGw89mOZjS0R3Nm2u26eqd+x4LIcI3n3ZgrgWdWJ6I4r+7zUSDm4/BDNPaF6gXLzvxBjwaZhIeJn/InHsXCdqCbXfEKG5U01nKWvRLwncsdn7vqrtdJSYXxVW/3sV4shuJ8+GOQph2+55WDLhHfI3jGQDa1Vl9A7PMdobAHESJ6LVAf9SWYS45nExwyMOrqQ8t54di9Vajl6utAOghy5dilmYxdpddf7HgiZGoioj6lQ5JdNhU9bfT+eQLy2pZix6+R+CJzhXgKElWPwQ5HYmh7bg= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-Network-Message-Id: a077171e-91a6-44bf-3e58-08d632daf829 X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Oct 2018 20:15:36.5554 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3559 Subject: Re: [dpdk-dev] [PATCH v3 1/7] hash: separate multi-writer from rw-concurrency 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: , X-List-Received-Date: Mon, 15 Oct 2018 20:15:38 -0000 Hi Yipeng, Thank you for the review, appreciate your efforts. Thank you, Honnappa > > > >RW concurrency is required with single writer and multiple reader > >usecase as well. Hence, multi-writer should not be enabled by default > >when RW concurrency is enabled. > > > >Fixes: f2e3001b53ec ("hash: support read/write concurrency") > >Cc: yipeng1.wang@intel.com > > > >Signed-off-by: Honnappa Nagarahalli > >Reviewed-by: Gavin Hu > >--- > > lib/librte_hash/rte_cuckoo_hash.c | 27 ++++++++++++++++----------- > >lib/librte_hash/rte_cuckoo_hash.h | 2 ++ > > test/test/test_hash_readwrite.c | 6 ++++-- > > 3 files changed, 22 insertions(+), 13 deletions(-) > > > >+ uint8_t writer_takes_lock; > >+ /**< Indicates if the writer threads need to take lock */ >=20 > [Wang, Yipeng] > Our understanding is that the difference between writer_takes_lock and > multi_writer_support flag now is that for the multi-writer case we still = have > the local cache for key-data pair slot. Please correct me if I am wrong. Yes, that is correct. Need for lock and need for local cache are separated.= =20 >=20 > But the name is confusing because writer_takes_lock implies multi-writer > support. Especially the comment here says that writer needs a lock, which > means multi-writer is supported. So conceptually it does not have differe= nt > meaning than the multi_writer_support by just reading the name. >=20 > If you want to distinguish these two implementation (with vs. without cac= he), > maybe change the name of multi-writer flag to use_local_cache flag? Agree, I will change the 'multi-writer' name to 'use_local_cache'. I will k= eep the 'writer_takes_lock' flag. > And the previous locking mechanism need to enable this flag for performan= ce > reasons, while the LF does not. > Or just keep the cache for both cases, and I don't think the local cache = will > add too much overhead. Agree, it might not make much of a performance difference. My goal is to re= duce the memory used when multi-writer is not enabled. >=20 > > rte_hash_function hash_func; /**< Function used to calculate hash. > */ > > uint32_t hash_func_init_val; /**< Init value used by hash_func. */ > > rte_hash_cmp_eq_t rte_hash_custom_cmp_eq; diff --git > >a/test/test/test_hash_readwrite.c b/test/test/test_hash_readwrite.c > >index 2a4f7b9..a8fadd0 100644 > >--- a/test/test/test_hash_readwrite.c > >+++ b/test/test/test_hash_readwrite.c > >@@ -122,10 +122,12 @@ init_params(int use_htm, int use_jhash) > > if (use_htm) > > hash_params.extra_flag =3D > > RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT | > >- RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY; > >+ RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY | > >+ RTE_HASH_EXTRA_FLAGS_MULTI_WRITER_ADD; > [Wang, Yipeng] > Could you double check that if current applications do not change their c= ode, > there is no functional issue will be introduced by this change, otherwise= this > would be an API change. > I believe it will have performance implication though. It is not advertised that multi-writer add is assumed when read-write concu= rrency is enabled. I think we should be ok. The functionality will be fine. Only difference is that the local-cache wil= l not be enabled without this flag. So, yes, there will be performance impl= ication. >=20 > Otherwise I am OK with this patch.