From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f66.google.com (mail-oi0-f66.google.com [209.85.218.66]) by dpdk.org (Postfix) with ESMTP id 045362B96 for ; Fri, 30 Mar 2018 10:07:28 +0200 (CEST) Received: by mail-oi0-f66.google.com with SMTP id e8-v6so7255297oii.4 for ; Fri, 30 Mar 2018 01:07:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=M+KG4s1FFveA9o2kvhmIeUWmkkzs8mtpy3XVsLErP0Y=; b=fzXsaiFjIzUdoAfmotF9l2LodZJcDEuTrnui38kDrPnqIISGICpZMa39qwaruwlyLs fL5ZV1DtqyAXkJKwKC+ZI+l7d08mwVnL7uSpwo4aM0rbWtV3DUHU5wDaWAADSYZQqhey TlK+EVY5UJSLCQGPZLJZ/9Gt5iPeqqbZ//unwsxJBx+20hycpLm84yDFwQ38XKMJBRCX ImWhLZcOR8NxEQSZH+1YRmmT9eCjxBCcdMPpv0UFs2EAXzuHHKguo42b42kaD6ldS6Mt EF5ilA+C9j5MZNifaK3GW0GgmYL/V+IrYmA0mqDaWMKg13mEbIlxt/L0EH1s1S83iEMY TQ9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=M+KG4s1FFveA9o2kvhmIeUWmkkzs8mtpy3XVsLErP0Y=; b=IYGS3nfHudpzP/HQ5zxi9Smc+sDQPivY4rvfjuRruyrepeS9WaUHXp7T+PUR955sfE SP0YXa1pqDd/rnz93xzKvSpvJGu8Dy4xAi9JwPhhZOCSYKNXO66XgsdAoX59E8j0sgZ6 2aJ5EgTxcRMD2Ef4xr48h+IW608LTQkz25tAQB6aftyh2k1dAbKZ6YqQMt+wZtDbd8xt xmZ/5cGJ01/TzbFnaGarwom/kqh914SUza66W/Po61yBp6SUqv/J8H95qS4kiykp8jcV kAnH88QtHC3wglbMcQM0MN/AfcF2LBZhE0ksCPGcCMFTM3MqaKvvC/xE+XdGc794AweN lONg== X-Gm-Message-State: ALQs6tBQKCdRlGgthdyc87h0jztRxLR9G8oDG4w9yRYF+NJwCFUM3nJm OA6vYtR0KMZnbr1cTLmI0MkhUZhdRlaHtG9X2KA= X-Google-Smtp-Source: AIpwx4+k0TaXna5qxQa/POU+Dqi0slIXDFwGZS0zXQK8GE6v+rHFVz6w+Vvtw9/tab3Tj7ZnH5TDGVaGNiiUOylRBgg= X-Received: by 10.202.104.41 with SMTP id d41mr713529oic.35.1522397248335; Fri, 30 Mar 2018 01:07:28 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.198.16 with HTTP; Fri, 30 Mar 2018 01:07:27 -0700 (PDT) In-Reply-To: <148da049-ee99-ce61-0261-59c3917033a1@redhat.com> References: <1522216165-19666-1-git-send-email-xiangxia.m.yue@gmail.com> <1522216165-19666-3-git-send-email-xiangxia.m.yue@gmail.com> <148da049-ee99-ce61-0261-59c3917033a1@redhat.com> From: Tonghao Zhang Date: Fri, 30 Mar 2018 16:07:27 +0800 Message-ID: To: Maxime Coquelin Cc: "Tan, Jianfeng" , "dev@dpdk.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH 2/2] vhost: add pipe event for optimizing negotiating X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Mar 2018 08:07:29 -0000 On Fri, Mar 30, 2018 at 3:57 PM, Maxime Coquelin wrote: > Hi Xiangxia, > > > On 03/29/2018 01:24 PM, Tonghao Zhang wrote: >> >> On Thu, Mar 29, 2018 at 3:32 PM, Tan, Jianfeng >> wrote: >>> >>> Hi Xiangxia, >>> >>>> -----Original Message----- >>>> From: xiangxia.m.yue@gmail.com [mailto:xiangxia.m.yue@gmail.com] >>>> Sent: Wednesday, March 28, 2018 1:49 PM >>>> To: Tan, Jianfeng >>>> Cc: dev@dpdk.org; Tonghao Zhang >>>> Subject: [PATCH 2/2] vhost: add pipe event for optimizing negotiating >>>> >>>> From: Tonghao Zhang >>>> >>>> When vhost=E2=80=94user connects qemu successfully, dpdk will call >>> >>> >>> Typo: "-" >> >> v2 will update it. :) >> >>>> the vhost_user_add_connection to add unix socket fd to poll. >>>> And fdset_add only set the socket fd to a fdentry while poll >>>> may sleep now. In a general case, this is no problem. But if >>>> we use hot update for vhost-user, most downtime of VMs network >>>> is 750+ms. This patch adds pipe event, so after connections are >>>> ok, dpdk rebuild the poll immediately. With this patch, the >>>> most downtime is 20~30ms. >>>> >>>> Signed-off-by: Tonghao Zhang >>>> --- >>>> lib/librte_vhost/fd_man.c | 49 >>>> +++++++++++++++++++++++++++++++++++++++++++++++ >>>> lib/librte_vhost/fd_man.h | 16 ++++++++++++++++ >>>> lib/librte_vhost/socket.c | 14 ++++++++++++++ >>>> 3 files changed, 79 insertions(+) >>>> >>>> diff --git a/lib/librte_vhost/fd_man.c b/lib/librte_vhost/fd_man.c >>>> index 181711c..7716757 100644 >>>> --- a/lib/librte_vhost/fd_man.c >>>> +++ b/lib/librte_vhost/fd_man.c >>>> @@ -15,6 +15,7 @@ >>>> #include >>>> >>>> #include "fd_man.h" >>>> +#include "vhost.h" >>> >>> >>> This is a nice finding and solution, however, I don't think we shall >>> include vhost header file in fd related files. Actually, I did not find= out >>> why you need to include this header file. >> >> >> Hi Jianfeng, thanks for your review. In the fdset_pipe_init function, >> I call the RTE_LOG with VHOST_CONFIG, when init pipe not successfully. >> So I included the vhost header file. > > > Maybe better to create VHOST_SOCKET in socket.c, than including vhost.h t= hat > creates a layer violation: > > #define RTE_LOGTYPE_VHOST_SOCKET RTE_LOGTYPE_USER1 > > Is it Ok for you? I so no need to resend, I can handle the change when > applying. yes, thanks. > > Thanks, > Maxime > >> >> >>> Thanks, >>> Jianfeng >>> >>>> >>>> #define FDPOLLERR (POLLERR | POLLHUP | POLLNVAL) >>>> >>>> @@ -272,3 +273,51 @@ >>>> >>>> return NULL; >>>> } >>>> + >>>> +static void >>>> +fdset_pipe_read_cb(int readfd, void *dat __rte_unused, >>>> + int *remove __rte_unused) >>>> +{ >>>> + char charbuf[16]; >>>> + read(readfd, charbuf, sizeof(charbuf)); >>>> +} >>>> + >>>> +void >>>> +fdset_pipe_uninit(struct fdset *fdset) >>>> +{ >>>> + fdset_del(fdset, fdset->u.readfd); >>>> + close(fdset->u.readfd); >>>> + close(fdset->u.writefd); >>>> +} >>>> + >>>> +int >>>> +fdset_pipe_init(struct fdset *fdset) >>>> +{ >>>> + int ret; >>>> + >>>> + if (pipe(fdset->u.pipefd) < 0) { >>>> + RTE_LOG(ERR, VHOST_CONFIG, >>>> + "failed to create pipe for vhost fdset\n"); >>>> + return -1; >>>> + } >>>> + >>>> + ret =3D fdset_add(fdset, fdset->u.readfd, >>>> + fdset_pipe_read_cb, NULL, NULL); >>>> + >>>> + if (ret < 0) { >>>> + RTE_LOG(ERR, VHOST_CONFIG, >>>> + "failed to add pipe readfd %d into vhost server >>>> fdset\n", >>>> + fdset->u.readfd); >>>> + >>>> + fdset_pipe_uninit(fdset); >>>> + return -1; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +void >>>> +fdset_pipe_notify(struct fdset *fdset) >>>> +{ >>>> + write(fdset->u.writefd, "1", 1); >>>> +} >>>> diff --git a/lib/librte_vhost/fd_man.h b/lib/librte_vhost/fd_man.h >>>> index 3a9276c..76a42fb 100644 >>>> --- a/lib/librte_vhost/fd_man.h >>>> +++ b/lib/librte_vhost/fd_man.h >>>> @@ -25,6 +25,16 @@ struct fdset { >>>> struct fdentry fd[MAX_FDS]; >>>> pthread_mutex_t fd_mutex; >>>> int num; /* current fd number of this fdset */ >>>> + >>>> + union pipefds { >>>> + struct { >>>> + int pipefd[2]; >>>> + }; >>>> + struct { >>>> + int readfd; >>>> + int writefd; >>>> + }; >>>> + } u; >>>> }; >>>> >>>> >>>> @@ -37,4 +47,10 @@ int fdset_add(struct fdset *pfdset, int fd, >>>> >>>> void *fdset_event_dispatch(void *arg); >>>> >>>> +int fdset_pipe_init(struct fdset *fdset); >>>> + >>>> +void fdset_pipe_uninit(struct fdset *fdset); >>>> + >>>> +void fdset_pipe_notify(struct fdset *fdset); >>>> + >>>> #endif >>>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c >>>> index 95bed78..795239c 100644 >>>> --- a/lib/librte_vhost/socket.c >>>> +++ b/lib/librte_vhost/socket.c >>>> @@ -231,6 +231,8 @@ struct vhost_user { >>>> pthread_mutex_lock(&vsocket->conn_mutex); >>>> TAILQ_INSERT_TAIL(&vsocket->conn_list, conn, next); >>>> pthread_mutex_unlock(&vsocket->conn_mutex); >>>> + >>>> + fdset_pipe_notify(&vhost_user.fdset); >>>> return; >>>> >>>> err: >>>> @@ -829,11 +831,23 @@ struct vhost_device_ops const * >>>> return -1; >>>> >>>> if (fdset_tid =3D=3D 0) { >>>> + /** >>>> + * create a pipe which will be waited by poll and notifi= ed >>>> to >>>> + * rebuild the wait list of poll. >>>> + */ >>>> + if (fdset_pipe_init(&vhost_user.fdset) < 0) { >>>> + RTE_LOG(ERR, VHOST_CONFIG, >>>> + "failed to create pipe for vhost >>>> fdset\n"); >>>> + return -1; >>>> + } >>>> + >>>> int ret =3D pthread_create(&fdset_tid, NULL, >>>> fdset_event_dispatch, >>>> &vhost_user.fdset); >>>> if (ret !=3D 0) { >>>> RTE_LOG(ERR, VHOST_CONFIG, >>>> "failed to create fdset handling >>>> thread"); >>>> + >>>> + fdset_pipe_uninit(&vhost_user.fdset); >>>> return -1; >>>> } else { >>>> snprintf(thread_name, RTE_MAX_THREAD_NAME_LEN, >>>> -- >>>> 1.8.3.1 >>> >>> >