From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BE6B7C43441 for ; Mon, 26 Nov 2018 04:02:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8ED6020855 for ; Mon, 26 Nov 2018 04:02:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8ED6020855 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726229AbeKZOyz (ORCPT ); Mon, 26 Nov 2018 09:54:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46104 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726124AbeKZOyz (ORCPT ); Mon, 26 Nov 2018 09:54:55 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C48F44E92B; Mon, 26 Nov 2018 04:02:07 +0000 (UTC) Received: from [10.72.12.50] (ovpn-12-50.pek2.redhat.com [10.72.12.50]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 22FC51974D; Mon, 26 Nov 2018 04:02:02 +0000 (UTC) Subject: Re: [PATCH net-next 3/3] vhost: don't touch avail ring if in_order is negotiated To: "Michael S. Tsirkin" Cc: kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20181123030016.4924-1-jasowang@redhat.com> <20181123030016.4924-4-jasowang@redhat.com> <20181123103750-mutt-send-email-mst@kernel.org> From: Jason Wang Message-ID: <99e6b6b0-3cc6-b100-1e60-aa837d293bc8@redhat.com> Date: Mon, 26 Nov 2018 12:01:59 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181123103750-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Mon, 26 Nov 2018 04:02:07 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018/11/23 下午11:41, Michael S. Tsirkin wrote: > On Fri, Nov 23, 2018 at 11:00:16AM +0800, Jason Wang wrote: >> Device use descriptors table in order, so there's no need to read >> index from available ring. This eliminate the cache contention on >> avail ring completely. > Well this isn't what the in order feature says in the spec. > > It forces the used ring to be in the same order as > the available ring. So I don't think you can skip > checking the available ring. Maybe I miss something. The spec (https://github.com/oasis-tcs/virtio-spec master) said: "If VIRTIO_F_IN_ORDER has been negotiated, driver uses descriptors in ring order: starting from offset 0 in the table, and wrapping around at the end of the table." Even if I was wrong, maybe it's time to force this consider the obvious improvement it brings? And maybe what you said is the reason that we only allow the following optimization only for packed ring? "notify the use of a batch of buffers to the driver by only writing out a single used descriptor with the Buffer ID corresponding to the last descriptor in the batch. " This seems another good optimization for packed ring as well. > And in fact depending on > ring size and workload, using all of descriptor buffer might > cause a slowdown. This is not the sin of in order but the size of the queue I believe? > Rather you should be able to get > about the same speedup, but from skipping checking > the used ring in virtio. Yes, I've made such changes in virtio-net pmd. But since we're testing it with vhost-kernel, the main contention was on available. So the improvement was not obvious. Thanks > > >> Virito-user + vhost_kernel + XDP_DROP gives about ~10% improvement on >> TX from 4.8Mpps to 5.3Mpps on Intel(R) Core(TM) i7-5600U CPU @ >> 2.60GHz. >> >> Signed-off-by: Jason Wang >> --- >> drivers/vhost/vhost.c | 19 ++++++++++++------- >> 1 file changed, 12 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c >> index 3a5f81a66d34..c8be151bc897 100644 >> --- a/drivers/vhost/vhost.c >> +++ b/drivers/vhost/vhost.c >> @@ -2002,6 +2002,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, >> __virtio16 avail_idx; >> __virtio16 ring_head; >> int ret, access; >> + bool in_order = vhost_has_feature(vq, VIRTIO_F_IN_ORDER); >> >> /* Check it isn't doing very strange things with descriptor numbers. */ >> last_avail_idx = vq->last_avail_idx; >> @@ -2034,15 +2035,19 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq, >> >> /* Grab the next descriptor number they're advertising, and increment >> * the index we've seen. */ >> - if (unlikely(vhost_get_avail(vq, ring_head, >> - &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) { >> - vq_err(vq, "Failed to read head: idx %d address %p\n", >> - last_avail_idx, >> - &vq->avail->ring[last_avail_idx % vq->num]); >> - return -EFAULT; >> + if (!in_order) { >> + if (unlikely(vhost_get_avail(vq, ring_head, >> + &vq->avail->ring[last_avail_idx & (vq->num - 1)]))) { >> + vq_err(vq, "Failed to read head: idx %d address %p\n", >> + last_avail_idx, >> + &vq->avail->ring[last_avail_idx % vq->num]); >> + return -EFAULT; >> + } >> + head = vhost16_to_cpu(vq, ring_head); >> + } else { >> + head = last_avail_idx & (vq->num - 1); >> } >> >> - head = vhost16_to_cpu(vq, ring_head); >> >> /* If their number is silly, that's an error. */ >> if (unlikely(head >= vq->num)) { >> -- >> 2.17.1 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net-next 3/3] vhost: don't touch avail ring if in_order is negotiated Date: Mon, 26 Nov 2018 12:01:59 +0800 Message-ID: <99e6b6b0-3cc6-b100-1e60-aa837d293bc8@redhat.com> References: <20181123030016.4924-1-jasowang@redhat.com> <20181123030016.4924-4-jasowang@redhat.com> <20181123103750-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org To: "Michael S. Tsirkin" Return-path: In-Reply-To: <20181123103750-mutt-send-email-mst@kernel.org> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org Ck9uIDIwMTgvMTEvMjMg5LiL5Y2IMTE6NDEsIE1pY2hhZWwgUy4gVHNpcmtpbiB3cm90ZToKPiBP biBGcmksIE5vdiAyMywgMjAxOCBhdCAxMTowMDoxNkFNICswODAwLCBKYXNvbiBXYW5nIHdyb3Rl Ogo+PiBEZXZpY2UgdXNlIGRlc2NyaXB0b3JzIHRhYmxlIGluIG9yZGVyLCBzbyB0aGVyZSdzIG5v IG5lZWQgdG8gcmVhZAo+PiBpbmRleCBmcm9tIGF2YWlsYWJsZSByaW5nLiBUaGlzIGVsaW1pbmF0 ZSB0aGUgY2FjaGUgY29udGVudGlvbiBvbgo+PiBhdmFpbCByaW5nIGNvbXBsZXRlbHkuCj4gV2Vs bCB0aGlzIGlzbid0IHdoYXQgdGhlIGluIG9yZGVyIGZlYXR1cmUgc2F5cyBpbiB0aGUgc3BlYy4K Pgo+IEl0IGZvcmNlcyB0aGUgdXNlZCByaW5nIHRvIGJlIGluIHRoZSBzYW1lIG9yZGVyIGFzCj4g dGhlIGF2YWlsYWJsZSByaW5nLiBTbyBJIGRvbid0IHRoaW5rIHlvdSBjYW4gc2tpcAo+IGNoZWNr aW5nIHRoZSBhdmFpbGFibGUgcmluZy4KCgpNYXliZSBJIG1pc3Mgc29tZXRoaW5nLiBUaGUgc3Bl YyAKKGh0dHBzOi8vZ2l0aHViLmNvbS9vYXNpcy10Y3MvdmlydGlvLXNwZWMgbWFzdGVyKSBzYWlk OiAiSWYgClZJUlRJT19GX0lOX09SREVSIGhhcyBiZWVuIG5lZ290aWF0ZWQsIGRyaXZlciB1c2Vz IGRlc2NyaXB0b3JzIGluIHJpbmcgCm9yZGVyOiBzdGFydGluZyBmcm9tIG9mZnNldCAwIGluIHRo ZSB0YWJsZSwgYW5kIHdyYXBwaW5nIGFyb3VuZCBhdCB0aGUgCmVuZCBvZiB0aGUgdGFibGUuIgoK RXZlbiBpZiBJIHdhcyB3cm9uZywgbWF5YmUgaXQncyB0aW1lIHRvIGZvcmNlIHRoaXMgY29uc2lk ZXIgdGhlIG9idmlvdXMgCmltcHJvdmVtZW50IGl0IGJyaW5ncz8gQW5kIG1heWJlIHdoYXQgeW91 IHNhaWQgaXMgdGhlIHJlYXNvbiB0aGF0IHdlIApvbmx5IGFsbG93IHRoZSBmb2xsb3dpbmcgb3B0 aW1pemF0aW9uIG9ubHkgZm9yIHBhY2tlZCByaW5nPwoKIm5vdGlmeSB0aGUgdXNlIG9mIGEgYmF0 Y2ggb2YgYnVmZmVycyB0byB0aGUgZHJpdmVyIGJ5IG9ubHkgd3JpdGluZyBvdXQgCmEgc2luZ2xl IHVzZWQgZGVzY3JpcHRvciB3aXRoIHRoZSBCdWZmZXIgSUQgY29ycmVzcG9uZGluZyB0byB0aGUg bGFzdCAKZGVzY3JpcHRvciBpbiB0aGUgYmF0Y2guICIKClRoaXMgc2VlbXMgYW5vdGhlciBnb29k IG9wdGltaXphdGlvbiBmb3IgcGFja2VkIHJpbmcgYXMgd2VsbC4KCgo+IEFuZCBpbiBmYWN0IGRl cGVuZGluZyBvbgo+IHJpbmcgc2l6ZSBhbmQgd29ya2xvYWQsIHVzaW5nIGFsbCBvZiBkZXNjcmlw dG9yIGJ1ZmZlciBtaWdodAo+IGNhdXNlIGEgc2xvd2Rvd24uCgoKVGhpcyBpcyBub3QgdGhlIHNp biBvZiBpbiBvcmRlciBidXQgdGhlIHNpemUgb2YgdGhlIHF1ZXVlIEkgYmVsaWV2ZT8KCgo+IFJh dGhlciB5b3Ugc2hvdWxkIGJlIGFibGUgdG8gZ2V0Cj4gYWJvdXQgdGhlIHNhbWUgc3BlZWR1cCwg YnV0IGZyb20gc2tpcHBpbmcgY2hlY2tpbmcKPiB0aGUgdXNlZCByaW5nIGluIHZpcnRpby4KCgpZ ZXMsIEkndmUgbWFkZSBzdWNoIGNoYW5nZXMgaW4gdmlydGlvLW5ldCBwbWQuIEJ1dCBzaW5jZSB3 ZSdyZSB0ZXN0aW5nIAppdCB3aXRoIHZob3N0LWtlcm5lbCwgdGhlIG1haW4gY29udGVudGlvbiB3 YXMgb24gYXZhaWxhYmxlLiBTbyB0aGUgCmltcHJvdmVtZW50IHdhcyBub3Qgb2J2aW91cy4KClRo YW5rcwoKCj4KPgo+PiBWaXJpdG8tdXNlciArIHZob3N0X2tlcm5lbCArIFhEUF9EUk9QIGdpdmVz IGFib3V0IH4xMCUgaW1wcm92ZW1lbnQgb24KPj4gVFggZnJvbSA0LjhNcHBzIHRvIDUuM01wcHMg b24gSW50ZWwoUikgQ29yZShUTSkgaTctNTYwMFUgQ1BVIEAKPj4gMi42MEdIei4KPj4KPj4gU2ln bmVkLW9mZi1ieTogSmFzb24gV2FuZyA8amFzb3dhbmdAcmVkaGF0LmNvbT4KPj4gLS0tCj4+ICAg ZHJpdmVycy92aG9zdC92aG9zdC5jIHwgMTkgKysrKysrKysrKysrLS0tLS0tLQo+PiAgIDEgZmls ZSBjaGFuZ2VkLCAxMiBpbnNlcnRpb25zKCspLCA3IGRlbGV0aW9ucygtKQo+Pgo+PiBkaWZmIC0t Z2l0IGEvZHJpdmVycy92aG9zdC92aG9zdC5jIGIvZHJpdmVycy92aG9zdC92aG9zdC5jCj4+IGlu ZGV4IDNhNWY4MWE2NmQzNC4uYzhiZTE1MWJjODk3IDEwMDY0NAo+PiAtLS0gYS9kcml2ZXJzL3Zo b3N0L3Zob3N0LmMKPj4gKysrIGIvZHJpdmVycy92aG9zdC92aG9zdC5jCj4+IEBAIC0yMDAyLDYg KzIwMDIsNyBAQCBpbnQgdmhvc3RfZ2V0X3ZxX2Rlc2Moc3RydWN0IHZob3N0X3ZpcnRxdWV1ZSAq dnEsCj4+ICAgCV9fdmlydGlvMTYgYXZhaWxfaWR4Owo+PiAgIAlfX3ZpcnRpbzE2IHJpbmdfaGVh ZDsKPj4gICAJaW50IHJldCwgYWNjZXNzOwo+PiArCWJvb2wgaW5fb3JkZXIgPSB2aG9zdF9oYXNf ZmVhdHVyZSh2cSwgVklSVElPX0ZfSU5fT1JERVIpOwo+PiAgIAo+PiAgIAkvKiBDaGVjayBpdCBp c24ndCBkb2luZyB2ZXJ5IHN0cmFuZ2UgdGhpbmdzIHdpdGggZGVzY3JpcHRvciBudW1iZXJzLiAq Lwo+PiAgIAlsYXN0X2F2YWlsX2lkeCA9IHZxLT5sYXN0X2F2YWlsX2lkeDsKPj4gQEAgLTIwMzQs MTUgKzIwMzUsMTkgQEAgaW50IHZob3N0X2dldF92cV9kZXNjKHN0cnVjdCB2aG9zdF92aXJ0cXVl dWUgKnZxLAo+PiAgIAo+PiAgIAkvKiBHcmFiIHRoZSBuZXh0IGRlc2NyaXB0b3IgbnVtYmVyIHRo ZXkncmUgYWR2ZXJ0aXNpbmcsIGFuZCBpbmNyZW1lbnQKPj4gICAJICogdGhlIGluZGV4IHdlJ3Zl IHNlZW4uICovCj4+IC0JaWYgKHVubGlrZWx5KHZob3N0X2dldF9hdmFpbCh2cSwgcmluZ19oZWFk LAo+PiAtCQkgICAgICZ2cS0+YXZhaWwtPnJpbmdbbGFzdF9hdmFpbF9pZHggJiAodnEtPm51bSAt IDEpXSkpKSB7Cj4+IC0JCXZxX2Vycih2cSwgIkZhaWxlZCB0byByZWFkIGhlYWQ6IGlkeCAlZCBh ZGRyZXNzICVwXG4iLAo+PiAtCQkgICAgICAgbGFzdF9hdmFpbF9pZHgsCj4+IC0JCSAgICAgICAm dnEtPmF2YWlsLT5yaW5nW2xhc3RfYXZhaWxfaWR4ICUgdnEtPm51bV0pOwo+PiAtCQlyZXR1cm4g LUVGQVVMVDsKPj4gKwlpZiAoIWluX29yZGVyKSB7Cj4+ICsJCWlmICh1bmxpa2VseSh2aG9zdF9n ZXRfYXZhaWwodnEsIHJpbmdfaGVhZCwKPj4gKwkJICAgICZ2cS0+YXZhaWwtPnJpbmdbbGFzdF9h dmFpbF9pZHggJiAodnEtPm51bSAtIDEpXSkpKSB7Cj4+ICsJCQl2cV9lcnIodnEsICJGYWlsZWQg dG8gcmVhZCBoZWFkOiBpZHggJWQgYWRkcmVzcyAlcFxuIiwKPj4gKwkJCQlsYXN0X2F2YWlsX2lk eCwKPj4gKwkJCQkmdnEtPmF2YWlsLT5yaW5nW2xhc3RfYXZhaWxfaWR4ICUgdnEtPm51bV0pOwo+ PiArCQkJcmV0dXJuIC1FRkFVTFQ7Cj4+ICsJCX0KPj4gKwkJaGVhZCA9IHZob3N0MTZfdG9fY3B1 KHZxLCByaW5nX2hlYWQpOwo+PiArCX0gZWxzZSB7Cj4+ICsJCWhlYWQgPSBsYXN0X2F2YWlsX2lk eCAmICh2cS0+bnVtIC0gMSk7Cj4+ICAgCX0KPj4gICAKPj4gLQloZWFkID0gdmhvc3QxNl90b19j cHUodnEsIHJpbmdfaGVhZCk7Cj4+ICAgCj4+ICAgCS8qIElmIHRoZWlyIG51bWJlciBpcyBzaWxs eSwgdGhhdCdzIGFuIGVycm9yLiAqLwo+PiAgIAlpZiAodW5saWtlbHkoaGVhZCA+PSB2cS0+bnVt KSkgewo+PiAtLSAKPj4gMi4xNy4xCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fClZpcnR1YWxpemF0aW9uIG1haWxpbmcgbGlzdApWaXJ0dWFsaXphdGlvbkBs aXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRwczovL2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5v cmcvbWFpbG1hbi9saXN0aW5mby92aXJ0dWFsaXphdGlvbg==