From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by dpdk.org (Postfix) with ESMTP id E0497312 for ; Fri, 9 May 2014 15:16:07 +0200 (CEST) Received: by mail-wi0-f181.google.com with SMTP id n15so1296266wiw.14 for ; Fri, 09 May 2014 06:16:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=OQ8/wnHeWfHpwJnsxd7dsQHmsq5iWZnIEUEPioBbc+M=; b=gietCP/Ozma21W41IbN+svJQjBtde+9rn2t+DgSgRnjvq6twnoT+tH2ZZidOQya6A+ BEqR2L/g7RDNuURsYTTFoXo0fjLkfvE2FYhpKm0PEG4qsJVQPrCybKh3ZDGbBMOD5FHR jad+oxrNYJezj0kWjf7kVG+lr7rWyo6et96xZSNKYaUdKQsp9i+SK2qWLtI9HSmxPrAA 9iAIg/v5OktnbyEJB6dm13QotBzbh/Yhg2fYFolzwIsQ7vlcISmEtNB6oU3xyFrry665 GV3cnqyY3PpyNdHVjRWgKp6uysQWdpnHVDxYmVXN6tt/qQF6/LRwEtc4u8yZmrTLYTGk mEzQ== X-Gm-Message-State: ALoCoQmWdBHLsvOz4KBwUJkqOuIFOQM17TcVKU7RORbtLqOwcYRyQXsKYpWBT68RlyCvDkxOUEx0 X-Received: by 10.180.218.35 with SMTP id pd3mr3354143wic.26.1399641374111; Fri, 09 May 2014 06:16:14 -0700 (PDT) Received: from alcyon.dev.6wind.com (6wind.net2.nerim.net. [213.41.180.237]) by mx.google.com with ESMTPSA id go1sm5086560wib.7.2014.05.09.06.16.12 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 May 2014 06:16:13 -0700 (PDT) From: David Marchand To: dev@dpdk.org Date: Fri, 9 May 2014 15:15:56 +0200 Message-Id: <1399641359-11267-5-git-send-email-david.marchand@6wind.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1399641359-11267-1-git-send-email-david.marchand@6wind.com> References: <1399641359-11267-1-git-send-email-david.marchand@6wind.com> Subject: [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak 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: Fri, 09 May 2014 13:16:08 -0000 A fd leak happens in pci_map_resource when multiple bars are mapped. Fix this by closing fd unconditionnally in this function and open the intr_handle fd in pci_uio_map_resource instead. Signed-off-by: David Marchand --- lib/librte_eal/bsdapp/eal/eal_pci.c | 60 +++++++++++++--------------- lib/librte_eal/linuxapp/eal/eal_pci.c | 71 ++++++++++++++++----------------- 2 files changed, 62 insertions(+), 69 deletions(-) diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c index a8945e4..94ae461 100644 --- a/lib/librte_eal/bsdapp/eal/eal_pci.c +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c @@ -119,8 +119,8 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev) /* map a particular resource from a file */ static void * -pci_map_resource(struct rte_pci_device *dev, void *requested_addr, - const char *devname, off_t offset, size_t size) +pci_map_resource(void *requested_addr, const char *devname, off_t offset, + size_t size) { int fd; void *mapaddr; @@ -130,7 +130,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr, */ fd = open(devname, O_RDWR); if (fd < 0) { - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", devname, strerror(errno)); goto fail; } @@ -138,35 +138,21 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr, /* Map the PCI memory resource of device */ mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset); + close(fd); if (mapaddr == MAP_FAILED || (requested_addr != NULL && mapaddr != requested_addr)) { RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):" - " %s (%p)\n", __func__, devname, fd, requested_addr, + " %s (%p)\n", __func__, devname, fd, requested_addr, (unsigned long)size, (unsigned long)offset, strerror(errno), mapaddr); - close(fd); goto fail; } - if (rte_eal_process_type() == RTE_PROC_PRIMARY) { - /* save fd if in primary process */ - dev->intr_handle.fd = fd; - dev->intr_handle.type = RTE_INTR_HANDLE_UIO; - } else { - /* fd is not needed in slave process, close it */ - dev->intr_handle.fd = -1; - dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - close(fd); - } - RTE_LOG(DEBUG, EAL, " PCI memory mapped at %p\n", mapaddr); return mapaddr; fail: - dev->intr_handle.fd = -1; - dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - return NULL; } @@ -179,19 +165,19 @@ pci_uio_map_secondary(struct rte_pci_device *dev) { size_t i; struct uio_resource *uio_res; - + TAILQ_FOREACH(uio_res, uio_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))) continue; - + for (i = 0; i != uio_res->nb_maps; i++) { - if (pci_map_resource(dev, uio_res->maps[i].addr, - uio_res->path, - (off_t)uio_res->maps[i].offset, - (size_t)uio_res->maps[i].size) != - uio_res->maps[i].addr) { + if (pci_map_resource(uio_res->maps[i].addr, + uio_res->path, + (off_t)uio_res->maps[i].offset, + (size_t)uio_res->maps[i].size) + != uio_res->maps[i].addr) { RTE_LOG(ERR, EAL, "Cannot mmap device resource\n"); return (-1); @@ -219,6 +205,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) struct uio_map *maps; dev->intr_handle.fd = -1; + dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; /* secondary processes - use already recorded details */ if (rte_eal_process_type() != RTE_PROC_PRIMARY) @@ -233,6 +220,15 @@ pci_uio_map_resource(struct rte_pci_device *dev) return -1; } + /* save fd if in primary process */ + dev->intr_handle.fd = open(devname, O_RDWR); + if (dev->intr_handle.fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", + devname, strerror(errno)); + return -1; + } + dev->intr_handle.type = RTE_INTR_HANDLE_UIO; + /* allocate the mapping details for secondary processes*/ if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) { RTE_LOG(ERR, EAL, @@ -246,7 +242,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) /* Map all BARs */ pagesz = sysconf(_SC_PAGESIZE); - + maps = uio_res->maps; for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) { @@ -254,16 +250,16 @@ pci_uio_map_resource(struct rte_pci_device *dev) /* skip empty BAR */ if ((phaddr = dev->mem_resource[i].phys_addr) == 0) continue; - + /* if matching map is found, then use it */ offset = i * pagesz; maps[j].offset = offset; maps[j].phaddr = dev->mem_resource[i].phys_addr; maps[j].size = dev->mem_resource[i].len; if (maps[j].addr != NULL || - (mapaddr = pci_map_resource(dev, - NULL, devname, (off_t)offset, - (size_t)maps[j].size)) == NULL) { + (mapaddr = pci_map_resource(NULL, devname, (off_t)offset, + (size_t)maps[j].size) + ) == NULL) { rte_free(uio_res); return (-1); } diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c index c006cf5..451fbd2 100644 --- a/lib/librte_eal/linuxapp/eal/eal_pci.c +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c @@ -305,8 +305,8 @@ pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev, /* map a particular resource from a file */ static void * -pci_map_resource(struct rte_pci_device *dev, void *requested_addr, - const char *devname, off_t offset, size_t size) +pci_map_resource(void *requested_addr, const char *devname, off_t offset, + size_t size) { int fd; void *mapaddr; @@ -317,8 +317,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr, * appear, so we wait some time before returning an error */ unsigned n; - fd = dev->intr_handle.fd; - for (n = 0; n < UIO_DEV_WAIT_TIMEOUT*10 && fd < 0; n++) { + for (n = 0; n < UIO_DEV_WAIT_TIMEOUT*10; n++) { errno = 0; if ((fd = open(devname, O_RDWR)) < 0 && errno != ENOENT) break; @@ -331,7 +330,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr, fd = open(devname, O_RDWR); #endif if (fd < 0) { - RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", devname, strerror(errno)); goto fail; } @@ -339,34 +338,21 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr, /* Map the PCI memory resource of device */ mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset); + close(fd); if (mapaddr == MAP_FAILED || (requested_addr != NULL && mapaddr != requested_addr)) { RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):" - " %s (%p)\n", __func__, devname, fd, requested_addr, + " %s (%p)\n", __func__, devname, fd, requested_addr, (unsigned long)size, (unsigned long)offset, strerror(errno), mapaddr); - close(fd); goto fail; } - if (rte_eal_process_type() == RTE_PROC_PRIMARY) { - /* save fd if in primary process */ - dev->intr_handle.fd = fd; - dev->intr_handle.type = RTE_INTR_HANDLE_UIO; - } else { - /* fd is not needed in slave process, close it */ - dev->intr_handle.fd = -1; - dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - close(fd); - } RTE_LOG(DEBUG, EAL, " PCI memory mapped at %p\n", mapaddr); return mapaddr; fail: - dev->intr_handle.fd = -1; - dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; - return NULL; } @@ -436,19 +422,19 @@ pci_uio_map_secondary(struct rte_pci_device *dev) { size_t i; struct uio_resource *uio_res; - + TAILQ_FOREACH(uio_res, uio_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))) continue; - + for (i = 0; i != uio_res->nb_maps; i++) { - if (pci_map_resource(dev, uio_res->maps[i].addr, - uio_res->path, - (off_t)uio_res->maps[i].offset, - (size_t)uio_res->maps[i].size) != - uio_res->maps[i].addr) { + if (pci_map_resource(uio_res->maps[i].addr, + uio_res->path, + (off_t)uio_res->maps[i].offset, + (size_t)uio_res->maps[i].size) + != uio_res->maps[i].addr) { RTE_LOG(ERR, EAL, "Cannot mmap device resource\n"); return (-1); @@ -596,6 +582,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) struct uio_map *maps; dev->intr_handle.fd = -1; + dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; /* secondary processes - use already recorded details */ if (rte_eal_process_type() != RTE_PROC_PRIMARY) @@ -608,6 +595,16 @@ pci_uio_map_resource(struct rte_pci_device *dev) "skipping\n", loc->domain, loc->bus, loc->devid, loc->function); return -1; } + rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num); + + /* save fd if in primary process */ + dev->intr_handle.fd = open(devname, O_RDWR); + if (dev->intr_handle.fd < 0) { + RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", + devname, strerror(errno)); + return -1; + } + dev->intr_handle.type = RTE_INTR_HANDLE_UIO; /* allocate the mapping details for secondary processes*/ if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) { @@ -616,7 +613,6 @@ pci_uio_map_resource(struct rte_pci_device *dev) return (-1); } - rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num); rte_snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname); memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr)); @@ -632,30 +628,31 @@ pci_uio_map_resource(struct rte_pci_device *dev) /* Map all BARs */ pagesz = sysconf(_SC_PAGESIZE); - + maps = uio_res->maps; for (i = 0; i != PCI_MAX_RESOURCE; i++) { - + /* skip empty BAR */ if ((phaddr = dev->mem_resource[i].phys_addr) == 0) continue; - + for (j = 0; j != nb_maps && (phaddr != maps[j].phaddr || dev->mem_resource[i].len != maps[j].size); j++) ; - + /* if matching map is found, then use it */ if (j != nb_maps) { offset = j * pagesz; if (maps[j].addr != NULL || - (mapaddr = pci_map_resource(dev, - NULL, devname, (off_t)offset, - (size_t)maps[j].size)) == NULL) { + (mapaddr = pci_map_resource(NULL, devname, + (off_t)offset, + (size_t)maps[j].size) + ) == NULL) { rte_free(uio_res); return (-1); } - + maps[j].addr = mapaddr; maps[j].offset = offset; dev->mem_resource[i].addr = mapaddr; -- 1.7.10.4