From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 7D102A493 for ; Sat, 27 Jan 2018 04:48:21 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Jan 2018 19:48:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,420,1511856000"; d="scan'208";a="13482053" Received: from jguo15x-mobl3.ccr.corp.intel.com (HELO [10.255.28.200]) ([10.255.28.200]) by fmsmga008.fm.intel.com with ESMTP; 26 Jan 2018 19:48:18 -0800 To: Bruce Richardson , thomas@monjalon.net References: <1516248723-16985-3-git-send-email-jia.guo@intel.com> <1516938577-27662-1-git-send-email-jia.guo@intel.com> <20180126165340.GA21468@bricha3-MOBL3.ger.corp.intel.com> Cc: stephen@networkplumber.org, gaetan.rivet@6wind.com, jingjing.wu@intel.com, motih@mellanox.com, ferruh.yigit@intel.com, konstantin.ananyev@intel.com, jblunck@infradead.org, shreyansh.jain@nxp.com, dev@dpdk.org, helin.zhang@intel.com From: "Guo, Jia" Message-ID: <4dba572b-38e1-33b8-2009-c7d977b8b012@intel.com> Date: Sat, 27 Jan 2018 11:48:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20180126165340.GA21468@bricha3-MOBL3.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH V13 1/3] eal: add uevent monitor api and callback func 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: Sat, 27 Jan 2018 03:48:22 -0000 On 1/27/2018 12:53 AM, Bruce Richardson wrote: > On Fri, Jan 26, 2018 at 11:49:35AM +0800, Jeff Guo wrote: >> This patch aim to add a general uevent mechanism in eal device layer, >> to enable all linux kernel object uevent monitoring, user could use these >> APIs to monitor and read out the device status info that sent from the >> kernel side, then corresponding to handle it, such as when detect hotplug >> uevent type, user could detach or attach the device, and more it benefit >> to use to do smoothly fail safe work. >> >> About uevent monitoring: >> a: add one epolling to poll the netlink socket, to monitor the uevent of >> the device. >> b: add enum of rte_eal_dev_event_type and struct of rte_eal_uevent. >> c: add below APIs in rte eal device layer. >> rte_dev_callback_register >> rte_dev_callback_unregister >> _rte_dev_callback_process >> rte_dev_event_monitor_start >> rte_dev_event_monitor_stop >> >> Signed-off-by: Jeff Guo > Hi Jeff, > > >> --- >> v13->v12: >> fix some logic issue and null check issue >> fix monitor stop func issue and bsp build issue > > >> +int >> +rte_dev_event_monitor_start(void) >> +{ >> + int ret; >> + struct rte_service_spec service; >> + const uint32_t sid = 0; >> + >> + if (!service_no_init) >> + return 0; >> + >> + slcore = rte_get_next_lcore(/* start core */ -1, >> + /* skip master */ 1, >> + /* wrap */ 0); >> + >> + 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. @tomas, do you agree with that above , or other suggestion, could it be got agreement all or let it improvement later? >> + memset(&service, 0, sizeof(service)); >> + snprintf(service.name, sizeof(service.name), DEV_EV_MNT_SERVICE_NAME); >> + >> + service.socket_id = rte_socket_id(); >> + service.callback = dev_uev_monitoring; >> + service.callback_userdata = NULL; >> + service.capabilities = 0; >> + ret = rte_service_component_register(&service, &sevice_id); >> + if (ret) { >> + RTE_LOG(ERR, EAL, "Failed to register service %s " >> + "err = %" PRId32, >> + service.name, ret); >> + return ret; >> + } >> + ret = rte_service_runstate_set(sid, 1); >> + if (ret) { >> + RTE_LOG(ERR, EAL, "Failed to set the runstate of " >> + "the service"); >> + goto err_done; >> + } >> + ret = rte_service_component_runstate_set(sevice_id, 1); >> + if (ret) { >> + RTE_LOG(ERR, EAL, "Failed to set the backend runstate" >> + " of a component"); >> + return ret; >> + } >> + ret = rte_service_map_lcore_set(sid, slcore, 1); >> + if (ret) { >> + RTE_LOG(ERR, EAL, "Failed to enable lcore 1 on " >> + "dev event monitor service"); >> + return ret; >> + } >> + rte_service_lcore_start(slcore); >> + service_no_init = false; >> + return 0; >> + >> +err_done: >> + rte_service_component_unregister(sevice_id); >> + return ret; >> +} >> + > Regards, > /Bruce