From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tan, Jianfeng" Subject: Re: [PATCH v2 3/4] eal: add synchronous multi-process communication Date: Tue, 16 Jan 2018 16:10:31 +0800 Message-ID: <5733adcf-ef47-2c07-a39c-7eda01add6e0@intel.com> References: <1512067450-59203-1-git-send-email-jianfeng.tan@intel.com> <1515643654-129489-1-git-send-email-jianfeng.tan@intel.com> <1515643654-129489-4-git-send-email-jianfeng.tan@intel.com> <2601191342CEEE43887BDE71AB9772588627E0E5@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: "Richardson, Bruce" , "thomas@monjalon.net" To: "Ananyev, Konstantin" , "dev@dpdk.org" , "Burakov, Anatoly" Return-path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 43F451B00B for ; Tue, 16 Jan 2018 09:10:34 +0100 (CET) In-Reply-To: <2601191342CEEE43887BDE71AB9772588627E0E5@irsmsx105.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Thank you, Konstantin and Anatoly firstly. Other comments are well received and I'll send out a new version. On 1/16/2018 8:00 AM, Ananyev, Konstantin wrote: > >> We need the synchronous way for multi-process communication, >> i.e., blockingly waiting for reply message when we send a request >> to the peer process. >> >> We add two APIs rte_eal_mp_request() and rte_eal_mp_reply() for >> such use case. By invoking rte_eal_mp_request(), a request message >> is sent out, and then it waits there for a reply message. The >> timeout is hard-coded 5 Sec. And the replied message will be copied >> in the parameters of this API so that the caller can decide how >> to translate those information (including params and fds). Note >> if a primary process owns multiple secondary processes, this API >> will fail. >> >> The API rte_eal_mp_reply() is always called by an mp action handler. >> Here we add another parameter for rte_eal_mp_t so that the action >> handler knows which peer address to reply. >> >> We use mutex in rte_eal_mp_request() to guarantee that only one >> request is on the fly for one pair of processes. > You don't need to do things in such strange and restrictive way. > Instead you can do something like that: > 1) Introduce new struct, list for it and mutex > struct sync_request { > int reply_received; > char dst[PATH_MAX]; > char reply[...]; > LIST_ENTRY(sync_request) next; > }; > > static struct > LIST_HEAD(list, sync_request); > pthread_mutex_t lock; > pthead_cond_t cond; > } sync_requests; > > 2) then at request() call: > Grab sync_requests.lock > Check do we already have a pending request for that destination, > If yes - the release the lock and returns with error. > - allocate and init new sync_request struct, set reply_received=0 > - do send_msg() > -then in a cycle: > pthread_cond_timed_wait(&sync_requests.cond, &sync_request.lock, ×pec); > - at return from it check if sync_request.reply_received == 1, if not > check if timeout expired and either return a failure or go to the start of the cycle. > > 3) at mp_handler() if REPLY received - grab sync_request.lock, > search through sync_requests.list for dst[] , > if found, then set it's reply_received=1, copy the received message into reply > and call pthread_cond_braodcast((&sync_requests.cond); The only benefit I can see is that now the sender can request to multiple receivers at the same time. And it makes things more complicated. Do we really need this? Thanks, Jianfeng