From 1eac307937a5b10794488c6df4aa7217d65ade14 Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Mon, 28 Aug 2017 13:57:20 +0200 Subject: [PATCH] rte_vhost: fix deadlock on rte_vhost_driver_unregister() 2 locks are executed in 2 places in opposite orders. Consider the following scenario, threads A and B: (A) * fdset_event_dispatch() start * pfdentry->busy = 1; (lock #1) * vhost_user_read_cb() start * vhost_destroy_device() start (B) * rte_vhost_driver_unregister() start * pthread_mutex_lock(&vsocket->conn_mutex); (lock #2) * fdset_del() * endless loop, waiting for pfdentry->busy == 0 (lock #1) (A) * vhost_destroy_device() end * pthread_mutex_lock(&vsocket->conn_mutex); (lock #2) (mutex already locked - deadlock at this point) Thread B has locked vsocket->conn_mutex and is in while(1) loop waiting for given fd to change it's busy flag to 0. Thread A would have to finish vhost_user_read_cb() in order to set busy flag back to 0, but that can't happen due to the vsocket->conn_mutex lock. This patch defers the fdset_del(), so that it's called outside of vsocket->conn_mutex. Change-Id: Ifb5d4699bdafe96a573444c11ad4eae3adc359f5 Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/375910 Tested-by: SPDK Automated Test System Reviewed-by: Pawel Wodkowski Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- lib/vhost/rte_vhost/socket.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/vhost/rte_vhost/socket.c b/lib/vhost/rte_vhost/socket.c index 4eea67893..54d5bc376 100644 --- a/lib/vhost/rte_vhost/socket.c +++ b/lib/vhost/rte_vhost/socket.c @@ -696,6 +696,8 @@ rte_vhost_driver_unregister(const char *path) int i; int count; struct vhost_user_connection *conn, *next; + struct vhost_user_connection_list del_conn_list = + TAILQ_HEAD_INITIALIZER(del_conn_list); pthread_mutex_lock(&vhost_user.mutex); @@ -717,16 +719,20 @@ rte_vhost_driver_unregister(const char *path) conn = next) { next = TAILQ_NEXT(conn, next); + TAILQ_REMOVE(&vsocket->conn_list, conn, next); + TAILQ_INSERT_TAIL(&del_conn_list, conn, next); + } + pthread_mutex_unlock(&vsocket->conn_mutex); + + TAILQ_FOREACH(conn, &del_conn_list, next) { fdset_del(&vhost_user.fdset, conn->connfd); RTE_LOG(INFO, VHOST_CONFIG, "free connfd = %d for device '%s'\n", conn->connfd, path); close(conn->connfd); vhost_destroy_device(conn->vid); - TAILQ_REMOVE(&vsocket->conn_list, conn, next); free(conn); } - pthread_mutex_unlock(&vsocket->conn_mutex); free(vsocket->path); free(vsocket);