Hi Ferruh, Sorry, I missed you comment last time. I will send an updated patch series after taking care of your suggested changes.
Regards, Rohit Raj Software Engineer | NXP India Private Limited Sent from Outlook Mobile ________________________________ From: Ferruh Yigit <ferruh.yi...@intel.com> Sent: Thursday, October 8, 2020 9:54:58 PM To: Rohit Raj <rohit....@nxp.com>; Ray Kinsella <m...@ashroe.eu>; Neil Horman <nhor...@tuxdriver.com> Cc: dev@dpdk.org <dev@dpdk.org> Subject: [EXT] Re: [dpdk-dev] [PATCH v3 1/3] eal: add API for bus close Caution: EXT Email 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?