From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f67.google.com (mail-oi0-f67.google.com [209.85.218.67]) by dpdk.org (Postfix) with ESMTP id D410B2B82 for ; Thu, 29 Mar 2018 13:24:01 +0200 (CEST) Received: by mail-oi0-f67.google.com with SMTP id 188-v6so4832257oih.8 for ; Thu, 29 Mar 2018 04:24:01 -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=iDRkTaZnD8pUOZ4iP8+WnbCACxhVxNLUC9p8o4UKTqg=; b=Sh03x52w4uTygHO4RmYFQZwa8ZQguKLu638dXJanKXuCRmkg5UfHPRbaIlGWPNUFPt eQBuU2exm/gSVQtdXohwTy5GUfNTGmMmyIke8vnNEZ6faFP+cztmLOOqWUaE/2RAiEV+ uPmKyjIqAP0770eHrcmf2uxJbxtZBw9lMAOUzfgzwwtjT8352AzWGEVZigt5B1a6CBHc CryXNCXo/4mJaelPe/WGOcnPVHVyJZEygy+NfA2i7j7ULQrmbtFFVsQti2RJhQQ0Aknd e+0cObyZMqwPEQuZ0RAo+OJNfei6x5L/11ODt42yycb7TAZyLS/QDuMYRgOV4kYyaQDn Iq1A== 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=iDRkTaZnD8pUOZ4iP8+WnbCACxhVxNLUC9p8o4UKTqg=; b=CYXzQmfoJ0K70jwoySobwBDcP/jkeazRdB48P6Q5azV3gXAUE7ZY0n+sZAQGGb1/Bc +QjU6MCnQy7t1rx1Rd3pMwZDEFRjtn+8z9rSwuD4z3CUA0wxfaeUqXv1S5pdLOwlnbUc 76W+0XGf8RU0ze7G0GBkylfFLZy8A7sl7sVwi8iXS+n2U9NPE4/qeVkz75LWWobLh9cA 6jvtR83OwBFMj0/3UJmpWj4zGSpo3Y7sK8/GYPik6jDNXyy2dl62LUdR7YGv97YJ6RB0 8VqbPyI5Acmy7WZrL0rpUqigjR6YASPtsruVn7xwvIOYoKxneos5R8djgA52Xl2iz3N5 jfpg== X-Gm-Message-State: ALQs6tDpI3XkznJii1JVtoeS0nMSMWN8/Pe2DmRHxgigoJdMCSaTLIJ8 HPnnJtkBGoz6m6p/NPLci3Vn5frhGSbqo9zZ2G8= X-Google-Smtp-Source: AIpwx48RjJyVTPGyWHbSRwBo/6+StXEXi+NqMQ+Vwr3agulZbm5HpwAjTmRwNFU3KvUWt5nDZhQoEoZ4lLk1/rg111o= X-Received: by 10.202.234.130 with SMTP id i124mr4366423oih.82.1522322641008; Thu, 29 Mar 2018 04:24:01 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.198.16 with HTTP; Thu, 29 Mar 2018 04:24:00 -0700 (PDT) In-Reply-To: References: <1522216165-19666-1-git-send-email-xiangxia.m.yue@gmail.com> <1522216165-19666-3-git-send-email-xiangxia.m.yue@gmail.com> From: Tonghao Zhang Date: Thu, 29 Mar 2018 19:24:00 +0800 Message-ID: To: "Tan, Jianfeng" Cc: "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: Thu, 29 Mar 2018 11:24:02 -0000 On Thu, Mar 29, 2018 at 3:32 PM, Tan, Jianfeng wro= te: > 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 incl= ude 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. > 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 notified= 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_d= ispatch, >> &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 >