* [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc @ 2018-04-20 1:21 Jianfeng Tan 2018-04-20 1:21 ` [dpdk-dev] [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
* [dpdk-dev] [PATCH 1/2] ipc: clearn up code 2018-04-20 1:21 [dpdk-dev] [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 ` [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [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 [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] ipc: clearn up code 2018-04-20 1:21 ` [dpdk-dev] [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: [dpdk-dev] [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
* [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 ` [dpdk-dev] [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: [dpdk-dev] [PATCH 2/2] ipc: fix timeout not properly handled in async 2018-04-20 1:21 ` [dpdk-dev] [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
* [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc 2018-04-20 1:21 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [dpdk-dev] [PATCH 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 1:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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
* [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code 2018-04-20 15:20 ` [dpdk-dev] [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 ` [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-23 20:48 ` [dpdk-dev] [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 [flat|nested] 11+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan @ 2018-04-20 15:20 ` Jianfeng Tan 2018-04-23 20:48 ` [dpdk-dev] [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 [flat|nested] 11+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [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 [dpdk-dev] [PATCH 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 1:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 1/2] ipc: clearn up code Jianfeng Tan 2018-04-20 15:20 ` [dpdk-dev] [PATCH v2 2/2] ipc: fix timeout not properly handled in async Jianfeng Tan 2018-04-23 20:48 ` [dpdk-dev] [PATCH v2 0/2] code clean up and fix in ipc Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).