From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f45.google.com (mail-pa0-f45.google.com [209.85.220.45]) by dpdk.org (Postfix) with ESMTP id BE09A68BE for ; Thu, 11 Dec 2014 04:11:14 +0100 (CET) Received: by mail-pa0-f45.google.com with SMTP id lf10so3506328pab.18 for ; Wed, 10 Dec 2014 19:11:14 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=esn3S1j9bLgQSO6/ISsuBU9wmXnGt/LhcNALzuTDTzI=; b=F58kYwiZ3sDpaPD5uVEjpBMsWiDELVMbqR4idIsXY5PknlVgQ6biW45fExgoN+1KT/ ccCurJ+sQ5T0wRJ3SdyL5UasnlnpZUEEm3SFFfVj6NgwKpJubuxgmQRYNH0lWfazg8vv eYdf2SIwajxt8SdBXu0U+3FD1G1yyC+oc5NQxpkJOOONyF/b7V9CSB7asj4KclTUlTSJ iV2rjspm9cDi70Ok+cyXA9R0is2Gbz/wnH1pzN8bB8hSx5xChPJn7X2y6AeeXDWoR05c m98IGPXpxHgO9ikV9hn4JnJNl0bGPhhn2pMm4dVuNXcP+rOerrUTvlLo/0hYnLLcJLB4 DmDg== X-Gm-Message-State: ALoCoQnBFOYl0GLjmKOE2IQIrV6eqJZpMuxS7tgYz9Fp7eytie8ahKOjhyxUC4qKnGsL3xR44FCF X-Received: by 10.68.216.167 with SMTP id or7mr12638180pbc.166.1418267474030; Wed, 10 Dec 2014 19:11:14 -0800 (PST) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id ye3sm5375090pbb.93.2014.12.10.19.11.11 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Wed, 10 Dec 2014 19:11:13 -0800 (PST) Message-ID: <54890B4F.5000306@igel.co.jp> Date: Thu, 11 Dec 2014 12:11:11 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: "Qiu, Michael" , "dev@dpdk.org" References: <1416474399-16851-1-git-send-email-mukawa@igel.co.jp> <1418106629-22227-1-git-send-email-mukawa@igel.co.jp> <1418106629-22227-4-git-send-email-mukawa@igel.co.jp> <533710CFB86FA344BFBF2D6802E60286C9E1CB@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <533710CFB86FA344BFBF2D6802E60286C9E1CB@SHSMSX101.ccr.corp.intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: "nakajima.yoshihiro@lab.ntt.co.jp" , "masutani.hitoshi@lab.ntt.co.jp" , "menrigh@brocade.com" Subject: Re: [dpdk-dev] [PATCH v3 03/28] eal/pci: Replace pci address comparison code by eal_compare_pci_addr X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Dec 2014 03:11:15 -0000 Hi Michael, (2014/12/09 23:22), Qiu, Michael wrote: > On 2014/12/9 14:32, Tetsuya Mukawa wrote: >> This patch replaces pci_addr_comparison() and memcmp() of pci addresses by >> eal_compare_pci_addr(). >> >> Signed-off-by: Tetsuya Mukawa >> --- >> lib/librte_eal/bsdapp/eal/eal_pci.c | 16 +--------------- >> lib/librte_eal/common/eal_common_pci.c | 2 +- >> lib/librte_eal/common/include/rte_pci.h | 29 +++++++++++++++++++++++++++++ >> lib/librte_eal/linuxapp/eal/eal_pci.c | 16 +--------------- >> lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 2 +- >> 5 files changed, 33 insertions(+), 32 deletions(-) >> >> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c >> index 74ecce7..7eda513 100644 >> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c >> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c >> @@ -270,20 +270,6 @@ pci_uio_map_resource(struct rte_pci_device *dev) >> return (0); >> } >> >> -/* Compare two PCI device addresses. */ >> -static int >> -pci_addr_comparison(struct rte_pci_addr *addr, struct rte_pci_addr *addr2) >> -{ >> - uint64_t dev_addr = (addr->domain << 24) + (addr->bus << 16) + (addr->devid << 8) + addr->function; >> - uint64_t dev_addr2 = (addr2->domain << 24) + (addr2->bus << 16) + (addr2->devid << 8) + addr2->function; >> - >> - if (dev_addr > dev_addr2) >> - return 1; >> - else >> - return 0; >> -} >> - >> - >> /* Scan one pci sysfs entry, and fill the devices list from it. */ >> static int >> pci_scan_one(int dev_pci_fd, struct pci_conf *conf) >> @@ -358,7 +344,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf) >> struct rte_pci_device *dev2 = NULL; >> >> TAILQ_FOREACH(dev2, &pci_device_list, next) { >> - if (pci_addr_comparison(&dev->addr, &dev2->addr)) >> + if (eal_compare_pci_addr(&dev->addr, &dev2->addr)) >> continue; >> else { >> TAILQ_INSERT_BEFORE(dev2, dev, next); >> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c >> index f3c7f71..f01f258 100644 >> --- a/lib/librte_eal/common/eal_common_pci.c >> +++ b/lib/librte_eal/common/eal_common_pci.c >> @@ -93,7 +93,7 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) >> if (devargs->type != RTE_DEVTYPE_BLACKLISTED_PCI && >> devargs->type != RTE_DEVTYPE_WHITELISTED_PCI) >> continue; >> - if (!memcmp(&dev->addr, &devargs->pci.addr, sizeof(dev->addr))) >> + if (eal_compare_pci_addr(&dev->addr, &devargs->pci.addr)) >> return devargs; >> } >> return NULL; >> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h >> index b819539..fe374a8 100644 >> --- a/lib/librte_eal/common/include/rte_pci.h >> +++ b/lib/librte_eal/common/include/rte_pci.h >> @@ -261,6 +261,35 @@ eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr) >> } >> #undef GET_PCIADDR_FIELD >> >> +/* Compare two PCI device addresses. */ >> +/** >> + * Utility function to compare two PCI device addresses. >> + * >> + * @param addr >> + * The PCI Bus-Device-Function address to compare >> + * @param addr2 >> + * The PCI Bus-Device-Function address to compare >> + * @return >> + * 0 on equal PCI address. >> + * Positive on addr is greater than addr2. >> + * Negative on addr is less than addr2. >> + */ >> +static inline int >> +eal_compare_pci_addr(struct rte_pci_addr *addr, struct rte_pci_addr *addr2) >> +{ >> + uint64_t dev_addr = (addr->domain << 24) + (addr->bus << 16) + >> + (addr->devid << 8) + addr->function; >> + uint64_t dev_addr2 = (addr2->domain << 24) + (addr2->bus << 16) + >> + (addr2->devid << 8) + addr2->function; >> + > Here I prefer: > > uint64_t dev_addr = (addr->domain << 24) | (addr->bus << 16) | (addr->devid << 8) | addr->function; Sure, I will change like above. Thanks, Tetsuya > > Thanks, > Michael >> + if (dev_addr > dev_addr2) >> + return 1; >> + else if (dev_addr < dev_addr2) >> + return -1; >> + else >> + return 0; >> +} >> + >> /** >> * Probe the PCI bus for registered drivers. >> * >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c >> index b5f5410..23a69e9 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c >> @@ -200,20 +200,6 @@ error: >> return -1; >> } >> >> -/* Compare two PCI device addresses. */ >> -static int >> -pci_addr_comparison(struct rte_pci_addr *addr, struct rte_pci_addr *addr2) >> -{ >> - uint64_t dev_addr = (addr->domain << 24) + (addr->bus << 16) + (addr->devid << 8) + addr->function; >> - uint64_t dev_addr2 = (addr2->domain << 24) + (addr2->bus << 16) + (addr2->devid << 8) + addr2->function; >> - >> - if (dev_addr > dev_addr2) >> - return 1; >> - else >> - return 0; >> -} >> - >> - >> /* Scan one pci sysfs entry, and fill the devices list from it. */ >> static int >> pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus, >> @@ -306,7 +292,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus, >> struct rte_pci_device *dev2 = NULL; >> >> TAILQ_FOREACH(dev2, &pci_device_list, next) { >> - if (pci_addr_comparison(&dev->addr, &dev2->addr)) >> + if (eal_compare_pci_addr(&dev->addr, &dev2->addr) != 0) >> continue; >> else { >> TAILQ_INSERT_BEFORE(dev2, dev, next); >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> index e53f06b..1da3507 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c >> @@ -123,7 +123,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev) >> TAILQ_FOREACH(uio_res, pci_res_list, next) { >> >> /* skip this element if it doesn't match our PCI address */ >> - if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr))) >> + if (eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr)) >> continue; >> >> for (i = 0; i != uio_res->nb_maps; i++) {