Re: [dpdk-dev] [RFC PATCH] avail idx update optimizations

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Apr 24, 2016 at 02:45:22AM +0000, Xie, Huawei wrote:
> Forget to cc the mailing list.
> 
> On 4/22/2016 9:53 PM, Xie, Huawei wrote:
> > Hi:
> >
> > This is a series of virtio/vhost idx/ring update optimizations for cache
> > to cache transfer. Actually I don't expect many of them as virtio/vhost
> > has already done quite right.

Hmm - is it a series or a single patch?

> > For this patch, in a VM2VM test, i observed ~6% performance increase.

Interesting. In that case, it seems likely that new ring layout
would give you an even bigger performance gain.
Could you take a look at tools/virtio/ringtest/ring.c
in latest Linux and tell me what do you think?
In particular, I know you looked at using vectored instructions
to do ring updates - would the layout in tools/virtio/ringtest/ring.c
interfere with that?

> > In VM1, run testpmd with txonly mode
> > In VM2, run testpmd with rxonly mode
> > In host, run testpmd(with two vhostpmds) with io forward
> >
> > Michael:
> > We have talked about this method when i tried the fixed ring.
> >
> >
> > On 4/22/2016 5:12 PM, Xie, Huawei wrote:
> >> eliminate unnecessary cache to cache transfer between virtio and vhost
> >> core

Yes I remember proposing this, but you probably should include the
explanation about why this works in he commit log:

- pre-format avail ring with expected descriptor index values
- as long as entries are consumed in-order, there's no
  need to modify the avail ring
- as long as avail ring is not modified, it can be
  valid in caches of both consumer and producer


> >>
> >> ---
> >>  drivers/net/virtio/virtqueue.h | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h
> >> index 4e9239e..8c46a83 100644
> >> --- a/drivers/net/virtio/virtqueue.h
> >> +++ b/drivers/net/virtio/virtqueue.h
> >> @@ -302,7 +302,8 @@ vq_update_avail_ring(struct virtqueue *vq, uint16_t desc_idx)
> >>  	 * descriptor.
> >>  	 */
> >>  	avail_idx = (uint16_t)(vq->vq_avail_idx & (vq->vq_nentries - 1));
> >> -	vq->vq_ring.avail->ring[avail_idx] = desc_idx;
> >> +	if (unlikely(vq->vq_ring.avail->ring[avail_idx] != desc_idx))
> >> +		vq->vq_ring.avail->ring[avail_idx] = desc_idx;
> >>  	vq->vq_avail_idx++;
> >>  }
> >>  
> >
>