Re: [dpdk-dev] [PATCH 3/3] lib/hash: adjust tbl_chng_cnt position

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

>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:honnappa.nagarahalli@arm.com]
>Sent: Tuesday, June 25, 2019 2:15 PM
>To: Wang, Yipeng1 <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>; Richardson, Bruce
><bruce.richardson@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; honnappa.nagarahalli@arm.com
>Cc: gavin.hu@arm.com; ruifeng.wang@arm.com; dev@dpdk.org; nd@arm.com; stable@dpdk.org
>Subject: [PATCH 3/3] lib/hash: adjust tbl_chng_cnt position
>
>tbl_chng_cnt is one of the first elements of the structure used in
>the lookup. Move it to the beginning of the cache line to gain
>performance.
>
>Fixes: e605a1d36 ("hash: add lock-free r/w concurrency")
>Cc: stable@dpdk.org
>
>Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>Reviewed-by: Gavin Hu <gavin.hu@arm.com>
>Tested-by: Ruifeng Wang <ruifeng.wang@arm.com>
>---
> lib/librte_hash/rte_cuckoo_hash.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/lib/librte_hash/rte_cuckoo_hash.h b/lib/librte_hash/rte_cuckoo_hash.h
>index fb19bb27d..af6451b5c 100644
>--- a/lib/librte_hash/rte_cuckoo_hash.h
>+++ b/lib/librte_hash/rte_cuckoo_hash.h
>@@ -170,7 +170,9 @@ struct rte_hash {
>
> 	/* Fields used in lookup */
>
>-	uint32_t key_len __rte_cache_aligned;
>+	uint32_t *tbl_chng_cnt __rte_cache_aligned;
>+	/**< Indicates if the hash table changed from last read. */
>+	uint32_t key_len;
> 	/**< Length of hash key. */
> 	uint8_t hw_trans_mem_support;
> 	/**< If hardware transactional memory is used. */
>@@ -218,8 +220,6 @@ struct rte_hash {
> 	 * is piggy-backed to freeing of the key index.
> 	 */
> 	uint32_t *ext_bkt_to_free;
>-	uint32_t *tbl_chng_cnt;
>-	/**< Indicates if the hash table changed from last read. */
> } __rte_cache_aligned;
>
> struct queue_node {
>--
>2.17.1

[Wang, Yipeng] 
I am not sure about this change. By moving counter to front, I think you seems push key_store out of the cache line. And key_store
Is also used in lookup (and more commonly).
My tests also show perf drop in many cases.