From mboxrd@z Thu Jan 1 00:00:00 1970 From: Honnappa Nagarahalli Subject: Re: [PATCH 0/4] Address reader-writer concurrency in rte_hash Date: Fri, 28 Sep 2018 21:11:18 +0000 Message-ID: References: <1536253938-192391-1-git-send-email-honnappa.nagarahalli@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" , "honnappa.nagarahalli@dpdk.org" , "Gavin Hu (Arm Technology China)" , Steve Capper , Ola Liljedahl , nd , "Gobriel, Sameh" To: "Wang, Yipeng1" , "Richardson, Bruce" , "De Lara Guarch, Pablo" Return-path: Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20086.outbound.protection.outlook.com [40.107.2.86]) by dpdk.org (Postfix) with ESMTP id C981D1B153 for ; Fri, 28 Sep 2018 23:11:20 +0200 (CEST) In-Reply-To: Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" >=20 > Hi Honnappa, >=20 > Reply inlined: Hi Yipeng, Thank you so much for reviewing. >=20 > >-----Original Message----- > > > > Currently, reader-writer concurrency problems in rte_hash are > > addressed using reader-writer locks. Use of reader-writer locks > > results in following issues: > > > > 1) In many of the use cases for the hash table, writer threads > > are running on control plane. If the writer is preempted while > > holding the lock, it will block the readers for an extended perio= d > > resulting in packet drops. This problem seems to apply for platfo= rms > > with transactional memory support as well because of the algorith= m > > used for rte_rwlock_write_lock_tm: > > > > static inline void > > rte_rwlock_write_lock_tm(rte_rwlock_t *rwl) > > { > > if (likely(rte_try_tm(&rwl->cnt))) > > return; > > rte_rwlock_write_lock(rwl); > > } > > > > i.e. there is a posibility of using rte_rwlock_write_lock in > > failure cases. > [Wang, Yipeng] In our test, TSX failure happens very rarely on a TSX > platform. But we agree that without TSX, the current rte_rwlock > implementation may make the writer to hold a lock for a period of time. >=20 > > 2) Reader-writer lock based solution does not address the following > > issue. > > rte_hash_lookup_xxx APIs return the index of the element in > > the key store. Application(reader) can use that index to referenc= e > > other data structures in its scope. Because of this, the > > index should not be freed till the application completes > > using the index. > [Wang, Yipeng] I agree on this use case. But I think we should provide n= ew > API functions for deletion to users who want this behavior, without > changing the meaning of current API if that is possible. In the lock-free algorithm, the rte_hash_delete API will not free the index= . The new API rte_hash_free will free the index. The solution for the algor= ithm with rw locks needs to be thought about. >=20 > > Current code: > > Cores Lookup Lookup > > with add > > 2 474 246 > > 4 935 579 > > 6 1387 1048 > > 8 1766 1480 > > 10 2119 1951 > > 12 2546 2441 > > > > With this patch: > > Cores Lookup Lookup > > with add > > 2 291 211 > > 4 297 196 > > 6 304 198 > > 8 309 202 > > 10 315 205 > > 12 319 209 > > > [Wang, Yipeng] It would be good if you could provide the platform > information on these results. Apologies, I should have done that. The machine I am using is: Intel(R) Xeo= n(R) CPU E5-2660 v4 @ 2.00GHz, 64G memory. This is a hacked test case which= is not upstreamed. In the case of 'Lookup with add' - I had equal number o= f threads calling 'rte_hash_add' and 'rte_hash_lookup'. In the case of 'Loo= kup' - a set of entries were added and all the threads called 'rte_hash_loo= kup'. Note that these are the numbers without htm. We have created another = test case which I will upstream as next version of this patch. I will publi= sh the numbers with that test case. So, you should be able to reproduce the= numbers with that test case. >=20 > Another comment is as you know we also have a new extension to rte_hash > to enable extendable buckets and partial-key hashing. Thanks for the > comments btw. Could you check if your lockless scheme also applies to > those extensions? Thank you for reminding me on this. I thought I had covered everything. On = a relook, I have missed few key issues. I will reply on the other email thr= ead.