From: David Marchand <david.marchand@6wind.com>
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak
Date: Fri, 9 May 2014 15:15:56 +0200 [thread overview]
Message-ID: <1399641359-11267-5-git-send-email-david.marchand@6wind.com> (raw)
In-Reply-To: <1399641359-11267-1-git-send-email-david.marchand@6wind.com>
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 <david.marchand@6wind.com>
---
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
next prev parent reply other threads:[~2014-05-09 13:16 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 2/7] pci: align bsd implementation on linux David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 3/7] pci: remove virtio-uio workaround David Marchand
2014-05-09 13:15 ` David Marchand [this message]
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 5/7] pci: pci_switch_module cleanup David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 6/7] pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 7/7] pci: remove deprecated RTE_EAL_UNBIND_PORTS option David Marchand
2014-05-13 14:29 ` [dpdk-dev] [PATCH v2 0/7] pci cleanup Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1399641359-11267-5-git-send-email-david.marchand@6wind.com \
--to=david.marchand@6wind.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).