From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Tan, Jianfeng" Subject: Re: [PATCH] pci: fix check uio bind Date: Sat, 21 Oct 2017 00:47:14 +0800 Message-ID: <38a126ec-7a6a-b43f-88b1-67e3b6477369@intel.com> References: <1508411909-63954-1-git-send-email-jianfeng.tan@intel.com> <20171019114225.GF3596@bidouze.vm.6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, santosh.shukla@caviumnetworks.com, jerin.jacob@caviumnetworks.com, anatoly.burakov@intel.com To: =?UTF-8?Q?Ga=c3=abtan_Rivet?= Return-path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 911661B26C for ; Fri, 20 Oct 2017 18:48:59 +0200 (CEST) In-Reply-To: <20171019114225.GF3596@bidouze.vm.6wind.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" Hi Gaëtan, On 10/19/2017 7:42 PM, Gaëtan Rivet wrote: > Hi Jianfeng, > > On Thu, Oct 19, 2017 at 11:18:29AM +0000, Jianfeng Tan wrote: >> When checking if any devices bound to uio, we did not exclud >> those which are blacklisted (or in the case that a whitelist >> is specified). >> >> This patch fixes it by only checking whitelisted devices. >> >> Fixes: 815c7deaed2d ("pci: get IOMMU class on Linux") >> >> Signed-off-by: Jianfeng Tan >> --- >> lib/librte_eal/linuxapp/eal/eal_pci.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c >> index b4dbf95..2b23d67 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c >> @@ -516,8 +516,26 @@ static inline int >> pci_one_device_bound_uio(void) >> { >> struct rte_pci_device *dev = NULL; >> + struct rte_devargs *devargs; >> + int check_all = 1; >> + int need_check; >> + >> + if (rte_pci_bus.bus.conf.scan_mode == RTE_BUS_SCAN_WHITELIST) >> + check_all = 0; >> >> FOREACH_DEVICE_ON_PCIBUS(dev) { >> + devargs = dev->device.devargs; >> + >> + need_check = 0; >> + if (check_all) > Unless I'm mistaken, you will check blacklisted devices as well here. Thank you for pointing out this. I was referring to rte_pci_probe(), which also only check "probe_all" and (devargs && RTE_DEV_WHITELISTED); but turns out it double checks the blacklisted devices in rte_pci_probe_one_driver(). I'll fix it. > The condition should be something like: > > if (check_all && devargs == NULL) > Which means that both ifs can be refactored as > > if ((check_all ^ (devargs != NULL)) == 0) > continue; > > Removing need_check. But it can be hard to read. Yes, I prefer to make it easy to understand. Please let me know if you are OK with below code (remove check_all): FOREACH_DEVICE_ON_PCIBUS(dev) { devargs = dev->device.devargs; need_check = 0; switch (rte_pci_bus.bus.conf.scan_mode) { case RTE_BUS_SCAN_UNDEFINED: need_check = 1; break; case RTE_BUS_SCAN_WHITELIST: if (devargs && devargs->policy == RTE_DEV_WHITELISTED) need_check = 1; break; case RTE_BUS_SCAN_BLACKLIST: if (!devargs || devargs->policy != RTE_DEV_BLACKLISTED) need_check = 1; break; } if (!need_check) continue; ... Thanks, Jianfeng