From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
To: Jianfeng Tan <jianfeng.tan@intel.com>, dev@dpdk.org
Cc: thomas@monjalon.net
Subject: Re: [dpdk-dev] [RFC 4/8] ipc: remove IPC thread for async request
Date: Fri, 20 Apr 2018 10:03:15 +0100 [thread overview]
Message-ID: <f5722292-e606-7b0f-b9c8-6343f9f00f2e@intel.com> (raw)
In-Reply-To: <1524150216-3407-5-git-send-email-jianfeng.tan@intel.com>
On 19-Apr-18 4:03 PM, Jianfeng Tan wrote:
> As discussed here, http://dpdk.org/dev/patchwork/patch/36579/,
> we remove IPC threads, rte_mp_handle and rte_mp_handle_async.
> This patch targets to remove thread rte_mp_handle_async.
>
> Previously, to handle replies for an async request, rte_mp_handle
> wakes up the rte_mp_handle_async thread to process through
> pending_requests.async_cond. Now, we change to handle that in
> rte_mp_handle context directly.
>
> To handle timeout events, for each async request which is sent,
> we set an alarm for it. If its reply is received before timeout,
> we will cancel the alarm when we handle the reply; otherwise,
> alarm will invoke the async_reply_handle() as the alarm callback.
>
> Cc: Anatoly Burakov <anatoly.burakov@intel.com>
>
> Suggested-by: Thomas Monjalon <thomas@monjalon.net>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
<...>
> @@ -299,9 +300,11 @@ process_msg(struct mp_msg_internal *m, struct sockaddr_un *s)
>
> 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 if (pending_req->type == REQUEST_TYPE_ASYNC) {
> + pthread_mutex_unlock(&pending_requests.lock);
> + async_reply_handle(pending_req);
> + pthread_mutex_lock(&pending_requests.lock);
There must be a better way to do this than to unlock mutex before
locking it again :) I haven't looked at implications of this suggestion
yet, but how about alarm calling a wrapper function that locks the
mutex, but leave async_reply_handle without any locking whatsoever?
> + }
> } else
> RTE_LOG(ERR, EAL, "Drop mp reply: %s\n", msg->name);
> pthread_mutex_unlock(&pending_requests.lock);
> @@ -450,115 +453,39 @@ trigger_async_action(struct pending_request *sr)
> free(sr->request);
> }
>
> -static struct pending_request *
> -check_trigger(struct timespec *ts)
<...>
> + ts_now.tv_nsec = now.tv_usec * 1000;
> + ts_now.tv_sec = now.tv_sec;
>
> - /* we've triggered a callback, but there may be
> - * more, so lock the list and check again.
> - */
> - pthread_mutex_lock(&pending_requests.lock);
> - }
> - } while (trigger);
> + pthread_mutex_lock(&pending_requests.lock);
> + action = process_async_request(req, &ts_now);
> + if (action == ACTION_NONE) {
> + pthread_mutex_unlock(&pending_requests.lock);
> + return;
> }
> + TAILQ_REMOVE(&pending_requests.requests, req, next);
> + pthread_mutex_unlock(&pending_requests.lock);
>
> - RTE_LOG(ERR, EAL, "ERROR: asynchronous requests disabled\n");
> + if (action == ACTION_TRIGGER)
> + trigger_async_action(req);
>
> - return NULL;
> + if (rte_eal_alarm_cancel(async_reply_handle, req) < 0 &&
> + rte_errno != EINPROGRESS)
> + RTE_LOG(ERR, EAL, "Failed to cancel alarm\n");
> +
Perhaps cancel the alarm before triggering callback?
--
Thanks,
Anatoly
next prev parent reply other threads:[~2018-04-20 9:03 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-19 15:03 [dpdk-dev] [RFC 0/8] remove IPC threads Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 1/8] ipc: clearn up code Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 2/8] ipc: fix timeout not properly handled in async Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 3/8] eal/linux: use glibc malloc in alarm Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 4/8] ipc: remove IPC thread for async request Jianfeng Tan
2018-04-20 9:03 ` Burakov, Anatoly [this message]
2018-04-19 15:03 ` [dpdk-dev] [RFC 5/8] eal/linux: use glibc malloc in interrupt handling Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 6/8] eal: bring forward init of " Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 7/8] eal: add IPC type for interrupt thread Jianfeng Tan
2018-04-19 15:03 ` [dpdk-dev] [RFC 8/8] ipc: remove IPC thread for message read Jianfeng Tan
2018-04-19 15:22 ` [dpdk-dev] [RFC 0/8] remove IPC threads Thomas Monjalon
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 0/6] Remove " Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 1/6] eal/linux: use glibc malloc in alarm Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 2/6] ipc: remove IPC thread for async requests Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 3/6] eal/linux: use glibc malloc in interrupt handling Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 4/6] eal: bring forward init of " Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 5/6] eal: add IPC type for interrupt thread Anatoly Burakov
2018-05-31 10:12 ` [dpdk-dev] [RFC v2 6/6] ipc: remove main IPC thread Anatoly Burakov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f5722292-e606-7b0f-b9c8-6343f9f00f2e@intel.com \
--to=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=jianfeng.tan@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).