[v4,1/5] eal: add API for bus close

Message ID 20201008153048.19369-1-rohit.raj@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v4,1/5] eal: add API for bus close |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Rohit Raj Oct. 8, 2020, 3:30 p.m. UTC
  From: Rohit Raj <rohit.raj@nxp.com>

As per the current code we have API for bus probe, but the
bus close API is missing. This breaks the multi process
scenarios as objects are not cleaned while terminating the
secondary processes.

This patch adds a new API rte_bus_close() for cleanup of
bus objects which were acquired during probe.

Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
---
 lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
 lib/librte_eal/include/rte_bus.h       | 27 +++++++++++++++++++++-
 lib/librte_eal/linux/eal.c             |  1 +
 lib/librte_eal/rte_eal_version.map     |  1 +
 4 files changed, 59 insertions(+), 2 deletions(-)
  

Comments

David Marchand Oct. 18, 2020, 9:21 a.m. UTC | #1
On Thu, Oct 8, 2020 at 5:31 PM <rohit.raj@nxp.com> wrote:
>
> From: Rohit Raj <rohit.raj@nxp.com>
>
> As per the current code we have API for bus probe, but the
> bus close API is missing. This breaks the multi process
> scenarios as objects are not cleaned while terminating the
> secondary processes.
>
> This patch adds a new API rte_bus_close() for cleanup of
> bus objects which were acquired during probe.
>
> Signed-off-by: Rohit Raj <rohit.raj@nxp.com>
> ---
>  lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
>  lib/librte_eal/include/rte_bus.h       | 27 +++++++++++++++++++++-
>  lib/librte_eal/linux/eal.c             |  1 +
>  lib/librte_eal/rte_eal_version.map     |  1 +
>  4 files changed, 59 insertions(+), 2 deletions(-)

This is a new EAL API, please update the release notes.


>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index baa5b532a..5fd7cf6c5 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>
>  #include <stdio.h>
> @@ -56,6 +56,36 @@ rte_bus_scan(void)
>         return 0;
>  }
>
> +int
> +rte_bus_close(void)
> +{
> +       int ret;
> +       struct rte_bus *bus, *vbus = NULL;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +               if (!strcmp(bus->name, "vdev")) {
> +                       vbus = bus;
> +                       continue;
> +               }
> +
> +               if (bus->close) {
> +                       ret = bus->close();
> +                       if (ret)
> +                               RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
> +                                       bus->name);
> +               }
> +       }
> +
> +       if (vbus && vbus->close) {
> +               ret = vbus->close();
> +               if (ret)
> +                       RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
> +                               vbus->name);
> +       }
> +
> +       return 0;

We should propagate that an error occurred so that the caller is aware
something went wrong.


