diff mbox

[dpdk-dev,v2,5/5] eal: fix race condition in IPC requests

Message ID fe5de52fe7f6c9735cee140d3687904acf4fbe27.1519940460.git.anatoly.burakov@intel.com (mailing list archive)
State Accepted, archived
Headers

Checks

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

Commit Message

Anatoly Burakov March 2, 2018, 8:41 a.m. UTC 
  Unlocking the action list before sending message and locking it
again aftterwards introduces a window where a response might
arrive before we have a chance to start waiting on a condition,
resulting in timeouts on valid messages.

Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
Cc: jianfeng.tan@intel.com

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---

Notes:
    v2: added this patch

 lib/librte_eal/common/eal_common_proc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Jianfeng Tan March 7, 2018, 2:06 p.m. UTC | #1 
On 3/2/2018 4:41 PM, Anatoly Burakov wrote:
> Unlocking the action list before sending message and locking it
> again aftterwards introduces a window where a response might

Typo: afterwards

> arrive before we have a chance to start waiting on a condition,
> resulting in timeouts on valid messages.
>
> Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> Cc: jianfeng.tan@intel.com
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>

Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>

Thank you for catching another bug :-)
Thomas Monjalon March 21, 2018, 8:51 a.m. UTC | #2 
07/03/2018 15:06, Tan, Jianfeng:
> 
> On 3/2/2018 4:41 PM, Anatoly Burakov wrote:
> > Unlocking the action list before sending message and locking it
> > again aftterwards introduces a window where a response might
> 
> Typo: afterwards
> 
> > arrive before we have a chance to start waiting on a condition,
> > resulting in timeouts on valid messages.
> >
> > Fixes: 783b6e54971d ("eal: add synchronous multi-process communication")
> > Cc: jianfeng.tan@intel.com
> >
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> 
> Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>
> 
> Thank you for catching another bug :-)

Series applied, thanks
diff mbox

Patch

diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c
index 3a1088e..da7930f 100644
--- a/lib/librte_eal/common/eal_common_proc.c
+++ b/lib/librte_eal/common/eal_common_proc.c
@@ -561,10 +561,10 @@  mp_request_one(const char *dst, struct rte_mp_msg *req,
 	exist = find_sync_request(dst, req->name);
 	if (!exist)
 		TAILQ_INSERT_TAIL(&sync_requests.requests, &sync_req, next);
-	pthread_mutex_unlock(&sync_requests.lock);
 	if (exist) {
 		RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name);
 		rte_errno = EEXIST;
+		pthread_mutex_unlock(&sync_requests.lock);
 		return -1;
 	}
 
@@ -578,7 +578,6 @@  mp_request_one(const char *dst, struct rte_mp_msg *req,
 
 	reply->nb_sent++;
 
-	pthread_mutex_lock(&sync_requests.lock);
 	do {
 		pthread_cond_timedwait(&sync_req.cond, &sync_requests.lock, ts);
 		/* Check spurious wakeups */