* [PATCH 0/2] code clean up and fix in ipc
@ 2018-04-20 1:21 Jianfeng Tan
2018-04-20 1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Jianfeng Tan @ 2018-04-20 1:21 UTC (permalink / raw)
To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan
Patch 1 is to rename some variable names for better readability.
Patch 2 is a bug fix.
Jianfeng Tan (2):
ipc: clearn up code
ipc: fix timeout not properly handled in async
lib/librte_eal/common/eal_common_proc.c | 88 ++++++++++++++++-----------------
1 file changed, 44 insertions(+), 44 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] ipc: clearn up code 2018-04-20 1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan @ 2018-04-20 1:21 ` Jianfeng Tan 2018-04-20 7:55 ` Burakov, Anatoly 2018-04-20 1:21 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2 siblings, 1 reply; 11+ messages in thread From: Jianfeng Tan @ 2018-04-20 1:21 UTC (permalink / raw) To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan Following below commit, we change some internal function and variable names: commit ce3a7312357b ("eal: rename IPC request as synchronous one") Also use calloc to supersede malloc + memset for code clean up. Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> --- lib/librte_eal/common/eal_common_proc.c | 82 +++++++++++++++------------------ 1 file changed, 38 insertions(+), 44 deletions(-) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index 2179f3d..070a075 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -108,7 +108,7 @@ mp_send(struct rte_mp_msg *msg, const char *peer, int type); static struct pending_request * -find_sync_request(const char *dst, const char *act_name) +find_pending_request(const char *dst, const char *act_name) { struct pending_request *r; @@ -282,7 +282,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) static void process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) { - struct pending_request *sync_req; + struct pending_request *pending_req; struct action_entry *entry; struct rte_mp_msg *msg = &m->msg; rte_mp_t action = NULL; @@ -291,15 +291,15 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) if (m->type == MP_REP || m->type == MP_IGN) { pthread_mutex_lock(&pending_requests.lock); - sync_req = find_sync_request(s->sun_path, msg->name); - if (sync_req) { - memcpy(sync_req->reply, msg, sizeof(*msg)); + pending_req = find_pending_request(s->sun_path, msg->name); + if (pending_req) { + memcpy(pending_req->reply, msg, sizeof(*msg)); /* -1 indicates that we've been asked to ignore */ - sync_req->reply_received = m->type == MP_REP ? 1 : -1; + pending_req->reply_received = m->type == MP_REP ? 1 : -1; - if (sync_req->type == REQUEST_TYPE_SYNC) - pthread_cond_signal(&sync_req->sync.cond); - else if (sync_req->type == REQUEST_TYPE_ASYNC) + if (pending_req->type == REQUEST_TYPE_SYNC) + pthread_cond_signal(&pending_req->sync.cond); + else if (pending_req->type == REQUEST_TYPE_ASYNC) pthread_cond_signal( &pending_requests.async_cond); } else @@ -322,6 +322,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) * yet ready to process this request. */ struct rte_mp_msg dummy; + memset(&dummy, 0, sizeof(dummy)); snprintf(dummy.name, sizeof(dummy.name), "%s", msg->name); @@ -854,30 +855,27 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, struct async_request_param *param) { struct rte_mp_msg *reply_msg; - struct pending_request *sync_req, *exist; + struct pending_request *pending_req, *exist; int ret; - sync_req = malloc(sizeof(*sync_req)); - reply_msg = malloc(sizeof(*reply_msg)); - if (sync_req == NULL || reply_msg == NULL) { + pending_req = calloc(1, sizeof(*pending_req)); + reply_msg = calloc(1, sizeof(*reply_msg)); + if (pending_req == NULL || reply_msg == NULL) { RTE_LOG(ERR, EAL, "Could not allocate space for sync request\n"); rte_errno = ENOMEM; ret = -1; goto fail; } - memset(sync_req, 0, sizeof(*sync_req)); - memset(reply_msg, 0, sizeof(*reply_msg)); - - sync_req->type = REQUEST_TYPE_ASYNC; - strcpy(sync_req->dst, dst); - sync_req->request = req; - sync_req->reply = reply_msg; - sync_req->async.param = param; + pending_req->type = REQUEST_TYPE_ASYNC; + strcpy(pending_req->dst, dst); + pending_req->request = req; + pending_req->reply = reply_msg; + pending_req->async.param = param; /* queue already locked by caller */ - exist = find_sync_request(dst, req->name); + exist = find_pending_request(dst, req->name); if (exist) { RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name); rte_errno = EEXIST; @@ -895,13 +893,13 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, ret = 0; goto fail; } - TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next); + TAILQ_INSERT_TAIL(&pending_requests.requests, pending_req, next); param->user_reply.nb_sent++; return 0; fail: - free(sync_req); + free(pending_req); free(reply_msg); return ret; } @@ -912,16 +910,16 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, { int ret; struct rte_mp_msg msg, *tmp; - struct pending_request sync_req, *exist; + struct pending_request pending_req, *exist; - sync_req.type = REQUEST_TYPE_SYNC; - sync_req.reply_received = 0; - strcpy(sync_req.dst, dst); - sync_req.request = req; - sync_req.reply = &msg; - pthread_cond_init(&sync_req.sync.cond, NULL); + pending_req.type = REQUEST_TYPE_SYNC; + pending_req.reply_received = 0; + strcpy(pending_req.dst, dst); + pending_req.request = req; + pending_req.reply = &msg; + pthread_cond_init(&pending_req.sync.cond, NULL); - exist = find_sync_request(dst, req->name); + exist = find_pending_request(dst, req->name); if (exist) { RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name); rte_errno = EEXIST; @@ -936,24 +934,24 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, } else if (ret == 0) return 0; - TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next); + TAILQ_INSERT_TAIL(&pending_requests.requests, &pending_req, next); reply->nb_sent++; do { - ret = pthread_cond_timedwait(&sync_req.sync.cond, + ret = pthread_cond_timedwait(&pending_req.sync.cond, &pending_requests.lock, ts); } while (ret != 0 && ret != ETIMEDOUT); - TAILQ_REMOVE(&pending_requests.requests, &sync_req, next); + TAILQ_REMOVE(&pending_requests.requests, &pending_req, next); - if (sync_req.reply_received == 0) { + if (pending_req.reply_received == 0) { RTE_LOG(ERR, EAL, "Fail to recv reply for request %s:%s\n", dst, req->name); rte_errno = ETIMEDOUT; return -1; } - if (sync_req.reply_received == -1) { + if (pending_req.reply_received == -1) { RTE_LOG(DEBUG, EAL, "Asked to ignore response\n"); /* not receiving this message is not an error, so decrement * number of sent messages @@ -1078,19 +1076,15 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts, rte_errno = errno; return -1; } - copy = malloc(sizeof(*copy)); - dummy = malloc(sizeof(*dummy)); - param = malloc(sizeof(*param)); + copy = calloc(1, sizeof(*copy)); + dummy = calloc(1, sizeof(*dummy)); + param = calloc(1, sizeof(*param)); if (copy == NULL || dummy == NULL || param == NULL) { RTE_LOG(ERR, EAL, "Failed to allocate memory for async reply\n"); rte_errno = ENOMEM; goto fail; } - memset(copy, 0, sizeof(*copy)); - memset(dummy, 0, sizeof(*dummy)); - memset(param, 0, sizeof(*param)); - /* copy message */ memcpy(copy, req, sizeof(*copy)); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ipc: clearn up code 2018-04-20 1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan @ 2018-04-20 7:55 ` Burakov, Anatoly 2018-04-20 14:41 ` Tan, Jianfeng 0 siblings, 1 reply; 11+ messages in thread From: Burakov, Anatoly @ 2018-04-20 7:55 UTC (permalink / raw) To: Jianfeng Tan, dev; +Cc: thomas On 20-Apr-18 2:21 AM, Jianfeng Tan wrote: > Following below commit, we change some internal function and variable > names: > commit ce3a7312357b ("eal: rename IPC request as synchronous one") > > Also use calloc to supersede malloc + memset for code clean up. > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > --- In general, Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> However, i think it would be good to have it rebased/applied on top of IPC coverity fixes, in particular because this patch uses strcpy instead of instead of strlcpy. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ipc: clearn up code 2018-04-20 7:55 ` Burakov, Anatoly @ 2018-04-20 14:41 ` Tan, Jianfeng 0 siblings, 0 replies; 11+ messages in thread From: Tan, Jianfeng @ 2018-04-20 14:41 UTC (permalink / raw) To: Burakov, Anatoly, dev; +Cc: thomas On 4/20/2018 3:55 PM, Burakov, Anatoly wrote: > On 20-Apr-18 2:21 AM, Jianfeng Tan wrote: >> Following below commit, we change some internal function and variable >> names: >> commit ce3a7312357b ("eal: rename IPC request as synchronous one") >> >> Also use calloc to supersede malloc + memset for code clean up. >> >> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> >> --- > > In general, > > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> > > However, i think it would be good to have it rebased/applied on top of > IPC coverity fixes, in particular because this patch uses strcpy > instead of instead of strlcpy. Will rebase on those fixes patches. And send the v2. Thanks, Jianfeng ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan @ 2018-04-20 1:21 ` Jianfeng Tan 2018-04-20 1:22 ` Tan, Jianfeng 2018-04-20 7:56 ` Burakov, Anatoly 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2 siblings, 2 replies; 11+ messages in thread From: Jianfeng Tan @ 2018-04-20 1:21 UTC (permalink / raw) To: dev; +Cc: thomas, anatoly.burakov, Jianfeng Tan In original implementation, timeout event for an async request will be ignored. As a result, an async request will never trigger the action if it cannot receive any reply any more. We fix this by counting timeout as a processed reply. Fixes: f05e26051c15 ("eal: add IPC asynchronous request") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> --- lib/librte_eal/common/eal_common_proc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index 070a075..27de16e 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr, const struct timespec *now) } else if (sr->reply_received == -1) { /* we were asked to ignore this process */ reply->nb_sent--; + } else if (timeout) { + /* count it as processed reponse, but don't increment + * nb_received. + */ + param->n_responses_processed++; } + free(sr->reply); last_msg = param->n_responses_processed == reply->nb_sent; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan @ 2018-04-20 1:22 ` Tan, Jianfeng 2018-04-20 7:56 ` Burakov, Anatoly 1 sibling, 0 replies; 11+ messages in thread From: Tan, Jianfeng @ 2018-04-20 1:22 UTC (permalink / raw) To: dev; +Cc: thomas, Burakov, Anatoly > -----Original Message----- > From: Tan, Jianfeng > Sent: Friday, April 20, 2018 9:22 AM > To: dev@dpdk.org > Cc: thomas@monjalon.net; Burakov, Anatoly; Tan, Jianfeng > Subject: [PATCH 2/2] ipc: fix timeout not properly handled in async > > In original implementation, timeout event for an async request > will be ignored. As a result, an async request will never > trigger the action if it cannot receive any reply any more. > > We fix this by counting timeout as a processed reply. > > Fixes: f05e26051c15 ("eal: add IPC asynchronous request") > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> > --- > lib/librte_eal/common/eal_common_proc.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/lib/librte_eal/common/eal_common_proc.c > b/lib/librte_eal/common/eal_common_proc.c > index 070a075..27de16e 100644 > --- a/lib/librte_eal/common/eal_common_proc.c > +++ b/lib/librte_eal/common/eal_common_proc.c > @@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr, > const struct timespec *now) > } else if (sr->reply_received == -1) { > /* we were asked to ignore this process */ > reply->nb_sent--; > + } else if (timeout) { > + /* count it as processed reponse, but don't increment Oops, a typo: s/reponse/response > + * nb_received. > + */ > + param->n_responses_processed++; > } > + > free(sr->reply); > > last_msg = param->n_responses_processed == reply->nb_sent; > -- > 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-20 1:22 ` Tan, Jianfeng @ 2018-04-20 7:56 ` Burakov, Anatoly 1 sibling, 0 replies; 11+ messages in thread From: Burakov, Anatoly @ 2018-04-20 7:56 UTC (permalink / raw) To: Jianfeng Tan, dev; +Cc: thomas On 20-Apr-18 2:21 AM, Jianfeng Tan wrote: > In original implementation, timeout event for an async request > will be ignored. As a result, an async request will never > trigger the action if it cannot receive any reply any more. > > We fix this by counting timeout as a processed reply. > > Fixes: f05e26051c15 ("eal: add IPC asynchronous request") > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> > --- Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> Thanks for catching this! -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/2] code clean up and fix in ipc 2018-04-20 1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 1:21 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan @ 2018-04-20 15:20 ` Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan ` (2 more replies) 2 siblings, 3 replies; 11+ messages in thread From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw) To: dev; +Cc: thomas, Jianfeng Tan v2: - Fix typo. - Rebase on below patches: http://dpdk.org/dev/patchwork/patch/38358/ http://dpdk.org/dev/patchwork/patch/38359/ http://dpdk.org/dev/patchwork/patch/38360/ Patch 1 is to rename some variable names for better readability. Patch 2 is a bug fix. Jianfeng Tan (2): ipc: clearn up code ipc: fix timeout not properly handled in async lib/librte_eal/common/eal_common_proc.c | 90 +++++++++++++++++---------------- 1 file changed, 46 insertions(+), 44 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] ipc: clearn up code 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan @ 2018-04-20 15:20 ` Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-23 20:48 ` [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon 2 siblings, 0 replies; 11+ messages in thread From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw) To: dev; +Cc: thomas, Jianfeng Tan Following below commit, we change some internal function and variable names: commit ce3a7312357b ("eal: rename IPC request as synchronous one") Also use calloc to supersede malloc + memset for code clean up. Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> --- lib/librte_eal/common/eal_common_proc.c | 84 ++++++++++++++++----------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index a329708..f8cb0e8 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -108,7 +108,7 @@ mp_send(struct rte_mp_msg *msg, const char *peer, int type); static struct pending_request * -find_sync_request(const char *dst, const char *act_name) +find_pending_request(const char *dst, const char *act_name) { struct pending_request *r; @@ -282,7 +282,7 @@ read_msg(struct mp_msg_internal *m, struct sockaddr_un *s) static void process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) { - struct pending_request *sync_req; + struct pending_request *pending_req; struct action_entry *entry; struct rte_mp_msg *msg = &m->msg; rte_mp_t action = NULL; @@ -291,15 +291,16 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) if (m->type == MP_REP || m->type == MP_IGN) { pthread_mutex_lock(&pending_requests.lock); - sync_req = find_sync_request(s->sun_path, msg->name); - if (sync_req) { - memcpy(sync_req->reply, msg, sizeof(*msg)); + pending_req = find_pending_request(s->sun_path, msg->name); + if (pending_req) { + memcpy(pending_req->reply, msg, sizeof(*msg)); /* -1 indicates that we've been asked to ignore */ - sync_req->reply_received = m->type == MP_REP ? 1 : -1; + pending_req->reply_received = + m->type == MP_REP ? 1 : -1; - if (sync_req->type == REQUEST_TYPE_SYNC) - pthread_cond_signal(&sync_req->sync.cond); - else if (sync_req->type == REQUEST_TYPE_ASYNC) + if (pending_req->type == REQUEST_TYPE_SYNC) + pthread_cond_signal(&pending_req->sync.cond); + else if (pending_req->type == REQUEST_TYPE_ASYNC) pthread_cond_signal( &pending_requests.async_cond); } else @@ -322,6 +323,7 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s) * yet ready to process this request. */ struct rte_mp_msg dummy; + memset(&dummy, 0, sizeof(dummy)); strlcpy(dummy.name, msg->name, sizeof(dummy.name)); mp_send(&dummy, s->sun_path, MP_IGN); @@ -855,30 +857,28 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, struct async_request_param *param) { struct rte_mp_msg *reply_msg; - struct pending_request *sync_req, *exist; + struct pending_request *pending_req, *exist; int ret; - sync_req = malloc(sizeof(*sync_req)); - reply_msg = malloc(sizeof(*reply_msg)); - if (sync_req == NULL || reply_msg == NULL) { + pending_req = calloc(1, sizeof(*pending_req)); + reply_msg = calloc(1, sizeof(*reply_msg)); + if (pending_req == NULL || reply_msg == NULL) { RTE_LOG(ERR, EAL, "Could not allocate space for sync request\n"); rte_errno = ENOMEM; ret = -1; goto fail; } - memset(sync_req, 0, sizeof(*sync_req)); - memset(reply_msg, 0, sizeof(*reply_msg)); - - sync_req->type = REQUEST_TYPE_ASYNC; - strlcpy(sync_req->dst, dst, sizeof(sync_req->dst)); - sync_req->request = req; - sync_req->reply = reply_msg; - sync_req->async.param = param; + pending_req->type = REQUEST_TYPE_ASYNC; + strlcpy(pending_req->dst, dst, sizeof(pending_req->dst)); + strcpy(pending_req->dst, dst); + pending_req->request = req; + pending_req->reply = reply_msg; + pending_req->async.param = param; /* queue already locked by caller */ - exist = find_sync_request(dst, req->name); + exist = find_pending_request(dst, req->name); if (exist) { RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name); rte_errno = EEXIST; @@ -896,13 +896,13 @@ mp_request_async(const char *dst, struct rte_mp_msg *req, ret = 0; goto fail; } - TAILQ_INSERT_TAIL(&pending_requests.requests, sync_req, next); + TAILQ_INSERT_TAIL(&pending_requests.requests, pending_req, next); param->user_reply.nb_sent++; return 0; fail: - free(sync_req); + free(pending_req); free(reply_msg); return ret; } @@ -913,16 +913,16 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, { int ret; struct rte_mp_msg msg, *tmp; - struct pending_request sync_req, *exist; + struct pending_request pending_req, *exist; - sync_req.type = REQUEST_TYPE_SYNC; - sync_req.reply_received = 0; - strlcpy(sync_req.dst, dst, sizeof(sync_req.dst)); - sync_req.request = req; - sync_req.reply = &msg; - pthread_cond_init(&sync_req.sync.cond, NULL); + pending_req.type = REQUEST_TYPE_SYNC; + pending_req.reply_received = 0; + strlcpy(pending_req.dst, dst, sizeof(pending_req.dst)); + pending_req.request = req; + pending_req.reply = &msg; + pthread_cond_init(&pending_req.sync.cond, NULL); - exist = find_sync_request(dst, req->name); + exist = find_pending_request(dst, req->name); if (exist) { RTE_LOG(ERR, EAL, "A pending request %s:%s\n", dst, req->name); rte_errno = EEXIST; @@ -937,24 +937,24 @@ mp_request_sync(const char *dst, struct rte_mp_msg *req, } else if (ret == 0) return 0; - TAILQ_INSERT_TAIL(&pending_requests.requests, &sync_req, next); + TAILQ_INSERT_TAIL(&pending_requests.requests, &pending_req, next); reply->nb_sent++; do { - ret = pthread_cond_timedwait(&sync_req.sync.cond, + ret = pthread_cond_timedwait(&pending_req.sync.cond, &pending_requests.lock, ts); } while (ret != 0 && ret != ETIMEDOUT); - TAILQ_REMOVE(&pending_requests.requests, &sync_req, next); + TAILQ_REMOVE(&pending_requests.requests, &pending_req, next); - if (sync_req.reply_received == 0) { + if (pending_req.reply_received == 0) { RTE_LOG(ERR, EAL, "Fail to recv reply for request %s:%s\n", dst, req->name); rte_errno = ETIMEDOUT; return -1; } - if (sync_req.reply_received == -1) { + if (pending_req.reply_received == -1) { RTE_LOG(DEBUG, EAL, "Asked to ignore response\n"); /* not receiving this message is not an error, so decrement * number of sent messages @@ -1079,19 +1079,15 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts, rte_errno = errno; return -1; } - copy = malloc(sizeof(*copy)); - dummy = malloc(sizeof(*dummy)); - param = malloc(sizeof(*param)); + copy = calloc(1, sizeof(*copy)); + dummy = calloc(1, sizeof(*dummy)); + param = calloc(1, sizeof(*param)); if (copy == NULL || dummy == NULL || param == NULL) { RTE_LOG(ERR, EAL, "Failed to allocate memory for async reply\n"); rte_errno = ENOMEM; goto fail; } - memset(copy, 0, sizeof(*copy)); - memset(dummy, 0, sizeof(*dummy)); - memset(param, 0, sizeof(*param)); - /* copy message */ memcpy(copy, req, sizeof(*copy)); -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] ipc: fix timeout not properly handled in async 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan @ 2018-04-20 15:20 ` Jianfeng Tan 2018-04-23 20:48 ` [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon 2 siblings, 0 replies; 11+ messages in thread From: Jianfeng Tan @ 2018-04-20 15:20 UTC (permalink / raw) To: dev; +Cc: thomas, Jianfeng Tan, Anatoly Burakov In original implementation, timeout event for an async request will be ignored. As a result, an async request will never trigger the action if it cannot receive any reply any more. We fix this by counting timeout as a processed reply. Fixes: f05e26051c15 ("eal: add IPC asynchronous request") Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com> --- lib/librte_eal/common/eal_common_proc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c index f8cb0e8..9136fb0 100644 --- a/lib/librte_eal/common/eal_common_proc.c +++ b/lib/librte_eal/common/eal_common_proc.c @@ -419,7 +419,13 @@ process_async_request(struct pending_request *sr, const struct timespec *now) } else if (sr->reply_received == -1) { /* we were asked to ignore this process */ reply->nb_sent--; + } else if (timeout) { + /* count it as processed response, but don't increment + * nb_received. + */ + param->n_responses_processed++; } + free(sr->reply); last_msg = param->n_responses_processed == reply->nb_sent; -- 2.7.4 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 0/2] code clean up and fix in ipc 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan @ 2018-04-23 20:48 ` Thomas Monjalon 2 siblings, 0 replies; 11+ messages in thread From: Thomas Monjalon @ 2018-04-23 20:48 UTC (permalink / raw) To: Jianfeng Tan; +Cc: dev 20/04/2018 17:20, Jianfeng Tan: > v2: > - Fix typo. > - Rebase on below patches: > http://dpdk.org/dev/patchwork/patch/38358/ > http://dpdk.org/dev/patchwork/patch/38359/ > http://dpdk.org/dev/patchwork/patch/38360/ > > Patch 1 is to rename some variable names for better readability. > > Patch 2 is a bug fix. > > Jianfeng Tan (2): > ipc: clearn up code > ipc: fix timeout not properly handled in async Applied, thanks ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-04-23 20:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-20 1:21 [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [PATCH 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 7:55 ` Burakov, Anatoly 2018-04-20 14:41 ` Tan, Jianfeng 2018-04-20 1:21 ` [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-20 1:22 ` Tan, Jianfeng 2018-04-20 7:56 ` Burakov, Anatoly 2018-04-20 15:20 ` [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 15:20 ` [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-23 20:48 ` [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.