Re: [dpdk-dev] [PATCH 0/4] Address reader-writer concurrency in rte_hash

["Wang, Yipeng1" <yipeng1.wang@intel.com>]

Hi Honnappa,

Reply inlined:

>-----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 period
>       resulting in packet drops. This problem seems to apply for platforms
>       with transactional memory support as well because of the algorithm
>       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.

>    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 reference
>       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 new API functions for deletion
to users who want this behavior,
without changing the meaning of current API if that is possible.

>    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.

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?