From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?5a2Z5paH5p2w?= Subject: Re: [PATCH] vhost: fix deadlock when vhost unregister Date: Thu, 14 Feb 2019 12:05:23 +0800 Message-ID: References: <20190128065549.98266-1-findtheonlyway@gmail.com> <341edf62-2e01-dcce-377c-11471b02c125@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org, stable@dpdk.org To: Maxime Coquelin Return-path: In-Reply-To: <341edf62-2e01-dcce-377c-11471b02c125@redhat.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Thanks, Maxime. Your description is better, My real name is Wenjie Sun. Signed-off-by: Wenjie Sun Maxime Coquelin =E4=BA=8E2019=E5=B9=B42=E6=9C= =888=E6=97=A5=E5=91=A8=E4=BA=94 =E4=B8=8B=E5=8D=8810:12=E5=86=99=E9=81=93= =EF=BC=9A > > > On 1/28/19 7:55 AM, 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. Unlock the > > vhost_user.mutexif fdset_try_del fail and relock it when retry. > > What about this wording: > > In rte_vhost_driver_unregister(), the connection fd is removed from > the fdset using fdset_try_del(). Call to this function may fail > if the corresponding fd is in busy state, indicating that event > dispatcher is executing the read or write callback on this fd. > When it happens, rte_vhost_driver_unregister() keeps trying to > remove the fd from the set until it is no more busy. > > This situation is causing a deadlock, because > rte_vhost_driver_unregister() keeps trying to remove the fd from > the set with vhost_user.mutex held, while the callback executed > by the dispatcher, vhost_user_read_cb(), also takes this mutex at > numerous places. > > The fix consists in releasing vhost_user.mutex between each retry > in vhost_driver_unregister(). > > > > > > Fixes: 8b4b949144b8 ("vhost: fix dead lock on closing in server mode") > > Cc: stable@dpdk.org > > > > Signed-off-by: sunwenjie > > We need your real name for legal reasons: > Signed-off-by: Surname Lastname > > No need to resubmit, I can handle the commit message fixup and > the fix looks good to me: > Reviewed-by: Maxime Coquelin > > As soon as I get your name in above format I will apply the patch in > Virtio tree. Thanks for submitting the fix. > > Maxime > > --- > > lib/librte_vhost/socket.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c > > index 9cf34ad17..9883b0491 100644 > > --- a/lib/librte_vhost/socket.c > > +++ b/lib/librte_vhost/socket.c > > @@ -961,13 +961,13 @@ 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]; > > > > if (!strcmp(vsocket->path, path)) { > > -again: > > pthread_mutex_lock(&vsocket->conn_mutex); > > for (conn =3D TAILQ_FIRST(&vsocket->conn_list); > > conn !=3D NULL; > > @@ -983,6 +983,7 @@ rte_vhost_driver_unregister(const char *path) > > conn->connfd) =3D=3D -1= ) { > > pthread_mutex_unlock( > > > &vsocket->conn_mutex); > > + > pthread_mutex_unlock(&vhost_user.mutex); > > goto again; > > } > > > > >