From patchwork Wed Jan 17 10:50:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?546L5b+X5YWL?= X-Patchwork-Id: 33911 X-Patchwork-Delegate: yuanhan.liu@linux.intel.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4585B1B03D; Wed, 17 Jan 2018 11:50:16 +0100 (CET) Received: from smtp.jd.com (smtp.jd.com [58.83.206.59]) by dpdk.org (Postfix) with ESMTP id ED06AA84C for ; Wed, 17 Jan 2018 11:50:13 +0100 (CET) Received: from HUB06.360buyAD.local (172.17.27.42) by HUB03.360buyAD.local (172.17.27.17) with Microsoft SMTP Server (TLS) id 14.3.361.1; Wed, 17 Jan 2018 18:50:14 +0800 Received: from MBX05.360buyAD.local ([fe80::7043:229a:8a1b:2ced]) by hub06.360buyAD.local ([fe80::a522:2f79:88c6:4981%15]) with mapi id 14.03.0382.000; Wed, 17 Jan 2018 18:50:07 +0800 From: =?utf-8?b?546L5b+X5YWL?= To: "dev@dpdk.org" , Yuanhan Liu , "huawei.xie@intel.com" Thread-Topic: [PATCH v3] lib/librte_vhost: move fdset_del out of conn_mutex Thread-Index: AQHTg7G/hdi8VC7wZ0Sg+wI3ZrxvgaN3+aAA Date: Wed, 17 Jan 2018 10:50:06 +0000 Message-ID: <6DAF063A35010343823807B082E5681F41D89369@mbx05.360buyAD.local> References: <1514887716-26343-1-git-send-email-wangzhike@jd.com> In-Reply-To: <1514887716-26343-1-git-send-email-wangzhike@jd.com> Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [172.17.27.83] MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3] lib/librte_vhost: move fdset_del out of conn_mutex 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Yuanhan, Huawei, Can you please spare some time for code review? Thanks. Br, Zhike -----Original Message----- From: 王志克 Sent: Tuesday, January 02, 2018 6:09 PM To: dev@dpdk.org Cc: 王志克 Subject: [PATCH v3] lib/librte_vhost: move fdset_del out of conn_mutex From: wang zhike v3: * Fix duplicate variable name, which leads to unexpected memory write. 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 --- lib/librte_vhost/socket.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) -- 1.8.3.1 diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c index 422da00..ea01327 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, fd_index; + 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 (fd_index = 0; fd_index < num_of_fds; fd_index++) + fdset_del(&vhost_user.fdset, del_fds[fd_index]); + 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);