From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 249A42956 for ; Sun, 24 Apr 2016 15:24:01 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP; 24 Apr 2016 06:24:01 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,527,1455004800"; d="scan'208";a="939100721" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga001.jf.intel.com with ESMTP; 24 Apr 2016 06:24:00 -0700 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 24 Apr 2016 06:24:00 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 24 Apr 2016 06:23:59 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.136]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.184]) with mapi id 14.03.0248.002; Sun, 24 Apr 2016 21:23:58 +0800 From: "Xie, Huawei" To: "Michael S. Tsirkin" CC: "dev@dpdk.org" , Stephen Hemminger , Tetsuya Mukawa , "Traynor, Kevin" , "Tan, Jianfeng" , Yuanhan Liu Thread-Topic: [RFC PATCH] avail idx update optimizations Thread-Index: AdGcnk4+ItYoYWyVTf2Hi+rA6pmHDg== Date: Sun, 24 Apr 2016 13:23:57 +0000 Message-ID: References: <1461259092-9309-1-git-send-email-huawei.xie@intel.com> <20160424120551-mutt-send-email-mst@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC PATCH] avail idx update optimizations X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 24 Apr 2016 13:24:02 -0000 On 4/24/2016 5:15 PM, Michael S. Tsirkin wrote:=0A= > On Sun, Apr 24, 2016 at 02:45:22AM +0000, Xie, Huawei wrote:=0A= >> Forget to cc the mailing list.=0A= >>=0A= >> On 4/22/2016 9:53 PM, Xie, Huawei wrote:=0A= >>> Hi:=0A= >>>=0A= >>> This is a series of virtio/vhost idx/ring update optimizations for cach= e=0A= >>> to cache transfer. Actually I don't expect many of them as virtio/vhost= =0A= >>> has already done quite right.=0A= > Hmm - is it a series or a single patch?=0A= =0A= Others in my mind is caching a copy of avail index in vhost. Use the=0A= cached copy if there are enough slots and then sync with the index in=0A= the ring.=0A= Haven't evaluated that idea.=0A= =0A= >>> For this patch, in a VM2VM test, i observed ~6% performance increase.= =0A= > Interesting. In that case, it seems likely that new ring layout=0A= > would give you an even bigger performance gain.=0A= > Could you take a look at tools/virtio/ringtest/ring.c=0A= > in latest Linux and tell me what do you think?=0A= > In particular, I know you looked at using vectored instructions=0A= > to do ring updates - would the layout in tools/virtio/ringtest/ring.c=0A= > interfere with that?=0A= =0A= Thanks. Would check. You know i have ever tried fixing avail ring in the=0A= virtio driver. In purely vhost->virtio test, it could have 2~3 times=0A= performance boost, but it isn't that obvious if with the physical nic=0A= involved, investigating that issue.=0A= I had planned to sync with you whether it is generic enough that we=0A= could have a negotiated feature, either for in order descriptor=0A= processing or like fixed avail ring. Would check your new ring layout.=0A= =0A= =0A= >=0A= >>> In VM1, run testpmd with txonly mode=0A= >>> In VM2, run testpmd with rxonly mode=0A= >>> In host, run testpmd(with two vhostpmds) with io forward=0A= >>>=0A= >>> Michael:=0A= >>> We have talked about this method when i tried the fixed ring.=0A= >>>=0A= >>>=0A= >>> On 4/22/2016 5:12 PM, Xie, Huawei wrote:=0A= >>>> eliminate unnecessary cache to cache transfer between virtio and vhost= =0A= >>>> core=0A= > Yes I remember proposing this, but you probably should include the=0A= > explanation about why this works in he commit log:=0A= >=0A= > - pre-format avail ring with expected descriptor index values=0A= > - as long as entries are consumed in-order, there's no=0A= > need to modify the avail ring=0A= > - as long as avail ring is not modified, it can be=0A= > valid in caches of both consumer and producer=0A= =0A= Yes, would add the explanation in the formal patch.=0A= =0A= =0A= >=0A= >>>> ---=0A= >>>> drivers/net/virtio/virtqueue.h | 3 ++-=0A= >>>> 1 file changed, 2 insertions(+), 1 deletion(-)=0A= >>>>=0A= >>>> diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtq= ueue.h=0A= >>>> index 4e9239e..8c46a83 100644=0A= >>>> --- a/drivers/net/virtio/virtqueue.h=0A= >>>> +++ b/drivers/net/virtio/virtqueue.h=0A= >>>> @@ -302,7 +302,8 @@ vq_update_avail_ring(struct virtqueue *vq, uint16_= t desc_idx)=0A= >>>> * descriptor.=0A= >>>> */=0A= >>>> avail_idx =3D (uint16_t)(vq->vq_avail_idx & (vq->vq_nentries - 1));= =0A= >>>> - vq->vq_ring.avail->ring[avail_idx] =3D desc_idx;=0A= >>>> + if (unlikely(vq->vq_ring.avail->ring[avail_idx] !=3D desc_idx))=0A= >>>> + vq->vq_ring.avail->ring[avail_idx] =3D desc_idx;=0A= >>>> vq->vq_avail_idx++;=0A= >>>> }=0A= >>>> =0A= =0A=