From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Guo, Jia" Subject: Re: [PATCH V13 1/3] eal: add uevent monitor api and callback func Date: Tue, 30 Jan 2018 20:20:42 +0800 Message-ID: <9767bc3a-7592-ec77-e849-b0f53044b4d3@intel.com> References: <1516248723-16985-3-git-send-email-jia.guo@intel.com> <20180126165340.GA21468@bricha3-MOBL3.ger.corp.intel.com> <4dba572b-38e1-33b8-2009-c7d977b8b012@intel.com> <2094855.jKUssMJQjQ@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, stephen@networkplumber.org, gaetan.rivet@6wind.com, jingjing.wu@intel.com, motih@mellanox.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, shreyansh.jain@nxp.com, helin.zhang@intel.com To: Thomas Monjalon , Bruce Richardson , harry.van.haaren@intel.com Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 0098A1B3FA for ; Tue, 30 Jan 2018 13:20:51 +0100 (CET) In-Reply-To: <2094855.jKUssMJQjQ@xps> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 1/30/2018 8:14 AM, Thomas Monjalon wrote: > 27/01/2018 04:48, Guo, Jia: >> On 1/27/2018 12:53 AM, Bruce Richardson wrote: >>> On Fri, Jan 26, 2018 at 11:49:35AM +0800, Jeff Guo wrote: >>>> + ret = rte_service_lcore_add(slcore); >>>> + if (ret) { >>>> + RTE_LOG(ERR, EAL, "dev event monitor lcore add fail"); >>>> + return ret; >>>> + } >>>> + >>> I don't think you should be taking another service core for this purpose >>> without the user asking for it. I also don't think service cores is the >>> right "tool" for monitoring the epoll. Rather than using a non-blocking >>> poll on a service core, I think you should look to reuse the existing >>> infrastructure for handling interrupts in the EAL, which relies on a >>> separate thread blocked on fd's awaiting input. >> bruce, seems that you might be see the other view of the mountain, so if >> service cores tools basically be born to need user knowledge and >> control it, and it is no need to add user to control service tool in the >> case, i thinks we might not use the existing interrupts infrastructure >> because it is the device uevent not interrupt as the same functional >> scope , we could use a separate thread which i have used before in v7 >> to specialize poll the uevent, please check v7 part to see if it is good. > The v7 was using pthread_create, so it was not the right solution. > >> @tomas, do you agree with that above , or other suggestion, could it be >> got agreement all or let it improvement later? > I have no issue about using rte_service. > I think the other events processing in EAL could use rte_service. > Maybe Harry has a different view? > > My main concerns are: > 1/ There is not enough review > 2/ The callback lookup is using device name from uevent > 3/ There is no reference to the rte_device struct > > Minor extra requirement: the new __rte_experimental should be used, > see http://dpdk.org/commit/77b7b81e32e please review my patch v14 , hope i can fix all your concern, about rte_device struct , i think if there is not better idea to handler the null struct issue, the device name should be use as experimental and i have verify that is ok for use.