Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/46672/?format=api
{ "id": 46672, "url": "http://patchwork.dpdk.org/api/patches/46672/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/patch/1539325918-125438-7-git-send-email-honnappa.nagarahalli@arm.com/", "project": { "id": 1, "url": "http://patchwork.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<1539325918-125438-7-git-send-email-honnappa.nagarahalli@arm.com>", "list_archive_url": "https://inbox.dpdk.org/dev/1539325918-125438-7-git-send-email-honnappa.nagarahalli@arm.com", "date": "2018-10-12T06:31:57", "name": "[v3,6/7] hash: enable lock-free reader-writer concurrency", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "44f72ef8b54d64a01f51aba01cded188a0a4f8bb", "submitter": { "id": 1045, "url": "http://patchwork.dpdk.org/api/people/1045/?format=api", "name": "Honnappa Nagarahalli", "email": "honnappa.nagarahalli@arm.com" }, "delegate": { "id": 1, "url": "http://patchwork.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patchwork.dpdk.org/project/dpdk/patch/1539325918-125438-7-git-send-email-honnappa.nagarahalli@arm.com/mbox/", "series": [ { "id": 1855, "url": "http://patchwork.dpdk.org/api/series/1855/?format=api", "web_url": "http://patchwork.dpdk.org/project/dpdk/list/?series=1855", "date": "2018-10-12T06:31:51", "name": "Address reader-writer concurrency in rte_hash", "version": 3, "mbox": "http://patchwork.dpdk.org/series/1855/mbox/" } ], "comments": "http://patchwork.dpdk.org/api/patches/46672/comments/", "check": "warning", "checks": "http://patchwork.dpdk.org/api/patches/46672/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id BFD8C1B14F;\n\tFri, 12 Oct 2018 08:32:24 +0200 (CEST)", "from foss.arm.com (foss.arm.com [217.140.101.70])\n\tby dpdk.org (Postfix) with ESMTP id 6E9F01B10D\n\tfor <dev@dpdk.org>; Fri, 12 Oct 2018 08:32:14 +0200 (CEST)", "from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249])\n\tby usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id DABE315BF;\n\tThu, 11 Oct 2018 23:32:13 -0700 (PDT)", "from 2p2660v4-1.austin.arm.com (2p2660v4-1.austin.arm.com\n\t[10.118.12.190])\n\tby usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id\n\t7544D3F5B3; Thu, 11 Oct 2018 23:32:13 -0700 (PDT)" ], "From": "Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>", "To": "bruce.richardson@intel.com,\n\tpablo.de.lara.guarch@intel.com", "Cc": "dev@dpdk.org, yipeng1.wang@intel.com, honnappa.nagarahalli@arm.com,\n\tdharmik.thakkar@arm.com, gavin.hu@arm.com, nd@arm.com", "Date": "Fri, 12 Oct 2018 01:31:57 -0500", "Message-Id": "<1539325918-125438-7-git-send-email-honnappa.nagarahalli@arm.com>", "X-Mailer": "git-send-email 2.7.4", "In-Reply-To": "<1539325918-125438-1-git-send-email-honnappa.nagarahalli@arm.com>", "References": "<1539325918-125438-1-git-send-email-honnappa.nagarahalli@arm.com>", "Subject": "[dpdk-dev] [PATCH v3 6/7] hash: enable lock-free reader-writer\n\tconcurrency", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://mails.dpdk.org/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://mails.dpdk.org/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://mails.dpdk.org/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Add the flag to enable reader-writer concurrency during\nrun time. The rte_hash_del_xxx APIs do not free the keystore\nelement when this flag is enabled. Hence a new API,\nrte_hash_free_key_with_position, to free the key store element\nis added.\n\nSigned-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>\nReviewed-by: Gavin Hu <gavin.hu@arm.com>\nReviewed-by: Ola Liljedahl <ola.liljedahl@arm.com>\nReviewed-by: Steve Capper <steve.capper@arm.com>\nReviewed-by: Yipeng Wang <yipeng1.wang@intel.com>\n---\n lib/librte_hash/rte_cuckoo_hash.c | 82 ++++++++++++++++++++++++------------\n lib/librte_hash/rte_cuckoo_hash.h | 2 +\n lib/librte_hash/rte_hash.h | 58 +++++++++++++++++++++----\n lib/librte_hash/rte_hash_version.map | 7 +++\n 4 files changed, 114 insertions(+), 35 deletions(-)", "diff": "diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c\nindex 262162c..4622ece 100644\n--- a/lib/librte_hash/rte_cuckoo_hash.c\n+++ b/lib/librte_hash/rte_cuckoo_hash.c\n@@ -143,6 +143,7 @@ rte_hash_create(const struct rte_hash_parameters *params)\n \tunsigned int writer_takes_lock = 0;\n \tunsigned int recycle_on_del = 1;\n \tuint32_t *tbl_chng_cnt = NULL;\n+\tunsigned int readwrite_concur_lf_support = 0;\n \n \trte_hash_function default_hash_func = (rte_hash_function)rte_jhash;\n \n@@ -162,6 +163,24 @@ rte_hash_create(const struct rte_hash_parameters *params)\n \t\treturn NULL;\n \t}\n \n+\t/* Validate correct usage of extra options */\n+\tif ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY) &&\n+\t (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF)) {\n+\t\trte_errno = EINVAL;\n+\t\tRTE_LOG(ERR, HASH, \"rte_hash_create choose rw concurrency or \"\n+\t\t\t\"rw concurrency lock free\\n\");\n+\t\treturn NULL;\n+\t}\n+\n+\tif ((params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF) &&\n+\t (params->extra_flag & RTE_HASH_EXTRA_FLAGS_EXT_TABLE)) {\n+\t\trte_errno = EINVAL;\n+\t\tRTE_LOG(ERR, HASH, \"rte_hash_create extendable bucket \"\n+\t\t\t\"feature not supported with rw concurrency \"\n+\t\t\t\"lock free\\n\");\n+\t\treturn NULL;\n+\t}\n+\n \t/* Check extra flags field to check extra options. */\n \tif (params->extra_flag & RTE_HASH_EXTRA_FLAGS_TRANS_MEM_SUPPORT)\n \t\thw_trans_mem_support = 1;\n@@ -182,6 +201,12 @@ rte_hash_create(const struct rte_hash_parameters *params)\n \tif (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL)\n \t\trecycle_on_del = 0;\n \n+\tif (params->extra_flag & RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF) {\n+\t\treadwrite_concur_lf_support = 1;\n+\t\t/* Disable freeing internal memory/index on delete */\n+\t\trecycle_on_del = 0;\n+\t}\n+\n \t/* Store all keys and leave the first entry as a dummy entry for lookup_bulk */\n \tif (multi_writer_support)\n \t\t/*\n@@ -378,6 +403,7 @@ rte_hash_create(const struct rte_hash_parameters *params)\n \th->ext_table_support = ext_table_support;\n \th->writer_takes_lock = writer_takes_lock;\n \th->recycle_on_del = recycle_on_del;\n+\th->readwrite_concur_lf_support = readwrite_concur_lf_support;\n \n #if defined(RTE_ARCH_X86)\n \tif (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE2))\n@@ -765,19 +791,21 @@ rte_hash_cuckoo_move_insert_mw(const struct rte_hash *h,\n \t\t\treturn -1;\n \t\t}\n \n-\t\t/* Inform the previous move. The current move need\n-\t\t * not be informed now as the current bucket entry\n-\t\t * is present in both primary and secondary.\n-\t\t * Since there is one writer, load acquires on\n-\t\t * tbl_chng_cnt are not required.\n-\t\t */\n-\t\t__atomic_store_n(h->tbl_chng_cnt,\n-\t\t\t\t *h->tbl_chng_cnt + 1,\n-\t\t\t\t __ATOMIC_RELEASE);\n-\t\t/* The stores to sig_alt and sig_current should not\n-\t\t * move above the store to tbl_chng_cnt.\n-\t\t */\n-\t\t__atomic_thread_fence(__ATOMIC_RELEASE);\n+\t\tif (h->readwrite_concur_lf_support) {\n+\t\t\t/* Inform the previous move. The current move need\n+\t\t\t * not be informed now as the current bucket entry\n+\t\t\t * is present in both primary and secondary.\n+\t\t\t * Since there is one writer, load acquires on\n+\t\t\t * tbl_chng_cnt are not required.\n+\t\t\t */\n+\t\t\t__atomic_store_n(h->tbl_chng_cnt,\n+\t\t\t\t\t *h->tbl_chng_cnt + 1,\n+\t\t\t\t\t __ATOMIC_RELEASE);\n+\t\t\t/* The stores to sig_alt and sig_current should not\n+\t\t\t * move above the store to tbl_chng_cnt.\n+\t\t\t */\n+\t\t\t__atomic_thread_fence(__ATOMIC_RELEASE);\n+\t\t}\n \n \t\t/* Need to swap current/alt sig to allow later\n \t\t * Cuckoo insert to move elements back to its\n@@ -795,19 +823,21 @@ rte_hash_cuckoo_move_insert_mw(const struct rte_hash *h,\n \t\tcurr_bkt = curr_node->bkt;\n \t}\n \n-\t/* Inform the previous move. The current move need\n-\t * not be informed now as the current bucket entry\n-\t * is present in both primary and secondary.\n-\t * Since there is one writer, load acquires on\n-\t * tbl_chng_cnt are not required.\n-\t */\n-\t__atomic_store_n(h->tbl_chng_cnt,\n-\t\t\t *h->tbl_chng_cnt + 1,\n-\t\t\t __ATOMIC_RELEASE);\n-\t/* The stores to sig_alt and sig_current should not\n-\t * move above the store to tbl_chng_cnt.\n-\t */\n-\t__atomic_thread_fence(__ATOMIC_RELEASE);\n+\tif (h->readwrite_concur_lf_support) {\n+\t\t/* Inform the previous move. The current move need\n+\t\t * not be informed now as the current bucket entry\n+\t\t * is present in both primary and secondary.\n+\t\t * Since there is one writer, load acquires on\n+\t\t * tbl_chng_cnt are not required.\n+\t\t */\n+\t\t__atomic_store_n(h->tbl_chng_cnt,\n+\t\t\t\t *h->tbl_chng_cnt + 1,\n+\t\t\t\t __ATOMIC_RELEASE);\n+\t\t/* The stores to sig_alt and sig_current should not\n+\t\t * move above the store to tbl_chng_cnt.\n+\t\t */\n+\t\t__atomic_thread_fence(__ATOMIC_RELEASE);\n+\t}\n \n \tcurr_bkt->sig_current[curr_slot] = sig;\n \t/* Release the new bucket entry */\ndiff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h\nindex 728cd17..0c23957 100644\n--- a/lib/librte_hash/rte_cuckoo_hash.h\n+++ b/lib/librte_hash/rte_cuckoo_hash.h\n@@ -175,6 +175,8 @@ struct rte_hash {\n \t * the deleted entry.\n \t * This flag is enabled by default.\n \t */\n+\tuint8_t readwrite_concur_lf_support;\n+\t/**< If read-write concurrency lock free support is enabled */\n \tuint8_t writer_takes_lock;\n \t/**< Indicates if the writer threads need to take lock */\n \trte_hash_function hash_func; /**< Function used to calculate hash. */\ndiff --git a/lib/librte_hash/rte_hash.h b/lib/librte_hash/rte_hash.h\nindex 3e5d336..22def5e 100644\n--- a/lib/librte_hash/rte_hash.h\n+++ b/lib/librte_hash/rte_hash.h\n@@ -44,9 +44,18 @@ extern \"C\" {\n \n /** Flag to disable freeing of internal memory/indices on hash delete.\n * Refer to rte_hash_del_xxx APIs for more details.\n+ * This is enabled by default when RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF\n+ * is enabled.\n */\n #define RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL 0x10\n \n+/** Flag to support lock free reader writer concurrency. Writer can be\n+ * single writer/multi writer.\n+ * Currently, extended bucket table feature is not supported with\n+ * this feature.\n+ */\n+#define RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF 0x20\n+\n /**\n * The type of hash value of a key.\n * It should be a value of at least 32bit with fully random pattern.\n@@ -132,7 +141,11 @@ void\n rte_hash_free(struct rte_hash *h);\n \n /**\n- * Reset all hash structure, by zeroing all entries\n+ * Reset all hash structure, by zeroing all entries.\n+ * When RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * it is application's responsibility to make sure that\n+ * none of the readers are referencing the hash table.\n+ *\n * @param h\n * Hash table to reset\n */\n@@ -156,6 +169,10 @@ rte_hash_count(const struct rte_hash *h);\n * and should only be called from one thread by default.\n * Thread safety can be enabled by setting flag during\n * table creation.\n+ * The writer needs to be aware if this API is called to update\n+ * an existing entry. The application should free any memory\n+ * allocated for the existing 'data' only after all the readers\n+ * have stopped referrencing it.\n *\n * @param h\n * Hash table to add the key to.\n@@ -178,6 +195,10 @@ rte_hash_add_key_data(const struct rte_hash *h, const void *key, void *data);\n * and should only be called from one thread by default.\n * Thread safety can be enabled by setting flag during\n * table creation.\n+ * The writer needs to be aware if this API is called to update\n+ * an existing entry. The application should free any memory\n+ * allocated for the existing 'data' only after all the readers\n+ * have stopped referencing it.\n *\n * @param h\n * Hash table to add the key to.\n@@ -243,10 +264,15 @@ rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t\n * and should only be called from one thread by default.\n * Thread safety can be enabled by setting flag during\n * table creation.\n- * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL is enabled,\n- * the hash library's internal memory/index will not be freed by this\n+ * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL or\n+ * RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * the hash library's internal memory will not be freed by this\n * API. rte_hash_free_key_with_position API must be called additionally\n- * to free the internal memory/index associated with the key.\n+ * to free any internal memory associated with the key.\n+ * If RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * rte_hash_free_key_with_position API should be called after all\n+ * the readers have stopped referencing the entry corresponding to\n+ * this key. RCU mechanisms can be used to determine such a state.\n *\n * @param h\n * Hash table to remove the key from.\n@@ -258,6 +284,8 @@ rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t\n * - A positive value that can be used by the caller as an offset into an\n * array of user data. This value is unique for this key, and is the same\n * value that was returned when the key was added.\n+ * When lock free concurrency is enabled, this value should be used\n+ * while calling the rte_hash_free_key_with_position API.\n */\n int32_t\n rte_hash_del_key(const struct rte_hash *h, const void *key);\n@@ -268,10 +296,15 @@ rte_hash_del_key(const struct rte_hash *h, const void *key);\n * and should only be called from one thread by default.\n * Thread safety can be enabled by setting flag during\n * table creation.\n- * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL is enabled,\n- * the hash library's internal memory/index will not be freed by this\n+ * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL or\n+ * RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * the hash library's internal memory will not be freed by this\n * API. rte_hash_free_key_with_position API must be called additionally\n- * to free the internal memory/index associated with the key.\n+ * to free any internal memory associated with the key.\n+ * If RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * rte_hash_free_key_with_position API should be called after all\n+ * the readers have stopped referencing the entry corresponding to\n+ * this key. RCU mechanisms can be used to determine such a state.\n *\n * @param h\n * Hash table to remove the key from.\n@@ -285,6 +318,8 @@ rte_hash_del_key(const struct rte_hash *h, const void *key);\n * - A positive value that can be used by the caller as an offset into an\n * array of user data. This value is unique for this key, and is the same\n * value that was returned when the key was added.\n+ * When lock free concurrency is enabled, this value should be used\n+ * while calling the rte_hash_free_key_with_position API.\n */\n int32_t\n rte_hash_del_key_with_hash(const struct rte_hash *h, const void *key, hash_sig_t sig);\n@@ -318,10 +353,15 @@ rte_hash_get_key_with_position(const struct rte_hash *h, const int32_t position,\n * of the key. This operation is not multi-thread safe and should\n * only be called from one thread by default. Thread safety\n * can be enabled by setting flag during table creation.\n- * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL is enabled,\n- * the hash library's internal memory/index must be freed using this API\n+ * If RTE_HASH_EXTRA_FLAGS_RECYCLE_ON_DEL or\n+ * RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * the hash library's internal memory must be freed using this API\n * after the key is deleted using rte_hash_del_key_xxx APIs.\n * This API does not validate if the key is already freed.\n+ * If RTE_HASH_EXTRA_FLAGS_RW_CONCURRENCY_LF is enabled,\n+ * this API should be called only after all the readers have stopped\n+ * referencing the entry corresponding to this key. RCU mechanisms can\n+ * be used to determine such a state.\n *\n * @param h\n * Hash table to free the key from.\ndiff --git a/lib/librte_hash/rte_hash_version.map b/lib/librte_hash/rte_hash_version.map\nindex e216ac8..734ae28 100644\n--- a/lib/librte_hash/rte_hash_version.map\n+++ b/lib/librte_hash/rte_hash_version.map\n@@ -53,3 +53,10 @@ DPDK_18.08 {\n \trte_hash_count;\n \n } DPDK_16.07;\n+\n+EXPERIMENTAL {\n+\tglobal:\n+\n+\trte_hash_free_key_with_position;\n+\n+};\n", "prefixes": [ "v3", "6/7" ] }