DPDK patches and discussions
 help / color / mirror / Atom feed
From: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
To: dev@dpdk.org
Cc: thomas@monjalon.net, Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Subject: [dpdk-dev] [PATCH 2/3] devargs: delay freeing previous devargs when overriding them
Date: Mon,  5 Nov 2018 08:04:46 +0100	[thread overview]
Message-ID: <20181105070447.67700-2-dariusz.stojaczyk@intel.com> (raw)
In-Reply-To: <20181105070447.67700-1-dariusz.stojaczyk@intel.com>

In eal hotplug path, the previous devargs may be still
referenced by device structs at the time the rte_devargs_insert()
is called. Those references are updated almost immediately
afterwards, but in cases something goes wrong and they cannot
be updated, we might want to still keep the old devargs around.

This patch modifies rte_devargs_insert() so that it returns
a pointer to previous devargs that are being overridden. In
case something in the EAL hotplug path goes wrong, we can now
remove the newly inserted devargs and re-insert the old ones.

Note: Functional changes will come in a subsequent patch. This
      one only extends the API.

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 drivers/bus/vdev/vdev.c                     | 12 +++++++++---
 lib/librte_eal/common/eal_common_dev.c      | 11 +++++++----
 lib/librte_eal/common/eal_common_devargs.c  | 21 ++++++++++++++-------
 lib/librte_eal/common/include/rte_devargs.h |  8 ++------
 4 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 9c66bdc78..bbdae2314 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -9,6 +9,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <sys/queue.h>
+#include <assert.h>
 
 #include <rte_eal.h>
 #include <rte_dev.h>
@@ -207,7 +208,7 @@ insert_vdev(const char *name, const char *args,
 		bool init)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
+	struct rte_devargs *devargs, *prev_devargs;
 	int ret;
 
 	if (name == NULL)
@@ -239,8 +240,13 @@ insert_vdev(const char *name, const char *args,
 	}
 
 	TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
-	if (init)
-		rte_devargs_insert(devargs);
+	if (init) {
+		rte_devargs_insert(devargs, &prev_devargs);
+		 /* any previous devargs should have been caught by the above
+		  * find_vdev(name) check
+		  */
+		assert(prev_devargs == NULL);
+	}
 
 	if (p_dev)
 		*p_dev = dev;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 62e9ed477..4cb424df1 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -131,7 +131,7 @@ int
 local_dev_probe(const char *devargs, struct rte_device **new_dev)
 {
 	struct rte_device *dev;
-	struct rte_devargs *da;
+	struct rte_devargs *da, *prev_da;
 	int ret;
 
 	*new_dev = NULL;
@@ -150,9 +150,12 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 		goto err_devarg;
 	}
 
-	ret = rte_devargs_insert(da);
-	if (ret)
-		goto err_devarg;
+	rte_devargs_insert(da, &prev_da);
+
+	if (prev_da != NULL) {
+		free(prev_da->args);
+		free(prev_da);
+	}
 
 	ret = da->bus->scan();
 	if (ret)
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index b7b9cb69e..c46365b69 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -262,16 +262,23 @@ rte_devargs_parsef(struct rte_devargs *da, const char *format, ...)
 	return ret;
 }
 
-int __rte_experimental
-rte_devargs_insert(struct rte_devargs *da)
+void __rte_experimental
+rte_devargs_insert(struct rte_devargs *da, struct rte_devargs **prev_da)
 {
-	int ret;
+	struct rte_devargs *d;
+	void *tmp;
+
+	*prev_da = NULL;
+	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
+		if (strcmp(d->bus->name, da->bus->name) == 0 &&
+		    strcmp(d->name, da->name) == 0) {
+			TAILQ_REMOVE(&devargs_list, d, next);
+			*prev_da = d;
+			break;
+		}
+	}
 
-	ret = rte_devargs_remove(da);
-	if (ret < 0)
-		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
-	return 0;
 }
 
 /* store a whitelist parameter for later parsing */
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index b1f121f83..753cf7f54 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -146,14 +146,10 @@ __attribute__((format(printf, 2, 0)));
  *
  * @param da
  *  The devargs structure to insert.
- *
- * @return
- *   - 0 on success
- *   - Negative on error.
  */
 __rte_experimental
-int
-rte_devargs_insert(struct rte_devargs *da);
+void
+rte_devargs_insert(struct rte_devargs *da, struct rte_devargs **prev_da);
 
 /**
  * Add a device to the user device list
-- 
2.17.1

  reply	other threads:[~2018-11-05  7:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-05  7:04 [dpdk-dev] [PATCH 1/3] bus/pci: update device devargs on each rescan Darek Stojaczyk
2018-11-05  7:04 ` Darek Stojaczyk [this message]
2018-11-05  7:30   ` [dpdk-dev] [PATCH 2/3] devargs: delay freeing previous devargs when overriding them Thomas Monjalon
2018-11-05  8:25     ` Stojaczyk, Dariusz
2018-11-05  9:46       ` Thomas Monjalon
2018-11-05 16:24         ` Gaëtan Rivet
2018-11-05  7:04 ` [dpdk-dev] [PATCH 3/3] eal: handle bus rescan failures during hotplug Darek Stojaczyk
2018-11-05 14:10 ` [dpdk-dev] [PATCH 1/3] bus/pci: update device devargs on each rescan Gaëtan Rivet
2018-11-05 14:52   ` Stojaczyk, Dariusz
2018-11-05 16:27     ` Gaëtan Rivet
2018-11-06  5:40 ` [dpdk-dev] [PATCH v2] " Dariusz Stojaczyk
2018-11-06 22:21   ` Zhang, Qi Z
2018-11-06 23:40     ` Gaëtan Rivet
2018-11-12  0:47       ` 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=20181105070447.67700-2-dariusz.stojaczyk@intel.com \
    --to=dariusz.stojaczyk@intel.com \
    --cc=dev@dpdk.org \
    --cc=thomas@monjalon.net \
    /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).