> > > >Add the flag to enable reader-writer concurrency during run time. The > >rte_hash_del_xxx APIs do not free the keystore element when this flag > >is enabled. Hence a new API, rte_hash_free_key_with_position, to free > >the key store element is added. > > > >+/** Flag to support lock free reader writer concurrency */ #define > >+RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF 0x08 > [Wang, Yipeng] It would be good to indicate that the lockless implementation > works for single writer multiple readers. Multi-writers are supported by using the rw-lock or transactional memory. Essentially, we still have single writer. This patch works fine with multi-writer as defined by ' MULTI_WRITER_ADD' flag. I have tested it as well. I will enable this test case in V2.
> Also, if people use a mix of the flags for example set both multiwriter and LF > flags, then I guess either we need to return an error or maybe multiwriter > should have higher priority. Currently the RW_CONCURRENCY will assume > MULTI_WRITER_ADD I think. As mentioned above, multi-writer and LF combination is supported. Yes, RW_CONCURRENCY currently assumes MULTI_WRITER_ADD. I think we should separate them. > >+ > > /** Signature of key that is stored internally. */ typedef uint32_t > > hash_sig_t; > > > >@@ -143,6 +148,11 @@ rte_hash_count(const struct rte_hash *h); > > * and should only be called from one thread by default. > > * Thread safety can be enabled by setting flag during > > * table creation. > >+ * When lock free reader writer concurrency is enabled, > >+ * if this API is called to update an existing entry, > >+ * the application should free any memory allocated for > >+ * previous 'data' only after all the readers have stopped > >+ * using previous 'data'. > [Wang, Yipeng] Could you be more specific on this description? > When add_key API is called, the users do not know if it will update an > existing > entry or inserting a new one, do they? I think, it will depend on the application. The applications I have worked on so far, added a hash entry as a result of receiving an event and updated it on receiving another event. I can change the comments to indicate that the applications need to be aware of add/update operations. > > > * > > * @param h > > * Hash table to add the key to. > >@@ -165,6 +175,11 @@ rte_hash_add_key_data(struct rte_hash *h, const > >void *key, void *data); > > * and should only be called from one thread by default. > > * Thread safety can be enabled by setting flag during > > * table creation. > >+ * When lock free reader writer concurrency is enabled, > >+ * if this API is called to update an existing entry, > >+ * the application should free any memory allocated for > >+ * previous 'data' only after all the readers have stopped > >+ * using previous 'data'. > > * > > * @param h > > * Hash table to add the key to. > >@@ -230,6 +245,12 @@ rte_hash_add_key_with_hash(struct rte_hash *h, > >const void *key, hash_sig_t sig); > > * and should only be called from one thread by default. > > * Thread safety can be enabled by setting flag during > > * table creation. > >+ * If lock free reader writer concurrency is enabled, > >+ * the hash library's internal memory for the deleted > >+ * key is not freed. It should be freed by calling > >+ * rte_hash_free_key_with_position API after all > >+ * the readers have stopped using the hash entry > >+ * corresponding to this key. > > * > > * @param h > > * Hash table to remove the key from. > >@@ -241,6 +262,8 @@ rte_hash_add_key_with_hash(struct rte_hash *h, > const void *key, hash_sig_t sig); > > * - A positive value that can be used by the caller as an offset into an > > * array of user data. This value is unique for this key, and is the > > same > > * value that was returned when the key was added. > >+ * When lock free concurrency is enabled, this value should be used > >+ * while calling the rte_hash_free_key_with_position API. > > */ > > int32_t > > rte_hash_del_key(const struct rte_hash *h, const void *key); @@ -251,6 > >+274,12 @@ rte_hash_del_key(const struct rte_hash *h, const void *key); > > * and should only be called from one thread by default. > > * Thread safety can be enabled by setting flag during > > * table creation. > >+ * If lock free reader writer concurrency is enabled, > >+ * the hash library's internal memory for the deleted > >+ * key is not freed. It should be freed by calling > >+ * rte_hash_free_key_with_position API after all > >+ * the readers have stopped using the hash entry > >+ * corresponding to this key. > > * > > * @param h > > * Hash table to remove the key from. > >@@ -264,6 +293,8 @@ rte_hash_del_key(const struct rte_hash *h, const > void *key); > > * - A positive value that can be used by the caller as an offset into an > > * array of user data. This value is unique for this key, and is the > > same > > * value that was returned when the key was added. > >+ * When lock free concurrency is enabled, this value should be used > >+ * while calling the rte_hash_free_key_with_position API. > > */ > > int32_t > > rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, > >hash_sig_t sig); @@ -290,6 +321,30 @@ > rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t > position, > > void **key); > > > [Wang, Yipeng] If possible, how about having a new delete function instead of > modifying the current one? > I think it does not need to be tied with the lockless implementation, it is > orthogonal to multi-threading implementation. > people using locks may still want this new deletion behavior. > If people want old behavior, they can call current API, otherwise they can > call > the new deletion function, followed by Rte_hash_free_key_with_position later. I like the terms 'delete' and 'free'. I am finding it hard to come up with a good name for the API. It will be on the lines of 'rte_hash_del_key_with_hash_no_free' - I do not like the name much. Instead, we could have a configuration flag for the hash table, 'RTE_HASH_EXTRA_FLAGS_FREE_MEM_ON_DEL'. If this is enabled, 'rte_hash_del_...' APIs will free the key store index and any internal memory. Enabling lock-free RW concurrency will enable this flag. User can enable this flag explicitly while not using lock-free RW concurrency as well.
