Re: [dpdk-dev] Missing an increment on vq->log_cache_nb_elem ?

[Maxime Coquelin <maxime.coquelin@redhat.com>]

Hi,

On 06/15/2018 02:32 PM, Ilya Maximets wrote:
>> Hi,
>>
>>
>> In the latest dpdk master branch, in the function *vhost_log_cache_page*:
>>
>>
>> static __rte_always_inline void
>> vhost_log_cache_page(struct virtio_net *dev, struct vhost_virtqueue *vq,
>> 			uint64_t page)
>> {
>> 	uint32_t bit_nr = page % (sizeof(unsigned long) << 3);
>> 	uint32_t offset = page / (sizeof(unsigned long) << 3);
>> 	int i;
>>
>> 	for (i = 0; i < vq->log_cache_nb_elem; i++) {
>> 		struct log_cache_entry *elem = vq->log_cache + i;
>>
>> 		if (elem->offset == offset) {
>> 			elem->val |= (1UL << bit_nr);
>> 			return;
>> 		}
>> 	}
>>
>> 	if (unlikely(i >= VHOST_LOG_CACHE_NR)) {
>> 		/*
>> 		 * No more room for a new log cache entry,
>> 		 * so write the dirty log map directly.
>> 		 */
>> 		rte_smp_wmb();
>> 		vhost_log_page((uint8_t *)(uintptr_t)dev->log_base, page);
>>
>> 		return;
>> 	}
>>
>> 	vq->log_cache[i].offset = offset;
>> 	vq->log_cache[i].val = (1UL << bit_nr);
>> }
>>
>> Did it just miss an increment on vq->log_cache_nb_elem ?
> 
> Hi. Thanks for pointing that.
> 
> I looked through the code and I see no updates of 'vq->log_cache_nb_elem'.
> Looks like it always equal to initial zero value.
> 
> Maxime, I'm afraid that this means that currently pages logging is not
> performed at all. Could you please check?
> 
> Best regards, Ilya Maximets.
> 

Thanks Ilya for the heads-up and HePeng for reporting the issue.
It is broken indeed... We didn't noticed corruption when running
our non-reg tests, I guess they have to be improved.

I will post a patch shortly to fix this.

HePeng, how does your name spells in the form "$firstname $lastname"?
I would need this info to credit you for reporting the issue.

Regards,
Maxime