From: "Wang, Yipeng1" <yipeng1.wang@intel.com>
To: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"gavin.hu@arm.com" <gavin.hu@arm.com>,
"steve.capper@arm.com" <steve.capper@arm.com>,
"ola.liljedahl@arm.com" <ola.liljedahl@arm.com>,
"nd@arm.com" <nd@arm.com>,
"Gobriel, Sameh" <sameh.gobriel@intel.com>
Subject: Re: [dpdk-dev] [PATCH 4/4] hash: enable lock-free reader-writer concurrency
Date: Fri, 28 Sep 2018 01:33:29 +0000 [thread overview]
Message-ID: <D2C4A16CA39F7F4E8E384D204491D7A6614D8226@FMSMSX151.amr.corp.intel.com> (raw)
In-Reply-To: <1536253938-192391-5-git-send-email-honnappa.nagarahalli@arm.com>
Reply inlined:
>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Honnappa Nagarahalli
>Sent: Thursday, September 6, 2018 10:12 AM
>To: Richardson, Bruce <bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
>Cc: dev@dpdk.org; honnappa.nagarahalli@dpdk.org; gavin.hu@arm.com; steve.capper@arm.com; ola.liljedahl@arm.com;
>nd@arm.com; Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>Subject: [dpdk-dev] [PATCH 4/4] hash: enable lock-free reader-writer concurrency
>
>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.
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.
>+
> /** 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?
> *
> * @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.
next prev parent reply other threads:[~2018-09-28 1:33 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-06 17:12 [dpdk-dev] [PATCH 0/4] Address reader-writer concurrency in rte_hash Honnappa Nagarahalli
2018-09-06 17:12 ` [dpdk-dev] [PATCH 1/4] hash: correct key store element alignment Honnappa Nagarahalli
2018-09-27 23:58 ` Wang, Yipeng1
2018-09-06 17:12 ` [dpdk-dev] [PATCH 2/4] hash: add memory ordering to avoid race conditions Honnappa Nagarahalli
2018-09-28 0:43 ` Wang, Yipeng1
2018-09-30 22:20 ` Honnappa Nagarahalli
2018-10-01 22:41 ` Wang, Yipeng1
2018-10-01 10:42 ` Ola Liljedahl
2018-10-02 1:52 ` Wang, Yipeng1
2018-09-06 17:12 ` [dpdk-dev] [PATCH 3/4] hash: fix rw concurrency while moving keys Honnappa Nagarahalli
2018-09-28 1:00 ` Wang, Yipeng1
2018-09-28 8:26 ` Bruce Richardson
2018-09-28 8:55 ` Van Haaren, Harry
2018-09-30 22:33 ` Honnappa Nagarahalli
2018-10-02 13:17 ` Van Haaren, Harry
2018-10-02 23:58 ` Wang, Yipeng1
2018-10-03 17:32 ` Honnappa Nagarahalli
2018-10-03 17:56 ` Wang, Yipeng1
2018-10-03 23:05 ` Ola Liljedahl
2018-10-04 3:32 ` Honnappa Nagarahalli
2018-10-04 3:54 ` Honnappa Nagarahalli
2018-10-04 19:16 ` Wang, Yipeng1
2018-09-30 23:05 ` Honnappa Nagarahalli
2018-10-01 22:56 ` Wang, Yipeng1
2018-10-03 0:16 ` Wang, Yipeng1
2018-10-03 17:39 ` Honnappa Nagarahalli
2018-09-06 17:12 ` [dpdk-dev] [PATCH 4/4] hash: enable lock-free reader-writer concurrency Honnappa Nagarahalli
2018-09-28 1:33 ` Wang, Yipeng1 [this message]
2018-10-01 4:11 ` Honnappa Nagarahalli
2018-10-01 23:54 ` Wang, Yipeng1
2018-10-11 5:24 ` Honnappa Nagarahalli
2018-09-14 21:18 ` [dpdk-dev] [PATCH 0/4] Address reader-writer concurrency in rte_hash Honnappa Nagarahalli
2018-09-26 14:36 ` Honnappa Nagarahalli
2018-09-27 23:45 ` Wang, Yipeng1
2018-09-28 21:11 ` Honnappa Nagarahalli
2018-10-02 0:30 ` Wang, Yipeng1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D2C4A16CA39F7F4E8E384D204491D7A6614D8226@FMSMSX151.amr.corp.intel.com \
--to=yipeng1.wang@intel.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=gavin.hu@arm.com \
--cc=honnappa.nagarahalli@arm.com \
--cc=nd@arm.com \
--cc=ola.liljedahl@arm.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=sameh.gobriel@intel.com \
--cc=steve.capper@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).