Re: [dpdk-dev] [PATCH v2 1/2] hash: add lock free support for extendable bucket

[Dharmik Thakkar <Dharmik.Thakkar@arm.com>]

+Honnappa

Hi Yipeng,

Thank you for the review!

> On Apr 1, 2019, at 1:20 PM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
> 
>> -----Original Message-----
>> From: Dharmik Thakkar [mailto:dharmik.thakkar@arm.com]
>> Sent: Monday, March 25, 2019 2:09 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>; Mcnamara, John
>> <john.mcnamara@intel.com>; Kovacevic, Marko <marko.kovacevic@intel.com>
>> Cc: dev@dpdk.org; Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Subject: [PATCH v2 1/2] hash: add lock free support for extendable bucket
>> 
>> This patch enables lock-free read-write concurrency support for
>> extendable bucket feature.
>> 
>> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> @@ -1072,7 +1082,15 @@ __rte_hash_add_key_with_hash(const struct rte_hash *h, const void *key,
>> 	bkt_id = (uint32_t)((uintptr_t)ext_bkt_id) - 1;
>> 	/* Use the first location of the new bucket */
>> 	(h->buckets_ext[bkt_id]).sig_current[0] = short_sig;
>> -	(h->buckets_ext[bkt_id]).key_idx[0] = new_idx;
>> +	/* Key can be of arbitrary length, so it is
>> +	 * not possible to store it atomically.
>> +	 * Hence the new key element's memory stores
>> +	 * (key as well as data) should be complete
>> +	 * before it is referenced.
>> +	 */
> [Wang, Yipeng] Minor issue: does this comment need to be fixed too? If so you could include my ack for next version.
Yes, I will fix it in the next version. 
> 
> Acked-by: Yipeng Wang  <yipeng1.wang@intel.com>
> 
> 
> Thanks
> Yipeng!
>