DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
@ 2020-07-08 12:03 Muhammad Bilal
  2020-07-08 14:11 ` Muhammad Bilal
  2020-09-12 19:53 ` Gaëtan Rivet
  0 siblings, 2 replies; 6+ messages in thread
From: Muhammad Bilal @ 2020-07-08 12:03 UTC (permalink / raw)
  To: ferruh.yigit, anatoly.burakov
  Cc: vipin.varghese, jgrajcia, jerinj, dev, stable, Muhammad Bilal

while using memif with app, the resources are not cleaned on exit,
So an error occurred on running it second time. The cause of this problem
is that remove() of memif driver is not called by rte_eal_cleanup() which
is counterpart of probe() called from rte_eal_init(). This is a case for
all other divers e.g pci, so to solve this problem I have added the
functionality of calling remove() function of all the driver attached to
devices on vdev and pci buses.

Bugzilla ID: 353
Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
---
 drivers/bus/pci/pci_common.c           | 23 +++++++++++++++++++++++
 drivers/bus/vdev/vdev.c                | 19 ++++++++++++++++++-
 lib/librte_eal/common/eal_common_bus.c | 18 ++++++++++++++++++
 lib/librte_eal/include/rte_bus.h       | 23 +++++++++++++++++++++++
 lib/librte_eal/linux/eal.c             |  2 ++
 5 files changed, 84 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 245d94f59..203b32a23 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -320,6 +320,28 @@ pci_probe(void)
 	return (probed && probed == failed) ? -1 : 0;
 }
 
+/*
+ * Scan the content of the PCI bus, and call the remove() function for
+ * all registered drivers of devices that have already been probed.
+ */
+static int
+pci_remove(void)
+{
+	struct rte_pci_device *dev = NULL;
+	int ret = 0;
+
+	FOREACH_DEVICE_ON_PCIBUS(dev) {
+		if (rte_dev_is_probed(&dev->device))
+			if (rte_pci_detach_dev(dev) != 0) {
+				RTE_LOG(INFO, EAL,
+					"failed to detach driver form %s\n",
+					dev->device.name);
+				ret = -1;
+			}
+	}
+	return ret;
+}
+
 /* dump one device */
 static int
 pci_dump_one_device(FILE *f, struct rte_pci_device *dev)
@@ -669,6 +691,7 @@ struct rte_pci_bus rte_pci_bus = {
 	.bus = {
 		.scan = rte_pci_scan,
 		.probe = pci_probe,
+		.remove = pci_remove,
 		.find_device = pci_find_device,
 		.plug = pci_plug,
 		.unplug = pci_unplug,
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index a89ea2353..692c936b9 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -546,6 +546,22 @@ vdev_unplug(struct rte_device *dev)
 	return rte_vdev_uninit(dev->name);
 }
 
+static int
+vdev_remove(void)
+{
+	struct rte_vdev_device *dev;
+	int ret = 0;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		if (vdev_remove_driver(dev) != 0) {
+			VDEV_LOG(INFO, "driver of %s is not removed\n",
+				dev->device.name);
+			ret = -1;
+		}
+	}
+	return ret;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
@@ -554,7 +570,8 @@ static struct rte_bus rte_vdev_bus = {
 	.unplug = vdev_unplug,
 	.parse = vdev_parse,
 	.dev_iterate = rte_vdev_dev_iterate,
-};
+	.remove = vdev_remove,
+	};
 
 RTE_REGISTER_BUS(vdev, rte_vdev_bus);
 
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index baa5b532a..bff95a0ae 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -85,6 +85,24 @@ rte_bus_probe(void)
 	return 0;
 }
 
+/* Remove all devices of all buses */
+int
+rte_bus_remove(void)
+{
+	int ret;
+	struct rte_bus *bus;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!strcmp(bus->name, "vdev") || !strcmp(bus->name, "pci")) {
+			ret = bus->remove();
+			if (ret)
+				RTE_LOG(INFO, EAL, "Bus (%s) remove failed.\n",
+					bus->name);
+		}
+	}
+	return 0;
+}
+
 /* Dump information of a single bus */
 static int
 bus_dump_one(FILE *f, struct rte_bus *bus)
diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
index d3034d0ed..c3e7e62c9 100644
--- a/lib/librte_eal/include/rte_bus.h
+++ b/lib/librte_eal/include/rte_bus.h
@@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
  */
 typedef int (*rte_bus_probe_t)(void);
 
+/**
+ * Implementation specific remove function which is responsible for unlinking
+ * devices on that bus from attached drivers.
+ *
+ * This is called while iterating over each registered bus.
+ *
+ * @return
+ *	0 for successful remove
+ *	!0 for any error while removing
+ */
+typedef int (*rte_bus_remove_t)(void);
+
 /**
  * Device iterator to find a device on a bus.
  *
@@ -248,6 +260,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_remove_t remove;	 /**< remove device on bus */
 	rte_bus_find_device_t find_device; /**< Find a device on the bus */
 	rte_bus_plug_t plug;         /**< Probe single device for drivers */
 	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
@@ -301,6 +314,16 @@ int rte_bus_scan(void);
  */
 int rte_bus_probe(void);
 
+/**
+ * For each device on the buses,call the driver-specific remove
+ * for device uninitialization.
+ *
+ * @return
+ *	 0 for successful match/probe
+ *	!0 otherwise
+ */
+int rte_bus_remove(void);
+
 /**
  * Dump information of all the buses registered with EAL.
  *
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index f162124a3..89a03d3bb 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1330,6 +1330,8 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
 int
 rte_eal_cleanup(void)
 {
+	/* Remove devices/drivers from all buses */
+	rte_bus_remove();
 	/* if we're in a primary process, we need to mark hugepages as freeable
 	 * so that finalization can release them back to the system.
 	 */
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
  2020-07-08 12:03 [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses Muhammad Bilal
@ 2020-07-08 14:11 ` Muhammad Bilal
  2020-09-12 19:53 ` Gaëtan Rivet
  1 sibling, 0 replies; 6+ messages in thread
From: Muhammad Bilal @ 2020-07-08 14:11 UTC (permalink / raw)
  To: ferruh.yigit, anatoly.burakov
  Cc: vipin.varghese, jgrajcia, jerinj, dev, stable

On Wed, Jul 8, 2020 at 5:04 PM Muhammad Bilal <m.bilal@emumba.com> wrote:
>
> while using memif with app, the resources are not cleaned on exit,
> So an error occurred on running it second time. The cause of this problem
> is that remove() of memif driver is not called by rte_eal_cleanup() which
> is counterpart of probe() called from rte_eal_init(). This is a case for
> all other divers e.g pci, so to solve this problem I have added the
> functionality of calling remove() function of all the driver attached to
> devices on vdev and pci buses.
>
Little correction
Bugzilla ID: 437
> Bugzilla ID: 353
> Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
> ---
>  drivers/bus/pci/pci_common.c           | 23 +++++++++++++++++++++++
>  drivers/bus/vdev/vdev.c                | 19 ++++++++++++++++++-
>  lib/librte_eal/common/eal_common_bus.c | 18 ++++++++++++++++++
>  lib/librte_eal/include/rte_bus.h       | 23 +++++++++++++++++++++++
>  lib/librte_eal/linux/eal.c             |  2 ++
>  5 files changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 245d94f59..203b32a23 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -320,6 +320,28 @@ pci_probe(void)
>         return (probed && probed == failed) ? -1 : 0;
>  }
>
> +/*
> + * Scan the content of the PCI bus, and call the remove() function for
> + * all registered drivers of devices that have already been probed.
> + */
> +static int
> +pci_remove(void)
> +{
> +       struct rte_pci_device *dev = NULL;
> +       int ret = 0;
> +
> +       FOREACH_DEVICE_ON_PCIBUS(dev) {
> +               if (rte_dev_is_probed(&dev->device))
> +                       if (rte_pci_detach_dev(dev) != 0) {
> +                               RTE_LOG(INFO, EAL,
> +                                       "failed to detach driver form %s\n",
> +                                       dev->device.name);
> +                               ret = -1;
> +                       }
> +       }
> +       return ret;
> +}
> +
>  /* dump one device */
>  static int
>  pci_dump_one_device(FILE *f, struct rte_pci_device *dev)
> @@ -669,6 +691,7 @@ struct rte_pci_bus rte_pci_bus = {
>         .bus = {
>                 .scan = rte_pci_scan,
>                 .probe = pci_probe,
> +               .remove = pci_remove,
>                 .find_device = pci_find_device,
>                 .plug = pci_plug,
>                 .unplug = pci_unplug,
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index a89ea2353..692c936b9 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -546,6 +546,22 @@ vdev_unplug(struct rte_device *dev)
>         return rte_vdev_uninit(dev->name);
>  }
>
> +static int
> +vdev_remove(void)
> +{
> +       struct rte_vdev_device *dev;
> +       int ret = 0;
> +
> +       TAILQ_FOREACH(dev, &vdev_device_list, next) {
> +               if (vdev_remove_driver(dev) != 0) {
> +                       VDEV_LOG(INFO, "driver of %s is not removed\n",
> +                               dev->device.name);
> +                       ret = -1;
> +               }
> +       }
> +       return ret;
> +}
> +
>  static struct rte_bus rte_vdev_bus = {
>         .scan = vdev_scan,
>         .probe = vdev_probe,
> @@ -554,7 +570,8 @@ static struct rte_bus rte_vdev_bus = {
>         .unplug = vdev_unplug,
>         .parse = vdev_parse,
>         .dev_iterate = rte_vdev_dev_iterate,
> -};
> +       .remove = vdev_remove,
> +       };
>
>  RTE_REGISTER_BUS(vdev, rte_vdev_bus);
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..bff95a0ae 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -85,6 +85,24 @@ rte_bus_probe(void)
>         return 0;
>  }
>
> +/* Remove all devices of all buses */
> +int
> +rte_bus_remove(void)
> +{
> +       int ret;
> +       struct rte_bus *bus;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +               if (!strcmp(bus->name, "vdev") || !strcmp(bus->name, "pci")) {
> +                       ret = bus->remove();
> +                       if (ret)
> +                               RTE_LOG(INFO, EAL, "Bus (%s) remove failed.\n",
> +                                       bus->name);
> +               }
> +       }
> +       return 0;
> +}
> +
>  /* Dump information of a single bus */
>  static int
>  bus_dump_one(FILE *f, struct rte_bus *bus)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..c3e7e62c9 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>   */
>  typedef int (*rte_bus_probe_t)(void);
>
> +/**
> + * Implementation specific remove function which is responsible for unlinking
> + * devices on that bus from attached drivers.
> + *
> + * This is called while iterating over each registered bus.
> + *
> + * @return
> + *     0 for successful remove
> + *     !0 for any error while removing
> + */
> +typedef int (*rte_bus_remove_t)(void);
> +
>  /**
>   * Device iterator to find a device on a bus.
>   *
> @@ -248,6 +260,7 @@ struct rte_bus {
>         const char *name;            /**< Name of the bus */
>         rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
>         rte_bus_probe_t probe;       /**< Probe devices on bus */
> +       rte_bus_remove_t remove;         /**< remove device on bus */
>         rte_bus_find_device_t find_device; /**< Find a device on the bus */
>         rte_bus_plug_t plug;         /**< Probe single device for drivers */
>         rte_bus_unplug_t unplug;     /**< Remove single device from driver */
> @@ -301,6 +314,16 @@ int rte_bus_scan(void);
>   */
>  int rte_bus_probe(void);
>
> +/**
> + * For each device on the buses,call the driver-specific remove
> + * for device uninitialization.
> + *
> + * @return
> + *      0 for successful match/probe
> + *     !0 otherwise
> + */
> +int rte_bus_remove(void);
> +
>  /**
>   * Dump information of all the buses registered with EAL.
>   *
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index f162124a3..89a03d3bb 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1330,6 +1330,8 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
>  int
>  rte_eal_cleanup(void)
>  {
> +       /* Remove devices/drivers from all buses */
> +       rte_bus_remove();
>         /* if we're in a primary process, we need to mark hugepages as freeable
>          * so that finalization can release them back to the system.
>          */
> --
> 2.25.1
>

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

