[{"id":1773381,"web_url":"http://patchwork.ozlabs.org/comment/1773381/","msgid":"<20170922090257.GB9243@stefanha-x1.localdomain>","list_archive_url":null,"date":"2017-09-22T09:02:57","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":2747,"url":"http://patchwork.ozlabs.org/api/people/2747/","name":"Stefan Hajnoczi","email":"stefanha@gmail.com"},"content":"On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n> index f87ec75..8424166d 100644\n> --- a/drivers/vhost/vhost.c\n> +++ b/drivers/vhost/vhost.c\n> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>  }\n>  EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>  \n> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> +\t\t\t\tstruct vring_used_elem *heads,\n> +\t\t\t\tu16 num, bool used_update)\n\nMissing doc comment.\n\n> +{\n> +\tint ret, ret2;\n> +\tu16 last_avail_idx, last_used_idx, total, copied;\n> +\t__virtio16 avail_idx;\n> +\tstruct vring_used_elem __user *used;\n> +\tint i;\n\nThe following variable names are a little confusing:\n\nlast_avail_idx vs vq->last_avail_idx.  last_avail_idx is a wrapped\navail->ring[] index, vq->last_avail_idx is a free-running counter.  The\nsame for last_used_idx vs vq->last_used_idx.\n\nnum argument vs vq->num.  The argument could be called nheads instead to\nmake it clear that this is heads[] and not the virtqueue size.\n\nNot a bug but it took me a while to figure out what was going on.","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"ABE9JZAe\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xz6xl0Z1zz9sRW\n\tfor <patchwork-incoming@ozlabs.org>;\n\tFri, 22 Sep 2017 19:04:59 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752070AbdIVJDE (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tFri, 22 Sep 2017 05:03:04 -0400","from mail-wm0-f65.google.com ([74.125.82.65]:38708 \"EHLO\n\tmail-wm0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751936AbdIVJDB (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Fri, 22 Sep 2017 05:03:01 -0400","by mail-wm0-f65.google.com with SMTP id x17so654374wmd.5;\n\tFri, 22 Sep 2017 02:03:00 -0700 (PDT)","from localhost ([51.15.41.238]) by smtp.gmail.com with ESMTPSA id\n\t10sm3124371wrt.59.2017.09.22.02.02.58\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 22 Sep 2017 02:02:58 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=YKUeDk+1G7YO1A2wX7Yc05QBNI7KRFRwN00STU2b+Mg=;\n\tb=ABE9JZAe8/WCgQM1VYWWzFNrRAkvDEsN/bb0RjWIuCj2sB5LFwgXAnzQNPMim52kyZ\n\t0F5KG0QM1fMtBDcKVCeV68O9eMMazMmx1pvb8iRqZhuiFVUWPgRhGP1YSmzx1z/MTHMH\n\tAFvC6IY8JC4DIsnGQrxkiGxh3MRwmj4fwRvz2UZc0eNf94dB61g1Zj/EKISTf/pHzICO\n\t28D6ZfuwXBkZn8v+OFV7QRQbVtDgmPVgpzYEc6x/lT2eid8JpXx6wVyctquOViyYGJD7\n\tOV+mFmQ5MiCY9u9EL2ghBigWG86QhRZabNhSR7XXpQxXXvQAuq9mhnTKWP0xOywUR85h\n\tMjVg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=YKUeDk+1G7YO1A2wX7Yc05QBNI7KRFRwN00STU2b+Mg=;\n\tb=mYKQIu3WPaS/LneHsDkoKDyz4kwfWmMAkkYe0vD7hOCQhxiqHjfuDjTdJ7TKDgkusi\n\t0nqhuNGBC3H3fPjgAaPAsmcNqgRYJNcGGBdFWLjmjwiIu1tM7siECLN9xcNjQlJTEVwU\n\t9BvjW7VNxAtazEuUGKk8LhexjeEKc/l0BGFP1xfuFH/TjhzABdNzNOrh2QccnQb2Kl10\n\tJmciRr0dq9NH9G3Qldy4dMmyLTu1jEVtcdIQMkIUrcozY0Akig3vp+f5lGJSD3qlObQw\n\ttU2K7X8rUvLf0DI/oDVl2xoQbobOTqFPzTy0IiwF1XqwysIAbw2jT7h0kWgUi4CV6G0B\n\tFNxg==","X-Gm-Message-State":"AHPjjUjvi3dE3goYzfdY7Tp99zJeZcRQa0D022RPphI14CYPJLLqnNPW\n\tBuEuO/4SC+qByTxu/6MJxHpkz5fw","X-Google-Smtp-Source":"AOwi7QDG0QAvjDyApBwuCGQUZUyCoNmHc3lHa4RmUQ31A5zrFOEgRf+Nk41WjgVvd/pLU27PlpICpw==","X-Received":"by 10.28.238.72 with SMTP id m69mr3577863wmh.63.1506070979660;\n\tFri, 22 Sep 2017 02:02:59 -0700 (PDT)","Date":"Fri, 22 Sep 2017 10:02:57 +0100","From":"Stefan Hajnoczi <stefanha@gmail.com>","To":"Jason Wang <jasowang@redhat.com>","Cc":"mst@redhat.com, virtualization@lists.linux-foundation.org,\n\tnetdev@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\n\tdesc index","Message-ID":"<20170922090257.GB9243@stefanha-x1.localdomain>","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1506067355-5771-3-git-send-email-jasowang@redhat.com>","User-Agent":"Mutt/1.8.3 (2017-05-23)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1774332,"web_url":"http://patchwork.ozlabs.org/comment/1774332/","msgid":"<33bdcceb-9f12-e5e9-4b4d-6d619e17a206@redhat.com>","list_archive_url":null,"date":"2017-09-25T02:04:13","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"On 2017年09月22日 17:02, Stefan Hajnoczi wrote:\n> On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n>> index f87ec75..8424166d 100644\n>> --- a/drivers/vhost/vhost.c\n>> +++ b/drivers/vhost/vhost.c\n>> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>>   }\n>>   EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>>   \n>> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n>> +\t\t\t\tstruct vring_used_elem *heads,\n>> +\t\t\t\tu16 num, bool used_update)\n> Missing doc comment.\n\nWill fix this.\n\n>\n>> +{\n>> +\tint ret, ret2;\n>> +\tu16 last_avail_idx, last_used_idx, total, copied;\n>> +\t__virtio16 avail_idx;\n>> +\tstruct vring_used_elem __user *used;\n>> +\tint i;\n> The following variable names are a little confusing:\n>\n> last_avail_idx vs vq->last_avail_idx.  last_avail_idx is a wrapped\n> avail->ring[] index, vq->last_avail_idx is a free-running counter.  The\n> same for last_used_idx vs vq->last_used_idx.\n>\n> num argument vs vq->num.  The argument could be called nheads instead to\n> make it clear that this is heads[] and not the virtqueue size.\n>\n> Not a bug but it took me a while to figure out what was going on.\n\nI admit the name is confusing. Let me try better ones in V2.\n\nThanks","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jasowang@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0nTQ6GzGz9t3t\n\tfor <patchwork-incoming@ozlabs.org>;\n\tMon, 25 Sep 2017 12:04:42 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753575AbdIYCEd (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tSun, 24 Sep 2017 22:04:33 -0400","from mx1.redhat.com ([209.132.183.28]:53790 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753388AbdIYCEb (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tSun, 24 Sep 2017 22:04:31 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 97670883CA;\n\tMon, 25 Sep 2017 02:04:31 +0000 (UTC)","from [10.72.12.85] (ovpn-12-85.pek2.redhat.com [10.72.12.85])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 45CF777C1F;\n\tMon, 25 Sep 2017 02:04:16 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 97670883CA","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","To":"Stefan Hajnoczi <stefanha@gmail.com>","Cc":"mst@redhat.com, virtualization@lists.linux-foundation.org,\n\tnetdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>\n\t<20170922090257.GB9243@stefanha-x1.localdomain>","From":"Jason Wang <jasowang@redhat.com>","Message-ID":"<33bdcceb-9f12-e5e9-4b4d-6d619e17a206@redhat.com>","Date":"Mon, 25 Sep 2017 10:04:13 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170922090257.GB9243@stefanha-x1.localdomain>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tMon, 25 Sep 2017 02:04:31 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1775800,"web_url":"http://patchwork.ozlabs.org/comment/1775800/","msgid":"<20170926221435-mutt-send-email-mst@kernel.org>","list_archive_url":null,"date":"2017-09-26T19:19:47","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":2235,"url":"http://patchwork.ozlabs.org/api/people/2235/","name":"Michael S. Tsirkin","email":"mst@redhat.com"},"content":"On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n> This patch introduces vhost_prefetch_desc_indices() which could batch\n> descriptor indices fetching and used ring updating. This intends to\n> reduce the cache misses of indices fetching and updating and reduce\n> cache line bounce when virtqueue is almost full. copy_to_user() was\n> used in order to benefit from modern cpus that support fast string\n> copy. Batched virtqueue processing will be the first user.\n> \n> Signed-off-by: Jason Wang <jasowang@redhat.com>\n> ---\n>  drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>  drivers/vhost/vhost.h |  3 +++\n>  2 files changed, 58 insertions(+)\n> \n> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n> index f87ec75..8424166d 100644\n> --- a/drivers/vhost/vhost.c\n> +++ b/drivers/vhost/vhost.c\n> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>  }\n>  EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>  \n> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> +\t\t\t\tstruct vring_used_elem *heads,\n> +\t\t\t\tu16 num, bool used_update)\n\nwhy do you need to combine used update with prefetch?\n\n> +{\n> +\tint ret, ret2;\n> +\tu16 last_avail_idx, last_used_idx, total, copied;\n> +\t__virtio16 avail_idx;\n> +\tstruct vring_used_elem __user *used;\n> +\tint i;\n> +\n> +\tif (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {\n> +\t\tvq_err(vq, \"Failed to access avail idx at %p\\n\",\n> +\t\t       &vq->avail->idx);\n> +\t\treturn -EFAULT;\n> +\t}\n> +\tlast_avail_idx = vq->last_avail_idx & (vq->num - 1);\n> +\tvq->avail_idx = vhost16_to_cpu(vq, avail_idx);\n> +\ttotal = vq->avail_idx - vq->last_avail_idx;\n> +\tret = total = min(total, num);\n> +\n> +\tfor (i = 0; i < ret; i++) {\n> +\t\tret2 = vhost_get_avail(vq, heads[i].id,\n> +\t\t\t\t      &vq->avail->ring[last_avail_idx]);\n> +\t\tif (unlikely(ret2)) {\n> +\t\t\tvq_err(vq, \"Failed to get descriptors\\n\");\n> +\t\t\treturn -EFAULT;\n> +\t\t}\n> +\t\tlast_avail_idx = (last_avail_idx + 1) & (vq->num - 1);\n> +\t}\n> +\n> +\tif (!used_update)\n> +\t\treturn ret;\n> +\n> +\tlast_used_idx = vq->last_used_idx & (vq->num - 1);\n> +\twhile (total) {\n> +\t\tcopied = min((u16)(vq->num - last_used_idx), total);\n> +\t\tret2 = vhost_copy_to_user(vq,\n> +\t\t\t\t\t  &vq->used->ring[last_used_idx],\n> +\t\t\t\t\t  &heads[ret - total],\n> +\t\t\t\t\t  copied * sizeof(*used));\n> +\n> +\t\tif (unlikely(ret2)) {\n> +\t\t\tvq_err(vq, \"Failed to update used ring!\\n\");\n> +\t\t\treturn -EFAULT;\n> +\t\t}\n> +\n> +\t\tlast_used_idx = 0;\n> +\t\ttotal -= copied;\n> +\t}\n> +\n> +\t/* Only get avail ring entries after they have been exposed by guest. */\n> +\tsmp_rmb();\n\nBarrier before return is a very confusing API. I guess it's designed to\nbe used in a specific way to make it necessary - but what is it?\n\n\n> +\treturn ret;\n> +}\n> +EXPORT_SYMBOL(vhost_prefetch_desc_indices);\n>  \n>  static int __init vhost_init(void)\n>  {\n> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h\n> index 39ff897..16c2cb6 100644\n> --- a/drivers/vhost/vhost.h\n> +++ b/drivers/vhost/vhost.h\n> @@ -228,6 +228,9 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to,\n>  ssize_t vhost_chr_write_iter(struct vhost_dev *dev,\n>  \t\t\t     struct iov_iter *from);\n>  int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled);\n> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> +\t\t\t\tstruct vring_used_elem *heads,\n> +\t\t\t\tu16 num, bool used_update);\n>  \n>  #define vq_err(vq, fmt, ...) do {                                  \\\n>  \t\tpr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \\\n> -- \n> 2.7.4","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx02.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=mst@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1rPW4Y6zz9t4b\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 05:19:59 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1030416AbdIZTTt (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 26 Sep 2017 15:19:49 -0400","from mx1.redhat.com ([209.132.183.28]:46848 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S965181AbdIZTTs (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tTue, 26 Sep 2017 15:19:48 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3DCB4883AE;\n\tTue, 26 Sep 2017 19:19:48 +0000 (UTC)","from redhat.com (ovpn-122-9.rdu2.redhat.com [10.10.122.9])\n\tby smtp.corp.redhat.com (Postfix) with SMTP id 9AD5160841;\n\tTue, 26 Sep 2017 19:19:47 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3DCB4883AE","Date":"Tue, 26 Sep 2017 22:19:47 +0300","From":"\"Michael S. Tsirkin\" <mst@redhat.com>","To":"Jason Wang <jasowang@redhat.com>","Cc":"virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, kvm@vger.kernel.org","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","Message-ID":"<20170926221435-mutt-send-email-mst@kernel.org>","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1506067355-5771-3-git-send-email-jasowang@redhat.com>","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.26]);\n\tTue, 26 Sep 2017 19:19:48 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1775964,"web_url":"http://patchwork.ozlabs.org/comment/1775964/","msgid":"<17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com>","list_archive_url":null,"date":"2017-09-27T00:35:47","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"On 2017年09月27日 03:19, Michael S. Tsirkin wrote:\n> On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n>> This patch introduces vhost_prefetch_desc_indices() which could batch\n>> descriptor indices fetching and used ring updating. This intends to\n>> reduce the cache misses of indices fetching and updating and reduce\n>> cache line bounce when virtqueue is almost full. copy_to_user() was\n>> used in order to benefit from modern cpus that support fast string\n>> copy. Batched virtqueue processing will be the first user.\n>>\n>> Signed-off-by: Jason Wang <jasowang@redhat.com>\n>> ---\n>>   drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>>   drivers/vhost/vhost.h |  3 +++\n>>   2 files changed, 58 insertions(+)\n>>\n>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n>> index f87ec75..8424166d 100644\n>> --- a/drivers/vhost/vhost.c\n>> +++ b/drivers/vhost/vhost.c\n>> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>>   }\n>>   EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>>   \n>> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n>> +\t\t\t\tstruct vring_used_elem *heads,\n>> +\t\t\t\tu16 num, bool used_update)\n> why do you need to combine used update with prefetch?\n\nFor better performance and I believe we don't care about the overhead \nwhen we meet errors in tx.\n\n>\n>> +{\n>> +\tint ret, ret2;\n>> +\tu16 last_avail_idx, last_used_idx, total, copied;\n>> +\t__virtio16 avail_idx;\n>> +\tstruct vring_used_elem __user *used;\n>> +\tint i;\n>> +\n>> +\tif (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {\n>> +\t\tvq_err(vq, \"Failed to access avail idx at %p\\n\",\n>> +\t\t       &vq->avail->idx);\n>> +\t\treturn -EFAULT;\n>> +\t}\n>> +\tlast_avail_idx = vq->last_avail_idx & (vq->num - 1);\n>> +\tvq->avail_idx = vhost16_to_cpu(vq, avail_idx);\n>> +\ttotal = vq->avail_idx - vq->last_avail_idx;\n>> +\tret = total = min(total, num);\n>> +\n>> +\tfor (i = 0; i < ret; i++) {\n>> +\t\tret2 = vhost_get_avail(vq, heads[i].id,\n>> +\t\t\t\t      &vq->avail->ring[last_avail_idx]);\n>> +\t\tif (unlikely(ret2)) {\n>> +\t\t\tvq_err(vq, \"Failed to get descriptors\\n\");\n>> +\t\t\treturn -EFAULT;\n>> +\t\t}\n>> +\t\tlast_avail_idx = (last_avail_idx + 1) & (vq->num - 1);\n>> +\t}\n>> +\n>> +\tif (!used_update)\n>> +\t\treturn ret;\n>> +\n>> +\tlast_used_idx = vq->last_used_idx & (vq->num - 1);\n>> +\twhile (total) {\n>> +\t\tcopied = min((u16)(vq->num - last_used_idx), total);\n>> +\t\tret2 = vhost_copy_to_user(vq,\n>> +\t\t\t\t\t  &vq->used->ring[last_used_idx],\n>> +\t\t\t\t\t  &heads[ret - total],\n>> +\t\t\t\t\t  copied * sizeof(*used));\n>> +\n>> +\t\tif (unlikely(ret2)) {\n>> +\t\t\tvq_err(vq, \"Failed to update used ring!\\n\");\n>> +\t\t\treturn -EFAULT;\n>> +\t\t}\n>> +\n>> +\t\tlast_used_idx = 0;\n>> +\t\ttotal -= copied;\n>> +\t}\n>> +\n>> +\t/* Only get avail ring entries after they have been exposed by guest. */\n>> +\tsmp_rmb();\n> Barrier before return is a very confusing API. I guess it's designed to\n> be used in a specific way to make it necessary - but what is it?\n\nLooks like a and we need do this after reading avail_idx.\n\nThanks\n\n>\n>\n>> +\treturn ret;\n>> +}\n>> +EXPORT_SYMBOL(vhost_prefetch_desc_indices);\n>>   \n>>   static int __init vhost_init(void)\n>>   {\n>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h\n>> index 39ff897..16c2cb6 100644\n>> --- a/drivers/vhost/vhost.h\n>> +++ b/drivers/vhost/vhost.h\n>> @@ -228,6 +228,9 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to,\n>>   ssize_t vhost_chr_write_iter(struct vhost_dev *dev,\n>>   \t\t\t     struct iov_iter *from);\n>>   int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled);\n>> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n>> +\t\t\t\tstruct vring_used_elem *heads,\n>> +\t\t\t\tu16 num, bool used_update);\n>>   \n>>   #define vq_err(vq, fmt, ...) do {                                  \\\n>>   \t\tpr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \\\n>> -- \n>> 2.7.4","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx05.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jasowang@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1zQF27qgz9t3x\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 27 Sep 2017 10:36:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1030765AbdI0Af4 (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 26 Sep 2017 20:35:56 -0400","from mx1.redhat.com ([209.132.183.28]:47764 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S967681AbdI0Afy (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tTue, 26 Sep 2017 20:35:54 -0400","from smtp.corp.redhat.com\n\t(int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 705F413AA4;\n\tWed, 27 Sep 2017 00:35:54 +0000 (UTC)","from [10.72.12.40] (ovpn-12-40.pek2.redhat.com [10.72.12.40])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id D8A9578A9F;\n\tWed, 27 Sep 2017 00:35:50 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 705F413AA4","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","To":"\"Michael S. Tsirkin\" <mst@redhat.com>","Cc":"virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, kvm@vger.kernel.org","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>\n\t<20170926221435-mutt-send-email-mst@kernel.org>","From":"Jason Wang <jasowang@redhat.com>","Message-ID":"<17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com>","Date":"Wed, 27 Sep 2017 08:35:47 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170926221435-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.79 on 10.5.11.14","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.29]);\n\tWed, 27 Sep 2017 00:35:54 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776675,"web_url":"http://patchwork.ozlabs.org/comment/1776675/","msgid":"<20170928012906-mutt-send-email-mst@kernel.org>","list_archive_url":null,"date":"2017-09-27T22:57:20","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":2235,"url":"http://patchwork.ozlabs.org/api/people/2235/","name":"Michael S. Tsirkin","email":"mst@redhat.com"},"content":"On Wed, Sep 27, 2017 at 08:35:47AM +0800, Jason Wang wrote:\n> \n> \n> On 2017年09月27日 03:19, Michael S. Tsirkin wrote:\n> > On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n> > > This patch introduces vhost_prefetch_desc_indices() which could batch\n> > > descriptor indices fetching and used ring updating. This intends to\n> > > reduce the cache misses of indices fetching and updating and reduce\n> > > cache line bounce when virtqueue is almost full. copy_to_user() was\n> > > used in order to benefit from modern cpus that support fast string\n> > > copy. Batched virtqueue processing will be the first user.\n> > > \n> > > Signed-off-by: Jason Wang <jasowang@redhat.com>\n> > > ---\n> > >   drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n> > >   drivers/vhost/vhost.h |  3 +++\n> > >   2 files changed, 58 insertions(+)\n> > > \n> > > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n> > > index f87ec75..8424166d 100644\n> > > --- a/drivers/vhost/vhost.c\n> > > +++ b/drivers/vhost/vhost.c\n> > > @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n> > >   }\n> > >   EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n> > > +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> > > +\t\t\t\tstruct vring_used_elem *heads,\n> > > +\t\t\t\tu16 num, bool used_update)\n> > why do you need to combine used update with prefetch?\n> \n> For better performance\n\n\nWhy is sticking a branch in there better than requesting the update\nconditionally from the caller?\n\n\n\n> and I believe we don't care about the overhead when\n> we meet errors in tx.\n\nThat's a separate question, I do not really understand how\nyou can fetch a descriptor and update the used ring at the same\ntime. This allows the guest to overwrite the buffer.\nI might be misunderstanding what is going on here though.\n\n\n> > \n> > > +{\n> > > +\tint ret, ret2;\n> > > +\tu16 last_avail_idx, last_used_idx, total, copied;\n> > > +\t__virtio16 avail_idx;\n> > > +\tstruct vring_used_elem __user *used;\n> > > +\tint i;\n> > > +\n> > > +\tif (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {\n> > > +\t\tvq_err(vq, \"Failed to access avail idx at %p\\n\",\n> > > +\t\t       &vq->avail->idx);\n> > > +\t\treturn -EFAULT;\n> > > +\t}\n> > > +\tlast_avail_idx = vq->last_avail_idx & (vq->num - 1);\n> > > +\tvq->avail_idx = vhost16_to_cpu(vq, avail_idx);\n> > > +\ttotal = vq->avail_idx - vq->last_avail_idx;\n> > > +\tret = total = min(total, num);\n> > > +\n> > > +\tfor (i = 0; i < ret; i++) {\n> > > +\t\tret2 = vhost_get_avail(vq, heads[i].id,\n> > > +\t\t\t\t      &vq->avail->ring[last_avail_idx]);\n> > > +\t\tif (unlikely(ret2)) {\n> > > +\t\t\tvq_err(vq, \"Failed to get descriptors\\n\");\n> > > +\t\t\treturn -EFAULT;\n> > > +\t\t}\n> > > +\t\tlast_avail_idx = (last_avail_idx + 1) & (vq->num - 1);\n> > > +\t}\n> > > +\n> > > +\tif (!used_update)\n> > > +\t\treturn ret;\n> > > +\n> > > +\tlast_used_idx = vq->last_used_idx & (vq->num - 1);\n> > > +\twhile (total) {\n> > > +\t\tcopied = min((u16)(vq->num - last_used_idx), total);\n> > > +\t\tret2 = vhost_copy_to_user(vq,\n> > > +\t\t\t\t\t  &vq->used->ring[last_used_idx],\n> > > +\t\t\t\t\t  &heads[ret - total],\n> > > +\t\t\t\t\t  copied * sizeof(*used));\n> > > +\n> > > +\t\tif (unlikely(ret2)) {\n> > > +\t\t\tvq_err(vq, \"Failed to update used ring!\\n\");\n> > > +\t\t\treturn -EFAULT;\n> > > +\t\t}\n> > > +\n> > > +\t\tlast_used_idx = 0;\n> > > +\t\ttotal -= copied;\n> > > +\t}\n> > > +\n> > > +\t/* Only get avail ring entries after they have been exposed by guest. */\n> > > +\tsmp_rmb();\n> > Barrier before return is a very confusing API. I guess it's designed to\n> > be used in a specific way to make it necessary - but what is it?\n> \n> Looks like a and we need do this after reading avail_idx.\n> \n> Thanks\n> \n> > \n> > \n> > > +\treturn ret;\n> > > +}\n> > > +EXPORT_SYMBOL(vhost_prefetch_desc_indices);\n> > >   static int __init vhost_init(void)\n> > >   {\n> > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h\n> > > index 39ff897..16c2cb6 100644\n> > > --- a/drivers/vhost/vhost.h\n> > > +++ b/drivers/vhost/vhost.h\n> > > @@ -228,6 +228,9 @@ ssize_t vhost_chr_read_iter(struct vhost_dev *dev, struct iov_iter *to,\n> > >   ssize_t vhost_chr_write_iter(struct vhost_dev *dev,\n> > >   \t\t\t     struct iov_iter *from);\n> > >   int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled);\n> > > +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> > > +\t\t\t\tstruct vring_used_elem *heads,\n> > > +\t\t\t\tu16 num, bool used_update);\n> > >   #define vq_err(vq, fmt, ...) do {                                  \\\n> > >   \t\tpr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \\\n> > > -- \n> > > 2.7.4","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=mst@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2YBC6tRYz9t6C\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 08:57:39 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752201AbdI0W5X (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 18:57:23 -0400","from mx1.redhat.com ([209.132.183.28]:36732 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751671AbdI0W5W (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tWed, 27 Sep 2017 18:57:22 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 3E1A9117AA9;\n\tWed, 27 Sep 2017 22:57:22 +0000 (UTC)","from redhat.com (ovpn-123-149.rdu2.redhat.com [10.10.123.149])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 6072E97BBC;\n\tWed, 27 Sep 2017 22:57:21 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 3E1A9117AA9","Date":"Thu, 28 Sep 2017 01:57:20 +0300","From":"\"Michael S. Tsirkin\" <mst@redhat.com>","To":"Jason Wang <jasowang@redhat.com>","Cc":"virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, kvm@vger.kernel.org","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","Message-ID":"<20170928012906-mutt-send-email-mst@kernel.org>","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>\n\t<20170926221435-mutt-send-email-mst@kernel.org>\n\t<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-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tWed, 27 Sep 2017 22:57:22 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776707,"web_url":"http://patchwork.ozlabs.org/comment/1776707/","msgid":"<CAF=yD-JC9g1gd-jsSjnLzbwAnk-LsPuj_JhF2NWf1rdxiWDwwA@mail.gmail.com>","list_archive_url":null,"date":"2017-09-28T00:47:25","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":67615,"url":"http://patchwork.ozlabs.org/api/people/67615/","name":"Willem de Bruijn","email":"willemdebruijn.kernel@gmail.com"},"content":"On Fri, Sep 22, 2017 at 4:02 AM, Jason Wang <jasowang@redhat.com> wrote:\n> This patch introduces vhost_prefetch_desc_indices() which could batch\n> descriptor indices fetching and used ring updating. This intends to\n> reduce the cache misses of indices fetching and updating and reduce\n> cache line bounce when virtqueue is almost full. copy_to_user() was\n> used in order to benefit from modern cpus that support fast string\n> copy. Batched virtqueue processing will be the first user.\n>\n> Signed-off-by: Jason Wang <jasowang@redhat.com>\n> ---\n>  drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>  drivers/vhost/vhost.h |  3 +++\n>  2 files changed, 58 insertions(+)\n>\n> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n> index f87ec75..8424166d 100644\n> --- a/drivers/vhost/vhost.c\n> +++ b/drivers/vhost/vhost.c\n> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>  }\n>  EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>\n> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n> +                               struct vring_used_elem *heads,\n> +                               u16 num, bool used_update)\n> +{\n> +       int ret, ret2;\n> +       u16 last_avail_idx, last_used_idx, total, copied;\n> +       __virtio16 avail_idx;\n> +       struct vring_used_elem __user *used;\n> +       int i;\n> +\n> +       if (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {\n> +               vq_err(vq, \"Failed to access avail idx at %p\\n\",\n> +                      &vq->avail->idx);\n> +               return -EFAULT;\n> +       }\n> +       last_avail_idx = vq->last_avail_idx & (vq->num - 1);\n> +       vq->avail_idx = vhost16_to_cpu(vq, avail_idx);\n> +       total = vq->avail_idx - vq->last_avail_idx;\n> +       ret = total = min(total, num);\n> +\n> +       for (i = 0; i < ret; i++) {\n> +               ret2 = vhost_get_avail(vq, heads[i].id,\n> +                                     &vq->avail->ring[last_avail_idx]);\n> +               if (unlikely(ret2)) {\n> +                       vq_err(vq, \"Failed to get descriptors\\n\");\n> +                       return -EFAULT;\n> +               }\n> +               last_avail_idx = (last_avail_idx + 1) & (vq->num - 1);\n> +       }\n\nThis is understandably very similar to the existing logic in vhost_get_vq_desc.\nCan that be extracted to a helper to avoid code duplication?\n\nPerhaps one helper to update vq->avail_idx and return num, and\nanother to call vhost_get_avail one or more times.\n\n> +\n> +       if (!used_update)\n> +               return ret;\n> +\n> +       last_used_idx = vq->last_used_idx & (vq->num - 1);\n> +       while (total) {\n> +               copied = min((u16)(vq->num - last_used_idx), total);\n> +               ret2 = vhost_copy_to_user(vq,\n> +                                         &vq->used->ring[last_used_idx],\n> +                                         &heads[ret - total],\n> +                                         copied * sizeof(*used));\n> +\n> +               if (unlikely(ret2)) {\n> +                       vq_err(vq, \"Failed to update used ring!\\n\");\n> +                       return -EFAULT;\n> +               }\n> +\n> +               last_used_idx = 0;\n> +               total -= copied;\n> +       }\n\nThis second part seems unrelated and could be a separate function?\n\nAlso, no need for ret2 and double assignment \"ret = total =\" if not\nmodifying total\nin the the second loop:\n\n  for (i = 0; i < total; ) {\n    ...\n    i += copied;\n  }","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Z5a27cYM\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2bdt66ZDz9t30\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 10:48:18 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752542AbdI1AsJ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tWed, 27 Sep 2017 20:48:09 -0400","from mail-oi0-f65.google.com ([209.85.218.65]:34673 \"EHLO\n\tmail-oi0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752445AbdI1AsH (ORCPT\n\t<rfc822;netdev@vger.kernel.org>); Wed, 27 Sep 2017 20:48:07 -0400","by mail-oi0-f65.google.com with SMTP id v188so10231357oia.1;\n\tWed, 27 Sep 2017 17:48:06 -0700 (PDT)","by 10.168.31.195 with HTTP; Wed, 27 Sep 2017 17:47:25 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=akRY8Pex4Fiuas7ft0kraEuuPlbiEpiVaxkDP3VEE+A=;\n\tb=Z5a27cYM0fWO0IMJ1K5GLfO/rFJcyJ5UcD0yaoElC1WUAmvndvBlXCUA0T0+6Exc5A\n\tTxONSZPTuua4dqqR7rAnftOvLMKYZp2UTCDxHiri98v8An1G8TPxqXWmau7x8FZrjpkF\n\t0OLhbs7ECQYJ6aw+yodn8LzVh7R8LUVSnQyn8a+JxDCkXKH1hdkc76CHe5apKvaWltc3\n\te16KQl0touj0lmxzSC6D4We1LQhZ+wB1XZz3WG0hR3eXxkZ5UUCfy9E0g+f9jCCmcFjD\n\tdI34iYKNiMH+0TBDdKqRHYHroS0jwNUtajdaxwfDAnnTEXKQ0xmrg51Yc/SYmcUrojl7\n\tpuPA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=akRY8Pex4Fiuas7ft0kraEuuPlbiEpiVaxkDP3VEE+A=;\n\tb=USqKt8Z1uVa+//BiHVRS0dCZ22DKl2FitqEVJCQx81Sur9veHa+Fjidwm55OOzLl6O\n\tzRoFysedw2OgBtLC+F8XGLxBOIAzcjaxTxbNNRbhmh9WdYggHx/wBy44FkWIwkrwRRLU\n\tZL9l/JkiC+WvV2z53N4BffXOEu8Z5FmLXXfZ5ebSVtOIxRka2+27VSU3vJtg8J2XET4Z\n\t2+4tYXB24LOQPyDpeCnNsGSkEQZ8FB//5/Yatr/6qLSriFVaKmsYpa7Vk4fnlunYElU9\n\tmkiw5EDvUBsg0zjTwkF6qeHfqoK5a/ejOZq5OxdoXf0sbW7a6F8NyB6eYnldV18okPgC\n\tAQUw==","X-Gm-Message-State":"AMCzsaUilySkVeCWLRbN4bhiTxvgKBJSlq07Oyr8886kG4yTMKGsghSH\n\t1OYNmIwYf/M/AwbeaDfdsJ6D2rhDv+V+CLwf5OE=","X-Google-Smtp-Source":"AOwi7QBwDvdgB6EofI/F2OJOWEmwepXbUddUiGC06HNW9tHnox6rmPTaC8Aie9+QtB4rWJYprmTqWEdiOhy9s+LBDcg=","X-Received":"by 10.157.80.24 with SMTP id a24mr453446oth.227.1506559686137;\n\tWed, 27 Sep 2017 17:48:06 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1506067355-5771-3-git-send-email-jasowang@redhat.com>","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>","From":"Willem de Bruijn <willemdebruijn.kernel@gmail.com>","Date":"Wed, 27 Sep 2017 20:47:25 -0400","Message-ID":"<CAF=yD-JC9g1gd-jsSjnLzbwAnk-LsPuj_JhF2NWf1rdxiWDwwA@mail.gmail.com>","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","To":"Jason Wang <jasowang@redhat.com>","Cc":"\"Michael S. Tsirkin\" <mst@redhat.com>,\n\tvirtualization@lists.linux-foundation.org,\n\tNetwork Development <netdev@vger.kernel.org>,\n\tLKML <linux-kernel@vger.kernel.org>, kvm@vger.kernel.org","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776816,"web_url":"http://patchwork.ozlabs.org/comment/1776816/","msgid":"<502eb735-3aea-3799-1f92-87697d4ef198@redhat.com>","list_archive_url":null,"date":"2017-09-28T07:18:45","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"On 2017年09月28日 06:57, Michael S. Tsirkin wrote:\n> On Wed, Sep 27, 2017 at 08:35:47AM +0800, Jason Wang wrote:\n>>\n>> On 2017年09月27日 03:19, Michael S. Tsirkin wrote:\n>>> On Fri, Sep 22, 2017 at 04:02:32PM +0800, Jason Wang wrote:\n>>>> This patch introduces vhost_prefetch_desc_indices() which could batch\n>>>> descriptor indices fetching and used ring updating. This intends to\n>>>> reduce the cache misses of indices fetching and updating and reduce\n>>>> cache line bounce when virtqueue is almost full. copy_to_user() was\n>>>> used in order to benefit from modern cpus that support fast string\n>>>> copy. Batched virtqueue processing will be the first user.\n>>>>\n>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>\n>>>> ---\n>>>>    drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>>>>    drivers/vhost/vhost.h |  3 +++\n>>>>    2 files changed, 58 insertions(+)\n>>>>\n>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n>>>> index f87ec75..8424166d 100644\n>>>> --- a/drivers/vhost/vhost.c\n>>>> +++ b/drivers/vhost/vhost.c\n>>>> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>>>>    }\n>>>>    EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>>>> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n>>>> +\t\t\t\tstruct vring_used_elem *heads,\n>>>> +\t\t\t\tu16 num, bool used_update)\n>>> why do you need to combine used update with prefetch?\n>> For better performance\n>\n> Why is sticking a branch in there better than requesting the update\n> conditionally from the caller?\n\nOk, I get your point, I can split the two functions.\n\n>\n>\n>\n>> and I believe we don't care about the overhead when\n>> we meet errors in tx.\n> That's a separate question, I do not really understand how\n> you can fetch a descriptor and update the used ring at the same\n> time. This allows the guest to overwrite the buffer.\n> I might be misunderstanding what is going on here though.\n\nWe don't update used idx, so guest can't overwrite the buffer I think?\n\nThanks","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx04.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jasowang@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2mJn0trwz9t5x\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 17:19:05 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751920AbdI1HSy (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 28 Sep 2017 03:18:54 -0400","from mx1.redhat.com ([209.132.183.28]:16858 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1750794AbdI1HSx (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tThu, 28 Sep 2017 03:18:53 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 06CD5344DB2;\n\tThu, 28 Sep 2017 07:18:53 +0000 (UTC)","from [10.72.12.139] (ovpn-12-139.pek2.redhat.com [10.72.12.139])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 4444A86E60;\n\tThu, 28 Sep 2017 07:18:47 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 06CD5344DB2","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","To":"\"Michael S. Tsirkin\" <mst@redhat.com>","Cc":"virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,\n\tlinux-kernel@vger.kernel.org, kvm@vger.kernel.org","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>\n\t<20170926221435-mutt-send-email-mst@kernel.org>\n\t<17e9c3a9-7759-a674-bc00-414eabfed118@redhat.com>\n\t<20170928012906-mutt-send-email-mst@kernel.org>","From":"Jason Wang <jasowang@redhat.com>","Message-ID":"<502eb735-3aea-3799-1f92-87697d4ef198@redhat.com>","Date":"Thu, 28 Sep 2017 15:18:45 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170928012906-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.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.28]);\n\tThu, 28 Sep 2017 07:18:53 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1776831,"web_url":"http://patchwork.ozlabs.org/comment/1776831/","msgid":"<4450795e-7967-8427-6b41-28fe4f912e51@redhat.com>","list_archive_url":null,"date":"2017-09-28T07:44:50","subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"On 2017年09月28日 08:47, Willem de Bruijn wrote:\n> On Fri, Sep 22, 2017 at 4:02 AM, Jason Wang <jasowang@redhat.com> wrote:\n>> This patch introduces vhost_prefetch_desc_indices() which could batch\n>> descriptor indices fetching and used ring updating. This intends to\n>> reduce the cache misses of indices fetching and updating and reduce\n>> cache line bounce when virtqueue is almost full. copy_to_user() was\n>> used in order to benefit from modern cpus that support fast string\n>> copy. Batched virtqueue processing will be the first user.\n>>\n>> Signed-off-by: Jason Wang <jasowang@redhat.com>\n>> ---\n>>   drivers/vhost/vhost.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n>>   drivers/vhost/vhost.h |  3 +++\n>>   2 files changed, 58 insertions(+)\n>>\n>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c\n>> index f87ec75..8424166d 100644\n>> --- a/drivers/vhost/vhost.c\n>> +++ b/drivers/vhost/vhost.c\n>> @@ -2437,6 +2437,61 @@ struct vhost_msg_node *vhost_dequeue_msg(struct vhost_dev *dev,\n>>   }\n>>   EXPORT_SYMBOL_GPL(vhost_dequeue_msg);\n>>\n>> +int vhost_prefetch_desc_indices(struct vhost_virtqueue *vq,\n>> +                               struct vring_used_elem *heads,\n>> +                               u16 num, bool used_update)\n>> +{\n>> +       int ret, ret2;\n>> +       u16 last_avail_idx, last_used_idx, total, copied;\n>> +       __virtio16 avail_idx;\n>> +       struct vring_used_elem __user *used;\n>> +       int i;\n>> +\n>> +       if (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {\n>> +               vq_err(vq, \"Failed to access avail idx at %p\\n\",\n>> +                      &vq->avail->idx);\n>> +               return -EFAULT;\n>> +       }\n>> +       last_avail_idx = vq->last_avail_idx & (vq->num - 1);\n>> +       vq->avail_idx = vhost16_to_cpu(vq, avail_idx);\n>> +       total = vq->avail_idx - vq->last_avail_idx;\n>> +       ret = total = min(total, num);\n>> +\n>> +       for (i = 0; i < ret; i++) {\n>> +               ret2 = vhost_get_avail(vq, heads[i].id,\n>> +                                     &vq->avail->ring[last_avail_idx]);\n>> +               if (unlikely(ret2)) {\n>> +                       vq_err(vq, \"Failed to get descriptors\\n\");\n>> +                       return -EFAULT;\n>> +               }\n>> +               last_avail_idx = (last_avail_idx + 1) & (vq->num - 1);\n>> +       }\n> This is understandably very similar to the existing logic in vhost_get_vq_desc.\n> Can that be extracted to a helper to avoid code duplication?\n>\n> Perhaps one helper to update vq->avail_idx and return num, and\n> another to call vhost_get_avail one or more times.\n\nYes it can.\n\n>\n>> +\n>> +       if (!used_update)\n>> +               return ret;\n>> +\n>> +       last_used_idx = vq->last_used_idx & (vq->num - 1);\n>> +       while (total) {\n>> +               copied = min((u16)(vq->num - last_used_idx), total);\n>> +               ret2 = vhost_copy_to_user(vq,\n>> +                                         &vq->used->ring[last_used_idx],\n>> +                                         &heads[ret - total],\n>> +                                         copied * sizeof(*used));\n>> +\n>> +               if (unlikely(ret2)) {\n>> +                       vq_err(vq, \"Failed to update used ring!\\n\");\n>> +                       return -EFAULT;\n>> +               }\n>> +\n>> +               last_used_idx = 0;\n>> +               total -= copied;\n>> +       }\n> This second part seems unrelated and could be a separate function?\n\nYes.\n\n>\n> Also, no need for ret2 and double assignment \"ret = total =\" if not\n> modifying total\n> in the the second loop:\n>\n>    for (i = 0; i < total; ) {\n>      ...\n>      i += copied;\n>    }\n\nRight, will do this in V2.\n\nThanks","headers":{"Return-Path":"<netdev-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jasowang@redhat.com"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y2mv657x6z9t5x\n\tfor <patchwork-incoming@ozlabs.org>;\n\tThu, 28 Sep 2017 17:45:22 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751941AbdI1HpJ (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tThu, 28 Sep 2017 03:45:09 -0400","from mx1.redhat.com ([209.132.183.28]:48354 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751437AbdI1HpI (ORCPT <rfc822;netdev@vger.kernel.org>);\n\tThu, 28 Sep 2017 03:45:08 -0400","from smtp.corp.redhat.com\n\t(int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 5864F98195;\n\tThu, 28 Sep 2017 07:45:08 +0000 (UTC)","from [10.72.12.139] (ovpn-12-139.pek2.redhat.com [10.72.12.139])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 04C848231C;\n\tThu, 28 Sep 2017 07:44:59 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 5864F98195","Subject":"Re: [PATCH net-next RFC 2/5] vhost: introduce helper to prefetch\n\tdesc index","To":"Willem de Bruijn <willemdebruijn.kernel@gmail.com>","Cc":"\"Michael S. Tsirkin\" <mst@redhat.com>,\n\tvirtualization@lists.linux-foundation.org,\n\tNetwork Development <netdev@vger.kernel.org>,\n\tLKML <linux-kernel@vger.kernel.org>, kvm@vger.kernel.org","References":"<1506067355-5771-1-git-send-email-jasowang@redhat.com>\n\t<1506067355-5771-3-git-send-email-jasowang@redhat.com>\n\t<CAF=yD-JC9g1gd-jsSjnLzbwAnk-LsPuj_JhF2NWf1rdxiWDwwA@mail.gmail.com>","From":"Jason Wang <jasowang@redhat.com>","Message-ID":"<4450795e-7967-8427-6b41-28fe4f912e51@redhat.com>","Date":"Thu, 28 Sep 2017 15:44:50 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<CAF=yD-JC9g1gd-jsSjnLzbwAnk-LsPuj_JhF2NWf1rdxiWDwwA@mail.gmail.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"8bit","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.12","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 28 Sep 2017 07:45:08 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<netdev.vger.kernel.org>","X-Mailing-List":"netdev@vger.kernel.org"}}]