DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal: fix tailq init for uio/vfio resources
@ 2015-03-11 17:25 David Marchand
  2015-03-12  7:36 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: David Marchand @ 2015-03-11 17:25 UTC (permalink / raw)
  To: dev

Commit a2348166ea18 ("tailq: move to dynamic tailq") introduced a bug in
uio/vfio resources list init.

These resources list were pointed at through a pointer initialised only once but
too early in the eal init (before tailqs init).

Fix this by "resolving" this pointer when used (which is well after tailqs
init).

Fixes: a2348166ea18 ("tailq: move to dynamic tailq")
Reported-by: Marvin Liu <yong.liu@intel.com>
Reported-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---

This patch is an alternative to the patch proposed by Marvin.
It has been tested only on Linux and with uio driver.

 lib/librte_eal/bsdapp/eal/eal_pci.c        |   11 +++++------
 lib/librte_eal/linuxapp/eal/eal_pci.c      |    8 --------
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |    1 -
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  |   17 +++++++++++++----
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   11 +++++++++--
 5 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 3a0fda5..fe3ef86 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -105,12 +105,10 @@ struct uio_resource {
 
 TAILQ_HEAD(uio_res_list, uio_resource);
 
-static struct uio_res_list *uio_res_list = NULL;
-
-static struct rte_tailq_elem rte_pci_tailq = {
-	.name = "PCI_RESOURCE_LIST",
+static struct rte_tailq_elem rte_uio_tailq = {
+	.name = "UIO_RESOURCE_LIST",
 };
-EAL_REGISTER_TAILQ(rte_pci_tailq)
+EAL_REGISTER_TAILQ(rte_uio_tailq)
 
 /* unbind kernel driver for this device */
 static int
@@ -165,6 +163,7 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 {
         size_t i;
         struct uio_resource *uio_res;
+	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
 
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
 
@@ -202,6 +201,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uint64_t pagesz;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct uio_resource *uio_res;
+	struct uio_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
 	struct uio_map *maps;
 
 	dev->intr_handle.fd = -1;
@@ -497,7 +497,6 @@ rte_eal_pci_init(void)
 {
 	TAILQ_INIT(&pci_driver_list);
 	TAILQ_INIT(&pci_device_list);
-	uio_res_list = RTE_TAILQ_CAST(rte_pci_tailq.head, uio_res_list);
 
 	/* for debug purposes, PCI can be disabled */
 	if (internal_config.no_pci)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index c42e843..83c589e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -56,13 +56,6 @@
  * IGB_UIO driver (or doesn't initialize, if the device wasn't bound to it).
  */
 
-struct mapped_pci_res_list *pci_res_list = NULL;
-
-static struct rte_tailq_elem rte_pci_tailq = {
-	.name = "PCI_RESOURCE_LIST",
-};
-EAL_REGISTER_TAILQ(rte_pci_tailq)
-
 /* unbind kernel driver for this device */
 static int
 pci_unbind_kernel_driver(struct rte_pci_device *dev)
@@ -770,7 +763,6 @@ rte_eal_pci_init(void)
 {
 	TAILQ_INIT(&pci_driver_list);
 	TAILQ_INIT(&pci_device_list);
-	pci_res_list = RTE_TAILQ_CAST(rte_pci_tailq.head, mapped_pci_res_list);
 
 	/* for debug purposes, PCI can be disabled */
 	if (internal_config.no_pci)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 6af84d1..aa7b755 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -58,7 +58,6 @@ struct mapped_pci_resource {
 };
 
 TAILQ_HEAD(mapped_pci_res_list, mapped_pci_resource);
-extern struct mapped_pci_res_list *pci_res_list;
 
 /*
  * Helper function to map PCI resources right after hugepages in virtual memory
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index ebbc2d3..2d1c69b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -41,6 +41,7 @@
 
 #include <rte_log.h>
 #include <rte_pci.h>
+#include <rte_eal_memconfig.h>
 #include <rte_common.h>
 #include <rte_malloc.h>
 
@@ -50,6 +51,10 @@
 
 void *pci_map_addr = NULL;
 
+static struct rte_tailq_elem rte_uio_tailq = {
+	.name = "UIO_RESOURCE_LIST",
+};
+EAL_REGISTER_TAILQ(rte_uio_tailq)
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
 
@@ -87,8 +92,9 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 {
 	int fd, i;
 	struct mapped_pci_resource *uio_res;
+	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
-	TAILQ_FOREACH(uio_res, pci_res_list, next) {
+	TAILQ_FOREACH(uio_res, uio_res_list, next) {
 
 		/* skip this element if it doesn't match our PCI address */
 		if (rte_eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr))
@@ -266,6 +272,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	uint64_t phaddr;
 	struct rte_pci_addr *loc = &dev->addr;
 	struct mapped_pci_resource *uio_res;
+	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 	struct pci_map *maps;
 
 	dev->intr_handle.fd = -1;
@@ -382,7 +389,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 
 	uio_res->nb_maps = map_idx;
 
-	TAILQ_INSERT_TAIL(pci_res_list, uio_res, next);
+	TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
 
 	return 0;
 }
@@ -405,11 +412,12 @@ static struct mapped_pci_resource *
 pci_uio_find_resource(struct rte_pci_device *dev)
 {
 	struct mapped_pci_resource *uio_res;
+	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
 	if (dev == NULL)
 		return NULL;
 
-	TAILQ_FOREACH(uio_res, pci_res_list, next) {
+	TAILQ_FOREACH(uio_res, uio_res_list, next) {
 
 		/* skip this element if it doesn't match our PCI address */
 		if (!rte_eal_compare_pci_addr(&uio_res->pci_addr, &dev->addr))
@@ -423,6 +431,7 @@ void
 pci_uio_unmap_resource(struct rte_pci_device *dev)
 {
 	struct mapped_pci_resource *uio_res;
+	struct mapped_pci_res_list *uio_res_list = RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
 
 	if (dev == NULL)
 		return;
@@ -436,7 +445,7 @@ pci_uio_unmap_resource(struct rte_pci_device *dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return pci_uio_unmap(uio_res);
 
-	TAILQ_REMOVE(pci_res_list, uio_res, next);
+	TAILQ_REMOVE(uio_res_list, uio_res, next);
 
 	/* unmap all resources */
 	pci_uio_unmap(uio_res);
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 9b0c151..aea1fb1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -64,6 +64,11 @@
 #define PAGE_SIZE   (sysconf(_SC_PAGESIZE))
 #define PAGE_MASK   (~(PAGE_SIZE - 1))
 
+static struct rte_tailq_elem rte_vfio_tailq = {
+	.name = "VFIO_RESOURCE_LIST",
+};
+EAL_REGISTER_TAILQ(rte_vfio_tailq)
+
 #define VFIO_DIR "/dev/vfio"
 #define VFIO_CONTAINER_PATH "/dev/vfio/vfio"
 #define VFIO_GROUP_FMT "/dev/vfio/%u"
@@ -546,6 +551,8 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	struct rte_pci_addr *loc = &dev->addr;
 	int i, ret, msix_bar;
 	struct mapped_pci_resource *vfio_res = NULL;
+	struct mapped_pci_res_list *vfio_res_list = RTE_TAILQ_CAST(rte_vfio_tailq.head, mapped_pci_res_list);
+
 	struct pci_map *maps;
 	uint32_t msix_table_offset = 0;
 	uint32_t msix_table_size = 0;
@@ -700,7 +707,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 				VFIO_PCI_BAR5_REGION_INDEX + 1);
 	} else {
 		/* if we're in a secondary process, just find our tailq entry */
-		TAILQ_FOREACH(vfio_res, pci_res_list, next) {
+		TAILQ_FOREACH(vfio_res, vfio_res_list, next) {
 			if (memcmp(&vfio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
 				continue;
 			break;
@@ -854,7 +861,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	}
 
 	if (internal_config.process_type == RTE_PROC_PRIMARY)
-		TAILQ_INSERT_TAIL(pci_res_list, vfio_res, next);
+		TAILQ_INSERT_TAIL(vfio_res_list, vfio_res, next);
 
 	return 0;
 }
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [dpdk-dev] [PATCH] eal: fix tailq init for uio/vfio resources
  2015-03-11 17:25 [dpdk-dev] [PATCH] eal: fix tailq init for uio/vfio resources David Marchand
@ 2015-03-12  7:36 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2015-03-12  7:36 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

2015-03-11 18:25, David Marchand:
> Commit a2348166ea18 ("tailq: move to dynamic tailq") introduced a bug in
> uio/vfio resources list init.
> 
> These resources list were pointed at through a pointer initialised only once but
> too early in the eal init (before tailqs init).
> 
> Fix this by "resolving" this pointer when used (which is well after tailqs
> init).
> 
> Fixes: a2348166ea18 ("tailq: move to dynamic tailq")
> Reported-by: Marvin Liu <yong.liu@intel.com>
> Reported-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
> 
> This patch is an alternative to the patch proposed by Marvin.
> It has been tested only on Linux and with uio driver.

Applied, thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-03-12  7:37 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-11 17:25 [dpdk-dev] [PATCH] eal: fix tailq init for uio/vfio resources David Marchand
2015-03-12  7:36 ` Thomas Monjalon

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).