[dpdk-dev] [PATCH v4 1/4] hash: fix race condition in iterate

Wang, Yipeng1 yipeng1.wang at intel.com
Tue Oct 2 02:17:59 CEST 2018


>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli at arm.com]
>> @@ -1317,16 +1317,19 @@ rte_hash_iterate(const struct rte_hash *h, const
>> void **key, void **data, uint32
>>  	bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES;
>>  	idx = *next % RTE_HASH_BUCKET_ENTRIES;
>>
>> +	__hash_rw_reader_lock(h);
>This does not work well with the lock-less changes I am making.  We should leave the lock in its original position. Instead change the
>while loop as follows:
>
>while ((position = h->buckets[bucket_idx].key_idx[idx]) == EMPTY_SLOT)
>
>>  	/* If current position is empty, go to the next one */
>>  	while (h->buckets[bucket_idx].key_idx[idx] == EMPTY_SLOT) {
>>  		(*next)++;
>>  		/* End of table */
>> -		if (*next == total_entries)
>> +		if (*next == total_entries) {
>> +			__hash_rw_reader_unlock(h);
>>  			return -ENOENT;
>> +		}
>>  		bucket_idx = *next / RTE_HASH_BUCKET_ENTRIES;
>>  		idx = *next % RTE_HASH_BUCKET_ENTRIES;
>>  	}
>> -	__hash_rw_reader_lock(h);
>> +
>>  	/* Get position of entry in key table */
>>  	position = h->buckets[bucket_idx].key_idx[idx];
>If we change the while loop as I suggested as above, we can remove this line.
>
>>  	next_key = (struct rte_hash_key *) ((char *)h->key_store +

[Wang, Yipeng] Sorry that I did not realize you already have it in your patch set and I agree.
Do you want to export it as a bug fix in your patch set? I will remove my change.

For the lock free, do we need to protect it with version counter? Imagine the following corner case:
While the iterator read the key and data, there is a writer deleted, removed, and recycled the key-data pair,
and write a new key and data into it. While the writer are writing, will the reader reads out wrong key/data, or
mismatched key/data?




More information about the dev mailing list