[{"id":1761775,"web_url":"http://patchwork.ozlabs.org/comment/1761775/","msgid":"<20170901184531-mutt-send-email-mst@kernel.org>","list_archive_url":null,"date":"2017-09-01T15:51:26","subject":"Re: [PATCH net] vhost_net: correctly check tx avail during rx busy\n\tpolling","submitter":{"id":2235,"url":"http://patchwork.ozlabs.org/api/people/2235/","name":"Michael S. Tsirkin","email":"mst@redhat.com"},"content":"On Fri, Sep 01, 2017 at 05:02:50PM +0800, Jason Wang wrote:\n> We check tx avail through vhost_enable_notify() in the past which is\n> wrong since it only checks whether or not guest has filled more\n> available buffer since last avail idx synchronization which was just\n> done by vhost_vq_avail_empty() before. What we really want is checking\n> pending buffers in the avail ring.\n\nThese are rx buffers, right? I'm not even sure why do we need to poll\nfor them. Running out of rx buffers is a slow path.\n\n> Fix this by calling\n> vhost_vq_avail_empty() instead.\n> \n> This issue could be noticed by doing netperf TCP_RR benchmark as\n> client from guest (but not host). With this fix, TCP_RR from guest to\n> localhost restores from 1375.91 trans per sec to 55235.28 trans per\n> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).\n> \n> Fixes: 030881372460 (\"vhost_net: basic polling support\")\n> Signed-off-by: Jason Wang \n> ---\n> - The patch is needed for -stable\n> ---\n> drivers/vhost/net.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c\n> index 06d0448..1b68253 100644\n> --- a/drivers/vhost/net.c\n> +++ b/drivers/vhost/net.c\n> @@ -634,7 +634,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)\n\nIn fact why does it poll the ring at all? I thought this function's\njob is to poll the socket, isn't it?\n\n\n> \n> \t\tpreempt_enable();\n> \n> -\t\tif (vhost_enable_notify(&net->dev, vq))\n> +\t\tif (!vhost_vq_avail_empty(&net->dev, vq))\n> \t\t\tvhost_poll_queue(&vq->poll);\n> \t\tmutex_unlock(&vq->mutex);\n\n\nAdding more contex:\n\n mutex_lock(&vq->mutex);\n vhost_disable_notify(&net->dev, vq);\n\n preempt_disable();\n endtime = busy_clock() + vq->busyloop_timeout;\n\n while (vhost_can_busy_poll(&net->dev, endtime) &&\n !sk_has_rx_data(sk) &&\n vhost_vq_avail_empty(&net->dev, vq))\n cpu_relax();\n \n preempt_enable();\n \n if (vhost_enable_notify(&net->dev, vq))\n vhost_poll_queue(&vq->poll);\n mutex_unlock(&vq->mutex);\n\n len = peek_head_len(rvq, sk);\n\n\nIf you drop this we'll exit the function with notifications\ndisabled. Seems wrong to me.\n\n\n> \n> -- \n> 2.7.4","headers":{"Return-Path":"","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=)","ext-mx03.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx03.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 3xkNyk0h8zz9t3k\n\tfor ;\n\tSat, 2 Sep 2017 01:51:41 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752100AbdIAPv3 (ORCPT );\n\tFri, 1 Sep 2017 11:51:29 -0400","from mx1.redhat.com ([209.132.183.28]:59816 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751039AbdIAPv1 (ORCPT );\n\tFri, 1 Sep 2017 11:51:27 -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 861267E434;\n\tFri, 1 Sep 2017 15:51:27 +0000 (UTC)","from redhat.com (ovpn-124-232.rdu2.redhat.com [10.10.124.232])\n\tby smtp.corp.redhat.com (Postfix) with SMTP id E9E7D5F93D;\n\tFri, 1 Sep 2017 15:51:26 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 861267E434","Date":"Fri, 1 Sep 2017 18:51:26 +0300","From":"\"Michael S. Tsirkin\" ","To":"Jason Wang ","Cc":"kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,\n\tnetdev@vger.kernel.org, linux-kernel@vger.kernel.org","Subject":"Re: [PATCH net] vhost_net: correctly check tx avail during rx busy\n\tpolling","Message-ID":"<20170901184531-mutt-send-email-mst@kernel.org>","References":"<1504256570-3488-1-git-send-email-jasowang@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1504256570-3488-1-git-send-email-jasowang@redhat.com>","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.27]);\n\tFri, 01 Sep 2017 15:51:27 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1762395,"web_url":"http://patchwork.ozlabs.org/comment/1762395/","msgid":"<7086b3c9-f81f-1c06-b484-abe0888f7fd5@redhat.com>","list_archive_url":null,"date":"2017-09-04T02:51:46","subject":"Re: [PATCH net] vhost_net: correctly check tx avail during rx busy\n\tpolling","submitter":{"id":5225,"url":"http://patchwork.ozlabs.org/api/people/5225/","name":"Jason Wang","email":"jasowang@redhat.com"},"content":"On 2017年09月01日 23:51, Michael S. Tsirkin wrote:\n> On Fri, Sep 01, 2017 at 05:02:50PM +0800, Jason Wang wrote:\n>> We check tx avail through vhost_enable_notify() in the past which is\n>> wrong since it only checks whether or not guest has filled more\n>> available buffer since last avail idx synchronization which was just\n>> done by vhost_vq_avail_empty() before. What we really want is checking\n>> pending buffers in the avail ring.\n> These are rx buffers, right? I'm not even sure why do we need to poll\n> for them. Running out of rx buffers is a slow path.\n\nActually it polls for tx buffer here. I admit the code (or probably the \nvariable name) is confusing here.\n\n>\n>> Fix this by calling\n>> vhost_vq_avail_empty() instead.\n>>\n>> This issue could be noticed by doing netperf TCP_RR benchmark as\n>> client from guest (but not host). With this fix, TCP_RR from guest to\n>> localhost restores from 1375.91 trans per sec to 55235.28 trans per\n>> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).\n>>\n>> Fixes: 030881372460 (\"vhost_net: basic polling support\")\n>> Signed-off-by: Jason Wang \n>> ---\n>> - The patch is needed for -stable\n>> ---\n>> drivers/vhost/net.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c\n>> index 06d0448..1b68253 100644\n>> --- a/drivers/vhost/net.c\n>> +++ b/drivers/vhost/net.c\n>> @@ -634,7 +634,7 @@ static int vhost_net_rx_peek_head_len(struct vhost_net *net, struct sock *sk)\n> In fact why does it poll the ring at all? I thought this function's\n> job is to poll the socket, isn't it?\n\nTx notification is disabled to try to avoid vmexits, so we poll tx avail \nbuffers too.\n\n>\n>\n>> \n>> \t\tpreempt_enable();\n>> \n>> -\t\tif (vhost_enable_notify(&net->dev, vq))\n>> +\t\tif (!vhost_vq_avail_empty(&net->dev, vq))\n>> \t\t\tvhost_poll_queue(&vq->poll);\n>> \t\tmutex_unlock(&vq->mutex);\n>\n> Adding more contex:\n>\n> mutex_lock(&vq->mutex);\n> vhost_disable_notify(&net->dev, vq);\n>\n> preempt_disable();\n> endtime = busy_clock() + vq->busyloop_timeout;\n>\n> while (vhost_can_busy_poll(&net->dev, endtime) &&\n> !sk_has_rx_data(sk) &&\n> vhost_vq_avail_empty(&net->dev, vq))\n> cpu_relax();\n> \n> preempt_enable();\n> \n> if (vhost_enable_notify(&net->dev, vq))\n> vhost_poll_queue(&vq->poll);\n> mutex_unlock(&vq->mutex);\n>\n> len = peek_head_len(rvq, sk);\n>\n>\n> If you drop this we'll exit the function with notifications\n> disabled. Seems wrong to me.\n\nYes, will fix this in V2.\n\nThanks\n\n>\n>> \n>> -- \n>> 2.7.4","headers":{"Return-Path":"","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=)","ext-mx10.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx10.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 3xlvWq63hLz9s7f\n\tfor ;\n\tMon, 4 Sep 2017 12:52:07 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753241AbdIDCv4 (ORCPT );\n\tSun, 3 Sep 2017 22:51:56 -0400","from mx1.redhat.com ([209.132.183.28]:36490 \"EHLO mx1.redhat.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1753124AbdIDCvz (ORCPT );\n\tSun, 3 Sep 2017 22:51:55 -0400","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\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 CAAA25F7BF;\n\tMon, 4 Sep 2017 02:51:54 +0000 (UTC)","from [10.72.12.102] (ovpn-12-102.pek2.redhat.com [10.72.12.102])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id BA5B162463;\n\tMon, 4 Sep 2017 02:51:50 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com CAAA25F7BF","Subject":"Re: [PATCH net] vhost_net: correctly check tx avail during rx busy\n\tpolling","To":"\"Michael S. Tsirkin\" ","Cc":"kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,\n\tnetdev@vger.kernel.org, linux-kernel@vger.kernel.org","References":"<1504256570-3488-1-git-send-email-jasowang@redhat.com>\n\t<20170901184531-mutt-send-email-mst@kernel.org>","From":"Jason Wang ","Message-ID":"<7086b3c9-f81f-1c06-b484-abe0888f7fd5@redhat.com>","Date":"Mon, 4 Sep 2017 10:51:46 +0800","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"<20170901184531-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.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tMon, 04 Sep 2017 02:51:54 +0000 (UTC)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]