From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 298D1199B0 for ; Tue, 9 Jan 2018 12:38:42 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 99A6420A94; Tue, 9 Jan 2018 06:38:41 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Tue, 09 Jan 2018 06:38:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=DRfPfg2SQBLpsowGdsB/RsnUbs TrOyjgxjk2Gd9+AK8=; b=Xnb4exyq1SywLgi7AnPD0gSqzANOUh7UsLN/HbHoVY F/mTzxPgGHRvYBUMELebnpfamO/WZVqQiTMMlsai7hpfXnuLbRvt4t6IGinhe0w4 jGe3srlDnMWgw6KOEiWlo7bosIF2SRec8+vPMahgODoWX7tqnOdX85tTWEhBdr20 M= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=DRfPfg 2SQBLpsowGdsB/RsnUbsTrOyjgxjk2Gd9+AK8=; b=TFht0wYysKLg/EXNDUSlHY fihmRaHlbrqpD4StR9l82UvyT1NmXHmQOKn/z9+VIoZOVep0NjjNMV7MNHjA/uLu G3i8U9RRXg6FQKNrmWKOw8e6gFA0zKzJIJTBrmeBdXCNHNvNEdmWEqwycPhPYHfc DjaWBC4ppAR0e7FhIZqOGybqNlQDpx0jbs45LUsT/fJVOTj/H0dQ9kDT9f5isFjk hTKHef02FxTOcXnKMREsRHB5C+4fv/9myjRcVmB54YRKkA+UpQHTy6flR7CXEMz5 Zk4A1NVAeN5vFNnbH56k3EGKktHuB+HXfRhEBpRLJD2T+JvReHoB9V91OxSbkauQ == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 3988124608; Tue, 9 Jan 2018 06:38:41 -0500 (EST) From: Thomas Monjalon To: "Guo, Jia" Cc: dev@dpdk.org, stephen@networkplumber.org, bruce.richardson@intel.com, ferruh.yigit@intel.com, gaetan.rivet@6wind.com, konstantin.ananyev@intel.com, shreyansh.jain@nxp.com, jingjing.wu@intel.com, helin.zhang@intel.com, motih@mellanox.com, harry.van.haaren@intel.com Date: Tue, 09 Jan 2018 12:38:17 +0100 Message-ID: <1930193.dgY3bQfKM7@xps> In-Reply-To: <6b97ebc5-1ec5-a724-a620-96b23b126d01@intel.com> References: <1509567405-27439-3-git-send-email-jia.guo@intel.com> <2000997.W0TFPrnL2l@xps> <6b97ebc5-1ec5-a724-a620-96b23b126d01@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v7 1/2] eal: add uevent monitor for hot plug 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: Tue, 09 Jan 2018 11:38:43 -0000 09/01/2018 09:25, Guo, Jia: > On 1/9/2018 8:39 AM, Thomas Monjalon wrote: > >> +int > >> +_rte_dev_callback_process(struct rte_device *device, > >> + enum rte_eal_dev_event_type event, > >> + void *cb_arg, void *ret_param) > > > > cb_arg must be an opaque parameter which is registered with the > > callback and passed later. No need as parameter of this function. > > > > ret_param is not needed at all. The kernel event will be just > > translated as rte_eal_dev_event_type (rte_dev_event after rename). > > suggest hold one to let new param, such as device info, add by > ret_param, so cb_arg have set when register and no use anymore, delete it. Sorry I don't understand. Please rephrase. > >> --- a/lib/librte_eal/common/include/rte_bus.h > >> +++ b/lib/librte_eal/common/include/rte_bus.h > >> /** > >> + * Device iterator to find a device on a bus. > >> + * > >> + * This function returns an rte_device if one of those held by the bus > >> + * matches the data passed as parameter. > >> + * > >> + * If the comparison function returns zero this function should stop iterating > >> + * over any more devices. To continue a search the device of a previous search > >> + * can be passed via the start parameter. > >> + * > >> + * @param cmp > >> + * the device name comparison function. > >> + * > >> + * @param data > >> + * Data to compare each device against. > >> + * > >> + * @param start > >> + * starting point for the iteration > >> + * > >> + * @return > >> + * The first device matching the data, NULL if none exists. > >> + */ > >> +typedef struct rte_device * > >> +(*rte_bus_find_device_by_name_t)(const struct rte_device *start, > >> + rte_dev_cmp_name_t cmp, > >> + const void *data); > > > > Why is it needed? There is already rte_bus_find_device_t. > > because the rte_bus_find_device_t just find a device structure in the > device list, but here need to find a device structure by device name > which come from uevent info. I don't understand how it is different? Looking at the code, it is a copy/paste except it is dedicated to name comparison. You can remove rte_bus_find_device_by_name_t and provide a comparison function which looks at name. > >> +int > >> +rte_eal_dev_monitor_enable(void); > > > > I suggest to drop this function which is just calling rte_dev_monitor_start. > > more discuss, i suggest keep on it , let rte_dev_monitor_start > separately stay on the platform code and let user commonly call > rte_eal_dev_monitor_enable. Then you may need a disable function. It will end up to be like start/stop. I think it is just redundant. If kept, please rename it to rte_dev_event_monitor_enable. > >> + /* create the host thread to wait/handle the uevent from kernel */ > >> + ret = pthread_create(&uev_monitor_thread, NULL, > >> + dev_uev_monitoring, NULL); > >> + return ret; > >> +} > > > > I think you should use rte_service for thread management. > > thanks for your info, such a good mechanism to use that i even not know > that before. i will study and use it. OK, good.