From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?5a2Z5paH5p2w?= Subject: Re: [PATCH] vhostuser: fix deadlock when vhost unregister Date: Fri, 11 Jan 2019 11:12:52 +0800 Message-ID: References: <20190108114509.31489-1-findtheonlyway@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org To: Maxime Coquelin Return-path: Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) by dpdk.org (Postfix) with ESMTP id C84FA2B83 for ; Fri, 11 Jan 2019 04:13:03 +0100 (CET) Received: by mail-ed1-f68.google.com with SMTP id a20so11919300edc.8 for ; Thu, 10 Jan 2019 19:13:03 -0800 (PST) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Will do, thanks! Maxime Coquelin =E4=BA=8E2019=E5=B9=B41=E6=9C= =8810=E6=97=A5=E5=91=A8=E5=9B=9B =E4=B8=8B=E5=8D=8810:48=E5=86=99=E9=81=93= =EF=BC=9A > Hi Sunwenjie, > > Thanks for your patch. > > Please resend with following contribution guidelines, that can be > found in doc/guides/contributing/. > > First, the commit message prefix should be vhost. > For other comments, please see inline: > > > On 1/8/19 12:45 PM, sunwenjie wrote: > > When rte_vhost_driver_unregister delete the connection fd, > fdset_try_del will > > always try and donot release the vhostuser.mutex if the fd is busy, > but the > > fdset_event_dispatch will set the fd to busy and call > vhost_user_msg_handler > > to get vhostuser.mutex, which will cause deadlock. > > > > The commit message should not be indented, and wrapped at 72 chars. > > > To fix it: > > Unlock the vhost_user.mutex if fdset_try_del fail and relock it wh= en > > retry. > > You need to add your Sined-off-by with your full name. > > > --- > > lib/librte_vhost/socket.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c > > index 9cf34ad17..a9effa115 100644 > > --- a/lib/librte_vhost/socket.c > > +++ b/lib/librte_vhost/socket.c > > @@ -961,13 +961,12 @@ rte_vhost_driver_unregister(const char *path) > > int count; > > struct vhost_user_connection *conn, *next; > > > > +again: > > pthread_mutex_lock(&vhost_user.mutex); > > > > for (i =3D 0; i < vhost_user.vsocket_cnt; i++) { > > struct vhost_user_socket *vsocket =3D vhost_user.vsockets= [i]; > > - > > Keep this new line, it is out of the scope of the change. > > > if (!strcmp(vsocket->path, path)) { > > -again: > > pthread_mutex_lock(&vsocket->conn_mutex); > > for (conn =3D TAILQ_FIRST(&vsocket->conn_list); > > conn !=3D NULL; > > @@ -981,6 +980,7 @@ rte_vhost_driver_unregister(const char *path) > > */ > > if (fdset_try_del(&vhost_user.fdset, > > conn->connfd) =3D=3D -1= ) { > > + > pthread_mutex_unlock(&vhost_user.mutex); > > pthread_mutex_unlock( > > > &vsocket->conn_mutex); > > goto again; > > > > Thanks, > Maxime >