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

[HePeng <xnhp0320@icloud.com>]

Hi, Maxime,

  My name is Peng He. 

  Thanks. 

> 在 2018年6月15日，下午9:24，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