From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch desc index Date: Thu, 28 Sep 2017 01:57:20 +0300 Message-ID: <20170928012906-mutt-send-email-mst@kernel.org> References: <1506067355-5771-1-git-send-email-jasowang@redhat.com> <1506067355-5771-3-git-send-email-jasowang@redhat.com> <20170926221435-mutt-send-email-mst@kernel.org> <17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com> 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 To: Jason Wang Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org List-Id: virtualization@lists.linuxfoundation.org T24gV2VkLCBTZXAgMjcsIDIwMTcgYXQgMDg6MzU6NDdBTSArMDgwMCwgSmFzb24gV2FuZyB3cm90 ZToKPiAKPiAKPiBPbiAyMDE35bm0MDnmnIgyN+aXpSAwMzoxOSwgTWljaGFlbCBTLiBUc2lya2lu IHdyb3RlOgo+ID4gT24gRnJpLCBTZXAgMjIsIDIwMTcgYXQgMDQ6MDI6MzJQTSArMDgwMCwgSmFz b24gV2FuZyB3cm90ZToKPiA+ID4gVGhpcyBwYXRjaCBpbnRyb2R1Y2VzIHZob3N0X3ByZWZldGNo X2Rlc2NfaW5kaWNlcygpIHdoaWNoIGNvdWxkIGJhdGNoCj4gPiA+IGRlc2NyaXB0b3IgaW5kaWNl cyBmZXRjaGluZyBhbmQgdXNlZCByaW5nIHVwZGF0aW5nLiBUaGlzIGludGVuZHMgdG8KPiA+ID4g cmVkdWNlIHRoZSBjYWNoZSBtaXNzZXMgb2YgaW5kaWNlcyBmZXRjaGluZyBhbmQgdXBkYXRpbmcg YW5kIHJlZHVjZQo+ID4gPiBjYWNoZSBsaW5lIGJvdW5jZSB3aGVuIHZpcnRxdWV1ZSBpcyBhbG1v c3QgZnVsbC4gY29weV90b191c2VyKCkgd2FzCj4gPiA+IHVzZWQgaW4gb3JkZXIgdG8gYmVuZWZp dCBmcm9tIG1vZGVybiBjcHVzIHRoYXQgc3VwcG9ydCBmYXN0IHN0cmluZwo+ID4gPiBjb3B5LiBC YXRjaGVkIHZpcnRxdWV1ZSBwcm9jZXNzaW5nIHdpbGwgYmUgdGhlIGZpcnN0IHVzZXIuCj4gPiA+ IAo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBKYXNvbiBXYW5nIDxqYXNvd2FuZ0ByZWRoYXQuY29tPgo+ ID4gPiAtLS0KPiA+ID4gICBkcml2ZXJzL3Zob3N0L3Zob3N0LmMgfCA1NSArKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ID4gICBkcml2ZXJzL3Zo b3N0L3Zob3N0LmggfCAgMyArKysKPiA+ID4gICAyIGZpbGVzIGNoYW5nZWQsIDU4IGluc2VydGlv bnMoKykKPiA+ID4gCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3Zob3N0L3Zob3N0LmMgYi9k cml2ZXJzL3Zob3N0L3Zob3N0LmMKPiA+ID4gaW5kZXggZjg3ZWM3NS4uODQyNDE2NmQgMTAwNjQ0 Cj4gPiA+IC0tLSBhL2RyaXZlcnMvdmhvc3Qvdmhvc3QuYwo+ID4gPiArKysgYi9kcml2ZXJzL3Zo b3N0L3Zob3N0LmMKPiA+ID4gQEAgLTI0MzcsNiArMjQzNyw2MSBAQCBzdHJ1Y3Qgdmhvc3RfbXNn X25vZGUgKnZob3N0X2RlcXVldWVfbXNnKHN0cnVjdCB2aG9zdF9kZXYgKmRldiwKPiA+ID4gICB9 Cj4gPiA+ICAgRVhQT1JUX1NZTUJPTF9HUEwodmhvc3RfZGVxdWV1ZV9tc2cpOwo+ID4gPiAraW50 IHZob3N0X3ByZWZldGNoX2Rlc2NfaW5kaWNlcyhzdHJ1Y3Qgdmhvc3RfdmlydHF1ZXVlICp2cSwK PiA+ID4gKwkJCQlzdHJ1Y3QgdnJpbmdfdXNlZF9lbGVtICpoZWFkcywKPiA+ID4gKwkJCQl1MTYg bnVtLCBib29sIHVzZWRfdXBkYXRlKQo+ID4gd2h5IGRvIHlvdSBuZWVkIHRvIGNvbWJpbmUgdXNl ZCB1cGRhdGUgd2l0aCBwcmVmZXRjaD8KPiAKPiBGb3IgYmV0dGVyIHBlcmZvcm1hbmNlCgoKV2h5 IGlzIHN0aWNraW5nIGEgYnJhbmNoIGluIHRoZXJlIGJldHRlciB0aGFuIHJlcXVlc3RpbmcgdGhl IHVwZGF0ZQpjb25kaXRpb25hbGx5IGZyb20gdGhlIGNhbGxlcj8KCgoKPiBhbmQgSSBiZWxpZXZl IHdlIGRvbid0IGNhcmUgYWJvdXQgdGhlIG92ZXJoZWFkIHdoZW4KPiB3ZSBtZWV0IGVycm9ycyBp biB0eC4KClRoYXQncyBhIHNlcGFyYXRlIHF1ZXN0aW9uLCBJIGRvIG5vdCByZWFsbHkgdW5kZXJz dGFuZCBob3cKeW91IGNhbiBmZXRjaCBhIGRlc2NyaXB0b3IgYW5kIHVwZGF0ZSB0aGUgdXNlZCBy aW5nIGF0IHRoZSBzYW1lCnRpbWUuIFRoaXMgYWxsb3dzIHRoZSBndWVzdCB0byBvdmVyd3JpdGUg dGhlIGJ1ZmZlci4KSSBtaWdodCBiZSBtaXN1bmRlcnN0YW5kaW5nIHdoYXQgaXMgZ29pbmcgb24g aGVyZSB0aG91Z2guCgoKPiA+IAo+ID4gPiArewo+ID4gPiArCWludCByZXQsIHJldDI7Cj4gPiA+ ICsJdTE2IGxhc3RfYXZhaWxfaWR4LCBsYXN0X3VzZWRfaWR4LCB0b3RhbCwgY29waWVkOwo+ID4g PiArCV9fdmlydGlvMTYgYXZhaWxfaWR4Owo+ID4gPiArCXN0cnVjdCB2cmluZ191c2VkX2VsZW0g X191c2VyICp1c2VkOwo+ID4gPiArCWludCBpOwo+ID4gPiArCj4gPiA+ICsJaWYgKHVubGlrZWx5 KHZob3N0X2dldF9hdmFpbCh2cSwgYXZhaWxfaWR4LCAmdnEtPmF2YWlsLT5pZHgpKSkgewo+ID4g PiArCQl2cV9lcnIodnEsICJGYWlsZWQgdG8gYWNjZXNzIGF2YWlsIGlkeCBhdCAlcFxuIiwKPiA+ ID4gKwkJICAgICAgICZ2cS0+YXZhaWwtPmlkeCk7Cj4gPiA+ICsJCXJldHVybiAtRUZBVUxUOwo+ ID4gPiArCX0KPiA+ID4gKwlsYXN0X2F2YWlsX2lkeCA9IHZxLT5sYXN0X2F2YWlsX2lkeCAmICh2 cS0+bnVtIC0gMSk7Cj4gPiA+ICsJdnEtPmF2YWlsX2lkeCA9IHZob3N0MTZfdG9fY3B1KHZxLCBh dmFpbF9pZHgpOwo+ID4gPiArCXRvdGFsID0gdnEtPmF2YWlsX2lkeCAtIHZxLT5sYXN0X2F2YWls X2lkeDsKPiA+ID4gKwlyZXQgPSB0b3RhbCA9IG1pbih0b3RhbCwgbnVtKTsKPiA+ID4gKwo+ID4g PiArCWZvciAoaSA9IDA7IGkgPCByZXQ7IGkrKykgewo+ID4gPiArCQlyZXQyID0gdmhvc3RfZ2V0 X2F2YWlsKHZxLCBoZWFkc1tpXS5pZCwKPiA+ID4gKwkJCQkgICAgICAmdnEtPmF2YWlsLT5yaW5n W2xhc3RfYXZhaWxfaWR4XSk7Cj4gPiA+ICsJCWlmICh1bmxpa2VseShyZXQyKSkgewo+ID4gPiAr CQkJdnFfZXJyKHZxLCAiRmFpbGVkIHRvIGdldCBkZXNjcmlwdG9yc1xuIik7Cj4gPiA+ICsJCQly ZXR1cm4gLUVGQVVMVDsKPiA+ID4gKwkJfQo+ID4gPiArCQlsYXN0X2F2YWlsX2lkeCA9IChsYXN0 X2F2YWlsX2lkeCArIDEpICYgKHZxLT5udW0gLSAxKTsKPiA+ID4gKwl9Cj4gPiA+ICsKPiA+ID4g KwlpZiAoIXVzZWRfdXBkYXRlKQo+ID4gPiArCQlyZXR1cm4gcmV0Owo+ID4gPiArCj4gPiA+ICsJ bGFzdF91c2VkX2lkeCA9IHZxLT5sYXN0X3VzZWRfaWR4ICYgKHZxLT5udW0gLSAxKTsKPiA+ID4g Kwl3aGlsZSAodG90YWwpIHsKPiA+ID4gKwkJY29waWVkID0gbWluKCh1MTYpKHZxLT5udW0gLSBs YXN0X3VzZWRfaWR4KSwgdG90YWwpOwo+ID4gPiArCQlyZXQyID0gdmhvc3RfY29weV90b191c2Vy KHZxLAo+ID4gPiArCQkJCQkgICZ2cS0+dXNlZC0+cmluZ1tsYXN0X3VzZWRfaWR4XSwKPiA+ID4g KwkJCQkJICAmaGVhZHNbcmV0IC0gdG90YWxdLAo+ID4gPiArCQkJCQkgIGNvcGllZCAqIHNpemVv ZigqdXNlZCkpOwo+ID4gPiArCj4gPiA+ICsJCWlmICh1bmxpa2VseShyZXQyKSkgewo+ID4gPiAr CQkJdnFfZXJyKHZxLCAiRmFpbGVkIHRvIHVwZGF0ZSB1c2VkIHJpbmchXG4iKTsKPiA+ID4gKwkJ CXJldHVybiAtRUZBVUxUOwo+ID4gPiArCQl9Cj4gPiA+ICsKPiA+ID4gKwkJbGFzdF91c2VkX2lk eCA9IDA7Cj4gPiA+ICsJCXRvdGFsIC09IGNvcGllZDsKPiA+ID4gKwl9Cj4gPiA+ICsKPiA+ID4g KwkvKiBPbmx5IGdldCBhdmFpbCByaW5nIGVudHJpZXMgYWZ0ZXIgdGhleSBoYXZlIGJlZW4gZXhw b3NlZCBieSBndWVzdC4gKi8KPiA+ID4gKwlzbXBfcm1iKCk7Cj4gPiBCYXJyaWVyIGJlZm9yZSBy ZXR1cm4gaXMgYSB2ZXJ5IGNvbmZ1c2luZyBBUEkuIEkgZ3Vlc3MgaXQncyBkZXNpZ25lZCB0bwo+ ID4gYmUgdXNlZCBpbiBhIHNwZWNpZmljIHdheSB0byBtYWtlIGl0IG5lY2Vzc2FyeSAtIGJ1dCB3 aGF0IGlzIGl0Pwo+IAo+IExvb2tzIGxpa2UgYSBhbmQgd2UgbmVlZCBkbyB0aGlzIGFmdGVyIHJl YWRpbmcgYXZhaWxfaWR4Lgo+IAo+IFRoYW5rcwo+IAo+ID4gCj4gPiAKPiA+ID4gKwlyZXR1cm4g cmV0Owo+ID4gPiArfQo+ID4gPiArRVhQT1JUX1NZTUJPTCh2aG9zdF9wcmVmZXRjaF9kZXNjX2lu ZGljZXMpOwo+ID4gPiAgIHN0YXRpYyBpbnQgX19pbml0IHZob3N0X2luaXQodm9pZCkKPiA+ID4g ICB7Cj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3Zob3N0L3Zob3N0LmggYi9kcml2ZXJzL3Zo b3N0L3Zob3N0LmgKPiA+ID4gaW5kZXggMzlmZjg5Ny4uMTZjMmNiNiAxMDA2NDQKPiA+ID4gLS0t IGEvZHJpdmVycy92aG9zdC92aG9zdC5oCj4gPiA+ICsrKyBiL2RyaXZlcnMvdmhvc3Qvdmhvc3Qu aAo+ID4gPiBAQCAtMjI4LDYgKzIyOCw5IEBAIHNzaXplX3Qgdmhvc3RfY2hyX3JlYWRfaXRlcihz dHJ1Y3Qgdmhvc3RfZGV2ICpkZXYsIHN0cnVjdCBpb3ZfaXRlciAqdG8sCj4gPiA+ICAgc3NpemVf dCB2aG9zdF9jaHJfd3JpdGVfaXRlcihzdHJ1Y3Qgdmhvc3RfZGV2ICpkZXYsCj4gPiA+ICAgCQkJ ICAgICBzdHJ1Y3QgaW92X2l0ZXIgKmZyb20pOwo+ID4gPiAgIGludCB2aG9zdF9pbml0X2Rldmlj ZV9pb3RsYihzdHJ1Y3Qgdmhvc3RfZGV2ICpkLCBib29sIGVuYWJsZWQpOwo+ID4gPiAraW50IHZo b3N0X3ByZWZldGNoX2Rlc2NfaW5kaWNlcyhzdHJ1Y3Qgdmhvc3RfdmlydHF1ZXVlICp2cSwKPiA+ ID4gKwkJCQlzdHJ1Y3QgdnJpbmdfdXNlZF9lbGVtICpoZWFkcywKPiA+ID4gKwkJCQl1MTYgbnVt LCBib29sIHVzZWRfdXBkYXRlKTsKPiA+ID4gICAjZGVmaW5lIHZxX2Vycih2cSwgZm10LCAuLi4p IGRvIHsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgXAo+ID4gPiAgIAkJcHJfZGVi dWcocHJfZm10KGZtdCksICMjX19WQV9BUkdTX18pOyAgICAgICBcCj4gPiA+IC0tIAo+ID4gPiAy LjcuNApfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpWaXJ0 dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlzdHMubGludXgtZm91bmRh dGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3JnL21haWxtYW4vbGlzdGlu Zm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752314AbdI0W5Y (ORCPT ); Wed, 27 Sep 2017 18:57:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36732 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751671AbdI0W5W (ORCPT ); Wed, 27 Sep 2017 18:57:22 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3E1A9117AA9 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=mst@redhat.com Date: Thu, 28 Sep 2017 01:57:20 +0300 From: "Michael S. Tsirkin" To: Jason Wang Cc: virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org Subject: Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch desc index Message-ID: <20170928012906-mutt-send-email-mst@kernel.org> References: <1506067355-5771-1-git-send-email-jasowang@redhat.com> <1506067355-5771-3-git-send-email-jasowang@redhat.com> <20170926221435-mutt-send-email-mst@kernel.org> <17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 27 Sep 2017 22:57:22 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 27, 2017 at 08:35:47AM +0800, Jason Wang wrote: > > > On 2017年09月27日 03:19, Michael S. Tsirkin wrote: > > On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote: > > > This patch introduces vhost_prefetch_desc_indices() which could batch > > > descriptor indices fetching and used ring updating. This intends to > > > reduce the cache misses of indices fetching and updating and reduce > > > cache line bounce when virtqueue is almost full. copy_to_user() was > > > used in order to benefit from modern cpus that support fast string > > > copy. Batched virtqueue processing will be the first user. > > > > > > Signed-off-by: Jason Wang > > > --- > > > drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++ > > > drivers/vhost/vhost.h | 3 +++ > > > 2 files changed, 58 insertions(+) > > > > > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c > > > index f87ec75..8424166d 100644 > > > --- a/drivers/vhost/vhost.c > > > +++ b/drivers/vhost/vhost.c > > > @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev, > > > } > > > EXPORT_SYMBOL_GPL(vhost_dequeue_msg); > > > +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq, > > > + struct vring_used_elem *heads, > > > + u16 num, bool used_update) > > why do you need to combine used update with prefetch? > > For better performance Why is sticking a branch in there better than requesting the update conditionally from the caller? > and I believe we don't care about the overhead when > we meet errors in tx. That's a separate question, I do not really understand how you can fetch a descriptor and update the used ring at the same time. This allows the guest to overwrite the buffer. I might be misunderstanding what is going on here though. > > > > > +{ > > > + int ret, ret2; > > > + u16 last_avail_idx, last_used_idx, total, copied; > > > + __virtio16 avail_idx; > > > + struct vring_used_elem __user *used; > > > + int i; > > > + > > > + if (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) { > > > + vq_err(vq, "Failed to access avail idx at %p\n", > > > + &vq->avail->idx); > > > + return -EFAULT; > > > + } > > > + last_avail_idx = vq->last_avail_idx & (vq->num - 1); > > > + vq->avail_idx = vhost16_to_cpu(vq, avail_idx); > > > + total = vq->avail_idx - vq->last_avail_idx; > > > + ret = total = min(total, num); > > > + > > > + for (i = 0; i < ret; i++) { > > > + ret2 = vhost_get_avail(vq, heads[i].id, > > > + &vq->avail->ring[last_avail_idx]); > > > + if (unlikely(ret2)) { > > > + vq_err(vq, "Failed to get descriptors\n"); > > > + return -EFAULT; > > > + } > > > + last_avail_idx = (last_avail_idx + 1) & (vq->num - 1); > > > + } > > > + > > > + if (!used_update) > > > + return ret; > > > + > > > + last_used_idx = vq->last_used_idx & (vq->num - 1); > > > + while (total) { > > > + copied = min((u16)(vq->num - last_used_idx), total); > > > + ret2 = vhost_copy_to_user(vq, > > > + &vq->used->ring[last_used_idx], > > > + &heads[ret - total], > > > + copied * sizeof(*used)); > > > + > > > + if (unlikely(ret2)) { > > > + vq_err(vq, "Failed to update used ring!\n"); > > > + return -EFAULT; > > > + } > > > + > > > + last_used_idx = 0; > > > + total -= copied; > > > + } > > > + > > > + /* Only get avail ring entries after they have been exposed by guest. */ > > > + smp_rmb(); > > Barrier before return is a very confusing API. I guess it's designed to > > be used in a specific way to make it necessary - but what is it? > > Looks like a and we need do this after reading avail_idx. > > Thanks > > > > > > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(vhost_prefetch_desc_indices); > > > static int __init vhost_init(void) > > > { > > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h > > > index 39ff897..16c2cb6 100644 > > > --- a/drivers/vhost/vhost.h > > > +++ b/drivers/vhost/vhost.h > > > @@ -228,6 +228,9 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to, > > > ssize_t vhost_chr_write_iter(struct vhost_dev *dev, > > > struct iov_iter *from); > > > int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled); > > > +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq, > > > + struct vring_used_elem *heads, > > > + u16 num, bool used_update); > > > #define vq_err(vq, fmt, ...) do { \ > > > pr_debug(pr_fmt(fmt), ##__VA_ARGS__); \ > > > -- > > > 2.7.4