From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anatoly Burakov Subject: [PATCH] eal/ipc: fix missing mutex unlocks on failed msg send Date: Fri, 13 Apr 2018 15:16:19 +0100 Message-ID: Cc: jianfeng.tan@intel.com, anatoly.burakov@intel.com To: dev@dpdk.org Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 78C1D1B898 for ; Fri, 13 Apr 2018 16:16:23 +0200 (CEST) List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Earlier fix for race condition introduced a bug where mutex wasn't unlocked if message failed to be sent. Fix all of this by moving locking out of mp_request_sync() altogether. Fixes: da5957821bdd ("eal: fix race condition in IPC request") Cc: anatoly.burakov@intel.com Signed-off-by: Anatoly Burakov --- lib/librte_eal/common/eal_common_proc.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index a8ca7b8..2d3bda3 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -919,12 +919,10 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, sync_req.reply = &msg; pthread_cond_init(&sync_req.sync.cond, NULL); - pthread_mutex_lock(&pending_requests.lock); exist = find_sync_request(dst, req->name); if (exist) { RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name); rte_errno = EEXIST; - pthread_mutex_unlock(&pending_requests.lock); return -1; } @@ -945,9 +943,7 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, &pending_requests.lock, ts); } while (ret != 0 && ret != ETIMEDOUT); - /* We got the lock now */ TAILQ_REMOVE(&pending_requests.requests, &sync_req, next); - pthread_mutex_unlock(&pending_requests.lock); if (sync_req.reply_received == 0) { RTE_LOG(ERR, EAL, "Fail to recv reply for request %s:%s\n", @@ -1006,8 +1002,12 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply, reply->msgs = NULL; /* for secondary process, send request to the primary process only */ - if (rte_eal_process_type() == RTE_PROC_SECONDARY) - return mp_request_sync(eal_mp_socket_path(), req, reply, &end); + if (rte_eal_process_type() == RTE_PROC_SECONDARY) { + pthread_mutex_lock(&pending_requests.lock); + ret = mp_request_sync(eal_mp_socket_path(), req, reply, &end); + pthread_mutex_unlock(&pending_requests.lock); + return ret; + } /* for primary process, broadcast request, and collect reply 1 by 1 */ mp_dir = opendir(mp_dir_path); @@ -1027,6 +1027,7 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply, return -1; } + pthread_mutex_lock(&pending_requests.lock); while ((ent = readdir(mp_dir))) { char path[PATH_MAX]; @@ -1036,9 +1037,13 @@ rte_mp_request_sync(struct rte_mp_msg *req, struct rte_mp_reply *reply, snprintf(path, sizeof(path), "%s/%s", mp_dir_path, ent->d_name); + /* unlocks the mutex while waiting for response, locks on + * recieve + */ if (mp_request_sync(path, req, reply, &end)) ret = -1; } + pthread_mutex_unlock(&pending_requests.lock); /* unlock the directory */ flock(dir_fd, LOCK_UN); -- 2.7.4