On 4/24/2016 5:15 PM, Michael S. Tsirkin wrote: > 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?
Others in my mind is caching a copy of avail index in vhost. Use the cached copy if there are enough slots and then sync with the index in the ring. Haven't evaluated that idea. >>> 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? Thanks. Would check. You know i have ever tried fixing avail ring in the virtio driver. In purely vhost->virtio test, it could have 2~3 times performance boost, but it isn't that obvious if with the physical nic involved, investigating that issue. I had planned to sync with you whether it is generic enough that we could have a negotiated feature, either for in order descriptor processing or like fixed avail ring. Would check your new ring layout. > >>> 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 Yes, would add the explanation in the formal patch. > >>>> --- >>>> 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++; >>>> } >>>>