On 9/30/2020 12:50 PM, Ferruh Yigit wrote:
On 8/26/2020 6:52 AM, rohit....@nxp.com wrote:
From: Rohit Raj <rohit....@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....@nxp.com>
---

v3:
* nit: combined nested if statements

v2:
* Moved rte_bus_close call to rte_eal_cleanup path.

  lib/librte_eal/common/eal_common_bus.c | 32 +++++++++++++++++++++++++-
  lib/librte_eal/include/rte_bus.h       | 25 +++++++++++++++++++-
  lib/librte_eal/linux/eal.c             |  1 +
  lib/librte_eal/rte_eal_version.map     |  3 +++
  4 files changed, 59 insertions(+), 2 deletions(-)

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;
+        }

This special treatment for 'vdev' bus is done in probe to be sure physically device port ids start from '0',  I guess we don't need to do this for 'close'.

+
+        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..af4787b18 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,18 @@ typedef int (*rte_bus_scan_t)(void);
   */
  typedef int (*rte_bus_probe_t)(void);
+/**
+ * Implementation specific close function which is responsible for closing
+ * devices on that bus.
+ *
+ * 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);
+

As I checked the 'rte_fslmc_bus->close()' ops, it iterates on all devices in the bus instead of doing a bus level close, in that case instead of adding a new 'close' bus operations, will it work if existing 'bus->unplug(dev)' used? Whatever done in the 'rte_fslmc_bus->close()' per device, can it be done under the 'fslmc_bus_unplug()'?

And in that case a 'rte_bus_remove()' API can be added which can call 'bus->unplug(dev)' for all buses and it will be beneficial for all buses, and it can fit well into the 'rte_eal_cleanup()'.

What do you think?

Hi Rohit,

I have seen new version has been sent today, I want to remind above question, can you please check it?

Reply via email to