> > > On Wed, Oct 10, 2018 at 02:48:05PM -0700, Yipeng Wang wrote: > > > > This commit improves the readwrite test to consider extendable > > > > table feature and add more functional tests to cover more corner cases. > > > > > > > > Signed-off-by: Yipeng Wang <yipeng1.w...@intel.com> --- > > > > test/test/test_hash_readwrite.c | 70 > > > > ++++++++++++++++++++++++++++++++++------- 1 file changed, 58 > > > > insertions(+), 12 deletions(-) > > > > > > > With the extension of this test case, and the addition of other test > > > cases by Honnappa in the other patch sets in this release, we are > > > building up quite a large set of hash table autotests, some of whose > > > meaning and use is a bit obscure. Are there any hash tests that you > > > feel could be removed at this point, to simplify things? > > > > > (this comment does not apply to this patch) Looks like your concern is > > about maintenance of the test code. > > IMO, we need to reduce the number of configuration flags in this library > which should reduce the number of test cases. > > The flags I think that are not necessary are: > > RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT - The tests prove that > this gives significant performance boost. IMO, if the platform supports it, it > should be enabled without user consent (I am not an expert on TSX). > > RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY - Most use cases require this > support. Only use case where this is not required is a single thread doing > both > inserts and lookups. Even if such a use case is valid, the lock over head > should > be small. > > > I agree with the idea. What I suggest is that only a single flag should be > needed, and that only for the uncommon case, i.e. where we do not need any > locking of the hash-table. Otherwise the hash should be thread safe by default > and using the most effective locking mechanism for the platform. > RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF - should take care of this.
> Unfortunately, doing this requires an ABI change, but since it only should > affect the create function, it should be doable with function versioning to > keep backward compatibility. > Looks simple enough. Version the rte_hash_create function and map the existing function to 18.08. The new version of the function will always enable hw_trans_mem_support and rw_concurrency. Should we check to see if these flags are set by the user and print a warning message about deprecation of these flags in the newer version of the function? > /Bruce
