From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id EC78B7CC9 for ; Fri, 20 Apr 2018 11:03:19 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Apr 2018 02:03:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,300,1520924400"; d="scan'208";a="192999663" Received: from aburakov-mobl.ger.corp.intel.com (HELO [10.252.23.81]) ([10.252.23.81]) by orsmga004.jf.intel.com with ESMTP; 20 Apr 2018 02:03:16 -0700 To: Jianfeng Tan , dev@dpdk.org Cc: thomas@monjalon.net References: <1524150216-3407-1-git-send-email-jianfeng.tan@intel.com> <1524150216-3407-5-git-send-email-jianfeng.tan@intel.com> From: "Burakov, Anatoly" Message-ID: Date: Fri, 20 Apr 2018 10:03:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1524150216-3407-5-git-send-email-jianfeng.tan@intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [RFC 4/8] ipc: remove IPC thread for async request X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Apr 2018 09:03:20 -0000 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 > > Suggested-by: Thomas Monjalon > Signed-off-by: Jianfeng Tan > --- <...> > @@ -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