* Re: [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
  2020-07-08 12:03 [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses Muhammad Bilal
  2020-07-08 14:11 ` Muhammad Bilal
@ 2020-09-12 19:53 ` Gaëtan Rivet
  2020-10-20 13:43   ` [dpdk-dev] [dpdk-stable] " David Marchand
  1 sibling, 1 reply; 6+ messages in thread
From: Gaëtan Rivet @ 2020-09-12 19:53 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: ferruh.yigit, anatoly.burakov, vipin.varghese, jgrajcia, jerinj,
	dev, stable

On 08/07/20 17:03 +0500, Muhammad Bilal wrote:
> while using memif with app, the resources are not cleaned on exit,
> So an error occurred on running it second time. The cause of this problem
> is that remove() of memif driver is not called by rte_eal_cleanup() which
> is counterpart of probe() called from rte_eal_init(). This is a case for
> all other divers e.g pci, so to solve this problem I have added the
> functionality of calling remove() function of all the driver attached to
> devices on vdev and pci buses.
> 

Hi Muhammad,

review inline.

> Bugzilla ID: 353
> Signed-off-by: Muhammad Bilal <m.bilal@emumba.com>
> ---
>  drivers/bus/pci/pci_common.c           | 23 +++++++++++++++++++++++
>  drivers/bus/vdev/vdev.c                | 19 ++++++++++++++++++-
>  lib/librte_eal/common/eal_common_bus.c | 18 ++++++++++++++++++
>  lib/librte_eal/include/rte_bus.h       | 23 +++++++++++++++++++++++
>  lib/librte_eal/linux/eal.c             |  2 ++
>  5 files changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
> index 245d94f59..203b32a23 100644
> --- a/drivers/bus/pci/pci_common.c
> +++ b/drivers/bus/pci/pci_common.c
> @@ -320,6 +320,28 @@ pci_probe(void)
>  	return (probed && probed == failed) ? -1 : 0;
>  }
>  
> +/*
> + * Scan the content of the PCI bus, and call the remove() function for
> + * all registered drivers of devices that have already been probed.
> + */

You are not scanning the content of the PCI bus here.
The PCI bus referred in this comment is the actual, physical one.
It is only scanned during rte_pci_scan.

You are iterating registered PCI devices and removing them.

> +static int
> +pci_remove(void)
> +{
> +	struct rte_pci_device *dev = NULL;
> +	int ret = 0;
> +
> +	FOREACH_DEVICE_ON_PCIBUS(dev) {
> +		if (rte_dev_is_probed(&dev->device))
> +			if (rte_pci_detach_dev(dev) != 0) {
> +				RTE_LOG(INFO, EAL,
> +					"failed to detach driver form %s\n",

s/form/from/

> +					dev->device.name);
> +				ret = -1;
> +			}

Memory is leaked from not freeing the rte_pci_device here.
The device iterator must be made safe (using TAILQ_FOREACH_SAFE)
and the dev pointer removed from the list and freed.

There might be other resources still open.

> +	}
> +	return ret;
> +}
> +
>  /* dump one device */
>  static int
>  pci_dump_one_device(FILE *f, struct rte_pci_device *dev)
> @@ -669,6 +691,7 @@ struct rte_pci_bus rte_pci_bus = {
>  	.bus = {
>  		.scan = rte_pci_scan,
>  		.probe = pci_probe,
> +		.remove = pci_remove,
>  		.find_device = pci_find_device,
>  		.plug = pci_plug,
>  		.unplug = pci_unplug,
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index a89ea2353..692c936b9 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -546,6 +546,22 @@ vdev_unplug(struct rte_device *dev)
>  	return rte_vdev_uninit(dev->name);
>  }
>  
> +static int
> +vdev_remove(void)
> +{
> +	struct rte_vdev_device *dev;
> +	int ret = 0;
> +
> +	TAILQ_FOREACH(dev, &vdev_device_list, next) {
> +		if (vdev_remove_driver(dev) != 0) {
> +			VDEV_LOG(INFO, "driver of %s is not removed\n",
> +				dev->device.name);
> +			ret = -1;
> +		}
> +	}

Same as PCI bus, it should be made safe and the dev pointer removed
from the TAILQ and freed as well.

> +	return ret;
> +}
> +
>  static struct rte_bus rte_vdev_bus = {
>  	.scan = vdev_scan,
>  	.probe = vdev_probe,
> @@ -554,7 +570,8 @@ static struct rte_bus rte_vdev_bus = {
>  	.unplug = vdev_unplug,
>  	.parse = vdev_parse,
>  	.dev_iterate = rte_vdev_dev_iterate,
> -};
> +	.remove = vdev_remove,
> +	};
>  
>  RTE_REGISTER_BUS(vdev, rte_vdev_bus);
>  
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..bff95a0ae 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -85,6 +85,24 @@ rte_bus_probe(void)
>  	return 0;
>  }
>  
> +/* Remove all devices of all buses */
> +int
> +rte_bus_remove(void)
> +{
> +	int ret;
> +	struct rte_bus *bus;
> +
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		if (!strcmp(bus->name, "vdev") || !strcmp(bus->name, "pci")) {

You should only check that the bus sets the ops, not how the bus is named.

> +			ret = bus->remove();
> +			if (ret)
> +				RTE_LOG(INFO, EAL, "Bus (%s) remove failed.\n",
> +					bus->name);
> +		}
> +	}
> +	return 0;
> +}
> +
>  /* Dump information of a single bus */
>  static int
>  bus_dump_one(FILE *f, struct rte_bus *bus)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..c3e7e62c9 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -67,6 +67,18 @@ typedef int (*rte_bus_scan_t)(void);
>   */
>  typedef int (*rte_bus_probe_t)(void);
>  
> +/**
> + * Implementation specific remove function which is responsible for unlinking
> + * devices on that bus from attached drivers.
> + *
> + * This is called while iterating over each registered bus.
> + *
> + * @return
> + *	0 for successful remove
> + *	!0 for any error while removing
> + */
> +typedef int (*rte_bus_remove_t)(void);
> +
>  /**
>   * Device iterator to find a device on a bus.
>   *
> @@ -248,6 +260,7 @@ struct rte_bus {
>  	const char *name;            /**< Name of the bus */
>  	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
>  	rte_bus_probe_t probe;       /**< Probe devices on bus */
> +	rte_bus_remove_t remove;	 /**< remove device on bus */

Missing capital R from 'remove', 'device' should be plural.
'Remove devices from the bus' would read a bit better.

>  	rte_bus_find_device_t find_device; /**< Find a device on the bus */
>  	rte_bus_plug_t plug;         /**< Probe single device for drivers */
>  	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
> @@ -301,6 +314,16 @@ int rte_bus_scan(void);
>   */
>  int rte_bus_probe(void);
>  
> +/**
> + * For each device on the buses,call the driver-specific remove

Missing a space after the comma.

> + * for device uninitialization.
> + *
> + * @return
> + *	 0 for successful match/probe

You missed replacing match/probe here.

> + *	!0 otherwise
> + */
> +int rte_bus_remove(void);
> +
>  /**
>   * Dump information of all the buses registered with EAL.
>   *
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index f162124a3..89a03d3bb 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1330,6 +1330,8 @@ mark_freeable(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
>  int
>  rte_eal_cleanup(void)
>  {
> +	/* Remove devices/drivers from all buses */
> +	rte_bus_remove();

You've made your API return an int, but ignored drivers return code and
always return 0. That could be fine, but you also ignore it here anyway.
At this point just make it void, unless some error handling can be done.

There are variable definitions following in this function, why is this
call first?

It seems to be called only from linux EAL, why not BSD and Windows as
well?

>  	/* if we're in a primary process, we need to mark hugepages as freeable
>  	 * so that finalization can release them back to the system.
>  	 */
> -- 
> 2.25.1
> 

-- 
Gaëtan

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
  2020-09-12 19:53 ` Gaëtan Rivet
@ 2020-10-20 13:43   ` David Marchand
  2020-10-25 21:28     ` Muhammad Bilal
  0 siblings, 1 reply; 6+ messages in thread
From: David Marchand @ 2020-10-20 13:43 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: Yigit, Ferruh, Burakov, Anatoly, Vipin Varghese, Jakub Grajciar,
	Jerin Jacob Kollanukkaran, dev, dpdk stable, Gaëtan Rivet

Hello,

On Sat, Sep 12, 2020 at 9:53 PM Gaëtan Rivet <grive@u256.net> wrote:
>
> On 08/07/20 17:03 +0500, Muhammad Bilal wrote:
> > while using memif with app, the resources are not cleaned on exit,
> > So an error occurred on running it second time. The cause of this problem
> > is that remove() of memif driver is not called by rte_eal_cleanup() which
> > is counterpart of probe() called from rte_eal_init(). This is a case for
> > all other divers e.g pci, so to solve this problem I have added the
> > functionality of calling remove() function of all the driver attached to
> > devices on vdev and pci buses.
> >
>
> Hi Muhammad,
>
> review inline.

There were comments from Gaetan, waiting for a v2.
Thanks.


-- 
David Marchand


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

* Re: [dpdk-dev] [dpdk-stable] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
  2020-10-20 13:43   ` [dpdk-dev] [dpdk-stable] " David Marchand
@ 2020-10-25 21:28     ` Muhammad Bilal
  2021-03-25 15:27       ` David Marchand
  0 siblings, 1 reply; 6+ messages in thread
From: Muhammad Bilal @ 2020-10-25 21:28 UTC (permalink / raw)
  To: David Marchand
  Cc: Yigit, Ferruh, Burakov, Anatoly, Vipin Varghese, Jakub Grajciar,
	Jerin Jacob Kollanukkaran, dev, dpdk stable, Gaëtan Rivet

On Tue, Oct 20, 2020 at 6:43 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> Hello,
Hi
>
> On Sat, Sep 12, 2020 at 9:53 PM Gaëtan Rivet <grive@u256.net> wrote:
> >
> > On 08/07/20 17:03 +0500, Muhammad Bilal wrote:
> > > while using memif with app, the resources are not cleaned on exit,
> > > So an error occurred on running it second time. The cause of this problem
> > > is that remove() of memif driver is not called by rte_eal_cleanup() which
> > > is counterpart of probe() called from rte_eal_init(). This is a case for
> > > all other divers e.g pci, so to solve this problem I have added the
> > > functionality of calling remove() function of all the driver attached to
> > > devices on vdev and pci buses.
> > >
> >
> > Hi Muhammad,
> >
> > review inline.
>
> There were comments from Gaetan, waiting for a v2.
I am working on required changes, and will update it soon.
Thanks
> Thanks.
>
>
> --
> David Marchand
>

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

* Re: [dpdk-dev] [dpdk-stable] [PATCH] EAL: Called remove() of drivers for vdev and pci buses
  2020-10-25 21:28     ` Muhammad Bilal
@ 2021-03-25 15:27       ` David Marchand
  0 siblings, 0 replies; 6+ messages in thread
From: David Marchand @ 2021-03-25 15:27 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: Yigit, Ferruh, Burakov, Anatoly, Vipin Varghese, Jakub Grajciar,
	Jerin Jacob Kollanukkaran, dev, dpdk stable, Gaëtan Rivet

On Sun, Oct 25, 2020 at 10:28 PM Muhammad Bilal <m.bilal@emumba.com> wrote:
>
> On Tue, Oct 20, 2020 at 6:43 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > Hello,
> Hi
> >
> > On Sat, Sep 12, 2020 at 9:53 PM Gaëtan Rivet <grive@u256.net> wrote:
> > >
> > > On 08/07/20 17:03 +0500, Muhammad Bilal wrote:
> > > > while using memif with app, the resources are not cleaned on exit,
> > > > So an error occurred on running it second time. The cause of this problem
> > > > is that remove() of memif driver is not called by rte_eal_cleanup() which
> > > > is counterpart of probe() called from rte_eal_init(). This is a case for
> > > > all other divers e.g pci, so to solve this problem I have added the
> > > > functionality of calling remove() function of all the driver attached to
> > > > devices on vdev and pci buses.
> > > >
> > >
> > > Hi Muhammad,
> > >
> > > review inline.
> >
> > There were comments from Gaetan, waiting for a v2.
> I am working on required changes, and will update it soon.

Any update?

Thanks.

-- 
David Marchand


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

end of thread, other threads:[~2021-03-25 15:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 12:03 [dpdk-dev] [PATCH] EAL: Called remove() of drivers for vdev and pci buses Muhammad Bilal
2020-07-08 14:11 ` Muhammad Bilal
2020-09-12 19:53 ` Gaëtan Rivet
2020-10-20 13:43   ` [dpdk-dev] [dpdk-stable] " David Marchand
2020-10-25 21:28     ` Muhammad Bilal
2021-03-25 15:27       ` David Marchand

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