From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4DF45A04B5; Wed, 30 Sep 2020 13:50:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 346F81DAFB; Wed, 30 Sep 2020 13:50:23 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 06A2B1C121 for ; Wed, 30 Sep 2020 13:50:21 +0200 (CEST) IronPort-SDR: H952bihcpaN981TgcfebFp7B+IKAcNSyfntNlT3uyQwFlwMU+ZcY2n0gkI4mGYVlwdxWDxf4yk iR7LZvPD2NZA== X-IronPort-AV: E=McAfee;i="6000,8403,9759"; a="162481707" X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="162481707" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 04:50:18 -0700 IronPort-SDR: iELRvfcFgZHiE8ykxtPgKYf+r+qXaB/hNQm+FZ3I2Hsv2bjIfoeNE8W9JIzR0UiLO8sUCn6orV shrO9SzrqHdw== X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="497174131" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.251.186]) ([10.213.251.186]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 04:50:17 -0700 To: rohit.raj@nxp.com, Ray Kinsella , Neil Horman Cc: dev@dpdk.org References: <20200824082414.30535-1-rohit.raj@nxp.com> <20200826055233.26075-1-rohit.raj@nxp.com> From: Ferruh Yigit Message-ID: <23bd31be-06a2-af9a-2b54-8e44cf9b2c79@intel.com> Date: Wed, 30 Sep 2020 12:50:13 +0100 MIME-Version: 1.0 In-Reply-To: <20200826055233.26075-1-rohit.raj@nxp.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v3 1/3] eal: add API for bus close X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 8/26/2020 6:52 AM, rohit.raj@nxp.com wrote: > From: Rohit Raj > > 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 > --- > > 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 > @@ -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?