From mboxrd@z Thu Jan 1 00:00:00 1970 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) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Oct 2017 09:48:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,405,1503385200"; d="scan'208";a="911978434" Received: from tanjianf-mobl.ccr.corp.intel.com (HELO [10.255.30.27]) ([10.255.30.27]) by FMSMGA003.fm.intel.com with ESMTP; 20 Oct 2017 09:47:15 -0700 To: =?UTF-8?Q?Ga=c3=abtan_Rivet?= References: <1508411909-63954-1-git-send-email-jianfeng.tan@intel.com> <20171019114225.GF3596@bidouze.vm.6wind.com> Cc: dev@dpdk.org, santosh.shukla@caviumnetworks.com, jerin.jacob@caviumnetworks.com, anatoly.burakov@intel.com From: "Tan, Jianfeng" Message-ID: <38a126ec-7a6a-b43f-88b1-67e3b6477369@intel.com> Date: Sat, 21 Oct 2017 00:47:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20171019114225.GF3596@bidouze.vm.6wind.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] pci: fix check uio bind 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: Fri, 20 Oct 2017 16:49:00 -0000 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