> +}
> +
>  /* Probe all devices of all buses */
>  int
>  rte_bus_probe(void)
> diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
> index d3034d0ed..7bf7e81b2 100644
> --- a/lib/librte_eal/include/rte_bus.h
> +++ b/lib/librte_eal/include/rte_bus.h
> @@ -1,5 +1,5 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright 2016 NXP
> + * Copyright 2016,2020 NXP
>   */
>
>  #ifndef _RTE_BUS_H_
> @@ -67,6 +67,20 @@ typedef int (*rte_bus_scan_t)(void);
>   */
>  typedef int (*rte_bus_probe_t)(void);
>
> +/**
> + * Implementation specific close function which is responsible for resetting all
> + * detected devices on the bus to a default state, closing UIO nodes or VFIO
> + * groups and also freeing any memory allocated during rte_bus_probe like
> + * private resources for device list.
> + *
> + * This is called while iterating over each registered bus.
> + *
> + * @return
> + *     0 for successful close
> + *     !0 for any error while closing
> + */
> +typedef int (*rte_bus_close_t)(void);
> +
>  /**
>   * Device iterator to find a device on a bus.
>   *
> @@ -248,6 +262,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_close_t close;       /**< Close devices 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 +316,16 @@ int rte_bus_scan(void);
>   */
>  int rte_bus_probe(void);
>
> +/**

Missing a banner:

+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice

> + * For each device on the buses, call the device specific close.
> + *
> + * @return
> + *      0 for successful close
> + *     !0 otherwise
> + */
> +__rte_experimental
> +int rte_bus_close(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 9cf0e2ec0..1e1256f07 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1358,6 +1358,7 @@ rte_eal_cleanup(void)
>
>         if (rte_eal_process_type() == RTE_PROC_PRIMARY)
>                 rte_memseg_walk(mark_freeable, NULL);
> +       rte_bus_close();
>         rte_service_finalize();
>         rte_mp_channel_cleanup();
>         rte_trace_save();

You can squash patch 4 and patch 5 in this patch so that we update all
OS-specific bits at once.


> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index a93dea9fe..8e226b297 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -400,6 +400,7 @@ EXPERIMENTAL {
>         # added in 20.11
>         __rte_eal_trace_generic_size_t;
>         rte_service_lcore_may_be_active;
> +       rte_bus_close;

Please, alphabetical order.


>  };
>
>  INTERNAL {
> --
> 2.17.1
>
  

Patch

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index baa5b532a..5fd7cf6c5 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2016 NXP
+ * Copyright 2016,2020 NXP
  */
 
 #include <stdio.h>
@@ -56,6 +56,36 @@  rte_bus_scan(void)
 	return 0;
 }
 
+int
+rte_bus_close(void)
+{
+	int ret;
+	struct rte_bus *bus, *vbus = NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!strcmp(bus->name, "vdev")) {
+			vbus = bus;
+			continue;
+		}
+
+		if (bus->close) {
+			ret = bus->close();
+			if (ret)
+				RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
+					bus->name);
+		}
+	}
+
+	if (vbus && vbus->close) {
+		ret = vbus->close();
+		if (ret)
+			RTE_LOG(ERR, EAL, "Bus (%s) close failed.\n",
+				vbus->name);
+	}
+
+	return 0;
+}
+
 /* Probe all devices of all buses */
 int
 rte_bus_probe(void)
diff --git a/lib/librte_eal/include/rte_bus.h b/lib/librte_eal/include/rte_bus.h
index d3034d0ed..7bf7e81b2 100644
--- a/lib/librte_eal/include/rte_bus.h
+++ b/lib/librte_eal/include/rte_bus.h
@@ -1,5 +1,5 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2016 NXP
+ * Copyright 2016,2020 NXP
  */
 
 #ifndef _RTE_BUS_H_
@@ -67,6 +67,20 @@  typedef int (*rte_bus_scan_t)(void);
  */
 typedef int (*rte_bus_probe_t)(void);
 
+/**
+ * Implementation specific close function which is responsible for resetting all
+ * detected devices on the bus to a default state, closing UIO nodes or VFIO
+ * groups and also freeing any memory allocated during rte_bus_probe like
+ * private resources for device list.
+ *
+ * This is called while iterating over each registered bus.
+ *
+ * @return
+ *	0 for successful close
+ *	!0 for any error while closing
+ */
+typedef int (*rte_bus_close_t)(void);
+
 /**
  * Device iterator to find a device on a bus.
  *
@@ -248,6 +262,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_close_t close;       /**< Close devices 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 +316,16 @@  int rte_bus_scan(void);
  */
 int rte_bus_probe(void);
 
+/**
+ * For each device on the buses, call the device specific close.
+ *
+ * @return
+ *	 0 for successful close
+ *	!0 otherwise
+ */
+__rte_experimental
+int rte_bus_close(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 9cf0e2ec0..1e1256f07 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -1358,6 +1358,7 @@  rte_eal_cleanup(void)
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY)
 		rte_memseg_walk(mark_freeable, NULL);
+	rte_bus_close();
 	rte_service_finalize();
 	rte_mp_channel_cleanup();
 	rte_trace_save();
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index a93dea9fe..8e226b297 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -400,6 +400,7 @@  EXPERIMENTAL {
 	# added in 20.11
 	__rte_eal_trace_generic_size_t;
 	rte_service_lcore_may_be_active;
+	rte_bus_close;
 };
 
 INTERNAL {