From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 6B4352BA7 for ; Thu, 28 Apr 2016 11:18:08 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga102.jf.intel.com with ESMTP; 28 Apr 2016 02:17:50 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,546,1455004800"; d="scan'208";a="954650741" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 28 Apr 2016 02:17:49 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 28 Apr 2016 02:17:49 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 28 Apr 2016 02:17:48 -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; Thu, 28 Apr 2016 17:17:47 +0800 From: "Xie, Huawei" To: "Michael S. Tsirkin" CC: "dev@dpdk.org" , Stephen Hemminger , Tetsuya Mukawa , "Traynor, Kevin" , "Tan, Jianfeng" , Yuanhan Liu Thread-Topic: [dpdk-dev] [RFC PATCH] avail idx update optimizations Thread-Index: AdGcnk4+ItYoYWyVTf2Hi+rA6pmHDg== Date: Thu, 28 Apr 2016 09:17:46 +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: Thu, 28 Apr 2016 09:18:09 -0000 On 4/24/2016 9:24 PM, Xie, Huawei wrote:=0A= > 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 cac= he=0A= >>>> to cache transfer. Actually I don't expect many of them as virtio/vhos= t=0A= >>>> has already done quite right.=0A= >> Hmm - is it a series or a single patch?=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= Tried cached vhost idx which gives a slight better perf, but will hold=0A= this patch, as i guess we couldn't blindly set this cached avail idx to=0A= 0, which might cause issue in live migration.=0A= =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= > 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= >>>> 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 vhos= t=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= > Yes, would add the explanation in the formal patch.=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/virt= queue.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= =0A=