ipc: fix missing mutex unlocks on failed send
authorAnatoly Burakov <anatoly.burakov@intel.com>
Fri, 13 Apr 2018 14:16:19 +0000 (15:16 +0100)
committerThomas Monjalon <thomas@monjalon.net>
Tue, 17 Apr 2018 08:23:05 +0000 (10:23 +0200)
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: stable@dpdk.org
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
lib/librte_eal/common/eal_common_proc.c

index 5b670df..2179f3d 100644 (file)
@@ -921,12 +921,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;
        }
 
@@ -947,9 +945,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",
@@ -1008,8 +1004,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);
@@ -1029,6 +1029,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];
 
@@ -1038,9 +1039,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 receive
+                */
                if (mp_request_sync(path, req, reply, &end))
                        ret = -1;
        }
+       pthread_mutex_unlock(&pending_requests.lock);
        /* unlock the directory */
        flock(dir_fd, LOCK_UN);