Re: [dpdk-dev] [PATCH v2 2/2] test/hash: lock-free rw concurrency test ext bkt

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

+Honnappa

Hi Yipeng,

Thank you for the review comments!

> On Apr 1, 2019, at 1:55 PM, Wang, Yipeng1 <yipeng1.wang@intel.com> wrote:
> 
> A little bit improvement on commit-message maybe needed.
> 
>> -----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>
>> Cc: dev@dpdk.org; Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Subject: [PATCH v2 2/2] test/hash: lock-free rw concurrency test ext bkt
> [Wang, Yipeng] *for* ext bucket
Exceeds 50 characters limit.
>> 
>> Add unit test to check for hash lookup and bulk-lookup perf.
> [Wang, Yipeng] for extendable bucket feature.
Will update in the next version.
>> Test with lock-free enabled and with lock-free disabled.
> [Wang, Yipeng] It is tested with both lock-free enabled and disabled case.
Will update in the next version.
> 
>> 
>> Test include:
>> 
>> - hash lookup on keys in ext bkt,
>> hash delete causing key-shifts of keys from ext bkt to secondary bkt
> [Wang, Yipeng] 
> Two test scenarios right? A bit of formatting..
> Tests include:
It is a single test wherein lookup perf is tested for with hash delete causing key-shifts.
I will put it as Test includes:
> - hash lookup on keys in ext bucket.
> - hash delete causing key-shifts of keys from ext bucket to secondary bucket.
>  
>> 
>> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> ---
>> +/*
>> + * Test lookup perf:
>> + * Reader(s) lookup keys present in the extendable bkt.
>> + */
>> +static int
>> +test_hash_add_ks_lookup_hit_extbkt(struct rwc_perf *rwc_perf_results,
>> +				int rwc_lf, int htm, int ext_bkt)
>> +{
>> +	unsigned int n, m;
>> +	uint64_t i;
>> +	int use_jhash = 0;
>> +	uint8_t write_type;
>> +	uint8_t read_type = READ_PASS_KEY_SHIFTS_EXTBKT;
>> +
>> +	rte_atomic64_init(&greads);
>> +	rte_atomic64_init(&gread_cycles);
>> +
>> +	if (init_params(rwc_lf, use_jhash, htm, ext_bkt) != 0)
>> +		goto err;
>> +	printf("\nTest: Hash add - key-shifts, read - hit (ext_bkt)\n");
>> +	for (m = 0; m < 2; m++) {
>> +		if (m == 1) {
>> +			printf("\n** With bulk-lookup **\n");
>> +			read_type |= BULK_LOOKUP;
>> +		}
>> +		for (n = 0; n < NUM_TEST; n++) {
>> +			unsigned int tot_lcore = rte_lcore_count();
>> +			if (tot_lcore < rwc_core_cnt[n] + 1)
>> +				goto finish;
>> +
>> +			printf("\nNumber of readers: %u\n", rwc_core_cnt[n]);
>> +
>> +			rte_atomic64_clear(&greads);
>> +			rte_atomic64_clear(&gread_cycles);
>> +
>> +			rte_hash_reset(tbl_rwc_test_param.h);
>> +			write_type = WRITE_NO_KEY_SHIFT;
>> +			if (write_keys(write_type) < 0)
>> +				goto err;
>> +			write_type = WRITE_KEY_SHIFT;
>> +			if (write_keys(write_type) < 0)
>> +				goto err;
>> +			writer_done = 0;
>> +			for (i = 1; i <= rwc_core_cnt[n]; i++)
>> +				rte_eal_remote_launch(test_rwc_reader,
>> +						(void *)(uintptr_t)read_type,
>> +							enabled_core_ids[i]);
>> +			for (i = 0; i < tbl_rwc_test_param.count_keys_ks_extbkt;
>> +			     i++) {
>> +				if (rte_hash_del_key(tbl_rwc_test_param.h,
>> +					tbl_rwc_test_param.keys_ks_extbkt + i)
>> +							< 0) {
>> +					printf("Delete Failed: %u\n",
>> +					tbl_rwc_test_param.keys_ks_extbkt[i]);
>> +					goto err;
>> +				}
>> +			}
>> +			writer_done = 1;
>> +			rte_eal_mp_wait_lcore();
> [Wang, Yipeng] Not requirement for this perf patch, but is it better to read the shifted key again
> Just to verify the logic? If not difficult please add.
Keys in ext bkt (including shifted keys) are being read in a loop while writer_done equals 0 i.e. while keys are being deleted.
Test fails if look up fails. Do you think there is still a need to add one more round of look up on all the keys in ext bkt (some of which have now been shifted to secondary bkts)?