diff mbox

[dpdk-dev,v2] lib/librte_vhost: move fdset_del out of conn_mutex

Message ID 1514424207-98179-1-git-send-email-wangzhike@jd.com (mailing list archive)
State Superseded, archived
Delegated to: Yuanhan Liu
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

王志克 Dec. 28, 2017, 1:23 a.m. UTC 
  From: wang zhike <wangzhike@jd.com>

v2:
* Move fdset_del before conn destroy.
* Fix coding style.

This patch fixes below race condition:
1. one thread calls: rte_vhost_driver_unregister->lock conn_mutex
   ->fdset_del->loop to check fd.busy.
2. another thread calls fdset_event_dispatch, and the busy flag is
   changed AFTER handling on the fd, i.e, rcb(). However, the rcb,
   such as vhost_user_read_cb() would try to retrieve the conn_mutex.

So issue is that the 1st thread will loop check the flag while holding
the mutex, while the 2nd thread would be blocked by mutex and can not
change the flag. Then dead lock is observed.

Signed-off-by: zhike wang <wangzhike@jd.com>
---
 lib/librte_vhost/socket.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
diff mbox

Patch

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 422da00..017f824 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -749,6 +749,9 @@  struct vhost_user_reconnect_list {
 		struct vhost_user_socket *vsocket = vhost_user.vsockets[i];
 
 		if (!strcmp(vsocket->path, path)) {
+			int del_fds[MAX_FDS];
+			int num_of_fds = 0, i;
+
 			if (vsocket->is_server) {
 				fdset_del(&vhost_user.fdset, vsocket->socket_fd);
 				close(vsocket->socket_fd);
@@ -757,13 +760,26 @@  struct vhost_user_reconnect_list {
 				vhost_user_remove_reconnect(vsocket);
 			}
 
+			/* fdset_del() must be called without conn_mutex. */
+			pthread_mutex_lock(&vsocket->conn_mutex);
+			for (conn = TAILQ_FIRST(&vsocket->conn_list);
+			     conn != NULL;
+			     conn = next) {
+				next = TAILQ_NEXT(conn, next);
+
+				del_fds[num_of_fds++] = conn->connfd;
+			}
+			pthread_mutex_unlock(&vsocket->conn_mutex);
+
+			for (i = 0; i < num_of_fds; i++)
+				fdset_del(&vhost_user.fdset, del_fds[i]);
+
 			pthread_mutex_lock(&vsocket->conn_mutex);
 			for (conn = TAILQ_FIRST(&vsocket->conn_list);
 			     conn != NULL;
 			     conn = next) {
 				next = TAILQ_NEXT(conn, next);
 
-				fdset_del(&vhost_user.fdset, conn->connfd);
 				RTE_LOG(INFO, VHOST_CONFIG,
 					"free connfd = %d for device '%s'\n",
 					conn->connfd, path);