Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/41187/?format=api
{ "id": 41187, "url": "http://patches.dpdk.org/api/patches/41187/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/patch/58e40d990bef3ab14ce0c26d6739cfce838ccfeb.1529071026.git.anatoly.burakov@intel.com/", "project": { "id": 1, "url": "http://patches.dpdk.org/api/projects/1/?format=api", "name": "DPDK", "link_name": "dpdk", "list_id": "dev.dpdk.org", "list_email": "dev@dpdk.org", "web_url": "http://core.dpdk.org", "scm_url": "git://dpdk.org/dpdk", "webscm_url": "http://git.dpdk.org/dpdk", "list_archive_url": "https://inbox.dpdk.org/dev", "list_archive_url_format": "https://inbox.dpdk.org/dev/{}", "commit_url_format": "" }, "msgid": "<58e40d990bef3ab14ce0c26d6739cfce838ccfeb.1529071026.git.anatoly.burakov@intel.com>", "list_archive_url": "https://inbox.dpdk.org/dev/58e40d990bef3ab14ce0c26d6739cfce838ccfeb.1529071026.git.anatoly.burakov@intel.com", "date": "2018-06-15T14:25:03", "name": "[3/8] ipc: remove IPC thread for async requests", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": true, "hash": "9fb5e19845cbd4e21f191421ccd8ca7dc44d1883", "submitter": { "id": 4, "url": "http://patches.dpdk.org/api/people/4/?format=api", "name": "Burakov, Anatoly", "email": "anatoly.burakov@intel.com" }, "delegate": { "id": 1, "url": "http://patches.dpdk.org/api/users/1/?format=api", "username": "tmonjalo", "first_name": "Thomas", "last_name": "Monjalon", "email": "thomas@monjalon.net" }, "mbox": "http://patches.dpdk.org/project/dpdk/patch/58e40d990bef3ab14ce0c26d6739cfce838ccfeb.1529071026.git.anatoly.burakov@intel.com/mbox/", "series": [ { "id": 145, "url": "http://patches.dpdk.org/api/series/145/?format=api", "web_url": "http://patches.dpdk.org/project/dpdk/list/?series=145", "date": "2018-06-15T14:25:00", "name": "Remove IPC threads", "version": 1, "mbox": "http://patches.dpdk.org/series/145/mbox/" } ], "comments": "http://patches.dpdk.org/api/patches/41187/comments/", "check": "success", "checks": "http://patches.dpdk.org/api/patches/41187/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<dev-bounces@dpdk.org>", "X-Original-To": "patchwork@dpdk.org", "Delivered-To": "patchwork@dpdk.org", "Received": [ "from [92.243.14.124] (localhost [127.0.0.1])\n\tby dpdk.org (Postfix) with ESMTP id E707E1D68C;\n\tFri, 15 Jun 2018 16:25:20 +0200 (CEST)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby dpdk.org (Postfix) with ESMTP id C55FB1D616\n\tfor <dev@dpdk.org>; Fri, 15 Jun 2018 16:25:11 +0200 (CEST)", "from orsmga006.jf.intel.com ([10.7.209.51])\n\tby orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t15 Jun 2018 07:25:10 -0700", "from irvmail001.ir.intel.com ([163.33.26.43])\n\tby orsmga006.jf.intel.com with ESMTP; 15 Jun 2018 07:25:09 -0700", "from sivswdev01.ir.intel.com (sivswdev01.ir.intel.com\n\t[10.237.217.45])\n\tby irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id\n\tw5FEP8Wa028182; Fri, 15 Jun 2018 15:25:08 +0100", "from sivswdev01.ir.intel.com (localhost [127.0.0.1])\n\tby sivswdev01.ir.intel.com with ESMTP id w5FEP8S9019456;\n\tFri, 15 Jun 2018 15:25:08 +0100", "(from aburakov@localhost)\n\tby sivswdev01.ir.intel.com with LOCAL id w5FEP8rL019452;\n\tFri, 15 Jun 2018 15:25:08 +0100" ], "X-Amp-Result": "SKIPPED(no attachment in message)", "X-Amp-File-Uploaded": "False", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.51,227,1526367600\"; d=\"scan'208\";a=\"50178069\"", "From": "Anatoly Burakov <anatoly.burakov@intel.com>", "To": "dev@dpdk.org", "Cc": "konstantin.ananyev@intel.com, thomas@monjalon.net,\n\tbruce.richardson@intel.com, Jianfeng Tan <jianfeng.tan@intel.com>", "Date": "Fri, 15 Jun 2018 15:25:03 +0100", "Message-Id": "<58e40d990bef3ab14ce0c26d6739cfce838ccfeb.1529071026.git.anatoly.burakov@intel.com>", "X-Mailer": "git-send-email 1.7.0.7", "In-Reply-To": [ "<cover.1529071026.git.anatoly.burakov@intel.com>", "<cover.1529071026.git.anatoly.burakov@intel.com>" ], "References": [ "<cover.1529071026.git.anatoly.burakov@intel.com>", "<cover.1529071026.git.anatoly.burakov@intel.com>" ], "Subject": "[dpdk-dev] [PATCH 3/8] ipc: remove IPC thread for async requests", "X-BeenThere": "dev@dpdk.org", "X-Mailman-Version": "2.1.15", "Precedence": "list", "List-Id": "DPDK patches and discussions <dev.dpdk.org>", "List-Unsubscribe": "<https://dpdk.org/ml/options/dev>,\n\t<mailto:dev-request@dpdk.org?subject=unsubscribe>", "List-Archive": "<http://dpdk.org/ml/archives/dev/>", "List-Post": "<mailto:dev@dpdk.org>", "List-Help": "<mailto:dev-request@dpdk.org?subject=help>", "List-Subscribe": "<https://dpdk.org/ml/listinfo/dev>,\n\t<mailto:dev-request@dpdk.org?subject=subscribe>", "Errors-To": "dev-bounces@dpdk.org", "Sender": "\"dev\" <dev-bounces@dpdk.org>" }, "content": "Previously, we were using two IPC threads - one to handle messages\nand synchronous requests, and another to handle asynchronous requests.\nTo handle replies for an async request, rte_mp_handle woke up the\nrte_mp_handle_async thread to process through pthread_cond variable.\n\nChange it to handle asynchronous messages within the main IPC thread.\nTo handle timeout events, for each async request which is sent,\nwe set an alarm for it. If its reply is received before timeout,\nwe will cancel the alarm when we handle the reply; otherwise,\nalarm will invoke the async_reply_handle() as the alarm callback.\n\nSigned-off-by: Jianfeng Tan <jianfeng.tan@intel.com>\nSigned-off-by: Anatoly Burakov <anatoly.burakov@intel.com>\nSuggested-by: Thomas Monjalon <thomas@monjalon.net>\n---\n\nNotes:\n RFC->RFCv2:\n - Rebased on latest code\n - Implemented comments to the original RFC\n\n lib/librte_eal/common/eal_common_proc.c | 201 +++++++++---------------\n 1 file changed, 70 insertions(+), 131 deletions(-)", "diff": "diff --git a/lib/librte_eal/common/eal_common_proc.c b/lib/librte_eal/common/eal_common_proc.c\nindex 707d8ab30..6f3366403 100644\n--- a/lib/librte_eal/common/eal_common_proc.c\n+++ b/lib/librte_eal/common/eal_common_proc.c\n@@ -20,6 +20,7 @@\n #include <sys/un.h>\n #include <unistd.h>\n \n+#include <rte_alarm.h>\n #include <rte_common.h>\n #include <rte_cycles.h>\n #include <rte_eal.h>\n@@ -94,11 +95,9 @@ TAILQ_HEAD(pending_request_list, pending_request);\n static struct {\n \tstruct pending_request_list requests;\n \tpthread_mutex_t lock;\n-\tpthread_cond_t async_cond;\n } pending_requests = {\n \t.requests = TAILQ_HEAD_INITIALIZER(pending_requests.requests),\n \t.lock = PTHREAD_MUTEX_INITIALIZER,\n-\t.async_cond = PTHREAD_COND_INITIALIZER\n \t/**< used in async requests only */\n };\n \n@@ -106,6 +105,16 @@ static struct {\n static int\n mp_send(struct rte_mp_msg *msg, const char *peer, int type);\n \n+/* for use with alarm callback */\n+static void\n+async_reply_handle(void *arg);\n+\n+/* for use with process_msg */\n+static struct pending_request *\n+async_reply_handle_thread_unsafe(void *arg);\n+\n+static void\n+trigger_async_action(struct pending_request *req);\n \n static struct pending_request *\n find_pending_request(const char *dst, const char *act_name)\n@@ -290,6 +299,8 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)\n \tRTE_LOG(DEBUG, EAL, \"msg: %s\\n\", msg->name);\n \n \tif (m->type == MP_REP || m->type == MP_IGN) {\n+\t\tstruct pending_request *req = NULL;\n+\n \t\tpthread_mutex_lock(&pending_requests.lock);\n \t\tpending_req = find_pending_request(s->sun_path, msg->name);\n \t\tif (pending_req) {\n@@ -301,11 +312,14 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)\n \t\t\tif (pending_req->type == REQUEST_TYPE_SYNC)\n \t\t\t\tpthread_cond_signal(&pending_req->sync.cond);\n \t\t\telse if (pending_req->type == REQUEST_TYPE_ASYNC)\n-\t\t\t\tpthread_cond_signal(\n-\t\t\t\t\t&pending_requests.async_cond);\n+\t\t\t\treq = async_reply_handle_thread_unsafe(\n+\t\t\t\t\t\tpending_req);\n \t\t} else\n \t\t\tRTE_LOG(ERR, EAL, \"Drop mp reply: %s\\n\", msg->name);\n \t\tpthread_mutex_unlock(&pending_requests.lock);\n+\n+\t\tif (req != NULL)\n+\t\t\ttrigger_async_action(req);\n \t\treturn;\n \t}\n \n@@ -365,7 +379,6 @@ timespec_cmp(const struct timespec *a, const struct timespec *b)\n }\n \n enum async_action {\n-\tACTION_NONE, /**< don't do anything */\n \tACTION_FREE, /**< free the action entry, but don't trigger callback */\n \tACTION_TRIGGER /**< trigger callback, then free action entry */\n };\n@@ -375,7 +388,7 @@ process_async_request(struct pending_request *sr, const struct timespec *now)\n {\n \tstruct async_request_param *param;\n \tstruct rte_mp_reply *reply;\n-\tbool timeout, received, last_msg;\n+\tbool timeout, last_msg;\n \n \tparam = sr->async.param;\n \treply = ¶m->user_reply;\n@@ -383,13 +396,6 @@ process_async_request(struct pending_request *sr, const struct timespec *now)\n \t/* did we timeout? */\n \ttimeout = timespec_cmp(¶m->end, now) <= 0;\n \n-\t/* did we receive a response? */\n-\treceived = sr->reply_received != 0;\n-\n-\t/* if we didn't time out, and we didn't receive a response, ignore */\n-\tif (!timeout && !received)\n-\t\treturn ACTION_NONE;\n-\n \t/* if we received a response, adjust relevant data and copy mesasge. */\n \tif (sr->reply_received == 1 && sr->reply) {\n \t\tstruct rte_mp_msg *msg, *user_msgs, *tmp;\n@@ -448,120 +454,60 @@ trigger_async_action(struct pending_request *sr)\n \tfree(sr->async.param->user_reply.msgs);\n \tfree(sr->async.param);\n \tfree(sr->request);\n+\tfree(sr);\n }\n \n static struct pending_request *\n-check_trigger(struct timespec *ts)\n+async_reply_handle_thread_unsafe(void *arg)\n {\n-\tstruct pending_request *next, *cur, *trigger = NULL;\n-\n-\tTAILQ_FOREACH_SAFE(cur, &pending_requests.requests, next, next) {\n-\t\tenum async_action action;\n-\t\tif (cur->type != REQUEST_TYPE_ASYNC)\n-\t\t\tcontinue;\n-\n-\t\taction = process_async_request(cur, ts);\n-\t\tif (action == ACTION_FREE) {\n-\t\t\tTAILQ_REMOVE(&pending_requests.requests, cur, next);\n-\t\t\tfree(cur);\n-\t\t} else if (action == ACTION_TRIGGER) {\n-\t\t\tTAILQ_REMOVE(&pending_requests.requests, cur, next);\n-\t\t\ttrigger = cur;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\treturn trigger;\n-}\n-\n-static void\n-wait_for_async_messages(void)\n-{\n-\tstruct pending_request *sr;\n-\tstruct timespec timeout;\n-\tbool timedwait = false;\n-\tbool nowait = false;\n-\tint ret;\n-\n-\t/* scan through the list and see if there are any timeouts that\n-\t * are earlier than our current timeout.\n-\t */\n-\tTAILQ_FOREACH(sr, &pending_requests.requests, next) {\n-\t\tif (sr->type != REQUEST_TYPE_ASYNC)\n-\t\t\tcontinue;\n-\t\tif (!timedwait || timespec_cmp(&sr->async.param->end,\n-\t\t\t\t&timeout) < 0) {\n-\t\t\tmemcpy(&timeout, &sr->async.param->end,\n-\t\t\t\tsizeof(timeout));\n-\t\t\ttimedwait = true;\n-\t\t}\n-\n-\t\t/* sometimes, we don't even wait */\n-\t\tif (sr->reply_received) {\n-\t\t\tnowait = true;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tif (nowait)\n-\t\treturn;\n-\n-\tdo {\n-\t\tret = timedwait ?\n-\t\t\tpthread_cond_timedwait(\n-\t\t\t\t&pending_requests.async_cond,\n-\t\t\t\t&pending_requests.lock,\n-\t\t\t\t&timeout) :\n-\t\t\tpthread_cond_wait(\n-\t\t\t\t&pending_requests.async_cond,\n-\t\t\t\t&pending_requests.lock);\n-\t} while (ret != 0 && ret != ETIMEDOUT);\n-\n-\t/* we've been woken up or timed out */\n-}\n-\n-static void *\n-async_reply_handle(void *arg __rte_unused)\n-{\n-\tstruct timeval now;\n+\tstruct pending_request *req = (struct pending_request *)arg;\n+\tenum async_action action;\n \tstruct timespec ts_now;\n-\twhile (1) {\n-\t\tstruct pending_request *trigger = NULL;\n+\tstruct timeval now;\n \n-\t\tpthread_mutex_lock(&pending_requests.lock);\n+\tif (gettimeofday(&now, NULL) < 0) {\n+\t\tRTE_LOG(ERR, EAL, \"Cannot get current time\\n\");\n+\t\tgoto no_trigger;\n+\t}\n+\tts_now.tv_nsec = now.tv_usec * 1000;\n+\tts_now.tv_sec = now.tv_sec;\n \n-\t\t/* we exit this function holding the lock */\n-\t\twait_for_async_messages();\n+\taction = process_async_request(req, &ts_now);\n \n-\t\tif (gettimeofday(&now, NULL) < 0) {\n-\t\t\tpthread_mutex_unlock(&pending_requests.lock);\n-\t\t\tRTE_LOG(ERR, EAL, \"Cannot get current time\\n\");\n-\t\t\tbreak;\n+\tTAILQ_REMOVE(&pending_requests.requests, req, next);\n+\n+\tif (rte_eal_alarm_cancel(async_reply_handle, req) < 0) {\n+\t\t/* if we failed to cancel the alarm because it's already in\n+\t\t * progress, don't proceed because otherwise we will end up\n+\t\t * handling the same message twice.\n+\t\t */\n+\t\tif (rte_errno == EINPROGRESS) {\n+\t\t\tRTE_LOG(DEBUG, EAL, \"Request handling is already in progress\\n\");\n+\t\t\tgoto no_trigger;\n \t\t}\n-\t\tts_now.tv_nsec = now.tv_usec * 1000;\n-\t\tts_now.tv_sec = now.tv_sec;\n-\n-\t\tdo {\n-\t\t\ttrigger = check_trigger(&ts_now);\n-\t\t\t/* unlock request list */\n-\t\t\tpthread_mutex_unlock(&pending_requests.lock);\n-\n-\t\t\tif (trigger) {\n-\t\t\t\ttrigger_async_action(trigger);\n-\t\t\t\tfree(trigger);\n-\n-\t\t\t\t/* we've triggered a callback, but there may be\n-\t\t\t\t * more, so lock the list and check again.\n-\t\t\t\t */\n-\t\t\t\tpthread_mutex_lock(&pending_requests.lock);\n-\t\t\t}\n-\t\t} while (trigger);\n+\t\tRTE_LOG(ERR, EAL, \"Failed to cancel alarm\\n\");\n \t}\n \n-\tRTE_LOG(ERR, EAL, \"ERROR: asynchronous requests disabled\\n\");\n-\n+\tif (action == ACTION_TRIGGER)\n+\t\treturn req;\n+no_trigger:\n+\tfree(req);\n \treturn NULL;\n }\n \n+static void\n+async_reply_handle(void *arg)\n+{\n+\tstruct pending_request *req;\n+\n+\tpthread_mutex_lock(&pending_requests.lock);\n+\treq = async_reply_handle_thread_unsafe(arg);\n+\tpthread_mutex_unlock(&pending_requests.lock);\n+\n+\tif (req != NULL)\n+\t\ttrigger_async_action(req);\n+}\n+\n static int\n open_socket_fd(void)\n {\n@@ -624,7 +570,7 @@ rte_mp_channel_init(void)\n {\n \tchar path[PATH_MAX];\n \tint dir_fd;\n-\tpthread_t mp_handle_tid, async_reply_handle_tid;\n+\tpthread_t mp_handle_tid;\n \n \t/* create filter path */\n \tcreate_socket_path(\"*\", path, sizeof(path));\n@@ -671,17 +617,6 @@ rte_mp_channel_init(void)\n \t\treturn -1;\n \t}\n \n-\tif (rte_ctrl_thread_create(&async_reply_handle_tid,\n-\t\t\t\"rte_mp_async\", NULL,\n-\t\t\tasync_reply_handle, NULL) < 0) {\n-\t\tRTE_LOG(ERR, EAL, \"failed to create mp thead: %s\\n\",\n-\t\t\tstrerror(errno));\n-\t\tclose(mp_fd);\n-\t\tclose(dir_fd);\n-\t\tmp_fd = -1;\n-\t\treturn -1;\n-\t}\n-\n \t/* unlock the directory */\n \tflock(dir_fd, LOCK_UN);\n \tclose(dir_fd);\n@@ -853,7 +788,7 @@ rte_mp_sendmsg(struct rte_mp_msg *msg)\n \n static int\n mp_request_async(const char *dst, struct rte_mp_msg *req,\n-\t\tstruct async_request_param *param)\n+\t\tstruct async_request_param *param, const struct timespec *ts)\n {\n \tstruct rte_mp_msg *reply_msg;\n \tstruct pending_request *pending_req, *exist;\n@@ -898,6 +833,13 @@ mp_request_async(const char *dst, struct rte_mp_msg *req,\n \n \tparam->user_reply.nb_sent++;\n \n+\tif (rte_eal_alarm_set(ts->tv_sec * 1000000 + ts->tv_nsec / 1000,\n+\t\t\t async_reply_handle, pending_req) < 0) {\n+\t\tRTE_LOG(ERR, EAL, \"Fail to set alarm for request %s:%s\\n\",\n+\t\t\tdst, req->name);\n+\t\trte_panic(\"Fix the above shit to properly free all memory\\n\");\n+\t}\n+\n \treturn 0;\n fail:\n \tfree(pending_req);\n@@ -1119,7 +1061,7 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,\n \n \t/* for secondary process, send request to the primary process only */\n \tif (rte_eal_process_type() == RTE_PROC_SECONDARY) {\n-\t\tret = mp_request_async(eal_mp_socket_path(), copy, param);\n+\t\tret = mp_request_async(eal_mp_socket_path(), copy, param, ts);\n \n \t\t/* if we didn't send anything, put dummy request on the queue */\n \t\tif (ret == 0 && reply->nb_sent == 0) {\n@@ -1162,7 +1104,7 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,\n \t\tsnprintf(path, sizeof(path), \"%s/%s\", mp_dir_path,\n \t\t\t ent->d_name);\n \n-\t\tif (mp_request_async(path, copy, param))\n+\t\tif (mp_request_async(path, copy, param, ts))\n \t\t\tret = -1;\n \t}\n \t/* if we didn't send anything, put dummy request on the queue */\n@@ -1171,9 +1113,6 @@ rte_mp_request_async(struct rte_mp_msg *req, const struct timespec *ts,\n \t\tdummy_used = true;\n \t}\n \n-\t/* trigger async request thread wake up */\n-\tpthread_cond_signal(&pending_requests.async_cond);\n-\n \t/* finally, unlock the queue */\n \tpthread_mutex_unlock(&pending_requests.lock);\n \n", "prefixes": [ "3/8" ] }